Unauthorized route migration for routes owned by obs-ux-management-te…

…am (elastic#198374) ### Authz API migration for unauthorized routes This PR migrates unauthorized routes owned by your team to a new security configuration. Please refer to the documentation for more information: [Authorization API](https://docs.elastic.dev/kibana-dev-docs/key-concepts/security-api-authorization) ### **Before migration:** ```ts router.get({ path: '/api/path', ... }, handler); ``` ### **After migration:** ```ts router.get({ path: '/api/path', security: { authz: { enabled: false, reason: 'This route is opted out from authorization because ...', }, }, ... }, handler); ``` ### What to do next? 1. Review the changes in this PR. 2. Elaborate on the reasoning to opt-out of authorization. 3. Routes without a compelling reason to opt-out of authorization should plan to introduce them as soon as possible. 2. You might need to update your tests to reflect the new security configuration: - If you have snapshot tests that include the route definition. ## Any questions? If you have any questions or need help with API authorization, please reach out to the `@elastic/kibana-security` team. --------- Co-authored-by: Dominique Belcher <[email protected]> Co-authored-by: Shahzad <[email protected]> (cherry picked from commit 0b193ec)
kibanamachine · Nov 21, 2024 · 9978abd · 9978abd
1 parent b42ac29
commit 9978abd
Show file tree

Hide file tree

Showing 11 changed files with 276 additions and 65 deletions.
diff --git a/...s/observability_solution/observability/server/lib/annotations/register_annotation_apis.ts b/...s/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/...ns/observability_solution/synthetics/server/routes/synthetics_service/run_once_monitor.ts b/...ns/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<any> => {
     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/...ns/observability_solution/synthetics/server/routes/synthetics_service/test_now_monitor.ts b/...ns/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<TestNowResponse> = () => ({
   method: 'POST',
@@ -33,48 +36,56 @@ export const testNowMonitorRoute: SyntheticsRestApiRouteFactory<TestNowResponse>
 export const triggerTestNow = async (
   monitorId: string,
   routeContext: RouteContext
-): Promise<TestNowResponse> => {
-  const { server, spaceId, syntheticsMonitorClient, savedObjectsClient } = routeContext;
+): Promise<TestNowResponse | IKibanaResponse<any>> => {
+  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
@@ -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
@@ -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/...plugins/observability_solution/uptime/server/legacy_uptime/routes/uptime_route_wrapper.ts b/...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
@@ -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,33 +72,28 @@ 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,
             },
           })
           .addVersion(
             {
               version: INITIAL_REST_VERSION,
+              security,
               validate: {
                 request: {
                   body: validate ? validate?.body : undefined,
@@ -117,14 +113,15 @@ export const initUptimeServer = (
           .put({
             access: 'public',
             description: `Update uptime settings`,
-            path: routeDefinition.path,
+            path,
             options: {
               tags: options?.tags,
             },
           })
           .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
@@ -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]'
+          );
         });
     });