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 index 65f5b41f91fba..b506933574d4a 100644 --- 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 @@ -7,20 +7,22 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import { Router, type RouterOptions } from './router'; +import type { ResponseToolkit, ResponseObject } from '@hapi/hapi'; import { loggingSystemMock } from '@kbn/core-logging-server-mocks'; import { isConfigSchema, schema } from '@kbn/config-schema'; -import { createFooValidation } from './router.test.util'; import { createRequestMock } from '@kbn/hapi-mocks/src/request'; +import { createFooValidation } from './router.test.util'; +import { Router, type RouterOptions } from './router'; import type { RouteValidatorRequestAndResponses } from '@kbn/core-http-server'; -const mockResponse: any = { +const mockResponse = { code: jest.fn().mockImplementation(() => mockResponse), header: jest.fn().mockImplementation(() => mockResponse), -}; -const mockResponseToolkit: any = { +} as unknown as jest.Mocked; + +const mockResponseToolkit = { response: jest.fn().mockReturnValue(mockResponse), -}; +} as unknown as jest.Mocked; const logger = loggingSystemMock.create().get(); const enhanceWithContext = (fn: (...args: any[]) => any) => fn.bind(null, {}); @@ -132,6 +134,42 @@ 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', + }, + validate: false, + }, + (context, req, res) => res.ok({ headers: { AAAA: 'test' } }) // with some fake headers + ); + router.post( + { + path: '/internal', + options: { + access: 'internal', + }, + validate: false, + }, + (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('constructs lazily provided validations once (idempotency)', async () => { const router = new Router('', logger, enhanceWithContext, routerOptions); const { fooValidation } = testValidation; 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 index 52363e7ea95be..d991ce2b2c4b9 100644 --- a/packages/core/http/core-http-router-server-internal/src/router.ts +++ b/packages/core/http/core-http-router-server-internal/src/router.ts @@ -33,13 +33,13 @@ import { validBodyOutput, getRequestValidation } from '@kbn/core-http-server'; import type { RouteSecurityGetter } from '@kbn/core-http-server'; import type { DeepPartial } from '@kbn/utility-types'; import { RouteValidator } from './validator'; -import { CoreVersionedRouter } from './versioned_router'; +import { ALLOWED_PUBLIC_VERSION, CoreVersionedRouter } from './versioned_router'; import { CoreKibanaRequest } from './request'; import { kibanaResponseFactory } from './response'; import { HapiResponseAdapter } from './response_adapter'; import { wrapErrors } from './error_wrapper'; import { Method } from './versioned_router/types'; -import { prepareRouteConfigValidation } from './util'; +import { getVersionHeader, injectVersionHeader, prepareRouteConfigValidation } from './util'; import { stripIllegalHttp2Headers } from './strip_illegal_http2_headers'; import { validRouteSecurity } from './security_route_config_validator'; import { InternalRouteConfig } from './route'; @@ -200,10 +200,11 @@ export class Router( route: InternalRouteConfig, handler: RequestHandler, - internalOptions: { isVersioned: boolean } = { isVersioned: false } + { isVersioned }: { isVersioned: boolean } = { isVersioned: false } ) => { route = prepareRouteConfigValidation(route); const routeSchemas = routeSchemasFromRouteConfig(route, method); + const isPublicUnversionedRoute = route.options?.access === 'public' && !isVersioned; this.routes.push({ handler: async (req, responseToolkit) => @@ -211,18 +212,19 @@ export class Router, route.options), /** Below is added for introspection */ validationSchemas: route.validate, - isVersioned: internalOptions.isVersioned, + isVersioned, }); }; @@ -266,10 +268,12 @@ export class Router { it('wraps only expected values in "once"', () => { @@ -49,3 +50,17 @@ describe('prepareResponseValidation', () => { expect(validation.response![500].body).toBeUndefined(); }); }); + +describe('injectResponseHeaders', () => { + it('injects an empty value as expected', () => { + const result = injectResponseHeaders({}, kibanaResponseFactory.ok()); + expect(result.options.headers).toEqual({}); + }); + it('merges values as expected', () => { + const result = injectResponseHeaders( + { foo: 'false', baz: 'true' }, + kibanaResponseFactory.ok({ headers: { foo: 'true', bar: 'false' } }) + ); + expect(result.options.headers).toEqual({ foo: 'false', bar: 'false', baz: 'true' }); + }); +}); diff --git a/packages/core/http/core-http-router-server-internal/src/util.ts b/packages/core/http/core-http-router-server-internal/src/util.ts index 0d1c8abb0e103..176d33b589880 100644 --- a/packages/core/http/core-http-router-server-internal/src/util.ts +++ b/packages/core/http/core-http-router-server-internal/src/util.ts @@ -14,6 +14,9 @@ import { type RouteMethod, type RouteValidator, } from '@kbn/core-http-server'; +import type { Mutable } from 'utility-types'; +import type { IKibanaResponse, ResponseHeaders } from '@kbn/core-http-server'; +import { ELASTIC_HTTP_VERSION_HEADER } from '@kbn/core-http-common'; import type { InternalRouteConfig } from './route'; function isStatusCode(key: string) { @@ -63,3 +66,29 @@ export function prepareRouteConfigValidation( } return config; } + +/** + * @note mutates the response object + * @internal + */ +export function injectResponseHeaders( + headers: ResponseHeaders, + response: IKibanaResponse +): IKibanaResponse { + const mutableResponse = response as Mutable; + mutableResponse.options.headers = { + ...mutableResponse.options.headers, + ...headers, + }; + return mutableResponse; +} + +export function getVersionHeader(version: string): ResponseHeaders { + return { + [ELASTIC_HTTP_VERSION_HEADER]: version, + }; +} + +export function injectVersionHeader(version: string, response: IKibanaResponse): IKibanaResponse { + return injectResponseHeaders(getVersionHeader(version), response); +} diff --git a/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.ts b/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.ts index 71ab30bbe8b80..e9a9e60de8193 100644 --- a/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.ts +++ b/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.ts @@ -38,7 +38,7 @@ import { readVersion, removeQueryVersion, } from './route_version_utils'; -import { injectResponseHeaders } from './inject_response_headers'; +import { getVersionHeader, injectVersionHeader } from '../util'; import { validRouteSecurity } from '../security_route_config_validator'; import { resolvers } from './handler_resolvers'; @@ -221,9 +221,7 @@ export class CoreVersionedRoute implements VersionedRoute { req.params = params; req.query = query; } catch (e) { - return res.badRequest({ - body: e.message, - }); + return res.badRequest({ body: e.message, headers: getVersionHeader(version) }); } } else { // Preserve behavior of not passing through unvalidated data @@ -252,12 +250,7 @@ export class CoreVersionedRoute implements VersionedRoute { } } - return injectResponseHeaders( - { - [ELASTIC_HTTP_VERSION_HEADER]: version, - }, - response - ); + return injectVersionHeader(version, response); }; private validateVersion(version: string) { diff --git a/packages/core/http/core-http-router-server-internal/src/versioned_router/inject_response_headers.ts b/packages/core/http/core-http-router-server-internal/src/versioned_router/inject_response_headers.ts deleted file mode 100644 index c27c92023f56e..0000000000000 --- a/packages/core/http/core-http-router-server-internal/src/versioned_router/inject_response_headers.ts +++ /dev/null @@ -1,27 +0,0 @@ -/* - * 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", the "GNU Affero General Public License v3.0 only", 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", the "GNU Affero General Public - * License v3.0 only", or the "Server Side Public License, v 1". - */ - -import type { Mutable } from 'utility-types'; -import type { IKibanaResponse } from '@kbn/core-http-server'; - -/** - * @note mutates the response object - * @internal - */ -export function injectResponseHeaders(headers: object, response: IKibanaResponse): IKibanaResponse { - const mutableResponse = response as Mutable; - mutableResponse.options = { - ...mutableResponse.options, - headers: { - ...mutableResponse.options.headers, - ...headers, - }, - }; - return mutableResponse; -} diff --git a/src/core/server/integration_tests/http/router.test.ts b/src/core/server/integration_tests/http/router.test.ts index 0b7bbb8ce55c3..c0a690e479e67 100644 --- a/src/core/server/integration_tests/http/router.test.ts +++ b/src/core/server/integration_tests/http/router.test.ts @@ -836,6 +836,82 @@ describe('Handler', () => { expect(body).toEqual(12); }); + + it('adds versioned header v2023-10-31 to public, unversioned routes', async () => { + const { server: innerServer, createRouter } = await server.setup(setupDeps); + const router = createRouter('/'); + + router.post( + { + path: '/public', + validate: { body: schema.object({ ok: schema.boolean() }) }, + options: { + access: 'public', + }, + }, + (context, req, res) => { + if (req.body.ok) { + return res.ok({ body: 'ok', headers: { test: 'this' } }); + } + return res.customError({ statusCode: 499, body: 'custom error' }); + } + ); + router.post( + { + path: '/internal', + validate: { body: schema.object({ ok: schema.boolean() }) }, + }, + (context, req, res) => { + return res.ok({ body: 'ok', headers: { test: 'this' } }); + } + ); + await server.start(); + + // Includes header if validation fails + { + const { headers } = await supertest(innerServer.listener) + .post('/public') + .send({ ok: null }) + .expect(400); + expect(headers).toMatchObject({ 'elastic-api-version': '2023-10-31' }); + } + + // Includes header if custom error + { + const { headers } = await supertest(innerServer.listener) + .post('/public') + .send({ ok: false }) + .expect(499); + expect(headers).toMatchObject({ 'elastic-api-version': '2023-10-31' }); + } + + // Includes header if OK + { + const { headers } = await supertest(innerServer.listener) + .post('/public') + .send({ ok: true }) + .expect(200); + expect(headers).toMatchObject({ 'elastic-api-version': '2023-10-31' }); + } + + // Internal unversioned routes do not include the header for OK + { + const { headers } = await supertest(innerServer.listener) + .post('/internal') + .send({ ok: true }) + .expect(200); + expect(headers).not.toMatchObject({ 'elastic-api-version': '2023-10-31' }); + } + + // Internal unversioned routes do not include the header for validation failures + { + const { headers } = await supertest(innerServer.listener) + .post('/internal') + .send({ ok: null }) + .expect(400); + expect(headers).not.toMatchObject({ 'elastic-api-version': '2023-10-31' }); + } + }); }); describe('handleLegacyErrors', () => { diff --git a/src/core/server/integration_tests/http/versioned_router.test.ts b/src/core/server/integration_tests/http/versioned_router.test.ts index 9f2b2625a6a7e..254337f82abcf 100644 --- a/src/core/server/integration_tests/http/versioned_router.test.ts +++ b/src/core/server/integration_tests/http/versioned_router.test.ts @@ -112,14 +112,12 @@ describe('Routing versioned requests', () => { await server.start(); - await expect(supertest.get('/my-path').expect(200)).resolves.toEqual( - expect.objectContaining({ - body: { v: '1' }, - header: expect.objectContaining({ - 'elastic-api-version': '2020-02-02', - }), - }) - ); + await expect(supertest.get('/my-path').expect(200)).resolves.toMatchObject({ + body: { v: '1' }, + header: expect.objectContaining({ + 'elastic-api-version': '2020-02-02', + }), + }); }); it('returns the expected output for badly formatted versions', async () => { @@ -137,11 +135,9 @@ describe('Routing versioned requests', () => { .set('Elastic-Api-Version', 'abc') .expect(400) .then(({ body }) => body) - ).resolves.toEqual( - expect.objectContaining({ - message: expect.stringMatching(/Invalid version/), - }) - ); + ).resolves.toMatchObject({ + message: expect.stringMatching(/Invalid version/), + }); }); it('returns the expected responses for failed validation', async () => { @@ -163,18 +159,14 @@ describe('Routing versioned requests', () => { await server.start(); await expect( - supertest - .post('/my-path') - .send({}) - .set('Elastic-Api-Version', '1') - .expect(400) - .then(({ body }) => body) - ).resolves.toEqual( - expect.objectContaining({ + supertest.post('/my-path').send({}).set('Elastic-Api-Version', '1').expect(400) + ).resolves.toMatchObject({ + body: { error: 'Bad Request', message: expect.stringMatching(/expected value of type/), - }) - ); + }, + headers: { 'elastic-api-version': '1' }, // includes version if validation failed + }); expect(captureErrorMock).not.toHaveBeenCalled(); }); @@ -193,7 +185,7 @@ describe('Routing versioned requests', () => { .set('Elastic-Api-Version', '2023-10-31') .expect(200) .then(({ header }) => header) - ).resolves.toEqual(expect.objectContaining({ 'elastic-api-version': '2023-10-31' })); + ).resolves.toMatchObject({ 'elastic-api-version': '2023-10-31' }); }); it('runs response validation when in dev', async () => { @@ -236,11 +228,9 @@ describe('Routing versioned requests', () => { .set('Elastic-Api-Version', '1') .expect(500) .then(({ body }) => body) - ).resolves.toEqual( - expect.objectContaining({ - message: expect.stringMatching(/Failed output validation/), - }) - ); + ).resolves.toMatchObject({ + message: expect.stringMatching(/Failed output validation/), + }); await expect( supertest @@ -248,11 +238,9 @@ describe('Routing versioned requests', () => { .set('Elastic-Api-Version', '2') .expect(500) .then(({ body }) => body) - ).resolves.toEqual( - expect.objectContaining({ - message: expect.stringMatching(/Failed output validation/), - }) - ); + ).resolves.toMatchObject({ + message: expect.stringMatching(/Failed output validation/), + }); // This should pass response validation await expect( @@ -261,11 +249,9 @@ describe('Routing versioned requests', () => { .set('Elastic-Api-Version', '3') .expect(200) .then(({ body }) => body) - ).resolves.toEqual( - expect.objectContaining({ - v: '3', - }) - ); + ).resolves.toMatchObject({ + v: '3', + }); expect(captureErrorMock).not.toHaveBeenCalled(); }); @@ -367,9 +353,7 @@ describe('Routing versioned requests', () => { .set('Elastic-Api-Version', '2020-02-02') .expect(500) .then(({ body }) => body) - ).resolves.toEqual( - expect.objectContaining({ message: expect.stringMatching(/No handlers registered/) }) - ); + ).resolves.toMatchObject({ message: expect.stringMatching(/No handlers registered/) }); expect(captureErrorMock).not.toHaveBeenCalled(); });