[HTTP] First pass of making Kibana work with internal restrictions en…

…forced (#162258) ## Summary When turning on `server.restrictInternalApis` a number of issues surfaced due to defaulting to internal resulting in `400`s for: * HTTP resources * Static assets via `registerStaticDir` * Use of `res.render(Html|Js|Css)` outside of HTTP resources This PR: * defaults our HTTP resources service to register routes by default `public`, same for static dirs. * Did an audit of all renderX usages, if outside of HTTP resources I added an explicit `access: public` * ...what else? ### Set `access: 'public'` for known set of "system" routes Method | Path | Comment -- | -- | -- GET | /api/status GET | /api/stats GET | /translations/{locale}.json GET | /api/fleet/agent_policies GET | /api/task_manager/_background_task_utilization GET | /internal/task_manager/_background_task_utilization GET | /internal/detection_engine/health/_cluster POST | /internal/detection_engine/health/_cluster GET | /internal/detection_engine/health/_space POST | /internal/detection_engine/health/_space POST | /internal/detection_engine/health/_rule POST | /internal/detection_engine/health/_setup GET | /bootstrap.js GET | /bootstrap-anonymous.js GET | \*\*/bundles/\* | Core's routes for serving JS & CSS bundles ## How to test Run this PR with `kibana.dev.yml` containing `server.restrictInternalApis: true` and navigate around Kibana UI checking that there are no `400`s in the network resources tab due to access restrictions. ## Notes * Either left a comment about why `access` was set public or a simple unit test to check that we are setting access for a given route ## To do - [x] Manually test Kibana - [x] Manually test with `interactiveSetup` plugin - [ ] Add integration and e2e test (will do in a follow up PR) Related: #162149 --------- Co-authored-by: kibanamachine <[email protected]>
elastic · Jul 26, 2023 · 32b5903 · 32b5903
1 parent ad542d0
commit 32b5903
Show file tree

Hide file tree

Showing 24 changed files with 152 additions and 19 deletions.
diff --git a/packages/core/apps/core-apps-server-internal/src/bundle_routes/bundle_route.test.ts b/packages/core/apps/core-apps-server-internal/src/bundle_routes/bundle_route.test.ts
@@ -42,6 +42,7 @@ describe('registerRouteForBundle', () => {
       {
         path: '/route-path/{path*}',
         options: {
+          access: 'public',
           authRequired: false,
         },
         validate: expect.any(Object),

diff --git a/packages/core/apps/core-apps-server-internal/src/bundle_routes/bundles_route.ts b/packages/core/apps/core-apps-server-internal/src/bundle_routes/bundles_route.ts
@@ -32,6 +32,7 @@ export function registerRouteForBundle(
       path: `${routePath}{path*}`,
       options: {
         authRequired: false,
+        access: 'public',
       },
       validate: {
         params: schema.object({

diff --git a/packages/core/apps/core-apps-server-internal/src/core_app.ts b/packages/core/apps/core-apps-server-internal/src/core_app.ts
@@ -93,18 +93,21 @@ export class CoreAppsService {
     const router = httpSetup.createRouter<InternalCoreAppsServiceRequestHandlerContext>('');
     const resources = coreSetup.httpResources.createRegistrar(router);
 
-    router.get({ path: '/', validate: false }, async (context, req, res) => {
-      const { uiSettings } = await context.core;
-      const defaultRoute = await uiSettings.client.get<string>('defaultRoute');
-      const basePath = httpSetup.basePath.get(req);
-      const url = `${basePath}${defaultRoute}`;
-
-      return res.redirected({
-        headers: {
-          location: url,
-        },
-      });
-    });
+    router.get(
+      { path: '/', validate: false, options: { access: 'public' } },
+      async (context, req, res) => {
+        const { uiSettings } = await context.core;
+        const defaultRoute = await uiSettings.client.get<string>('defaultRoute');
+        const basePath = httpSetup.basePath.get(req);
+        const url = `${basePath}${defaultRoute}`;
+
+        return res.redirected({
+          headers: {
+            location: url,
+          },
+        });
+      }
+    );
 
     this.registerCommonDefaultRoutes({
       basePath: coreSetup.http.basePath,

diff --git a/packages/core/http/core-http-resources-server-internal/src/http_resources_service.test.ts b/packages/core/http/core-http-resources-server-internal/src/http_resources_service.test.ts
@@ -62,6 +62,20 @@ describe('HttpResources service', () => {
           register = await initializer();
         });
 
+        it('registration defaults to "public" access', () => {
+          register(routeConfig, async (ctx, req, res) => res.ok());
+          const [[registeredRouteConfig]] = router.get.mock.calls;
+          expect(registeredRouteConfig.options?.access).toBe('public');
+        });
+
+        it('registration can set access to "internal"', () => {
+          register({ ...routeConfig, options: { access: 'internal' } }, async (ctx, req, res) =>
+            res.ok()
+          );
+          const [[registeredRouteConfig]] = router.get.mock.calls;
+          expect(registeredRouteConfig.options?.access).toBe('internal');
+        });
+
         describe('renderCoreApp', () => {
           it('formats successful response', async () => {
             register(routeConfig, async (ctx, req, res) => {

diff --git a/packages/core/http/core-http-resources-server-internal/src/http_resources_service.ts b/packages/core/http/core-http-resources-server-internal/src/http_resources_service.ts
@@ -85,12 +85,21 @@ export class HttpResourcesService implements CoreService<InternalHttpResourcesSe
         route: RouteConfig<P, Q, B, 'get'>,
         handler: HttpResourcesRequestHandler<P, Q, B, Context>
       ) => {
-        return router.get<P, Q, B>(route, (context, request, response) => {
-          return handler(context as Context, request, {
-            ...response,
-            ...this.createResponseToolkit(deps, context, request, response),
-          });
-        });
+        return router.get<P, Q, B>(
+          {
+            ...route,
+            options: {
+              access: 'public',
+              ...route.options,
+            },
+          },
+          (context, request, response) => {
+            return handler(context as Context, request, {
+              ...response,
+              ...this.createResponseToolkit(deps, context, request, response),
+            });
+          }
+        );
       },
     };
   }

diff --git a/packages/core/http/core-http-server-internal/src/http_server.test.ts b/packages/core/http/core-http-server-internal/src/http_server.test.ts
@@ -6,6 +6,14 @@
  * Side Public License, v 1.
  */
 
+jest.mock('@kbn/server-http-tools', () => {
+  const module = jest.requireActual('@kbn/server-http-tools');
+  return {
+    ...module,
+    createServer: jest.fn(module.createServer),
+  };
+});
+
 import { Server } from 'http';
 import { rm, mkdtemp, readFile, writeFile } from 'fs/promises';
 import supertest from 'supertest';
@@ -23,6 +31,7 @@ import type {
   RequestHandlerContextBase,
 } from '@kbn/core-http-server';
 import { Router, type RouterOptions } from '@kbn/core-http-router-server-internal';
+import { createServer } from '@kbn/server-http-tools';
 import { HttpConfig } from './http_config';
 import { HttpServer } from './http_server';
 import { Readable } from 'stream';
@@ -1506,6 +1515,29 @@ describe('setup contract', () => {
       }
     });
 
+    test('registers routes with expected options', async () => {
+      const { registerStaticDir } = await server.setup(config);
+      expect(createServer).toHaveBeenCalledTimes(1);
+      const [{ value: myServer }] = (createServer as jest.Mock).mock.results;
+      jest.spyOn(myServer, 'route');
+      expect(myServer.route).toHaveBeenCalledTimes(0);
+      registerStaticDir('/static/{path*}', assetFolder);
+      expect(myServer.route).toHaveBeenCalledTimes(1);
+      expect(myServer.route).toHaveBeenNthCalledWith(
+        1,
+        expect.objectContaining({
+          options: {
+            app: { access: 'public' },
+            auth: false,
+            cache: {
+              privacy: 'public',
+              otherwise: 'must-revalidate',
+            },
+          },
+        })
+      );
+    });
+
     test('does not throw if called after stop', async () => {
       const { registerStaticDir } = await server.setup(config);
       await server.stop();

diff --git a/packages/core/http/core-http-server-internal/src/http_server.ts b/packages/core/http/core-http-server-internal/src/http_server.ts
@@ -587,6 +587,7 @@ export class HttpServer {
         },
       },
       options: {
+        app: { access: 'public' },
         auth: false,
         cache: {
           privacy: 'public',

diff --git a/packages/core/i18n/core-i18n-server-internal/src/routes/translations.test.ts b/packages/core/i18n/core-i18n-server-internal/src/routes/translations.test.ts
@@ -0,0 +1,23 @@
+/*
+ * 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 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import { mockRouter } from '@kbn/core-http-router-server-mocks';
+import { registerTranslationsRoute } from './translations';
+
+describe('registerTranslationsRoute', () => {
+  test('registers route with expected options', () => {
+    const router = mockRouter.create();
+    registerTranslationsRoute(router, 'en');
+    expect(router.get).toHaveBeenCalledTimes(1);
+    expect(router.get).toHaveBeenNthCalledWith(
+      1,
+      expect.objectContaining({ options: { access: 'public', authRequired: false } }),
+      expect.any(Function)
+    );
+  });
+});
diff --git a/packages/core/i18n/core-i18n-server-internal/src/routes/translations.ts b/packages/core/i18n/core-i18n-server-internal/src/routes/translations.ts
@@ -28,6 +28,7 @@ export const registerTranslationsRoute = (router: IRouter, locale: string) => {
         }),
       },
       options: {
+        access: 'public',
         authRequired: false,
       },
     },

diff --git a/packages/core/i18n/core-i18n-server-internal/tsconfig.json b/packages/core/i18n/core-i18n-server-internal/tsconfig.json
@@ -25,6 +25,7 @@
     "@kbn/i18n",
     "@kbn/std",
     "@kbn/repo-packages",
+    "@kbn/core-http-router-server-mocks",
   ],
   "exclude": [
     "target/**/*",

diff --git a/...e/rendering/core-rendering-server-internal/src/bootstrap/register_bootstrap_route.test.ts b/...e/rendering/core-rendering-server-internal/src/bootstrap/register_bootstrap_route.test.ts
@@ -0,0 +1,31 @@
+/*
+ * 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 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import { registerBootstrapRoute } from './register_bootstrap_route';
+import { mockRouter } from '@kbn/core-http-router-server-mocks';
+
+describe('registerBootstrapRoute', () => {
+  test('register with expected options', () => {
+    const router = mockRouter.create();
+    const renderer = jest.fn();
+    registerBootstrapRoute({ router, renderer });
+    expect(router.get).toHaveBeenCalledTimes(2);
+    expect(router.get).toHaveBeenNthCalledWith(
+      1,
+      expect.objectContaining({ options: { access: 'public', tags: ['api'] } }),
+      expect.any(Function)
+    );
+    expect(router.get).toHaveBeenNthCalledWith(
+      2,
+      expect.objectContaining({
+        options: { access: 'public', tags: ['api'], authRequired: 'optional' },
+      }),
+      expect.any(Function)
+    );
+  });
+});
diff --git a/...s/core/rendering/core-rendering-server-internal/src/bootstrap/register_bootstrap_route.ts b/...s/core/rendering/core-rendering-server-internal/src/bootstrap/register_bootstrap_route.ts
@@ -21,6 +21,7 @@ export const registerBootstrapRoute = ({
       path: '/bootstrap.js',
       options: {
         tags: ['api'],
+        access: 'public',
       },
       validate: false,
     },
@@ -44,6 +45,7 @@ export const registerBootstrapRoute = ({
       options: {
         authRequired: 'optional',
         tags: ['api'],
+        access: 'public',
       },
       validate: false,
     },

diff --git a/packages/core/status/core-status-server-internal/src/routes/status.ts b/packages/core/status/core-status-server-internal/src/routes/status.ts
@@ -83,6 +83,7 @@ export const registerStatusRoute = ({
       options: {
         authRequired: 'optional',
         tags: ['api'], // ensures that unauthenticated calls receive a 401 rather than a 302 redirect to login page
+        access: 'public', // needs to be public to allow access from "system" users like k8s readiness probes.
       },
       validate: {
         query: schema.object(

diff --git a/packages/core/status/core-status-server-internal/src/routes/status_preboot.ts b/packages/core/status/core-status-server-internal/src/routes/status_preboot.ts
@@ -17,6 +17,7 @@ export const registerPrebootStatusRoute = ({ router }: { router: IRouter }) => {
       options: {
         authRequired: false,
         tags: ['api'],
+        access: 'public', // needs to be public to allow access from "system" users like k8s readiness probes.
       },
       validate: false,
     },

diff --git a/packages/core/status/core-status-server-internal/src/status_service.test.ts b/packages/core/status/core-status-server-internal/src/status_service.test.ts
@@ -82,7 +82,7 @@ describe('StatusService', () => {
       expect(prebootRouterMock.get).toHaveBeenCalledWith(
         {
           path: '/api/status',
-          options: { authRequired: false, tags: ['api'] },
+          options: { authRequired: false, tags: ['api'], access: 'public' },
           validate: false,
         },
         expect.any(Function)

diff --git a/src/plugins/home/public/application/load_tutorials.js b/src/plugins/home/public/application/load_tutorials.js
@@ -9,12 +9,14 @@
 import _ from 'lodash';
 import { getServices } from './kibana_services';
 import { i18n } from '@kbn/i18n';
+import { X_ELASTIC_INTERNAL_ORIGIN_REQUEST } from '@kbn/core-http-common';
 
 const baseUrl = getServices().addBasePath('/api/kibana/home/tutorials');
 const headers = new Headers();
 headers.append('Accept', 'application/json');
 headers.append('Content-Type', 'application/json');
 headers.append('kbn-xsrf', 'kibana');
+headers.append(X_ELASTIC_INTERNAL_ORIGIN_REQUEST, 'kibana');
 
 let tutorials = [];
 let tutorialsLoaded = false;

diff --git a/src/plugins/home/tsconfig.json b/src/plugins/home/tsconfig.json
@@ -32,6 +32,7 @@
     "@kbn/storybook",
     "@kbn/cloud-chat-provider-plugin",
     "@kbn/shared-ux-router",
+    "@kbn/core-http-common",
   ],
   "exclude": [
     "target/**/*",

diff --git a/src/plugins/usage_collection/server/routes/stats/stats.ts b/src/plugins/usage_collection/server/routes/stats/stats.ts
@@ -56,6 +56,7 @@ export function registerStatsRoute({
       options: {
         authRequired: !config.allowAnonymous,
         tags: ['api'], // ensures that unauthenticated calls receive a 401 rather than a 302 redirect to login page
+        access: 'public', // needs to be public to allow access from "system" users like metricbeat.
       },
       validate: {
         query: schema.object({

diff --git a/x-pack/plugins/fleet/server/routes/agent_policy/index.ts b/x-pack/plugins/fleet/server/routes/agent_policy/index.ts
@@ -42,6 +42,7 @@ export const registerRoutes = (router: FleetAuthzRouter) => {
     {
       path: AGENT_POLICY_API_ROUTES.LIST_PATTERN,
       validate: GetAgentPoliciesRequestSchema,
+      options: { access: 'public' },
       fleetAuthz: {
         fleet: { readAgentPolicies: true },
       },

diff --git a/...ule_monitoring/api/detection_engine_health/get_cluster_health/get_cluster_health_route.ts b/...ule_monitoring/api/detection_engine_health/get_cluster_health/get_cluster_health_route.ts
@@ -38,6 +38,7 @@ export const getClusterHealthRoute = (router: SecuritySolutionPluginRouter) => {
       validate: {},
       options: {
         tags: ['access:securitySolution'],
+        access: 'public', // must be public to enable "system" users to collect data
       },
     },
     async (context, request, response) => {
@@ -61,6 +62,7 @@ export const getClusterHealthRoute = (router: SecuritySolutionPluginRouter) => {
       },
       options: {
         tags: ['access:securitySolution'],
+        access: 'public', // must be public to enable "system" users to collect data
       },
     },
     async (context, request, response) => {

diff --git a/...gine/rule_monitoring/api/detection_engine_health/get_rule_health/get_rule_health_route.ts b/...gine/rule_monitoring/api/detection_engine_health/get_rule_health/get_rule_health_route.ts
@@ -36,6 +36,7 @@ export const getRuleHealthRoute = (router: SecuritySolutionPluginRouter) => {
       },
       options: {
         tags: ['access:securitySolution'],
+        access: 'public', // must be public to enable "system" users to collect data
       },
     },
     async (context, request, response) => {

diff --git a/...ne/rule_monitoring/api/detection_engine_health/get_space_health/get_space_health_route.ts b/...ne/rule_monitoring/api/detection_engine_health/get_space_health/get_space_health_route.ts
@@ -38,6 +38,7 @@ export const getSpaceHealthRoute = (router: SecuritySolutionPluginRouter) => {
       validate: {},
       options: {
         tags: ['access:securitySolution'],
+        access: 'public', // must be public to enable "system" users to collect data
       },
     },
     async (context, request, response) => {
@@ -61,6 +62,7 @@ export const getSpaceHealthRoute = (router: SecuritySolutionPluginRouter) => {
       },
       options: {
         tags: ['access:securitySolution'],
+        access: 'public', // must be public to enable "system" users to collect data
       },
     },
     async (context, request, response) => {

diff --git a/.../detection_engine/rule_monitoring/api/detection_engine_health/setup/setup_health_route.ts b/.../detection_engine/rule_monitoring/api/detection_engine_health/setup/setup_health_route.ts
@@ -23,6 +23,7 @@ export const setupHealthRoute = (router: SecuritySolutionPluginRouter) => {
       validate: {},
       options: {
         tags: ['access:securitySolution'],
+        access: 'public', // must be public to enable "system" users to collect data
       },
     },
     async (context, request, response) => {

diff --git a/x-pack/plugins/task_manager/server/routes/background_task_utilization.ts b/x-pack/plugins/task_manager/server/routes/background_task_utilization.ts
@@ -115,6 +115,7 @@ export function backgroundTaskUtilizationRoute(
         // options: { tags: ['access:taskManager'] },
         validate: false,
         options: {
+          access: 'public', // access must be public to allow "system" users, like metrics collectors, to access these routes
           authRequired: routeOption.isAuthenticated ?? true,
         },
       },
-Original file line number
+Diff line change
@@ Expand Up @@
             }),
           },
           options: {
+            access: 'public',
             authRequired: false,
           },
         },
@@ Expand Down @@