From 0c12dc1c58fdd917fbf8dd693ad9ebef6ac2a8cb Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Wed, 30 Oct 2024 14:52:13 +0000 Subject: [PATCH 1/8] [Authz] Migrated unauthorized routes owned by obs-ux-management-team --- .../annotations/register_annotation_apis.ts | 36 +++++++++++++++++++ .../synthetics/server/server.ts | 24 +++++++++++++ .../server/legacy_uptime/uptime_server.ts | 12 +++++++ 3 files changed, 72 insertions(+) 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..26e4d8c7a3fde 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 is opted out from authorization', + }, + }, validate: { body: unknowns, }, @@ -110,6 +116,12 @@ export function registerAnnotationAPIs({ router.put( { path: '/api/observability/annotation/{id}', + security: { + authz: { + enabled: false, + reason: 'This route is opted out from authorization', + }, + }, validate: { body: unknowns, }, @@ -125,6 +137,12 @@ export function registerAnnotationAPIs({ router.delete( { path: '/api/observability/annotation/{id}', + security: { + authz: { + enabled: false, + reason: 'This route is opted out from authorization', + }, + }, validate: { params: unknowns, }, @@ -140,6 +158,12 @@ export function registerAnnotationAPIs({ router.get( { path: '/api/observability/annotation/{id}', + security: { + authz: { + enabled: false, + reason: 'This route is opted out from authorization', + }, + }, validate: { params: unknowns, }, @@ -155,6 +179,12 @@ export function registerAnnotationAPIs({ router.get( { path: '/api/observability/annotation/find', + security: { + authz: { + enabled: false, + reason: 'This route is opted out from authorization', + }, + }, validate: { query: unknowns, }, @@ -170,6 +200,12 @@ export function registerAnnotationAPIs({ router.get( { path: '/api/observability/annotation/permissions', + security: { + authz: { + enabled: false, + reason: 'This route is opted out from authorization', + }, + }, validate: { query: unknowns, }, diff --git a/x-pack/plugins/observability_solution/synthetics/server/server.ts b/x-pack/plugins/observability_solution/synthetics/server/server.ts index 77937c57590e7..c1bed95325e3d 100644 --- a/x-pack/plugins/observability_solution/synthetics/server/server.ts +++ b/x-pack/plugins/observability_solution/synthetics/server/server.ts @@ -77,6 +77,12 @@ export const initSyntheticsServer = ( .addVersion( { version: '2023-10-31', + security: { + authz: { + enabled: false, + reason: 'This route is opted out from authorization', + }, + }, validate: validation ?? false, }, handler @@ -94,6 +100,12 @@ export const initSyntheticsServer = ( .addVersion( { version: '2023-10-31', + security: { + authz: { + enabled: false, + reason: 'This route is opted out from authorization', + }, + }, validate: validation ?? false, }, handler @@ -111,6 +123,12 @@ export const initSyntheticsServer = ( .addVersion( { version: '2023-10-31', + security: { + authz: { + enabled: false, + reason: 'This route is opted out from authorization', + }, + }, validate: validation ?? false, }, handler @@ -128,6 +146,12 @@ export const initSyntheticsServer = ( .addVersion( { version: '2023-10-31', + security: { + authz: { + enabled: false, + reason: 'This route is opted out from authorization', + }, + }, validate: validation ?? false, }, handler 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..ce7a7ebd40d2f 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 @@ -98,6 +98,12 @@ export const initUptimeServer = ( .addVersion( { version: INITIAL_REST_VERSION, + security: { + authz: { + enabled: false, + reason: 'This route is opted out from authorization', + }, + }, validate: { request: { body: validate ? validate?.body : undefined, @@ -125,6 +131,12 @@ export const initUptimeServer = ( .addVersion( { version: INITIAL_REST_VERSION, + security: { + authz: { + enabled: false, + reason: 'This route is opted out from authorization', + }, + }, validate: { request: { body: validate ? validate?.body : undefined, From 6e92ad6cee3200bb0f9ce44c2d957dc46c5b10d3 Mon Sep 17 00:00:00 2001 From: Dominique Belcher Date: Fri, 1 Nov 2024 15:58:36 -0400 Subject: [PATCH 2/8] make write access a required param for Synthetics route schema --- .../observability_solution/synthetics/server/routes/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/observability_solution/synthetics/server/routes/types.ts b/x-pack/plugins/observability_solution/synthetics/server/routes/types.ts index fa481d16b92bf..49d965543c396 100644 --- a/x-pack/plugins/observability_solution/synthetics/server/routes/types.ts +++ b/x-pack/plugins/observability_solution/synthetics/server/routes/types.ts @@ -37,7 +37,7 @@ export type SupportedMethod = 'GET' | 'POST' | 'PUT' | 'DELETE'; */ export interface UMServerRoute { method: SupportedMethod; - writeAccess?: boolean; + writeAccess: boolean; handler: T; validation?: VersionedRouteValidation; streamHandler?: ( From 9ec831d7f2b6282c850f04aa304d21c971a5a75f Mon Sep 17 00:00:00 2001 From: Dominique Belcher Date: Mon, 4 Nov 2024 12:35:46 -0500 Subject: [PATCH 3/8] adjust synthetics routes --- .../synthetics/server/routes/types.ts | 2 +- .../synthetics/server/server.ts | 47 ++++--------------- .../server/synthetics_route_wrapper.ts | 6 ++- .../apis/synthetics/add_monitor.ts | 4 +- 4 files changed, 17 insertions(+), 42 deletions(-) diff --git a/x-pack/plugins/observability_solution/synthetics/server/routes/types.ts b/x-pack/plugins/observability_solution/synthetics/server/routes/types.ts index 49d965543c396..fa481d16b92bf 100644 --- a/x-pack/plugins/observability_solution/synthetics/server/routes/types.ts +++ b/x-pack/plugins/observability_solution/synthetics/server/routes/types.ts @@ -37,7 +37,7 @@ export type SupportedMethod = 'GET' | 'POST' | 'PUT' | 'DELETE'; */ export interface UMServerRoute { method: SupportedMethod; - writeAccess: boolean; + writeAccess?: boolean; handler: T; validation?: VersionedRouteValidation; streamHandler?: ( diff --git a/x-pack/plugins/observability_solution/synthetics/server/server.ts b/x-pack/plugins/observability_solution/synthetics/server/server.ts index c1bed95325e3d..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,19 +68,11 @@ export const initSyntheticsServer = ( .get({ access: 'public', path: routeDefinition.path, - options: { - tags: options?.tags, - }, }) .addVersion( { version: '2023-10-31', - security: { - authz: { - enabled: false, - reason: 'This route is opted out from authorization', - }, - }, + security, validate: validation ?? false, }, handler @@ -93,19 +83,11 @@ export const initSyntheticsServer = ( .put({ access: 'public', path: routeDefinition.path, - options: { - tags: options?.tags, - }, }) .addVersion( { version: '2023-10-31', - security: { - authz: { - enabled: false, - reason: 'This route is opted out from authorization', - }, - }, + security, validate: validation ?? false, }, handler @@ -116,19 +98,11 @@ export const initSyntheticsServer = ( .post({ access: 'public', path: routeDefinition.path, - options: { - tags: options?.tags, - }, }) .addVersion( { version: '2023-10-31', - security: { - authz: { - enabled: false, - reason: 'This route is opted out from authorization', - }, - }, + security, validate: validation ?? false, }, handler @@ -146,12 +120,7 @@ export const initSyntheticsServer = ( .addVersion( { version: '2023-10-31', - security: { - authz: { - enabled: false, - reason: 'This route is opted out from authorization', - }, - }, + 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/test/api_integration/apis/synthetics/add_monitor.ts b/x-pack/test/api_integration/apis/synthetics/add_monitor.ts index 01e5c175ee7d6..9e9b36fdbfa22 100644 --- a/x-pack/test/api_integration/apis/synthetics/add_monitor.ts +++ b/x-pack/test/api_integration/apis/synthetics/add_monitor.ts @@ -301,7 +301,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]' + ); }); }); From 1cb250bef0c6bed84ce65b0dd4063c7897ee4aa2 Mon Sep 17 00:00:00 2001 From: Dominique Belcher Date: Mon, 4 Nov 2024 12:59:23 -0500 Subject: [PATCH 4/8] adjust uptime security --- .../routes/uptime_route_wrapper.ts | 11 ++++--- .../server/legacy_uptime/uptime_server.ts | 33 ++++--------------- 2 files changed, 13 insertions(+), 31 deletions(-) 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 ce7a7ebd40d2f..906c5a5bada7e 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 @@ -5,8 +5,6 @@ * 2.0. */ -import { Logger } from '@kbn/core/server'; -import { IRuleDataClient } from '@kbn/rule-registry-plugin/server'; import { getRequestValidation } from '@kbn/core-http-server'; import { INITIAL_REST_VERSION } from '../../common/constants'; import { DynamicSettingsSchema } from './routes/dynamic_settings'; @@ -36,12 +34,10 @@ export type UMServerLibs = typeof libs; export const initUptimeServer = ( server: UptimeServerSetup, plugins: UptimeCorePluginsSetup, - ruleDataClient: IRuleDataClient, - logger: Logger, 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 +46,7 @@ export const initUptimeServer = ( path, validate, options, + security, }; switch (method) { @@ -71,26 +68,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,12 +89,7 @@ export const initUptimeServer = ( .addVersion( { version: INITIAL_REST_VERSION, - security: { - authz: { - enabled: false, - reason: 'This route is opted out from authorization', - }, - }, + security, validate: { request: { body: validate ? validate?.body : undefined, @@ -123,7 +109,7 @@ export const initUptimeServer = ( .put({ access: 'public', description: `Update uptime settings`, - path: routeDefinition.path, + path, options: { tags: options?.tags, }, @@ -131,12 +117,7 @@ export const initUptimeServer = ( .addVersion( { version: INITIAL_REST_VERSION, - security: { - authz: { - enabled: false, - reason: 'This route is opted out from authorization', - }, - }, + security, validate: { request: { body: validate ? validate?.body : undefined, From 42c1a5375c2310afc70c3e2aca89b83ebca71d90 Mon Sep 17 00:00:00 2001 From: Dominique Belcher Date: Mon, 4 Nov 2024 13:41:27 -0500 Subject: [PATCH 5/8] add authorization delegation notes --- .../lib/annotations/register_annotation_apis.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) 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 26e4d8c7a3fde..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 @@ -98,7 +98,7 @@ export function registerAnnotationAPIs({ security: { authz: { enabled: false, - reason: 'This route is opted out from authorization', + reason: 'This route delegates authorization to Elasticsearch', }, }, validate: { @@ -119,7 +119,7 @@ export function registerAnnotationAPIs({ security: { authz: { enabled: false, - reason: 'This route is opted out from authorization', + reason: 'This route delegates authorization to Elasticsearch', }, }, validate: { @@ -140,7 +140,7 @@ export function registerAnnotationAPIs({ security: { authz: { enabled: false, - reason: 'This route is opted out from authorization', + reason: 'This route delegates authorization to Elasticsearch', }, }, validate: { @@ -161,7 +161,7 @@ export function registerAnnotationAPIs({ security: { authz: { enabled: false, - reason: 'This route is opted out from authorization', + reason: 'This route delegates authorization to Elasticsearch', }, }, validate: { @@ -182,7 +182,7 @@ export function registerAnnotationAPIs({ security: { authz: { enabled: false, - reason: 'This route is opted out from authorization', + reason: 'This route delegates authorization to Elasticsearch', }, }, validate: { @@ -203,7 +203,7 @@ export function registerAnnotationAPIs({ security: { authz: { enabled: false, - reason: 'This route is opted out from authorization', + reason: 'This route delegates authorization to Elasticsearch', }, }, validate: { From 77045e3c3f3aff4e59be862aafdae348553fb5b3 Mon Sep 17 00:00:00 2001 From: Dominique Belcher Date: Mon, 4 Nov 2024 14:34:52 -0500 Subject: [PATCH 6/8] adjust types --- .../uptime/server/legacy_uptime/uptime_server.ts | 4 ++++ 1 file changed, 4 insertions(+) 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 906c5a5bada7e..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 @@ -5,6 +5,8 @@ * 2.0. */ +import { Logger } from '@kbn/core/server'; +import { IRuleDataClient } from '@kbn/rule-registry-plugin/server'; import { getRequestValidation } from '@kbn/core-http-server'; import { INITIAL_REST_VERSION } from '../../common/constants'; import { DynamicSettingsSchema } from './routes/dynamic_settings'; @@ -34,6 +36,8 @@ export type UMServerLibs = typeof libs; export const initUptimeServer = ( server: UptimeServerSetup, plugins: UptimeCorePluginsSetup, + ruleDataClient: IRuleDataClient, + logger: Logger, router: UptimeRouter ) => { legacyUptimeRestApiRoutes.forEach((route) => { From 66f73a1bef987de248d4806afb6266914f317032 Mon Sep 17 00:00:00 2001 From: shahzad31 Date: Tue, 12 Nov 2024 12:52:10 +0100 Subject: [PATCH 7/8] add api security control tests --- .../synthetics_service/run_once_monitor.ts | 4 + .../synthetics_service/test_now_monitor.ts | 70 ++++---- .../api_integration/apis/synthetics/index.ts | 1 + .../synthetics_monitor_test_service.ts | 5 +- .../synthetics/synthetics_api_security.ts | 162 ++++++++++++++++++ 5 files changed, 210 insertions(+), 32 deletions(-) create mode 100644 x-pack/test/api_integration/apis/synthetics/synthetics_api_security.ts 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..a446f6cf5c0c7 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,7 @@ */ import { schema } from '@kbn/config-schema'; import { v4 as uuidv4 } from 'uuid'; +import { SavedObjectsErrorHelpers } from '@kbn/core-saved-objects-server'; import { getDecryptedMonitor } from '../../saved_objects/synthetics_monitor'; import { PrivateLocationAttributes } from '../../runtime_types/private_locations'; import { RouteContext, SyntheticsRestApiRouteFactory } from '../types'; @@ -14,6 +15,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', @@ -34,47 +36,55 @@ export const triggerTestNow = async ( monitorId: string, routeContext: RouteContext ): Promise => { - const { server, spaceId, syntheticsMonitorClient, savedObjectsClient } = routeContext; + 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/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..49f21b9df5680 --- /dev/null +++ b/x-pack/test/api_integration/apis/synthetics/synthetics_api_security.ts @@ -0,0 +1,162 @@ +/* + * 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 supertest = getService('supertest'); + 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]', + }); + } + }); + }); +} From f291ac60a73b931df1e639ffdbb0ef64d6b9a000 Mon Sep 17 00:00:00 2001 From: shahzad31 Date: Tue, 12 Nov 2024 13:25:31 +0100 Subject: [PATCH 8/8] fix types --- .../server/routes/synthetics_service/test_now_monitor.ts | 3 ++- .../api_integration/apis/synthetics/synthetics_api_security.ts | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) 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 a446f6cf5c0c7..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 @@ -7,6 +7,7 @@ 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'; @@ -35,7 +36,7 @@ export const testNowMonitorRoute: SyntheticsRestApiRouteFactory export const triggerTestNow = async ( monitorId: string, routeContext: RouteContext -): Promise => { +): Promise> => { const { server, spaceId, syntheticsMonitorClient, savedObjectsClient, response } = routeContext; try { 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 index 49f21b9df5680..ddd0d4b209940 100644 --- a/x-pack/test/api_integration/apis/synthetics/synthetics_api_security.ts +++ b/x-pack/test/api_integration/apis/synthetics/synthetics_api_security.ts @@ -18,7 +18,6 @@ export default function ({ getService }: FtrProviderContext) { describe('SyntheticsAPISecurity', function () { this.tags('skipCloud'); - const supertest = getService('supertest'); const supertestWithoutAuth = getService('supertestWithoutAuth'); const monitorTestService = new SyntheticsMonitorTestService(getService);