NP licensing plugin improvements

src/core/public/http/http_service.mock.ts

@@ -21,10 +21,10 @@ import { HttpService } from './http_service';
 import { HttpSetup } from './types';
 import { BehaviorSubject } from 'rxjs';
 import { BasePath } from './base_path_service';
-import { AnonymousPaths } from './anonymous_paths';
 
 type ServiceSetupMockType = jest.Mocked<HttpSetup> & {
   basePath: BasePath;
+  anonymousPaths: jest.Mocked<HttpSetup['anonymousPaths']>;
 };
 
 const createServiceMock = ({ basePath = '' } = {}): ServiceSetupMockType => ({
@@ -37,7 +37,10 @@ const createServiceMock = ({ basePath = '' } = {}): ServiceSetupMockType => ({
   delete: jest.fn(),
   options: jest.fn(),
   basePath: new BasePath(basePath),
-  anonymousPaths: new AnonymousPaths(new BasePath(basePath)),
+  anonymousPaths: {
+    register: jest.fn(),
+    isAnonymous: jest.fn(),
+  },
   addLoadingCount: jest.fn(),
   getLoadingCount$: jest.fn().mockReturnValue(new BehaviorSubject(0)),
   stop: jest.fn(),

src/core/public/mocks.ts

@@ -18,7 +18,7 @@
  */
 import { applicationServiceMock } from './application/application_service.mock';
 import { chromeServiceMock } from './chrome/chrome_service.mock';
-import { CoreContext, CoreSetup, CoreStart, PluginInitializerContext, NotificationsSetup } from '.';
+import { CoreContext, PluginInitializerContext } from '.';
 import { docLinksServiceMock } from './doc_links/doc_links_service.mock';
 import { fatalErrorsServiceMock } from './fatal_errors/fatal_errors_service.mock';
 import { httpServiceMock } from './http/http_service.mock';
@@ -42,7 +42,7 @@ export { overlayServiceMock } from './overlays/overlay_service.mock';
 export { uiSettingsServiceMock } from './ui_settings/ui_settings_service.mock';
 
 function createCoreSetupMock({ basePath = '' } = {}) {
-  const mock: MockedKeys<CoreSetup> & { notifications: MockedKeys<NotificationsSetup> } = {
+  const mock = {
     application: applicationServiceMock.createSetupContract(),
     context: contextServiceMock.createSetupContract(),
     fatalErrors: fatalErrorsServiceMock.createSetupContract(),
@@ -58,7 +58,7 @@ function createCoreSetupMock({ basePath = '' } = {}) {
 }
 
 function createCoreStartMock({ basePath = '' } = {}) {
-  const mock: MockedKeys<CoreStart> & { notifications: MockedKeys<NotificationsSetup> } = {
+  const mock = {
     application: applicationServiceMock.createStartContract(),
     chrome: chromeServiceMock.createStartContract(),
     docLinks: docLinksServiceMock.createStartContract(),

src/core/server/http/http_server.ts

@@ -17,16 +17,17 @@
  * under the License.
  */
 
-import { Request, Server } from 'hapi';
+import { Server } from 'hapi';
 
 import { Logger, LoggerFactory } from '../logging';
 import { HttpConfig } from './http_config';
 import { createServer, getListenerOptions, getServerOptions } from './http_tools';
 import { adoptToHapiAuthFormat, AuthenticationHandler } from './lifecycle/auth';
 import { adoptToHapiOnPostAuthFormat, OnPostAuthHandler } from './lifecycle/on_post_auth';
 import { adoptToHapiOnPreAuthFormat, OnPreAuthHandler } from './lifecycle/on_pre_auth';
+import { adoptToHapiOnPreResponseFormat, OnPreResponseHandler } from './lifecycle/on_pre_response';
 
-import { ResponseHeaders, IRouter } from './router';
+import { IRouter } from './router';
 import {
   SessionStorageCookieOptions,
   createCookieSessionStorageFactory,
@@ -49,6 +50,7 @@ export interface HttpServerSetup {
   registerAuth: HttpServiceSetup['registerAuth'];
   registerOnPreAuth: HttpServiceSetup['registerOnPreAuth'];
   registerOnPostAuth: HttpServiceSetup['registerOnPostAuth'];
+  registerOnPreResponse: HttpServiceSetup['registerOnPreResponse'];
   isTlsEnabled: HttpServiceSetup['isTlsEnabled'];
   auth: {
     get: GetAuthState;
@@ -101,6 +103,7 @@ export class HttpServer {
       registerRouter: this.registerRouter.bind(this),
       registerOnPreAuth: this.registerOnPreAuth.bind(this),
       registerOnPostAuth: this.registerOnPostAuth.bind(this),
+      registerOnPreResponse: this.registerOnPreResponse.bind(this),
       createCookieSessionStorageFactory: <T>(cookieOptions: SessionStorageCookieOptions<T>) =>
         this.createCookieSessionStorageFactory(cookieOptions, config.basePath),
       registerAuth: this.registerAuth.bind(this),
@@ -203,6 +206,14 @@ export class HttpServer {
     this.server.ext('onRequest', adoptToHapiOnPreAuthFormat(fn, this.log));
   }
 
+  private registerOnPreResponse(fn: OnPreResponseHandler) {
+    if (this.server === undefined) {
+      throw new Error('Server is not created yet');
+    }
+
+    this.server.ext('onPreResponse', adoptToHapiOnPreResponseFormat(fn, this.log));
+  }
+
   private async createCookieSessionStorageFactory<T>(
     cookieOptions: SessionStorageCookieOptions<T>,
     basePath?: string
@@ -260,39 +271,9 @@ export class HttpServer {
     // https://github.com/hapijs/hapi/blob/master/API.md#-serverauthdefaultoptions
     this.server.auth.default('session');
 
-    this.server.ext('onPreResponse', (request, t) => {
+    this.registerOnPreResponse((request, responseInfo, t) => {
       const authResponseHeaders = this.authResponseHeaders.get(request);
-      this.extendResponseWithHeaders(request, authResponseHeaders);
-      return t.continue;
-    });
-  }
-
-  private extendResponseWithHeaders(request: Request, headers?: ResponseHeaders) {
-    const response = request.response;
-    if (!headers || !response) return;
-
-    if (response instanceof Error) {
-      this.findHeadersIntersection(response.output.headers, headers);
-      // hapi wraps all error response in Boom object internally
-      response.output.headers = {
-        ...response.output.headers,
-        ...(headers as any), // hapi types don't specify string[] as valid value
-      };
-    } else {
-      for (const [headerName, headerValue] of Object.entries(headers)) {
-        this.findHeadersIntersection(response.headers, headers);
-        response.header(headerName, headerValue as any); // hapi types don't specify string[] as valid value
-      }
-    }
-  }
-
-  // NOTE: responseHeaders contains not a full list of response headers, but only explicitly set on a response object.
-  // any headers added by hapi internally, like `content-type`, `content-length`, etc. do not present here.
-  private findHeadersIntersection(responseHeaders: ResponseHeaders, headers: ResponseHeaders) {
-    Object.keys(headers).forEach(headerName => {
-      if (responseHeaders[headerName] !== undefined) {
-        this.log.warn(`Server rewrites a response header [${headerName}].`);
-      }
+      return t.next({ headers: authResponseHeaders });
     });
   }
 }

src/core/server/http/http_service.mock.ts

@@ -25,6 +25,7 @@ import { AuthToolkit } from './lifecycle/auth';
 import { sessionStorageMock } from './cookie_session_storage.mocks';
 import { IRouter } from './router';
 import { OnPostAuthToolkit } from './lifecycle/on_post_auth';
+import { OnPreResponseToolkit } from './lifecycle/on_pre_response';
 
 type ServiceSetupMockType = jest.Mocked<InternalHttpServiceSetup> & {
   basePath: jest.Mocked<InternalHttpServiceSetup['basePath']>;
@@ -62,6 +63,7 @@ const createSetupContractMock = () => {
     registerAuth: jest.fn(),
     registerOnPostAuth: jest.fn(),
     registerRouteHandlerContext: jest.fn(),
+    registerOnPreResponse: jest.fn(),
     createRouter: jest.fn(),
     basePath: createBasePathMock(),
     auth: {
@@ -103,12 +105,17 @@ const createAuthToolkitMock = (): jest.Mocked<AuthToolkit> => ({
   authenticated: jest.fn(),
 });
 
+const createOnPreResponseToolkitMock = (): jest.Mocked<OnPreResponseToolkit> => ({
+  next: jest.fn(),
+});
+
 export const httpServiceMock = {
   create: createHttpServiceMock,
   createBasePath: createBasePathMock,
   createSetupContract: createSetupContractMock,
   createOnPreAuthToolkit: createOnPreAuthToolkitMock,
   createOnPostAuthToolkit: createOnPostAuthToolkitMock,
+  createOnPreResponseToolkit: createOnPreResponseToolkitMock,
   createAuthToolkit: createAuthToolkitMock,
   createRouter: createRouterMock,
 };

src/core/server/http/index.ts

@@ -64,6 +64,7 @@ export {
   AuthResultType,
 } from './lifecycle/auth';
 export { OnPostAuthHandler, OnPostAuthToolkit } from './lifecycle/on_post_auth';
+export { OnPreResponseHandler, OnPreResponseToolkit } from './lifecycle/on_pre_response';
 export { SessionStorageFactory, SessionStorage } from './session_storage';
 export {
   SessionStorageCookieOptions,

src/core/server/http/integration_tests/lifecycle.test.ts

@@ -913,3 +913,157 @@ describe('Auth', () => {
       .expect(200, { customField: 'undefined' });
   });
 });
+
+describe('OnPreResponse', () => {
+  it('supports registering response inceptors', async () => {
+    const { registerOnPreResponse, server: innerServer, createRouter } = await server.setup(
+      setupDeps
+    );
+    const router = createRouter('/');
+
+    router.get({ path: '/', validate: false }, (context, req, res) => res.ok({ body: 'ok' }));
+
+    const callingOrder: string[] = [];
+    registerOnPreResponse((req, res, t) => {
+      callingOrder.push('first');
+      return t.next();
+    });
+
+    registerOnPreResponse((req, res, t) => {
+      callingOrder.push('second');
+      return t.next();
+    });
+    await server.start();
+
+    await supertest(innerServer.listener)
+      .get('/')
+      .expect(200, 'ok');
+
+    expect(callingOrder).toEqual(['first', 'second']);
+  });
+
+  it('supports additional headers attachments', async () => {
+    const { registerOnPreResponse, server: innerServer, createRouter } = await server.setup(
+      setupDeps
+    );
+    const router = createRouter('/');
+
+    router.get({ path: '/', validate: false }, (context, req, res) => res.ok());
+
+    registerOnPreResponse((req, res, t) =>
+      t.next({
+        headers: {
+          'x-kibana-header': 'value',
+        },
+      })
+    );
+    await server.start();
+
+    const result = await supertest(innerServer.listener)
+      .get('/')
+      .expect(200);
+
+    expect(result.header['x-kibana-header']).toBe('value');
+  });
+
+  it('logs a warning if interceptor rewrites response header', async () => {
+    const { registerOnPreResponse, server: innerServer, createRouter } = await server.setup(
+      setupDeps
+    );
+    const router = createRouter('/');
+
+    router.get({ path: '/', validate: false }, (context, req, res) =>
+      res.ok({
+        headers: { 'x-kibana-header': 'value' },
+      })
+    );
+    registerOnPreResponse((req, res, t) =>
+      t.next({
+        headers: { 'x-kibana-header': 'value' },
+      })
+    );
+    await server.start();
+
+    await supertest(innerServer.listener)
+      .get('/')
+      .expect(200);
+
+    expect(loggingServiceMock.collect(logger).warn).toMatchInlineSnapshot(`
+      Array [
+        Array [
+          "Server rewrites a response header [x-kibana-header].",
+        ],
+      ]
+    `);
+  });
+
+  it("doesn't expose error details if interceptor throws", async () => {
+    const { registerOnPreResponse, server: innerServer, createRouter } = await server.setup(
+      setupDeps
+    );
+    const router = createRouter('/');
+
+    router.get({ path: '/', validate: false }, (context, req, res) => res.ok(undefined));
+    registerOnPreResponse((req, res, t) => {
+      throw new Error('reason');
+    });
+    await server.start();
+
+    const result = await supertest(innerServer.listener)
+      .get('/')
+      .expect(500);
+
+    expect(result.body.message).toBe('An internal server error occurred.');
+    expect(loggingServiceMock.collect(logger).error).toMatchInlineSnapshot(`
+      Array [
+        Array [
+          [Error: reason],
+        ],
+      ]
+    `);
+  });
+
+  it('returns internal error if interceptor returns unexpected result', async () => {
+    const { registerOnPreResponse, server: innerServer, createRouter } = await server.setup(
+      setupDeps
+    );
+    const router = createRouter('/');
+
+    router.get({ path: '/', validate: false }, (context, req, res) => res.ok());
+    registerOnPreResponse((req, res, t) => ({} as any));
+    await server.start();
+
+    const result = await supertest(innerServer.listener)
+      .get('/')
+      .expect(500);
+
+    expect(result.body.message).toBe('An internal server error occurred.');
+    expect(loggingServiceMock.collect(logger).error).toMatchInlineSnapshot(`
+      Array [
+        Array [
+          [Error: Unexpected result from OnPreResponse. Expected OnPreResponseResult, but given: [object Object].],
+        ],
+      ]
+    `);
+  });
+
+  it('cannot change response statusCode', async () => {
+    const { registerOnPreResponse, server: innerServer, createRouter } = await server.setup(
+      setupDeps
+    );
+    const router = createRouter('/');
+
+    registerOnPreResponse((req, res, t) => {
+      res.statusCode = 500;
+      return t.next();
+    });
+
+    router.get({ path: '/', validate: false }, (context, req, res) => res.ok({ body: 'ok' }));
+
+    await server.start();
+
+    await supertest(innerServer.listener)
+      .get('/')
+      .expect(200);
+  });
+});