Skip to content

Commit

Permalink
[Http] Added version header to unversioned, public routes (#195464)
Browse files Browse the repository at this point in the history
(cherry picked from commit 61251bf)
  • Loading branch information
jloleysens committed Oct 9, 2024
1 parent 3add8e4 commit 139739c
Show file tree
Hide file tree
Showing 8 changed files with 214 additions and 92 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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<ResponseObject>;

const mockResponseToolkit = {
response: jest.fn().mockReturnValue(mockResponse),
};
} as unknown as jest.Mocked<ResponseToolkit>;

const logger = loggingSystemMock.create().get();
const enhanceWithContext = (fn: (...args: any[]) => any) => fn.bind(null, {});
Expand Down Expand Up @@ -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;
Expand Down
26 changes: 20 additions & 6 deletions packages/core/http/core-http-router-server-internal/src/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -200,29 +200,31 @@ export class Router<Context extends RequestHandlerContextBase = RequestHandlerCo
<P, Q, B>(
route: InternalRouteConfig<P, Q, B, Method>,
handler: RequestHandler<P, Q, B, Context, Method>,
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) =>
await this.handle({
routeSchemas,
request: req,
responseToolkit,
isPublicUnversionedRoute,
handler: this.enhanceWithContext(handler),
}),
method,
path: getRouteFullPath(this.routerPath, route.path),
options: validOptions(method, route),
// For the versioned route security is validated in the versioned router
security: internalOptions.isVersioned
security: isVersioned
? route.security
: validRouteSecurity(route.security as DeepPartial<RouteSecurity>, route.options),
/** Below is added for introspection */
validationSchemas: route.validate,
isVersioned: internalOptions.isVersioned,
isVersioned,
});
};

Expand Down Expand Up @@ -266,10 +268,12 @@ export class Router<Context extends RequestHandlerContextBase = RequestHandlerCo
routeSchemas,
request,
responseToolkit,
isPublicUnversionedRoute,
handler,
}: {
request: Request;
responseToolkit: ResponseToolkit;
isPublicUnversionedRoute: boolean;
handler: RequestHandlerEnhanced<
P,
Q,
Expand All @@ -295,11 +299,21 @@ export class Router<Context extends RequestHandlerContextBase = RequestHandlerCo
>;
} catch (error) {
this.logError('400 Bad Request', 400, { request, error });
return hapiResponseAdapter.toBadRequest(error.message);
const response = hapiResponseAdapter.toBadRequest(error.message);
if (isPublicUnversionedRoute) {
response.output.headers = {
...response.output.headers,
...getVersionHeader(ALLOWED_PUBLIC_VERSION),
};
}
return response;
}

try {
const kibanaResponse = await handler(kibanaRequest, kibanaResponseFactory);
if (isPublicUnversionedRoute) {
injectVersionHeader(ALLOWED_PUBLIC_VERSION, kibanaResponse);
}
if (kibanaRequest.protocol === 'http2' && kibanaResponse.options.headers) {
kibanaResponse.options.headers = stripIllegalHttp2Headers({
headers: kibanaResponse.options.headers,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@

import { schema } from '@kbn/config-schema';
import { RouteValidator } from '@kbn/core-http-server';
import { prepareResponseValidation } from './util';
import { injectResponseHeaders, prepareResponseValidation } from './util';
import { kibanaResponseFactory } from './response';

describe('prepareResponseValidation', () => {
it('wraps only expected values in "once"', () => {
Expand Down Expand Up @@ -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' });
});
});
29 changes: 29 additions & 0 deletions packages/core/http/core-http-router-server-internal/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -63,3 +66,29 @@ export function prepareRouteConfigValidation<P, Q, B>(
}
return config;
}

/**
* @note mutates the response object
* @internal
*/
export function injectResponseHeaders(
headers: ResponseHeaders,
response: IKibanaResponse
): IKibanaResponse {
const mutableResponse = response as Mutable<IKibanaResponse>;
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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down

This file was deleted.

76 changes: 76 additions & 0 deletions src/core/server/integration_tests/http/router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Loading

0 comments on commit 139739c

Please sign in to comment.