Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ describe('registerRouteForBundle', () => {
options: {
access: 'public',
authRequired: false,
httpResource: true,
},
validate: expect.any(Object),
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export function registerRouteForBundle(
{
path: `${routePath}{path*}`,
options: {
httpResource: true,
authRequired: false,
access: 'public',
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ export class HttpResourcesService implements CoreService<InternalHttpResourcesSe
access: 'public',
excludeFromOAS: true,
...route.options,
httpResource: true,
},
},
(context, request, response) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
20 changes: 12 additions & 8 deletions packages/core/http/core-http-router-server-internal/src/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
});
Expand Down Expand Up @@ -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,
Expand All @@ -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),
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
},
};

Expand Down
14 changes: 14 additions & 0 deletions packages/core/http/core-http-server/src/router/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export const registerTranslationsRoute = ({
},
options: {
access: 'public',
httpResource: true,
authRequired: false,
},
},
Expand Down
18 changes: 18 additions & 0 deletions src/core/server/integration_tests/http/versioned_router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) });
});
});
}