[8.x] [Http] Make HTTP resource routes respond without the versioned header (#195940) (#196324)

# Backport This will backport the following commits from `main` to `8.x`: - [[Http] Make HTTP resource routes respond without the versioned header (#195940)](https://github.com/elastic/kibana/pull/195940)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Jean-Louis Leysens <jeanlouis.leysens@elastic.co>
2025-04-24 01:38:56 -04:00 · 2024-10-16 03:10:28 +11:00 · 2024-10-16 03:10:28 +11:00 · 0158fec36f
commit 0158fec36f
parent c156cb3816
12 changed files with 153 additions and 40 deletions
--- 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
@ -45,6 +45,7 @@ describe('registerRouteForBundle', () => {
        options: {
          access: 'public',
          authRequired: false,
+          httpResource: true,
        },
        validate: expect.any(Object),
      },
--- 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: {
+        httpResource: true,
        authRequired: false,
        access: 'public',
      },
--- 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
@ -61,6 +61,25 @@ describe('HttpResources service', () => {
          expect(registeredRouteConfig.options?.access).toBe('public');
        });

+        it('registration does not allow changing "httpResource"', () => {
+          register(
+            { ...routeConfig, options: { ...routeConfig.options, httpResource: undefined } },
+            async (ctx, req, res) => res.ok()
+          );
+          register(
+            { ...routeConfig, options: { ...routeConfig.options, httpResource: true } },
+            async (ctx, req, res) => res.ok()
+          );
+          register(
+            { ...routeConfig, options: { ...routeConfig.options, httpResource: false } },
+            async (ctx, req, res) => res.ok()
+          );
+          const [[first], [second], [third]] = router.get.mock.calls;
+          expect(first.options?.httpResource).toBe(true);
+          expect(second.options?.httpResource).toBe(true);
+          expect(third.options?.httpResource).toBe(true);
+        });
+
        it('registration can set access to "internal"', () => {
          register({ ...routeConfig, options: { access: 'internal' } }, async (ctx, req, res) =>
            res.ok()
--- 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
@ -91,6 +91,7 @@ export class HttpResourcesService implements CoreService<InternalHttpResourcesSe
              access: 'public',
              excludeFromOAS: true,
              ...route.options,
+              httpResource: true,
            },
          },
          (context, request, response) => {
--- a/packages/core/http/core-http-router-server-internal/src/router.test.ts
+++ b/packages/core/http/core-http-router-server-internal/src/router.test.ts
@ -134,40 +134,76 @@ describe('Router', () => {
    }
  );

-  it('adds versioned header v2023-10-31 to public, unversioned routes', async () => {
-    const router = new Router('', logger, enhanceWithContext, routerOptions);
-    router.post(
-      {
-        path: '/public',
-        options: {
-          access: 'public',
+  describe('elastic-api-version header', () => {
+    it('adds the header to public, unversioned routes', async () => {
+      const router = new Router('', logger, enhanceWithContext, routerOptions);
+      router.post(
+        {
+          path: '/public',
+          options: {
+            access: 'public',
+          },
+          validate: false,
        },
-        validate: false,
-      },
-      (context, req, res) => res.ok({ headers: { AAAA: 'test' } }) // with some fake headers
-    );
-    router.post(
-      {
-        path: '/internal',
-        options: {
-          access: 'internal',
+        (context, req, res) => res.ok({ headers: { AAAA: 'test' } }) // with some fake headers
+      );
+      router.post(
+        {
+          path: '/internal',
+          options: {
+            access: 'internal',
+          },
+          validate: false,
        },
-        validate: false,
-      },
-      (context, req, res) => res.ok()
-    );
-    const [{ handler: publicHandler }, { handler: internalHandler }] = router.getRoutes();
+        (context, req, res) => res.ok()
+      );
+      const [{ handler: publicHandler }, { handler: internalHandler }] = router.getRoutes();

-    await publicHandler(createRequestMock(), mockResponseToolkit);
-    expect(mockResponse.header).toHaveBeenCalledTimes(2);
-    const [first, second] = mockResponse.header.mock.calls
-      .concat()
-      .sort(([k1], [k2]) => k1.localeCompare(k2));
-    expect(first).toEqual(['AAAA', 'test']);
-    expect(second).toEqual(['elastic-api-version', '2023-10-31']);
+      await publicHandler(createRequestMock(), mockResponseToolkit);
+      expect(mockResponse.header).toHaveBeenCalledTimes(2);
+      const [first, second] = mockResponse.header.mock.calls
+        .concat()
+        .sort(([k1], [k2]) => k1.localeCompare(k2));
+      expect(first).toEqual(['AAAA', 'test']);
+      expect(second).toEqual(['elastic-api-version', '2023-10-31']);

-    await internalHandler(createRequestMock(), mockResponseToolkit);
-    expect(mockResponse.header).toHaveBeenCalledTimes(2); // no additional calls
+      await internalHandler(createRequestMock(), mockResponseToolkit);
+      expect(mockResponse.header).toHaveBeenCalledTimes(2); // no additional calls
+    });
+
+    it('does not add the header to public http resource routes', async () => {
+      const router = new Router('', logger, enhanceWithContext, routerOptions);
+      router.post(
+        {
+          path: '/public',
+          options: {
+            access: 'public',
+          },
+          validate: false,
+        },
+        (context, req, res) => res.ok()
+      );
+      router.post(
+        {
+          path: '/public-resource',
+          options: {
+            access: 'public',
+            httpResource: true,
+          },
+          validate: false,
+        },
+        (context, req, res) => res.ok()
+      );
+      const [{ handler: publicHandler }, { handler: resourceHandler }] = router.getRoutes();
+
+      await publicHandler(createRequestMock(), mockResponseToolkit);
+      expect(mockResponse.header).toHaveBeenCalledTimes(1);
+      const [headersTuple] = mockResponse.header.mock.calls;
+      expect(headersTuple).toEqual(['elastic-api-version', '2023-10-31']);
+
+      await resourceHandler(createRequestMock(), mockResponseToolkit);
+      expect(mockResponse.header).toHaveBeenCalledTimes(1); // no additional calls
+    });
  });

  it('constructs lazily provided validations once (idempotency)', async () => {
--- a/packages/core/http/core-http-router-server-internal/src/router.ts
+++ b/packages/core/http/core-http-router-server-internal/src/router.ts
@ -149,6 +149,7 @@ export interface RouterOptions {
 export interface InternalRegistrarOptions {
  isVersioned: boolean;
 }
+
 /** @internal */
 export type VersionedRouteConfig<P, Q, B, M extends RouteMethod> = Omit<
  RouteConfig<P, Q, B, M>,
@ -201,11 +202,15 @@ export class Router<Context extends RequestHandlerContextBase = RequestHandlerCo
      <P, Q, B>(
        route: InternalRouteConfig<P, Q, B, Method>,
        handler: RequestHandler<P, Q, B, Context, Method>,
-        { isVersioned }: { isVersioned: boolean } = { isVersioned: false }
+        { isVersioned }: InternalRegistrarOptions = { isVersioned: false }
      ) => {
        route = prepareRouteConfigValidation(route);
        const routeSchemas = routeSchemasFromRouteConfig(route, method);
-        const isPublicUnversionedRoute = route.options?.access === 'public' && !isVersioned;
+        const isPublicUnversionedApi =
+          !isVersioned &&
+          route.options?.access === 'public' &&
+          // We do not consider HTTP resource routes as APIs
+          route.options?.httpResource !== true;

        this.routes.push({
          handler: async (req, responseToolkit) =>
@ -213,7 +218,7 @@ export class Router<Context extends RequestHandlerContextBase = RequestHandlerCo
              routeSchemas,
              request: req,
              responseToolkit,
-              isPublicUnversionedRoute,
+              isPublicUnversionedApi,
              handler: this.enhanceWithContext(handler),
            }),
          method,
@ -223,7 +228,6 @@ export class Router<Context extends RequestHandlerContextBase = RequestHandlerCo
          security: isVersioned
            ? route.security
            : validRouteSecurity(route.security as DeepPartial<RouteSecurity>, route.options),
-          /** Below is added for introspection */
          validationSchemas: route.validate,
          isVersioned,
        });
@ -269,12 +273,12 @@ export class Router<Context extends RequestHandlerContextBase = RequestHandlerCo
    routeSchemas,
    request,
    responseToolkit,
-    isPublicUnversionedRoute,
+    isPublicUnversionedApi,
    handler,
  }: {
    request: Request;
    responseToolkit: ResponseToolkit;
-    isPublicUnversionedRoute: boolean;
+    isPublicUnversionedApi: boolean;
    handler: RequestHandlerEnhanced<
      P,
      Q,
@ -301,7 +305,7 @@ export class Router<Context extends RequestHandlerContextBase = RequestHandlerCo
    } catch (error) {
      this.logError('400 Bad Request', 400, { request, error });
      const response = hapiResponseAdapter.toBadRequest(error.message);
-      if (isPublicUnversionedRoute) {
+      if (isPublicUnversionedApi) {
        response.output.headers = {
          ...response.output.headers,
          ...getVersionHeader(ALLOWED_PUBLIC_VERSION),
@ -312,7 +316,7 @@ export class Router<Context extends RequestHandlerContextBase = RequestHandlerCo

    try {
      const kibanaResponse = await handler(kibanaRequest, kibanaResponseFactory);
-      if (isPublicUnversionedRoute) {
+      if (isPublicUnversionedApi) {
        injectVersionHeader(ALLOWED_PUBLIC_VERSION, kibanaResponse);
      }
      if (kibanaRequest.protocol === 'http2' && kibanaResponse.options.headers) {
--- a/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.test.ts
+++ b/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.test.ts
@ -138,6 +138,9 @@ describe('Versioned route', () => {
        tags: ['access:test'],
        timeout: { payload: 60_000, idleSocket: 10_000 },
        xsrfRequired: false,
+        excludeFromOAS: true,
+        httpResource: true,
+        summary: `test`,
      },
    };

--- a/packages/core/http/core-http-server/src/router/route.ts
+++ b/packages/core/http/core-http-server/src/router/route.ts
@ -307,6 +307,20 @@ export interface RouteConfigOptions<Method extends RouteMethod> {
   * @remarks This will be surfaced in OAS documentation.
   */
  security?: RouteSecurity;
+
+  /**
+   * Whether this endpoint is being used to serve generated or static HTTP resources
+   * like JS, CSS or HTML. _Do not set to `true` for HTTP APIs._
+   *
+   * @note Unless you need this setting for a special case, rather use the
+   *       {@link HttpResources} service exposed to plugins directly.
+   *
+   * @note This is not a security feature. It may affect aspects of the HTTP
+   *       response like headers.
+   *
+   * @default false
+   */
+  httpResource?: boolean;
 }

 /**
--- 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
@ -24,7 +24,7 @@ describe('registerTranslationsRoute', () => {
      1,
      expect.objectContaining({
        path: '/translations/{locale}.json',
-        options: { access: 'public', authRequired: false },
+        options: { access: 'public', authRequired: false, httpResource: true },
      }),
      expect.any(Function)
    );
@ -32,7 +32,7 @@ describe('registerTranslationsRoute', () => {
      2,
      expect.objectContaining({
        path: '/translations/XXXX/{locale}.json',
-        options: { access: 'public', authRequired: false },
+        options: { access: 'public', authRequired: false, httpResource: true },
      }),
      expect.any(Function)
    );
--- a/packages/core/i18n/core-i18n-server-internal/src/routes/translations.ts
+++ b/packages/core/i18n/core-i18n-server-internal/src/routes/translations.ts
@ -45,6 +45,7 @@ export const registerTranslationsRoute = ({
          },
          options: {
            access: 'public',
+            httpResource: true,
            authRequired: false,
          },
        },
--- a/src/core/server/integration_tests/http/versioned_router.test.ts
+++ b/src/core/server/integration_tests/http/versioned_router.test.ts
@ -188,6 +188,24 @@ describe('Routing versioned requests', () => {
    ).resolves.toMatchObject({ 'elastic-api-version': '2023-10-31' });
  });

+  it('returns the version in response headers, even for HTTP resources', async () => {
+    router.versioned
+      .get({ path: '/my-path', access: 'public', options: { httpResource: true } })
+      .addVersion({ validate: false, version: '2023-10-31' }, async (ctx, req, res) => {
+        return res.ok({ body: { foo: 'bar' } });
+      });
+
+    await server.start();
+
+    await expect(
+      supertest
+        .get('/my-path')
+        .set('Elastic-Api-Version', '2023-10-31')
+        .expect(200)
+        .then(({ header }) => header)
+    ).resolves.toMatchObject({ 'elastic-api-version': '2023-10-31' });
+  });
+
  it('runs response validation when in dev', async () => {
    router.versioned
      .get({ path: '/my-path', access: 'internal' })
--- a/src/core/server/integration_tests/http_resources/http_resources_service.test.ts
+++ b/src/core/server/integration_tests/http_resources/http_resources_service.test.ts
@ -198,5 +198,20 @@ function applyTestsWithDisableUnsafeEvalSetTo(disableUnsafeEval: boolean) {
        expect(response.text).toBe('window.alert(42);');
      });
    });
+
+    it('responses do not contain the elastic-api-version header', async () => {
+      const { http, httpResources } = await root.setup();
+
+      const router = http.createRouter('');
+      const resources = httpResources.createRegistrar(router);
+      const htmlBody = `<p>HtMlr00lz</p>`;
+      resources.register({ path: '/render-html', validate: false }, (context, req, res) =>
+        res.renderHtml({ body: htmlBody })
+      );
+
+      await root.start();
+      const { header } = await request.get(root, '/render-html').expect(200);
+      expect(header).not.toMatchObject({ 'elastic-api-version': expect.any(String) });
+    });
  });
 }