From 44be16fedb043ed47c642d9854efcf7ca750a6db Mon Sep 17 00:00:00 2001 From: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Date: Fri, 22 Nov 2024 10:59:31 +1100 Subject: [PATCH] [8.x] Unauthorized route migration for routes owned by obs-ux-management-team (#198374) (#201298) # Backport This will backport the following commits from `main` to `8.x`: - [Unauthorized route migration for routes owned by obs-ux-management-team (#198374)](https://github.com/elastic/kibana/pull/198374) ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) --- .../annotations/register_annotation_apis.ts | 36 ++++ .../synthetics_service/run_once_monitor.ts | 4 + .../synthetics_service/test_now_monitor.ts | 73 ++++---- .../synthetics/server/server.ts | 23 +-- .../server/synthetics_route_wrapper.ts | 6 +- .../routes/uptime_route_wrapper.ts | 11 +- .../server/legacy_uptime/uptime_server.ts | 17 +- .../apis/synthetics/add_monitor.ts | 4 +- .../api_integration/apis/synthetics/index.ts | 1 + .../synthetics_monitor_test_service.ts | 5 +- .../synthetics/synthetics_api_security.ts | 161 ++++++++++++++++++ 11 files changed, 276 insertions(+), 65 deletions(-) create mode 100644 x-pack/test/api_integration/apis/synthetics/synthetics_api_security.ts diff --git a/x-pack/plugins/observability_solution/observability/server/lib/annotations/register_annotation_apis.ts b/x-pack/plugins/observability_solution/observability/server/lib/annotations/register_annotation_apis.ts index 59ae964ce8831..5df2ed8d0e682 100644 --- a/x-pack/plugins/observability_solution/observability/server/lib/annotations/register_annotation_apis.ts +++ b/x-pack/plugins/observability_solution/observability/server/lib/annotations/register_annotation_apis.ts @@ -95,6 +95,12 @@ export function registerAnnotationAPIs({ router.post( { path: '/api/observability/annotation', + security: { + authz: { + enabled: false, + reason: 'This route delegates authorization to Elasticsearch', + }, + }, validate: { body: unknowns, }, @@ -110,6 +116,12 @@ export function registerAnnotationAPIs({ router.put( { path: '/api/observability/annotation/{id}', + security: { + authz: { + enabled: false, + reason: 'This route delegates authorization to Elasticsearch', + }, + }, validate: { body: unknowns, }, @@ -125,6 +137,12 @@ export function registerAnnotationAPIs({ router.delete( { path: '/api/observability/annotation/{id}', + security: { + authz: { + enabled: false, + reason: 'This route delegates authorization to Elasticsearch', + }, + }, validate: { params: unknowns, }, @@ -140,6 +158,12 @@ export function registerAnnotationAPIs({ router.get( { path: '/api/observability/annotation/{id}', + security: { + authz: { + enabled: false, + reason: 'This route delegates authorization to Elasticsearch', + }, + }, validate: { params: unknowns, }, @@ -155,6 +179,12 @@ export function registerAnnotationAPIs({ router.get( { path: '/api/observability/annotation/find', + security: { + authz: { + enabled: false, + reason: 'This route delegates authorization to Elasticsearch', + }, + }, validate: { query: unknowns, }, @@ -170,6 +200,12 @@ export function registerAnnotationAPIs({ router.get( { path: '/api/observability/annotation/permissions', + security: { + authz: { + enabled: false, + reason: 'This route delegates authorization to Elasticsearch', + }, + }, validate: { query: unknowns, }, diff --git a/x-pack/plugins/observability_solution/synthetics/server/routes/synthetics_service/run_once_monitor.ts b/x-pack/plugins/observability_solution/synthetics/server/routes/synthetics_service/run_once_monitor.ts index 6f7b3427f96f0..2af3a10f39750 100644 --- a/x-pack/plugins/observability_solution/synthetics/server/routes/synthetics_service/run_once_monitor.ts +++ b/x-pack/plugins/observability_solution/synthetics/server/routes/synthetics_service/run_once_monitor.ts @@ -6,6 +6,7 @@ */ import { schema } from '@kbn/config-schema'; import { DEFAULT_SPACE_ID } from '@kbn/spaces-plugin/common'; +import { isEmpty } from 'lodash'; import { PrivateLocationAttributes } from '../../runtime_types/private_locations'; import { getPrivateLocationsForMonitor } from '../monitor_cruds/add_monitor/utils'; import { SyntheticsRestApiRouteFactory } from '../types'; @@ -31,6 +32,9 @@ export const runOnceSyntheticsMonitorRoute: SyntheticsRestApiRouteFactory = () = }): Promise => { const monitor = request.body as MonitorFields; const { monitorId } = request.params; + if (isEmpty(monitor)) { + return response.badRequest({ body: { message: 'Monitor data is empty.' } }); + } const validationResult = validateMonitor(monitor); diff --git a/x-pack/plugins/observability_solution/synthetics/server/routes/synthetics_service/test_now_monitor.ts b/x-pack/plugins/observability_solution/synthetics/server/routes/synthetics_service/test_now_monitor.ts index 3f878b10ac8f8..d1a1513ae85c8 100644 --- a/x-pack/plugins/observability_solution/synthetics/server/routes/synthetics_service/test_now_monitor.ts +++ b/x-pack/plugins/observability_solution/synthetics/server/routes/synthetics_service/test_now_monitor.ts @@ -6,6 +6,8 @@ */ import { schema } from '@kbn/config-schema'; import { v4 as uuidv4 } from 'uuid'; +import { SavedObjectsErrorHelpers } from '@kbn/core-saved-objects-server'; +import { IKibanaResponse } from '@kbn/core-http-server'; import { getDecryptedMonitor } from '../../saved_objects/synthetics_monitor'; import { PrivateLocationAttributes } from '../../runtime_types/private_locations'; import { RouteContext, SyntheticsRestApiRouteFactory } from '../types'; @@ -14,6 +16,7 @@ import { ConfigKey, MonitorFields } from '../../../common/runtime_types'; import { SYNTHETICS_API_URLS } from '../../../common/constants'; import { normalizeSecrets } from '../../synthetics_service/utils/secrets'; import { getPrivateLocationsForMonitor } from '../monitor_cruds/add_monitor/utils'; +import { getMonitorNotFoundResponse } from './service_errors'; export const testNowMonitorRoute: SyntheticsRestApiRouteFactory = () => ({ method: 'POST', @@ -33,48 +36,56 @@ export const testNowMonitorRoute: SyntheticsRestApiRouteFactory export const triggerTestNow = async ( monitorId: string, routeContext: RouteContext -): Promise => { - const { server, spaceId, syntheticsMonitorClient, savedObjectsClient } = routeContext; +): Promise> => { + const { server, spaceId, syntheticsMonitorClient, savedObjectsClient, response } = routeContext; - const monitorWithSecrets = await getDecryptedMonitor(server, monitorId, spaceId); - const normalizedMonitor = normalizeSecrets(monitorWithSecrets); + try { + const monitorWithSecrets = await getDecryptedMonitor(server, monitorId, spaceId); + const normalizedMonitor = normalizeSecrets(monitorWithSecrets); - const { [ConfigKey.SCHEDULE]: schedule, [ConfigKey.LOCATIONS]: locations } = - monitorWithSecrets.attributes; + const { [ConfigKey.SCHEDULE]: schedule, [ConfigKey.LOCATIONS]: locations } = + monitorWithSecrets.attributes; - const privateLocations: PrivateLocationAttributes[] = await getPrivateLocationsForMonitor( - savedObjectsClient, - normalizedMonitor.attributes - ); - const testRunId = uuidv4(); + const privateLocations: PrivateLocationAttributes[] = await getPrivateLocationsForMonitor( + savedObjectsClient, + normalizedMonitor.attributes + ); + const testRunId = uuidv4(); - const [, errors] = await syntheticsMonitorClient.testNowConfigs( - { - monitor: normalizedMonitor.attributes as MonitorFields, - id: monitorId, - testRunId, - }, - savedObjectsClient, - privateLocations, - spaceId - ); + const [, errors] = await syntheticsMonitorClient.testNowConfigs( + { + monitor: normalizedMonitor.attributes as MonitorFields, + id: monitorId, + testRunId, + }, + savedObjectsClient, + privateLocations, + spaceId + ); + + if (errors && errors?.length > 0) { + return { + errors, + testRunId, + schedule, + locations, + configId: monitorId, + monitor: normalizedMonitor.attributes, + }; + } - if (errors && errors?.length > 0) { return { - errors, testRunId, schedule, locations, configId: monitorId, monitor: normalizedMonitor.attributes, }; - } + } catch (getErr) { + if (SavedObjectsErrorHelpers.isNotFoundError(getErr)) { + return getMonitorNotFoundResponse(response, monitorId); + } - return { - testRunId, - schedule, - locations, - configId: monitorId, - monitor: normalizedMonitor.attributes, - }; + throw getErr; + } }; diff --git a/x-pack/plugins/observability_solution/synthetics/server/server.ts b/x-pack/plugins/observability_solution/synthetics/server/server.ts index 77937c57590e7..9ba4341ecad30 100644 --- a/x-pack/plugins/observability_solution/synthetics/server/server.ts +++ b/x-pack/plugins/observability_solution/synthetics/server/server.ts @@ -21,7 +21,7 @@ export const initSyntheticsServer = ( ) => { const { router } = server; syntheticsAppRestApiRoutes.forEach((route) => { - const { method, options, handler, validate, path } = syntheticsRouteWrapper( + const { method, options, handler, validate, path, security } = syntheticsRouteWrapper( createSyntheticsRouteWithAuth(route), server, syntheticsMonitorClient @@ -30,6 +30,7 @@ export const initSyntheticsServer = ( const routeDefinition = { path, validate, + security, options, }; @@ -52,11 +53,8 @@ export const initSyntheticsServer = ( }); syntheticsAppPublicRestApiRoutes.forEach((route) => { - const { method, options, handler, validate, path, validation } = syntheticsRouteWrapper( - createSyntheticsRouteWithAuth(route), - server, - syntheticsMonitorClient - ); + const { method, options, handler, validate, path, validation, security } = + syntheticsRouteWrapper(createSyntheticsRouteWithAuth(route), server, syntheticsMonitorClient); const routeDefinition = { path, @@ -70,13 +68,11 @@ export const initSyntheticsServer = ( .get({ access: 'public', path: routeDefinition.path, - options: { - tags: options?.tags, - }, }) .addVersion( { version: '2023-10-31', + security, validate: validation ?? false, }, handler @@ -87,13 +83,11 @@ export const initSyntheticsServer = ( .put({ access: 'public', path: routeDefinition.path, - options: { - tags: options?.tags, - }, }) .addVersion( { version: '2023-10-31', + security, validate: validation ?? false, }, handler @@ -104,13 +98,11 @@ export const initSyntheticsServer = ( .post({ access: 'public', path: routeDefinition.path, - options: { - tags: options?.tags, - }, }) .addVersion( { version: '2023-10-31', + security, validate: validation ?? false, }, handler @@ -128,6 +120,7 @@ export const initSyntheticsServer = ( .addVersion( { version: '2023-10-31', + security, validate: validation ?? false, }, handler diff --git a/x-pack/plugins/observability_solution/synthetics/server/synthetics_route_wrapper.ts b/x-pack/plugins/observability_solution/synthetics/server/synthetics_route_wrapper.ts index a0785b4486912..7091f85ac3f43 100644 --- a/x-pack/plugins/observability_solution/synthetics/server/synthetics_route_wrapper.ts +++ b/x-pack/plugins/observability_solution/synthetics/server/synthetics_route_wrapper.ts @@ -20,9 +20,13 @@ export const syntheticsRouteWrapper: SyntheticsRouteWrapper = ( ) => ({ ...uptimeRoute, options: { - tags: ['access:uptime-read', ...(uptimeRoute?.writeAccess ? ['access:uptime-write'] : [])], ...(uptimeRoute.options ?? {}), }, + security: { + authz: { + requiredPrivileges: ['uptime-read', ...(uptimeRoute?.writeAccess ? ['uptime-write'] : [])], + }, + }, handler: async (context, request, response) => { const { elasticsearch, savedObjects, uiSettings } = await context.core; diff --git a/x-pack/plugins/observability_solution/uptime/server/legacy_uptime/routes/uptime_route_wrapper.ts b/x-pack/plugins/observability_solution/uptime/server/legacy_uptime/routes/uptime_route_wrapper.ts index 2590db8524105..58b20dba1c471 100644 --- a/x-pack/plugins/observability_solution/uptime/server/legacy_uptime/routes/uptime_route_wrapper.ts +++ b/x-pack/plugins/observability_solution/uptime/server/legacy_uptime/routes/uptime_route_wrapper.ts @@ -12,11 +12,12 @@ import { UptimeEsClient } from '../lib/lib'; export const uptimeRouteWrapper: UMKibanaRouteWrapper = (uptimeRoute, server) => ({ ...uptimeRoute, options: { - tags: [ - 'oas-tag:uptime', - 'access:uptime-read', - ...(uptimeRoute?.writeAccess ? ['access:uptime-write'] : []), - ], + tags: ['oas-tag:uptime'], + }, + security: { + authz: { + requiredPrivileges: ['uptime-read', ...(uptimeRoute?.writeAccess ? ['uptime-write'] : [])], + }, }, handler: async (context, request, response) => { const coreContext = await context.core; diff --git a/x-pack/plugins/observability_solution/uptime/server/legacy_uptime/uptime_server.ts b/x-pack/plugins/observability_solution/uptime/server/legacy_uptime/uptime_server.ts index b8a9b56c2a909..32feb62fcb0fd 100644 --- a/x-pack/plugins/observability_solution/uptime/server/legacy_uptime/uptime_server.ts +++ b/x-pack/plugins/observability_solution/uptime/server/legacy_uptime/uptime_server.ts @@ -41,7 +41,7 @@ export const initUptimeServer = ( router: UptimeRouter ) => { legacyUptimeRestApiRoutes.forEach((route) => { - const { method, options, handler, validate, path } = uptimeRouteWrapper( + const { method, options, handler, validate, path, security } = uptimeRouteWrapper( createRouteWithAuth(libs, route), server ); @@ -50,6 +50,7 @@ export const initUptimeServer = ( path, validate, options, + security, }; switch (method) { @@ -71,26 +72,20 @@ export const initUptimeServer = ( }); legacyUptimePublicRestApiRoutes.forEach((route) => { - const { method, options, handler, path, ...rest } = uptimeRouteWrapper( + const { method, options, handler, path, security, ...rest } = uptimeRouteWrapper( createRouteWithAuth(libs, route), server ); const validate = rest.validate ? getRequestValidation(rest.validate) : rest.validate; - const routeDefinition = { - path, - validate, - options, - }; - switch (method) { case 'GET': router.versioned .get({ access: 'public', description: `Get uptime settings`, - path: routeDefinition.path, + path, options: { tags: options?.tags, }, @@ -98,6 +93,7 @@ export const initUptimeServer = ( .addVersion( { version: INITIAL_REST_VERSION, + security, validate: { request: { body: validate ? validate?.body : undefined, @@ -117,7 +113,7 @@ export const initUptimeServer = ( .put({ access: 'public', description: `Update uptime settings`, - path: routeDefinition.path, + path, options: { tags: options?.tags, }, @@ -125,6 +121,7 @@ export const initUptimeServer = ( .addVersion( { version: INITIAL_REST_VERSION, + security, validate: { request: { body: validate ? validate?.body : undefined, diff --git a/x-pack/test/api_integration/apis/synthetics/add_monitor.ts b/x-pack/test/api_integration/apis/synthetics/add_monitor.ts index aee2e1e2c7212..6be47ba8d9619 100644 --- a/x-pack/test/api_integration/apis/synthetics/add_monitor.ts +++ b/x-pack/test/api_integration/apis/synthetics/add_monitor.ts @@ -304,7 +304,9 @@ export default function ({ getService }: FtrProviderContext) { .send(httpMonitorJson); expect(apiResponse.status).eql(403); - expect(apiResponse.body.message).eql('Forbidden'); + expect(apiResponse.body.message).eql( + 'API [POST /api/synthetics/monitors] is unauthorized for user, this action is granted by the Kibana privileges [uptime-write]' + ); }); }); diff --git a/x-pack/test/api_integration/apis/synthetics/index.ts b/x-pack/test/api_integration/apis/synthetics/index.ts index 15e76126e9555..27c0febf5939d 100644 --- a/x-pack/test/api_integration/apis/synthetics/index.ts +++ b/x-pack/test/api_integration/apis/synthetics/index.ts @@ -16,6 +16,7 @@ export default function ({ getService, loadTestFile }: FtrProviderContext) { await esDeleteAllIndices('synthetics*'); }); + loadTestFile(require.resolve('./synthetics_api_security')); loadTestFile(require.resolve('./edit_monitor_public_api')); loadTestFile(require.resolve('./add_monitor_public_api')); loadTestFile(require.resolve('./synthetics_enablement')); diff --git a/x-pack/test/api_integration/apis/synthetics/services/synthetics_monitor_test_service.ts b/x-pack/test/api_integration/apis/synthetics/services/synthetics_monitor_test_service.ts index 498da8c6b1800..d1dda60c8d7c8 100644 --- a/x-pack/test/api_integration/apis/synthetics/services/synthetics_monitor_test_service.ts +++ b/x-pack/test/api_integration/apis/synthetics/services/synthetics_monitor_test_service.ts @@ -175,7 +175,7 @@ export class SyntheticsMonitorTestService { } } - async addsNewSpace() { + async addsNewSpace(uptimePermissions: string[] = ['all']) { const username = 'admin'; const password = `${username}-password`; const roleName = 'uptime-role'; @@ -190,7 +190,8 @@ export class SyntheticsMonitorTestService { kibana: [ { feature: { - uptime: ['all'], + uptime: uptimePermissions, + slo: ['all'], }, spaces: ['*'], }, diff --git a/x-pack/test/api_integration/apis/synthetics/synthetics_api_security.ts b/x-pack/test/api_integration/apis/synthetics/synthetics_api_security.ts new file mode 100644 index 0000000000000..ddd0d4b209940 --- /dev/null +++ b/x-pack/test/api_integration/apis/synthetics/synthetics_api_security.ts @@ -0,0 +1,161 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { + syntheticsAppPublicRestApiRoutes, + syntheticsAppRestApiRoutes, +} from '@kbn/synthetics-plugin/server/routes'; +import expect from '@kbn/expect'; +import { SYNTHETICS_API_URLS } from '@kbn/synthetics-plugin/common/constants'; +import { FtrProviderContext } from '../../ftr_provider_context'; +import { SyntheticsMonitorTestService } from './services/synthetics_monitor_test_service'; + +export default function ({ getService }: FtrProviderContext) { + describe('SyntheticsAPISecurity', function () { + this.tags('skipCloud'); + + const supertestWithoutAuth = getService('supertestWithoutAuth'); + + const monitorTestService = new SyntheticsMonitorTestService(getService); + const kibanaServer = getService('kibanaServer'); + + const assertPermissions = async ( + method: 'GET' | 'POST' | 'PUT' | 'DELETE', + path: string, + options: { + statusCodes: number[]; + SPACE_ID: string; + username: string; + password: string; + writeAccess?: boolean; + tags?: string; + } + ) => { + let resp; + const { statusCodes, SPACE_ID, username, password, writeAccess } = options; + const tags = !writeAccess ? '[uptime-read]' : options.tags ?? '[uptime-read,uptime-write]'; + const getStatusMessage = (respStatus: string) => + `Expected ${statusCodes?.join( + ',' + )}, got ${respStatus} status code doesn't match, for path: ${path} and method ${method}`; + + const getBodyMessage = (tg?: string) => + `API [${method} ${path}] is unauthorized for user, this action is granted by the Kibana privileges ${ + tg ?? tags + }`; + + const verifyPermissionsBody = (res: any, msg: string) => { + if (res.status === 403 && !res.body.message.includes('MissingIndicesPrivileges:')) { + expect(decodeURIComponent(res.body.message)).to.eql(msg); + } + }; + + switch (method) { + case 'GET': + resp = await supertestWithoutAuth + .get(`/s/${SPACE_ID}${path}`) + .auth(username, password) + .set('kbn-xsrf', 'true') + .send({}); + expect(statusCodes.includes(resp.status)).to.eql(true, getStatusMessage(resp.status)); + verifyPermissionsBody(resp, getBodyMessage('[uptime-read]')); + break; + case 'PUT': + resp = await supertestWithoutAuth + .put(`/s/${SPACE_ID}${path}`) + .auth(username, password) + .set('kbn-xsrf', 'true') + .send({}); + expect(statusCodes.includes(resp.status)).to.eql(true, getStatusMessage(resp.status)); + verifyPermissionsBody(resp, getBodyMessage()); + break; + case 'POST': + resp = await supertestWithoutAuth + .post(`/s/${SPACE_ID}${path}`) + .auth(username, password) + .set('kbn-xsrf', 'true') + .send({}); + expect(statusCodes.includes(resp.status)).to.eql(true, getStatusMessage(resp.status)); + verifyPermissionsBody(resp, getBodyMessage()); + break; + case 'DELETE': + resp = await supertestWithoutAuth + .delete(`/s/${SPACE_ID}${path}`) + .auth(username, password) + .set('kbn-xsrf', 'true') + .send({}); + expect(statusCodes.includes(resp.status)).to.eql(true, getStatusMessage(resp.status)); + verifyPermissionsBody(resp, getBodyMessage()); + break; + } + + return resp; + }; + + before(async () => { + await kibanaServer.savedObjects.cleanStandardList(); + }); + + const allRoutes = syntheticsAppRestApiRoutes.concat(syntheticsAppPublicRestApiRoutes); + + it('throws permissions errors for un-auth user', async () => { + const { SPACE_ID, username, password } = await monitorTestService.addsNewSpace([]); + + for (const routeFn of allRoutes) { + const route = routeFn(); + await assertPermissions(route.method, route.path, { + statusCodes: [403], + SPACE_ID, + username, + password, + writeAccess: route.writeAccess ?? true, + }); + } + }); + + it('throws permissions errors for read user', async () => { + const { SPACE_ID, username, password } = await monitorTestService.addsNewSpace(['read']); + + for (const routeFn of allRoutes) { + const route = routeFn(); + if (route.writeAccess === false) { + continue; + } + await assertPermissions(route.method, route.path, { + statusCodes: [200, 403, 500, 400, 404], + SPACE_ID, + username, + password, + writeAccess: route.writeAccess ?? true, + tags: '[uptime-write]', + }); + } + }); + + it('no permissions errors for all user', async () => { + const { SPACE_ID, username, password } = await monitorTestService.addsNewSpace(['all']); + + for (const routeFn of allRoutes) { + const route = routeFn(); + if ( + (route.method === 'DELETE' && route.path === SYNTHETICS_API_URLS.SYNTHETICS_ENABLEMENT) || + SYNTHETICS_API_URLS.SYNTHETICS_PROJECT_APIKEY + ) { + continue; + } + await assertPermissions(route.method, route.path, { + statusCodes: [400, 200, 404], + SPACE_ID, + username, + password, + writeAccess: route.writeAccess ?? true, + tags: '[uptime-write]', + }); + } + }); + }); +}