From 484351bdac07e38a18a53b089e25cc4c3ff4f439 Mon Sep 17 00:00:00 2001 From: Mikhail Shustov Date: Wed, 26 Jun 2019 10:04:34 +0200 Subject: [PATCH] KibanaRequest provides headers as a property. (#39506) * use property instead of method. not all header names are known * fix tag name and re-generate docs --- ...server.kibanarequest.getfilteredheaders.md | 22 ----------- ...ana-plugin-server.kibanarequest.headers.md | 7 +++- .../kibana-plugin-server.kibanarequest.md | 12 +----- src/core/server/http/http_server.test.ts | 39 +++---------------- .../integration_tests/http_service.test.ts | 9 ++--- src/core/server/http/router/request.test.ts | 38 ++++++------------ src/core/server/http/router/request.ts | 29 +++++--------- src/core/server/server.api.md | 2 - 8 files changed, 35 insertions(+), 123 deletions(-) delete mode 100644 docs/development/core/server/kibana-plugin-server.kibanarequest.getfilteredheaders.md diff --git a/docs/development/core/server/kibana-plugin-server.kibanarequest.getfilteredheaders.md b/docs/development/core/server/kibana-plugin-server.kibanarequest.getfilteredheaders.md deleted file mode 100644 index ca0918c53562..000000000000 --- a/docs/development/core/server/kibana-plugin-server.kibanarequest.getfilteredheaders.md +++ /dev/null @@ -1,22 +0,0 @@ - - -[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [KibanaRequest](./kibana-plugin-server.kibanarequest.md) > [getFilteredHeaders](./kibana-plugin-server.kibanarequest.getfilteredheaders.md) - -## KibanaRequest.getFilteredHeaders() method - -Signature: - -```typescript -getFilteredHeaders(headersToKeep: string[]): Pick, string>; -``` - -## Parameters - -| Parameter | Type | Description | -| --- | --- | --- | -| headersToKeep | string[] | | - -Returns: - -`Pick, string>` - diff --git a/docs/development/core/server/kibana-plugin-server.kibanarequest.headers.md b/docs/development/core/server/kibana-plugin-server.kibanarequest.headers.md index e2e350d33c28..8bd50e23608d 100644 --- a/docs/development/core/server/kibana-plugin-server.kibanarequest.headers.md +++ b/docs/development/core/server/kibana-plugin-server.kibanarequest.headers.md @@ -4,10 +4,15 @@ ## KibanaRequest.headers property -This property will be removed in future version of this class, please use the `getFilteredHeaders` method instead +Readonly copy of incoming request headers. Signature: ```typescript readonly headers: Headers; ``` + +## Remarks + +This property will contain a `filtered` copy of request headers. + diff --git a/docs/development/core/server/kibana-plugin-server.kibanarequest.md b/docs/development/core/server/kibana-plugin-server.kibanarequest.md index ae658c8ae603..a9622e4319d5 100644 --- a/docs/development/core/server/kibana-plugin-server.kibanarequest.md +++ b/docs/development/core/server/kibana-plugin-server.kibanarequest.md @@ -23,19 +23,9 @@ export declare class KibanaRequestBody | | -| [headers](./kibana-plugin-server.kibanarequest.headers.md) | | Headers | This property will be removed in future version of this class, please use the getFilteredHeaders method instead | +| [headers](./kibana-plugin-server.kibanarequest.headers.md) | | Headers | Readonly copy of incoming request headers. | | [params](./kibana-plugin-server.kibanarequest.params.md) | | Params | | | [query](./kibana-plugin-server.kibanarequest.query.md) | | Query | | | [route](./kibana-plugin-server.kibanarequest.route.md) | | RecursiveReadonly<KibanaRequestRoute> | | | [url](./kibana-plugin-server.kibanarequest.url.md) | | Url | | -## Methods - -| Method | Modifiers | Description | -| --- | --- | --- | -| [getFilteredHeaders(headersToKeep)](./kibana-plugin-server.kibanarequest.getfilteredheaders.md) | | | - -## Remarks - -The `headers` property will be deprecated and removed in future versions of this class. Please use the `getFilteredHeaders` method to acesss the list of headers available - diff --git a/src/core/server/http/http_server.test.ts b/src/core/server/http/http_server.test.ts index 88533504758b..8698a3db7743 100644 --- a/src/core/server/http/http_server.test.ts +++ b/src/core/server/http/http_server.test.ts @@ -406,35 +406,6 @@ test('handles deleting', async () => { }); }); -test('filtered headers', async () => { - expect.assertions(1); - - const router = new Router('/foo'); - - let filteredHeaders: any; - - router.get({ path: '/', validate: false }, (req, res) => { - filteredHeaders = req.getFilteredHeaders(['x-kibana-foo', 'host']); - - return res.noContent(); - }); - - const { registerRouter, server: innerServer } = await server.setup(config); - registerRouter(router); - - await server.start(); - - await supertest(innerServer.listener) - .get('/foo/?bar=quux') - .set('x-kibana-foo', 'bar') - .set('x-kibana-bar', 'quux'); - - expect(filteredHeaders).toEqual({ - host: `127.0.0.1:${config.port}`, - 'x-kibana-foo': 'bar', - }); -}); - describe('with `basepath: /bar` and `rewriteBasePath: false`', () => { let configWithBasePath: HttpConfig; let innerServerListener: Server; @@ -756,7 +727,7 @@ describe('#registerAuth', () => { ]); }); - it('is the only place with access to the authorization header', async () => { + it.skip('is the only place with access to the authorization header', async () => { const token = 'Basic: user:password'; const { registerAuth, @@ -768,26 +739,26 @@ describe('#registerAuth', () => { let fromRegisterOnPreAuth; await registerOnPreAuth((req, t) => { - fromRegisterOnPreAuth = req.getFilteredHeaders(['authorization']); + fromRegisterOnPreAuth = req.headers.authorization; return t.next(); }); let fromRegisterAuth; await registerAuth((req, t) => { - fromRegisterAuth = req.getFilteredHeaders(['authorization']); + fromRegisterAuth = req.headers.authorization; return t.authenticated(); }, cookieOptions); let fromRegisterOnPostAuth; await registerOnPostAuth((req, t) => { - fromRegisterOnPostAuth = req.getFilteredHeaders(['authorization']); + fromRegisterOnPostAuth = req.headers.authorization; return t.next(); }); let fromRouteHandler; const router = new Router(''); router.get({ path: '/', validate: false }, (req, res) => { - fromRouteHandler = req.getFilteredHeaders(['authorization']); + fromRouteHandler = req.headers.authorization; return res.ok({ content: 'ok' }); }); registerRouter(router); diff --git a/src/core/server/http/integration_tests/http_service.test.ts b/src/core/server/http/integration_tests/http_service.test.ts index d1345b088443..b35d20a58ff5 100644 --- a/src/core/server/http/integration_tests/http_service.test.ts +++ b/src/core/server/http/integration_tests/http_service.test.ts @@ -59,8 +59,7 @@ describe('http service', () => { it('runs auth for legacy routes and proxy request to legacy server route handlers', async () => { const { http } = await root.setup(); const { sessionStorageFactory } = await http.registerAuth((req, t) => { - const headers = req.getFilteredHeaders(['authorization']); - if (headers.authorization) { + if (req.headers.authorization) { const user = { id: '42' }; const sessionStorage = sessionStorageFactory.asScoped(req); sessionStorage.set({ value: user, expires: Date.now() + sessionDurationMs }); @@ -90,8 +89,7 @@ describe('http service', () => { const token = 'Basic: name:password'; const { http } = await root.setup(); const { sessionStorageFactory } = await http.registerAuth((req, t) => { - const headers = req.getFilteredHeaders(['authorization']); - if (headers.authorization) { + if (req.headers.authorization) { const user = { id: '42' }; const sessionStorage = sessionStorageFactory.asScoped(req); sessionStorage.set({ value: user, expires: Date.now() + sessionDurationMs }); @@ -129,8 +127,7 @@ describe('http service', () => { const { http } = await root.setup(); const { sessionStorageFactory } = await http.registerAuth((req, t) => { - const headers = req.getFilteredHeaders(['authorization']); - if (headers.authorization) { + if (req.headers.authorization) { const sessionStorage = sessionStorageFactory.asScoped(req); sessionStorage.set({ value: user, expires: Date.now() + sessionDurationMs }); return t.authenticated({ state: user }); diff --git a/src/core/server/http/router/request.test.ts b/src/core/server/http/router/request.test.ts index 6aad68e48638..ebb7ffa7a6fc 100644 --- a/src/core/server/http/router/request.test.ts +++ b/src/core/server/http/router/request.test.ts @@ -30,41 +30,25 @@ describe('KibanaRequest', () => { }); }); - describe('#getFilteredHeaders', () => { - it('returns request headers', () => { + describe('headers property', () => { + it('provides a frozen copy of request headers', () => { + const rawRequestHeaders = { custom: 'one' }; const request = httpServerMock.createRawRequest({ - headers: { custom: 'one' }, + headers: rawRequestHeaders, }); const kibanaRequest = KibanaRequest.from(request); - expect(kibanaRequest.getFilteredHeaders(['custom'])).toEqual({ - custom: 'one', - }); + + expect(kibanaRequest.headers).toEqual({ custom: 'one' }); + expect(kibanaRequest.headers).not.toBe(rawRequestHeaders); + expect(Object.isFrozen(kibanaRequest.headers)).toBe(true); }); - it('normalizes a header name', () => { - const request = httpServerMock.createRawRequest({ - headers: { custom: 'one' }, - }); - const kibanaRequest = KibanaRequest.from(request); - expect(kibanaRequest.getFilteredHeaders(['CUSTOM'])).toEqual({ - custom: 'one', - }); - }); - - it('returns an empty object is no headers were specified', () => { - const request = httpServerMock.createRawRequest({ - headers: { custom: 'one' }, - }); - const kibanaRequest = KibanaRequest.from(request); - expect(kibanaRequest.getFilteredHeaders([])).toEqual({}); - }); - - it("doesn't expose authorization header by default", () => { + it.skip("doesn't expose authorization header by default", () => { const request = httpServerMock.createRawRequest({ headers: { custom: 'one', authorization: 'token' }, }); const kibanaRequest = KibanaRequest.from(request); - expect(kibanaRequest.getFilteredHeaders(['custom', 'authorization'])).toEqual({ + expect(kibanaRequest.headers).toEqual({ custom: 'one', }); }); @@ -74,7 +58,7 @@ describe('KibanaRequest', () => { headers: { custom: 'one', authorization: 'token' }, }); const kibanaRequest = KibanaRequest.from(request, undefined, false); - expect(kibanaRequest.getFilteredHeaders(['custom', 'authorization'])).toEqual({ + expect(kibanaRequest.headers).toEqual({ custom: 'one', authorization: 'token', }); diff --git a/src/core/server/http/router/request.ts b/src/core/server/http/router/request.ts index 8e683b3c92f7..7a1f4903b1cf 100644 --- a/src/core/server/http/router/request.ts +++ b/src/core/server/http/router/request.ts @@ -18,11 +18,12 @@ */ import { Url } from 'url'; -import { ObjectType, TypeOf } from '@kbn/config-schema'; import { Request } from 'hapi'; +import { ObjectType, TypeOf } from '@kbn/config-schema'; + import { deepFreeze, RecursiveReadonly } from '../../../utils'; -import { filterHeaders, Headers } from './headers'; +import { Headers } from './headers'; import { RouteMethod, RouteSchemas, RouteConfigOptions } from './route'; const requestSymbol = Symbol('request'); @@ -37,15 +38,8 @@ export interface KibanaRequestRoute { options: Required; } -const secretHeaders = ['authorization']; /** * Kibana specific abstraction for an incoming request. - * - * @remarks - * The `headers` property will be deprecated and removed in future versions - * of this class. Please use the `getFilteredHeaders` method to acesss the - * list of headers available - * * @public * */ export class KibanaRequest { @@ -104,8 +98,9 @@ export class KibanaRequest { public readonly url: Url; public readonly route: RecursiveReadonly; /** - * This property will be removed in future version of this class, please - * use the `getFilteredHeaders` method instead + * Readonly copy of incoming request headers. + * @remarks + * This property will contain a `filtered` copy of request headers. */ public readonly headers: Headers; @@ -117,10 +112,12 @@ export class KibanaRequest { readonly params: Params, readonly query: Query, readonly body: Body, + // @ts-ignore we will use this flag as soon as http request proxy is supported in the core + // until that time we have to expose all the headers private readonly withoutSecretHeaders: boolean ) { this.url = request.url; - this.headers = request.headers; + this.headers = deepFreeze({ ...request.headers }); // prevent Symbol exposure via Object.getOwnPropertySymbols() Object.defineProperty(this, requestSymbol, { @@ -131,14 +128,6 @@ export class KibanaRequest { this.route = deepFreeze(this.getRouteInfo()); } - public getFilteredHeaders(headersToKeep: string[]) { - return filterHeaders( - this[requestSymbol].headers, - headersToKeep, - this.withoutSecretHeaders ? secretHeaders : [] - ); - } - private getRouteInfo() { const request = this[requestSymbol]; return { diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index 086d0f4bef1a..a1c8d4a8e2b5 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -189,8 +189,6 @@ export class KibanaRequest { // // @internal static from

(req: Request, routeSchemas?: RouteSchemas, withoutSecretHeaders?: boolean): KibanaRequest; - // (undocumented) - getFilteredHeaders(headersToKeep: string[]): Pick, string>; readonly headers: Headers; // (undocumented) readonly params: Params;