[8.x] [Core] log on external requests to internal routes (#195696) (#195974)

# Backport This will backport the following commits from `main` to `8.x`: - [[Core] log on external requests to internal routes (#195696)](https://github.com/elastic/kibana/pull/195696)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Christiane (Tina) Heiligers <christiane.heiligers@elastic.co>
2025-04-23 17:28:26 -04:00 · 2024-10-12 06:57:52 +11:00 · 2024-10-12 06:57:52 +11:00 · bf56b4ea3a
commit bf56b4ea3a
parent 7d25b26536
5 changed files with 164 additions and 110 deletions
--- a/packages/core/http/core-http-server-internal/src/lifecycle_handlers.test.ts
+++ b/packages/core/http/core-http-server-internal/src/lifecycle_handlers.test.ts
@ -19,6 +19,7 @@ import type {
 } from '@kbn/core-http-server';
 import { mockRouter } from '@kbn/core-http-router-server-mocks';
 import {
+  INTERNAL_API_RESTRICTED_LOGGER_NAME,
  createBuildNrMismatchLoggerPreResponseHandler,
  createCustomHeadersPreResponseHandler,
  createRestrictInternalRoutesPostAuthHandler,
@ -267,11 +268,19 @@ describe('versionCheck post-auth handler', () => {
 describe('restrictInternal post-auth handler', () => {
  let toolkit: ToolkitMock;
  let responseFactory: ReturnType<typeof mockRouter.createResponseFactory>;
+  let logger: jest.Mocked<Logger>;
+  let config: HttpConfig;

  beforeEach(() => {
    toolkit = createToolkit();
    responseFactory = mockRouter.createResponseFactory();
+    logger = loggerMock.create();
+    config = createConfig({
+      name: 'my-server-name',
+      restrictInternalApis: true,
+    });
  });
+
  const createForgeRequest = (
    access: 'internal' | 'public',
    headers: Record<string, string> | undefined = {},
@ -298,92 +307,115 @@ describe('restrictInternal post-auth handler', () => {
    expect(result).toBe('next');
  };

-  describe('when restriction is enabled', () => {
-    const config = createConfig({
-      name: 'my-server-name',
-      restrictInternalApis: true,
-    });
-
-    it('returns a bad request if called without internal origin header for internal API', () => {
-      const handler = createRestrictInternalRoutesPostAuthHandler(config as HttpConfig);
-      const request = createForgeRequest('internal');
-
-      responseFactory.badRequest.mockReturnValue('badRequest' as any);
-
-      const result = handler(request, responseFactory, toolkit);
-
-      expect(toolkit.next).not.toHaveBeenCalled();
-      expect(responseFactory.badRequest.mock.calls[0][0]?.body).toMatchInlineSnapshot(
-        `"uri [/internal/some-path] with method [get] exists but is not available with the current configuration"`
-      );
-      expect(result).toBe('badRequest');
-    });
-
-    it('forward the request to the next interceptor if called with internal origin header for internal API', () => {
-      const handler = createRestrictInternalRoutesPostAuthHandler(config as HttpConfig);
-      const request = createForgeRequest('internal', { 'x-elastic-internal-origin': 'Kibana' });
-      createForwardSuccess(handler, request);
-    });
-
-    it('forward the request to the next interceptor if called with internal origin header for public APIs', () => {
-      const handler = createRestrictInternalRoutesPostAuthHandler(config as HttpConfig);
-      const request = createForgeRequest('public', { 'x-elastic-internal-origin': 'Kibana' });
-      createForwardSuccess(handler, request);
-    });
-
-    it('forward the request to the next interceptor if called without internal origin header for public APIs', () => {
-      const handler = createRestrictInternalRoutesPostAuthHandler(config as HttpConfig);
-      const request = createForgeRequest('public');
-      createForwardSuccess(handler, request);
-    });
-
-    it('forward the request to the next interceptor if called with internal origin query param for internal API', () => {
-      const handler = createRestrictInternalRoutesPostAuthHandler(config as HttpConfig);
-      const request = createForgeRequest('internal', undefined, { elasticInternalOrigin: 'true' });
-      createForwardSuccess(handler, request);
-    });
-
-    it('forward the request to the next interceptor if called with internal origin query param for public APIs', () => {
-      const handler = createRestrictInternalRoutesPostAuthHandler(config as HttpConfig);
-      const request = createForgeRequest('internal', undefined, { elasticInternalOrigin: 'true' });
-      createForwardSuccess(handler, request);
-    });
-
-    it('forward the request to the next interceptor if called without internal origin query param for public APIs', () => {
-      const handler = createRestrictInternalRoutesPostAuthHandler(config as HttpConfig);
-      const request = createForgeRequest('public');
-      createForwardSuccess(handler, request);
-    });
+  it('injects a logger prefix', () => {
+    createRestrictInternalRoutesPostAuthHandler(config, logger);
+    expect(logger.get).toHaveBeenCalledTimes(1);
+    expect(logger.get).toHaveBeenCalledWith(`server`, INTERNAL_API_RESTRICTED_LOGGER_NAME);
  });

-  describe('when restriction is not enabled', () => {
-    const config = createConfig({
-      name: 'my-server-name',
-      restrictInternalApis: false,
-    });
-    it('forward the request to the next interceptor if called without internal origin header for internal APIs', () => {
-      const handler = createRestrictInternalRoutesPostAuthHandler(config as HttpConfig);
-      const request = createForgeRequest('internal');
-      createForwardSuccess(handler, request);
-    });
+  it('when enabled, does not log deprecation warning for internal API access restriction', () => {
+    createRestrictInternalRoutesPostAuthHandler(config, logger);
+    expect(logger.warn).not.toHaveBeenCalled();
+  });

-    it('forward the request to the next interceptor if called with internal origin header for internal API', () => {
-      const handler = createRestrictInternalRoutesPostAuthHandler(config as HttpConfig);
-      const request = createForgeRequest('internal', { 'x-elastic-internal-origin': 'Kibana' });
-      createForwardSuccess(handler, request);
-    });
+  it('when enabled, returns a bad request if called without internal origin header for internal API', () => {
+    const handler = createRestrictInternalRoutesPostAuthHandler(config, logger);
+    const request = createForgeRequest('internal');

-    it('forward the request to the next interceptor if called without internal origin header for public APIs', () => {
-      const handler = createRestrictInternalRoutesPostAuthHandler(config as HttpConfig);
-      const request = createForgeRequest('public');
-      createForwardSuccess(handler, request);
-    });
+    responseFactory.badRequest.mockReturnValue('badRequest' as any);

-    it('forward the request to the next interceptor if called with internal origin header for public APIs', () => {
-      const handler = createRestrictInternalRoutesPostAuthHandler(config as HttpConfig);
-      const request = createForgeRequest('public', { 'x-elastic-internal-origin': 'Kibana' });
-      createForwardSuccess(handler, request);
-    });
+    const result = handler(request, responseFactory, toolkit);
+
+    expect(toolkit.next).not.toHaveBeenCalled();
+    expect(responseFactory.badRequest.mock.calls[0][0]?.body).toMatchInlineSnapshot(
+      `"uri [/internal/some-path] with method [get] exists but is not available with the current configuration"`
+    );
+    expect(result).toBe('badRequest');
+  });
+
+  it('when enabled, forward the request to the next interceptor if called with internal origin header for internal API', () => {
+    const handler = createRestrictInternalRoutesPostAuthHandler(config, logger);
+    const request = createForgeRequest('internal', { 'x-elastic-internal-origin': 'Kibana' });
+    createForwardSuccess(handler, request);
+  });
+
+  it('when enabled, forward the request to the next interceptor if called with internal origin header for public APIs', () => {
+    const handler = createRestrictInternalRoutesPostAuthHandler(config, logger);
+    const request = createForgeRequest('public', { 'x-elastic-internal-origin': 'Kibana' });
+    createForwardSuccess(handler, request);
+  });
+
+  it('when enabled, forward the request to the next interceptor if called without internal origin header for public APIs', () => {
+    const handler = createRestrictInternalRoutesPostAuthHandler(config, logger);
+    const request = createForgeRequest('public');
+    createForwardSuccess(handler, request);
+  });
+
+  it('when enabled, forward the request to the next interceptor if called with internal origin query param for internal API', () => {
+    const handler = createRestrictInternalRoutesPostAuthHandler(config, logger);
+    const request = createForgeRequest('internal', undefined, { elasticInternalOrigin: 'true' });
+    createForwardSuccess(handler, request);
+  });
+
+  it('when enabled, forward the request to the next interceptor if called with internal origin query param for public APIs', () => {
+    const handler = createRestrictInternalRoutesPostAuthHandler(config, logger);
+    const request = createForgeRequest('internal', undefined, { elasticInternalOrigin: 'true' });
+    createForwardSuccess(handler, request);
+  });
+
+  it('when enabled, forward the request to the next interceptor if called without internal origin query param for public APIs', () => {
+    const handler = createRestrictInternalRoutesPostAuthHandler(config, logger);
+    const request = createForgeRequest('public');
+    createForwardSuccess(handler, request);
+  });
+
+  it('when not enabled, logs deprecation warning for internal API access restriction', () => {
+    const handler = createRestrictInternalRoutesPostAuthHandler(
+      { ...config, restrictInternalApis: false },
+      logger
+    );
+    const request = createForgeRequest('internal');
+    createForwardSuccess(handler, request);
+    expect(logger.warn).toHaveBeenCalledTimes(1);
+    expect(logger.warn).toHaveBeenCalledWith(
+      `Access to uri [/internal/some-path] with method [get] is deprecated`
+    );
+  });
+
+  it('when not enabled, forward the request to the next interceptor if called without internal origin header for internal APIs', () => {
+    const handler = createRestrictInternalRoutesPostAuthHandler(
+      { ...config, restrictInternalApis: false },
+      logger
+    );
+    const request = createForgeRequest('internal');
+    createForwardSuccess(handler, request);
+  });
+
+  it('when not enabled, forward the request to the next interceptor if called with internal origin header for internal API', () => {
+    const handler = createRestrictInternalRoutesPostAuthHandler(
+      { ...config, restrictInternalApis: false },
+      logger
+    );
+    const request = createForgeRequest('internal', { 'x-elastic-internal-origin': 'Kibana' });
+    createForwardSuccess(handler, request);
+  });
+
+  it('when not enabled, forward the request to the next interceptor if called without internal origin header for public APIs', () => {
+    const handler = createRestrictInternalRoutesPostAuthHandler(
+      { ...config, restrictInternalApis: false },
+      logger
+    );
+    const request = createForgeRequest('public');
+    createForwardSuccess(handler, request);
+  });
+
+  it('when not enabled, forward the request to the next interceptor if called with internal origin header for public APIs', () => {
+    const handler = createRestrictInternalRoutesPostAuthHandler(
+      { ...config, restrictInternalApis: false },
+      logger
+    );
+    const request = createForgeRequest('public', { 'x-elastic-internal-origin': 'Kibana' });
+    createForwardSuccess(handler, request);
  });
 });

--- a/packages/core/http/core-http-server-internal/src/lifecycle_handlers.ts
+++ b/packages/core/http/core-http-server-internal/src/lifecycle_handlers.ts
@ -45,18 +45,35 @@ export const createXsrfPostAuthHandler = (config: HttpConfig): OnPostAuthHandler
  };
 };

+/**
+ * This should remain part of the logger prefix so that we can notify/track
+ * when we see this logged for observability purposes.
+ */
+export const INTERNAL_API_RESTRICTED_LOGGER_NAME = 'kbn-internal-api-restricted';
 export const createRestrictInternalRoutesPostAuthHandler = (
-  config: HttpConfig
+  config: HttpConfig,
+  log: Logger
 ): OnPostAuthHandler => {
  const isRestrictionEnabled = config.restrictInternalApis;
+  log = log.get('server', `${INTERNAL_API_RESTRICTED_LOGGER_NAME}`);

  return (request, response, toolkit) => {
    const isInternalRoute = request.route.options.access === 'internal';
-    if (isRestrictionEnabled && isInternalRoute && !request.isInternalApiRequest) {
-      // throw 400
-      return response.badRequest({
-        body: `uri [${request.url.pathname}] with method [${request.route.method}] exists but is not available with the current configuration`,
-      });
+    if (isInternalRoute && !request.isInternalApiRequest) {
+      if (!isRestrictionEnabled) {
+        // warn if the restriction is not enforced
+        log.warn(
+          `Access to uri [${request.url.pathname}] with method [${request.route.method}] is deprecated`
+        );
+      } else {
+        log.error(
+          `Access to uri [${request.url.pathname}] with method [${request.route.method}] is not available with the current configuration`
+        );
+        // throw 400
+        return response.badRequest({
+          body: `uri [${request.url.pathname}] with method [${request.route.method}] exists but is not available with the current configuration`,
+        });
+      }
    }
    return toolkit.next();
  };
--- a/packages/core/http/core-http-server-internal/src/register_lifecycle_handlers.ts
+++ b/packages/core/http/core-http-server-internal/src/register_lifecycle_handlers.ts
@ -38,5 +38,5 @@ export const registerCoreHandlers = (
    );
  }
  // add check on header if the route is internal
-  registrar.registerOnPostAuth(createRestrictInternalRoutesPostAuthHandler(config)); // strictly speaking, we should have access to route.options.access from the request on postAuth
+  registrar.registerOnPostAuth(createRestrictInternalRoutesPostAuthHandler(config, log)); // strictly speaking, we should have access to route.options.access from the request on postAuth
 };
--- a/src/core/server/integration_tests/http/lifecycle.test.ts
+++ b/src/core/server/integration_tests/http/lifecycle.test.ts
@ -1019,11 +1019,9 @@ describe('Auth', () => {
    const response = await supertest(innerServer.listener).get('/').expect(200);

    expect(response.header['www-authenticate']).toBe('from auth interceptor');
-    expect(loggingSystemMock.collect(logger).warn).toMatchInlineSnapshot(`
+    expect(loggingSystemMock.collect(logger).warn[1]).toMatchInlineSnapshot(`
      Array [
-        Array [
-          "onPreResponseHandler rewrote a response header [www-authenticate].",
-        ],
+        "onPreResponseHandler rewrote a response header [www-authenticate].",
      ]
    `);
  });
@ -1054,6 +1052,9 @@ describe('Auth', () => {
    expect(response.header['www-authenticate']).toBe('from auth interceptor');
    expect(loggingSystemMock.collect(logger).warn).toMatchInlineSnapshot(`
      Array [
+        Array [
+          "Access to uri [/] with method [get] is deprecated",
+        ],
        Array [
          "onPreResponseHandler rewrote a response header [www-authenticate].",
        ],
@ -1288,11 +1289,9 @@ describe('OnPreResponse', () => {

    await supertest(innerServer.listener).get('/').expect(200);

-    expect(loggingSystemMock.collect(logger).warn).toMatchInlineSnapshot(`
+    expect(loggingSystemMock.collect(logger).warn[1]).toMatchInlineSnapshot(`
      Array [
-        Array [
-          "onPreResponseHandler rewrote a response header [x-kibana-header].",
-        ],
+        "onPreResponseHandler rewrote a response header [x-kibana-header].",
      ]
    `);
  });
--- a/src/core/server/integration_tests/http/lifecycle_handlers.test.ts
+++ b/src/core/server/integration_tests/http/lifecycle_handlers.test.ts
@ -27,7 +27,7 @@ const nameHeader = 'kbn-name';
 const allowlistedTestPath = '/xsrf/test/route/whitelisted';
 const xsrfDisabledTestPath = '/xsrf/test/route/disabled';
 const kibanaName = 'my-kibana-name';
-const internalProductHeader = 'x-elastic-internal-origin';
+const internalOriginHeader = 'x-elastic-internal-origin';
 const internalProductQueryParam = 'elasticInternalOrigin';
 const setupDeps = {
  context: contextServiceMock.createSetupContract(),
@ -279,30 +279,30 @@ describe('core lifecycle handlers', () => {
        .expect(400);
    });

-    it('accepts requests with the internal product header to internal routes', async () => {
+    it('accepts requests with the internal origin header to internal routes', async () => {
      await supertest(innerServer.listener)
        .get(testInternalRoute)
-        .set(internalProductHeader, 'anything')
+        .set(internalOriginHeader, 'anything')
        .query({ myValue: 'test' })
        .expect(200, 'ok()');
    });

-    it('accepts requests with the internal product header to public routes', async () => {
+    it('accepts requests with the internal origin header to public routes', async () => {
      await supertest(innerServer.listener)
        .get(testPublicRoute)
-        .set(internalProductHeader, 'anything')
+        .set(internalOriginHeader, 'anything')
        .query({ myValue: 'test' })
        .expect(200, 'ok()');
    });

-    it('accepts requests with the internal product query param to internal routes', async () => {
+    it('accepts requests with the internal origin query param to internal routes', async () => {
      await supertest(innerServer.listener)
        .get(testInternalRoute)
        .query({ [internalProductQueryParam]: 'anything', myValue: 'test' })
        .expect(200, 'ok()');
    });

-    it('accepts requests with the internal product query param to public routes', async () => {
+    it('accepts requests with the internal origin query param to public routes', async () => {
      await supertest(innerServer.listener)
        .get(testInternalRoute)
        .query({ [internalProductQueryParam]: 'anything', myValue: 'test' })
@ -315,10 +315,12 @@ describe('core lifecycle handlers with restrict internal routes enforced', () =>
  let server: HttpService;
  let innerServer: HttpServerSetup['server'];
  let router: IRouter;
+  let logger: jest.Mocked<Logger>;

  beforeEach(async () => {
+    logger = loggerMock.create();
    const configService = createConfigService({ server: { restrictInternalApis: true } });
-    server = createHttpService({ configService });
+    server = createHttpService({ configService, logger });

    await server.preboot({ context: contextServiceMock.createPrebootContract() });
    const serverSetup = await server.setup(setupDeps);
@ -349,16 +351,20 @@ describe('core lifecycle handlers with restrict internal routes enforced', () =>
      await server.start();
    });

-    it('request requests without the internal product header to internal routes', async () => {
+    it('rejects requests without the internal product header to internal routes', async () => {
      const result = await supertest(innerServer.listener).get(testInternalRoute).expect(400);
      expect(result.body.error).toBe('Bad Request');
+      expect(logger.warn).toHaveBeenCalledTimes(0);
+      expect(logger.error).toHaveBeenCalledTimes(1);
    });

    it('accepts requests with the internal product header to internal routes', async () => {
      await supertest(innerServer.listener)
        .get(testInternalRoute)
-        .set(internalProductHeader, 'anything')
+        .set(internalOriginHeader, 'anything')
        .expect(200, 'ok()');
+      expect(logger.warn).toHaveBeenCalledTimes(0);
+      expect(logger.error).toHaveBeenCalledTimes(0);
    });
  });
 });
@ -418,8 +424,8 @@ describe('core lifecycle handlers with no strict client version check', () => {
      .set(KIBANA_BUILD_NR_HEADER, '12345')
      .expect(500, /nok/);

-    expect(logger.warn).toHaveBeenCalledTimes(1);
-    const [[message]] = logger.warn.mock.calls;
+    expect(logger.warn).toHaveBeenCalledTimes(2);
+    const message = logger.warn.mock.calls[1][0];
    expect(message).toMatch(
      /^Client build \(12345\) is newer than this Kibana server build \(1234\)/
    );
@ -430,8 +436,8 @@ describe('core lifecycle handlers with no strict client version check', () => {
      .set(KIBANA_BUILD_NR_HEADER, '123')
      .expect(500, /nok/);

-    expect(logger.warn).toHaveBeenCalledTimes(1);
-    const [[message]] = logger.warn.mock.calls;
+    expect(logger.warn).toHaveBeenCalledTimes(2);
+    const message = logger.warn.mock.calls[1][0];
    expect(message).toMatch(
      /^Client build \(123\) is older than this Kibana server build \(1234\)/
    );