From e90578ccda8cca43882fe22691d7b316d8521181 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Mon, 23 Sep 2024 16:53:31 +0200 Subject: [PATCH] [HTTP] Set explicit access for `public` HTTP APIs (#192554) ## Summary We will be enforcing restricted access to internal HTTP APIs [from 9.0](https://github.com/elastic/kibana/issues/186781). This PR is part 1 of audit checking that our public APIs have their access tag set explicitly to ensure they are still available to end users after we start enforcing HTTP API restrictions. APIs reviewed in this PR ([docs](https://www.elastic.co/guide/en/kibana/current/dashboard-import-api.html)): Screenshot 2024-09-11 at 11 25 55 ## Note to reviewers This audit is focussed on set `access: 'public'` where needed. Per the screenshot our public-facing documentation is taken as the source of truth for which APIs should be public. This may differ per offering so please consider whether a given HTTP API should be public on both serverless and stateful offerings. ## Risks * If we miss an API that should be public, end users will encounter a `400` response when they try to use the HTTP API on 9.0 * If we set an API's access to "public" it will not have the same restrictions applied to it. (cherry picked from commit 3fa5bdf8732101812a656ec954e2a8d779838938) --- .../src/routes/index.ts | 8 +++++++- .../src/routes/legacy_import_export/export.ts | 10 +++++++++- .../src/routes/legacy_import_export/import.ts | 10 +++++++++- .../routes/legacy_import_export/export.test.ts | 1 + .../routes/legacy_import_export/import.test.ts | 1 + .../server/routes/authorization/roles/delete.ts | 1 + .../security/server/routes/authorization/roles/get.ts | 1 + .../server/routes/authorization/roles/get_all.ts | 1 + .../server/routes/authorization/roles/post.ts | 1 + .../security/server/routes/authorization/roles/put.ts | 1 + .../routes/session_management/invalidate.test.ts | 1 + .../server/routes/session_management/invalidate.ts | 1 + .../server/routes/api/external/copy_to_space.ts | 11 ++++++++++- .../spaces/server/routes/api/external/delete.ts | 3 ++- .../routes/api/external/disable_legacy_url_aliases.ts | 3 ++- .../plugins/spaces/server/routes/api/external/get.ts | 3 ++- .../spaces/server/routes/api/external/get_all.ts | 3 ++- .../routes/api/external/get_shareable_references.ts | 3 ++- .../plugins/spaces/server/routes/api/external/post.ts | 1 + .../plugins/spaces/server/routes/api/external/put.ts | 1 + .../routes/api/external/update_objects_spaces.ts | 3 ++- 21 files changed, 58 insertions(+), 10 deletions(-) diff --git a/packages/core/saved-objects/core-saved-objects-server-internal/src/routes/index.ts b/packages/core/saved-objects/core-saved-objects-server-internal/src/routes/index.ts index f435e29a22eef..48ac75e045148 100644 --- a/packages/core/saved-objects/core-saved-objects-server-internal/src/routes/index.ts +++ b/packages/core/saved-objects/core-saved-objects-server-internal/src/routes/index.ts @@ -84,8 +84,14 @@ export function registerRoutes({ maxImportPayloadBytes: config.maxImportPayloadBytes, coreUsageData, logger, + access: internalOnServerless, + }); + registerLegacyExportRoute(legacyRouter, { + kibanaVersion, + coreUsageData, + logger, + access: internalOnServerless, }); - registerLegacyExportRoute(legacyRouter, { kibanaVersion, coreUsageData, logger }); const internalRouter = http.createRouter( '/internal/saved_objects/' diff --git a/packages/core/saved-objects/core-saved-objects-server-internal/src/routes/legacy_import_export/export.ts b/packages/core/saved-objects/core-saved-objects-server-internal/src/routes/legacy_import_export/export.ts index cdb04e5afc697..f3cf776f7d977 100644 --- a/packages/core/saved-objects/core-saved-objects-server-internal/src/routes/legacy_import_export/export.ts +++ b/packages/core/saved-objects/core-saved-objects-server-internal/src/routes/legacy_import_export/export.ts @@ -11,6 +11,7 @@ import moment from 'moment'; import { schema } from '@kbn/config-schema'; import type { Logger } from '@kbn/logging'; import type { InternalCoreUsageDataSetup } from '@kbn/core-usage-data-base-server-internal'; +import type { RouteAccess } from '@kbn/core-http-server'; import type { InternalSavedObjectRouter } from '../../internal_types'; import { exportDashboards } from './lib'; @@ -20,7 +21,13 @@ export const registerLegacyExportRoute = ( kibanaVersion, coreUsageData, logger, - }: { kibanaVersion: string; coreUsageData: InternalCoreUsageDataSetup; logger: Logger } + access, + }: { + kibanaVersion: string; + coreUsageData: InternalCoreUsageDataSetup; + logger: Logger; + access: RouteAccess; + } ) => { router.get( { @@ -31,6 +38,7 @@ export const registerLegacyExportRoute = ( }), }, options: { + access, tags: ['api'], }, }, diff --git a/packages/core/saved-objects/core-saved-objects-server-internal/src/routes/legacy_import_export/import.ts b/packages/core/saved-objects/core-saved-objects-server-internal/src/routes/legacy_import_export/import.ts index 186e1cc26dc63..e45ef3205af18 100644 --- a/packages/core/saved-objects/core-saved-objects-server-internal/src/routes/legacy_import_export/import.ts +++ b/packages/core/saved-objects/core-saved-objects-server-internal/src/routes/legacy_import_export/import.ts @@ -11,6 +11,7 @@ import { schema } from '@kbn/config-schema'; import type { Logger } from '@kbn/logging'; import type { SavedObject } from '@kbn/core-saved-objects-server'; import type { InternalCoreUsageDataSetup } from '@kbn/core-usage-data-base-server-internal'; +import type { RouteAccess } from '@kbn/core-http-server'; import type { InternalSavedObjectRouter } from '../../internal_types'; import { importDashboards } from './lib'; @@ -20,7 +21,13 @@ export const registerLegacyImportRoute = ( maxImportPayloadBytes, coreUsageData, logger, - }: { maxImportPayloadBytes: number; coreUsageData: InternalCoreUsageDataSetup; logger: Logger } + access, + }: { + maxImportPayloadBytes: number; + coreUsageData: InternalCoreUsageDataSetup; + logger: Logger; + access: RouteAccess; + } ) => { router.post( { @@ -38,6 +45,7 @@ export const registerLegacyImportRoute = ( }), }, options: { + access, tags: ['api'], body: { maxBytes: maxImportPayloadBytes, diff --git a/src/core/server/integration_tests/saved_objects/routes/legacy_import_export/export.test.ts b/src/core/server/integration_tests/saved_objects/routes/legacy_import_export/export.test.ts index b3660a0aae549..5481fbc807276 100644 --- a/src/core/server/integration_tests/saved_objects/routes/legacy_import_export/export.test.ts +++ b/src/core/server/integration_tests/saved_objects/routes/legacy_import_export/export.test.ts @@ -62,6 +62,7 @@ describe('POST /api/dashboards/export', () => { kibanaVersion: 'mockversion', coreUsageData, logger: loggerMock.create(), + access: 'public', }); handlerContext.savedObjects.client.bulkGet diff --git a/src/core/server/integration_tests/saved_objects/routes/legacy_import_export/import.test.ts b/src/core/server/integration_tests/saved_objects/routes/legacy_import_export/import.test.ts index 388773eea1bed..8157c77e936a9 100644 --- a/src/core/server/integration_tests/saved_objects/routes/legacy_import_export/import.test.ts +++ b/src/core/server/integration_tests/saved_objects/routes/legacy_import_export/import.test.ts @@ -62,6 +62,7 @@ describe('POST /api/dashboards/import', () => { maxImportPayloadBytes: 26214400, coreUsageData, logger: loggerMock.create(), + access: 'public', }); handlerContext.savedObjects.client.bulkCreate.mockResolvedValueOnce({ diff --git a/x-pack/plugins/security/server/routes/authorization/roles/delete.ts b/x-pack/plugins/security/server/routes/authorization/roles/delete.ts index 8a481c4b60cbf..022e574181425 100644 --- a/x-pack/plugins/security/server/routes/authorization/roles/delete.ts +++ b/x-pack/plugins/security/server/routes/authorization/roles/delete.ts @@ -16,6 +16,7 @@ export function defineDeleteRolesRoutes({ router }: RouteDefinitionParams) { { path: '/api/security/role/{name}', options: { + access: 'public', summary: `Delete a role`, }, validate: { diff --git a/x-pack/plugins/security/server/routes/authorization/roles/get.ts b/x-pack/plugins/security/server/routes/authorization/roles/get.ts index 9109d89d956fd..ec2208341dc16 100644 --- a/x-pack/plugins/security/server/routes/authorization/roles/get.ts +++ b/x-pack/plugins/security/server/routes/authorization/roles/get.ts @@ -22,6 +22,7 @@ export function defineGetRolesRoutes({ { path: '/api/security/role/{name}', options: { + access: 'public', summary: `Get a role`, }, validate: { diff --git a/x-pack/plugins/security/server/routes/authorization/roles/get_all.ts b/x-pack/plugins/security/server/routes/authorization/roles/get_all.ts index a7d995f0a2639..ed31aedba7f31 100644 --- a/x-pack/plugins/security/server/routes/authorization/roles/get_all.ts +++ b/x-pack/plugins/security/server/routes/authorization/roles/get_all.ts @@ -22,6 +22,7 @@ export function defineGetAllRolesRoutes({ { path: '/api/security/role', options: { + access: 'public', summary: `Get all roles`, }, validate: false, diff --git a/x-pack/plugins/security/server/routes/authorization/roles/post.ts b/x-pack/plugins/security/server/routes/authorization/roles/post.ts index 07b9886c4072c..37967d208bf3a 100644 --- a/x-pack/plugins/security/server/routes/authorization/roles/post.ts +++ b/x-pack/plugins/security/server/routes/authorization/roles/post.ts @@ -43,6 +43,7 @@ export function defineBulkCreateOrUpdateRolesRoutes({ { path: '/api/security/roles', options: { + access: 'public', summary: 'Create or update roles', }, validate: { diff --git a/x-pack/plugins/security/server/routes/authorization/roles/put.ts b/x-pack/plugins/security/server/routes/authorization/roles/put.ts index 57271235add36..6175ba6f4d64f 100644 --- a/x-pack/plugins/security/server/routes/authorization/roles/put.ts +++ b/x-pack/plugins/security/server/routes/authorization/roles/put.ts @@ -24,6 +24,7 @@ export function definePutRolesRoutes({ { path: '/api/security/role/{name}', options: { + access: 'public', summary: `Create or update a role`, }, validate: { diff --git a/x-pack/plugins/security/server/routes/session_management/invalidate.test.ts b/x-pack/plugins/security/server/routes/session_management/invalidate.test.ts index ccd42b45718f8..fbc14015d80c1 100644 --- a/x-pack/plugins/security/server/routes/session_management/invalidate.test.ts +++ b/x-pack/plugins/security/server/routes/session_management/invalidate.test.ts @@ -44,6 +44,7 @@ describe('Invalidate sessions routes', () => { it('correctly defines route.', () => { expect(routeConfig.options).toEqual({ + access: 'public', summary: 'Invalidate user sessions', tags: ['access:sessionManagement'], }); diff --git a/x-pack/plugins/security/server/routes/session_management/invalidate.ts b/x-pack/plugins/security/server/routes/session_management/invalidate.ts index 1cf65bb9191c6..c7d27b835edf2 100644 --- a/x-pack/plugins/security/server/routes/session_management/invalidate.ts +++ b/x-pack/plugins/security/server/routes/session_management/invalidate.ts @@ -34,6 +34,7 @@ export function defineInvalidateSessionsRoutes({ router, getSession }: RouteDefi }), }, options: { + access: 'public', tags: ['access:sessionManagement'], summary: `Invalidate user sessions`, }, diff --git a/x-pack/plugins/spaces/server/routes/api/external/copy_to_space.ts b/x-pack/plugins/spaces/server/routes/api/external/copy_to_space.ts index c237c86b0b91a..f1f1f22b55e32 100644 --- a/x-pack/plugins/spaces/server/routes/api/external/copy_to_space.ts +++ b/x-pack/plugins/spaces/server/routes/api/external/copy_to_space.ts @@ -24,13 +24,21 @@ const areObjectsUnique = (objects: SavedObjectIdentifier[]) => _.uniqBy(objects, (o: SavedObjectIdentifier) => `${o.type}:${o.id}`).length === objects.length; export function initCopyToSpacesApi(deps: ExternalRouteDeps) { - const { router, getSpacesService, usageStatsServicePromise, getStartServices, log } = deps; + const { + router, + getSpacesService, + usageStatsServicePromise, + getStartServices, + log, + isServerless, + } = deps; const usageStatsClientPromise = usageStatsServicePromise.then(({ getClient }) => getClient()); router.post( { path: '/api/spaces/_copy_saved_objects', options: { + access: isServerless ? 'internal' : 'public', tags: ['access:copySavedObjectsToSpaces'], description: `Copy saved objects to spaces`, }, @@ -148,6 +156,7 @@ export function initCopyToSpacesApi(deps: ExternalRouteDeps) { { path: '/api/spaces/_resolve_copy_saved_objects_errors', options: { + access: isServerless ? 'internal' : 'public', tags: ['access:copySavedObjectsToSpaces'], description: `Resolve conflicts copying saved objects`, }, diff --git a/x-pack/plugins/spaces/server/routes/api/external/delete.ts b/x-pack/plugins/spaces/server/routes/api/external/delete.ts index 30a6f85d6994b..6ede0fa220043 100644 --- a/x-pack/plugins/spaces/server/routes/api/external/delete.ts +++ b/x-pack/plugins/spaces/server/routes/api/external/delete.ts @@ -15,12 +15,13 @@ import { wrapError } from '../../../lib/errors'; import { createLicensedRouteHandler } from '../../lib'; export function initDeleteSpacesApi(deps: ExternalRouteDeps) { - const { router, log, getSpacesService } = deps; + const { router, log, getSpacesService, isServerless } = deps; router.delete( { path: '/api/spaces/space/{id}', options: { + access: isServerless ? 'internal' : 'public', description: `Delete a space`, }, validate: { diff --git a/x-pack/plugins/spaces/server/routes/api/external/disable_legacy_url_aliases.ts b/x-pack/plugins/spaces/server/routes/api/external/disable_legacy_url_aliases.ts index 67bce1bfb742a..6b3c70eb64ffa 100644 --- a/x-pack/plugins/spaces/server/routes/api/external/disable_legacy_url_aliases.ts +++ b/x-pack/plugins/spaces/server/routes/api/external/disable_legacy_url_aliases.ts @@ -12,13 +12,14 @@ import { wrapError } from '../../../lib/errors'; import { createLicensedRouteHandler } from '../../lib'; export function initDisableLegacyUrlAliasesApi(deps: ExternalRouteDeps) { - const { router, getSpacesService, usageStatsServicePromise, log } = deps; + const { router, getSpacesService, usageStatsServicePromise, log, isServerless } = deps; const usageStatsClientPromise = usageStatsServicePromise.then(({ getClient }) => getClient()); router.post( { path: '/api/spaces/_disable_legacy_url_aliases', options: { + access: isServerless ? 'internal' : 'public', description: `Disable legacy URL aliases`, }, validate: { diff --git a/x-pack/plugins/spaces/server/routes/api/external/get.ts b/x-pack/plugins/spaces/server/routes/api/external/get.ts index 99bc646994b5c..dce169449c99a 100644 --- a/x-pack/plugins/spaces/server/routes/api/external/get.ts +++ b/x-pack/plugins/spaces/server/routes/api/external/get.ts @@ -13,12 +13,13 @@ import { wrapError } from '../../../lib/errors'; import { createLicensedRouteHandler } from '../../lib'; export function initGetSpaceApi(deps: ExternalRouteDeps) { - const { router, getSpacesService } = deps; + const { router, getSpacesService, isServerless } = deps; router.get( { path: '/api/spaces/space/{id}', options: { + access: isServerless ? 'internal' : 'public', description: `Get a space`, }, validate: { diff --git a/x-pack/plugins/spaces/server/routes/api/external/get_all.ts b/x-pack/plugins/spaces/server/routes/api/external/get_all.ts index 5b972f6491860..603dc3dfe45ba 100644 --- a/x-pack/plugins/spaces/server/routes/api/external/get_all.ts +++ b/x-pack/plugins/spaces/server/routes/api/external/get_all.ts @@ -13,12 +13,13 @@ import { wrapError } from '../../../lib/errors'; import { createLicensedRouteHandler } from '../../lib'; export function initGetAllSpacesApi(deps: ExternalRouteDeps) { - const { router, log, getSpacesService } = deps; + const { router, log, getSpacesService, isServerless } = deps; router.get( { path: '/api/spaces/space', options: { + access: isServerless ? 'internal' : 'public', description: `Get all spaces`, }, validate: { diff --git a/x-pack/plugins/spaces/server/routes/api/external/get_shareable_references.ts b/x-pack/plugins/spaces/server/routes/api/external/get_shareable_references.ts index 687a51b28f9c8..93a210cd82b3e 100644 --- a/x-pack/plugins/spaces/server/routes/api/external/get_shareable_references.ts +++ b/x-pack/plugins/spaces/server/routes/api/external/get_shareable_references.ts @@ -12,12 +12,13 @@ import { wrapError } from '../../../lib/errors'; import { createLicensedRouteHandler } from '../../lib'; export function initGetShareableReferencesApi(deps: ExternalRouteDeps) { - const { router, getStartServices } = deps; + const { router, getStartServices, isServerless } = deps; router.post( { path: '/api/spaces/_get_shareable_references', options: { + access: isServerless ? 'internal' : 'public', description: `Get shareable references`, }, validate: { diff --git a/x-pack/plugins/spaces/server/routes/api/external/post.ts b/x-pack/plugins/spaces/server/routes/api/external/post.ts index 31dda1285f06a..db3cc2c1cdcae 100644 --- a/x-pack/plugins/spaces/server/routes/api/external/post.ts +++ b/x-pack/plugins/spaces/server/routes/api/external/post.ts @@ -21,6 +21,7 @@ export function initPostSpacesApi(deps: ExternalRouteDeps) { { path: '/api/spaces/space', options: { + access: isServerless ? 'internal' : 'public', description: `Create a space`, }, validate: { diff --git a/x-pack/plugins/spaces/server/routes/api/external/put.ts b/x-pack/plugins/spaces/server/routes/api/external/put.ts index e1defabee7103..4612aedb4c151 100644 --- a/x-pack/plugins/spaces/server/routes/api/external/put.ts +++ b/x-pack/plugins/spaces/server/routes/api/external/put.ts @@ -21,6 +21,7 @@ export function initPutSpacesApi(deps: ExternalRouteDeps) { { path: '/api/spaces/space/{id}', options: { + access: isServerless ? 'internal' : 'public', description: `Update a space`, }, validate: { diff --git a/x-pack/plugins/spaces/server/routes/api/external/update_objects_spaces.ts b/x-pack/plugins/spaces/server/routes/api/external/update_objects_spaces.ts index 8222ceba0813c..68b89d0934cf1 100644 --- a/x-pack/plugins/spaces/server/routes/api/external/update_objects_spaces.ts +++ b/x-pack/plugins/spaces/server/routes/api/external/update_objects_spaces.ts @@ -14,7 +14,7 @@ import { SPACE_ID_REGEX } from '../../../lib/space_schema'; import { createLicensedRouteHandler } from '../../lib'; export function initUpdateObjectsSpacesApi(deps: ExternalRouteDeps) { - const { router, getStartServices } = deps; + const { router, getStartServices, isServerless } = deps; const spacesSchema = schema.arrayOf( schema.string({ @@ -37,6 +37,7 @@ export function initUpdateObjectsSpacesApi(deps: ExternalRouteDeps) { { path: '/api/spaces/_update_objects_spaces', options: { + access: isServerless ? 'internal' : 'public', description: `Update saved objects in spaces`, }, validate: {