[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)](#195940)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Jean-Louis Leysens <[email protected]>
elastic · Oct 15, 2024 · 0158fec · 0158fec
1 parent c156cb3
commit 0158fec
Show file tree

Hide file tree

Showing 12 changed files with 153 additions and 40 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
@@ -45,6 +45,7 @@ describe('registerRouteForBundle', () => {
         options: {
           access: 'public',
           authRequired: false,
+          httpResource: true,
         },
         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: {
+        httpResource: true,
         authRequired: false,
         access: 'public',
       },

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
@@ -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()

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
@@ -91,6 +91,7 @@ export class HttpResourcesService implements CoreService<InternalHttpResourcesSe
               access: 'public',
               excludeFromOAS: true,
               ...route.options,
+              httpResource: true,
             },
           },
           (context, request, response) => {

diff --git 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 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(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(1);
+      const [headersTuple] = mockResponse.header.mock.calls;
+      expect(headersTuple).toEqual(['elastic-api-version', '2023-10-31']);
 
-    await internalHandler(createRequestMock(), mockResponseToolkit);
-    expect(mockResponse.header).toHaveBeenCalledTimes(2); // no additional calls
+      await resourceHandler(createRequestMock(), mockResponseToolkit);
+      expect(mockResponse.header).toHaveBeenCalledTimes(1); // no additional calls
+    });
   });
 
   it('constructs lazily provided validations once (idempotency)', async () => {

diff --git 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,19 +202,23 @@ 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) =>
             await this.handle({
               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) {

diff --git a/...e/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.test.ts b/...e/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`,
       },
     };
 

diff --git 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;
 }
 
 /**

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
@@ -24,15 +24,15 @@ describe('registerTranslationsRoute', () => {
       1,
       expect.objectContaining({
         path: '/translations/{locale}.json',
-        options: { access: 'public', authRequired: false },
+        options: { access: 'public', authRequired: false, httpResource: true },
       }),
       expect.any(Function)
     );
     expect(router.get).toHaveBeenNthCalledWith(
       2,
       expect.objectContaining({
         path: '/translations/XXXX/{locale}.json',
-        options: { access: 'public', authRequired: false },
+        options: { access: 'public', authRequired: false, httpResource: true },
       }),
       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
@@ -45,6 +45,7 @@ export const registerTranslationsRoute = ({
           },
           options: {
             access: 'public',
+            httpResource: true,
             authRequired: false,
           },
         },

diff --git 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' })

diff --git 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) });
+    });
   });
 }