From 89a1eff4669dbf5c871fee05abd90a4a423fa011 Mon Sep 17 00:00:00 2001 From: Mikhail Shustov Date: Wed, 26 Feb 2020 13:04:45 +0100 Subject: [PATCH 01/29] add authRequred: 'optional' --- src/core/server/http/http_server.ts | 27 ++++++++++-- src/core/server/http/lifecycle/auth.ts | 58 ++++++++++++++++++++++---- src/core/server/http/router/request.ts | 27 +++++++++++- src/core/server/http/router/route.ts | 2 +- 4 files changed, 99 insertions(+), 15 deletions(-) diff --git a/src/core/server/http/http_server.ts b/src/core/server/http/http_server.ts index 025ab2bf56ac2d..860e8feaa9c727 100644 --- a/src/core/server/http/http_server.ts +++ b/src/core/server/http/http_server.ts @@ -27,7 +27,7 @@ import { adoptToHapiOnPostAuthFormat, OnPostAuthHandler } from './lifecycle/on_p import { adoptToHapiOnPreAuthFormat, OnPreAuthHandler } from './lifecycle/on_pre_auth'; import { adoptToHapiOnPreResponseFormat, OnPreResponseHandler } from './lifecycle/on_pre_response'; -import { IRouter } from './router'; +import { IRouter, RouteConfigOptions } from './router'; import { SessionStorageCookieOptions, createCookieSessionStorageFactory, @@ -148,15 +148,15 @@ export class HttpServer { this.log.debug(`registering route handler for [${route.path}]`); // Hapi does not allow payload validation to be specified for 'head' or 'get' requests const validate = ['head', 'get'].includes(route.method) ? undefined : { payload: true }; - const { authRequired = true, tags, body = {} } = route.options; + const { authRequired, tags, body = {} } = route.options; const { accepts: allow, maxBytes, output, parse } = body; + this.server.route({ handler: route.handler, method: route.method, path: route.path, options: { - // Enforcing the comparison with true because plugins could overwrite the auth strategy by doing `options: { authRequired: authStrategy as any }` - auth: authRequired === true ? undefined : false, + auth: this.getAuthOption(authRequired), tags: tags ? Array.from(tags) : undefined, // TODO: This 'validate' section can be removed once the legacy platform is completely removed. // We are telling Hapi that NP routes can accept any payload, so that it can bypass the default @@ -190,6 +190,20 @@ export class HttpServer { this.server = undefined; } + private getAuthOption( + authRequired: RouteConfigOptions['authRequired'] = true + ): false | { mode: 'required' | 'optional' } { + if (this.authRegistered) { + if (authRequired === true) { + return { mode: 'required' }; + } + if (authRequired === 'optional') { + return { mode: 'optional' }; + } + } + return false; + } + private setupBasePathRewrite(config: HttpConfig, basePathService: BasePath) { if (config.basePath === undefined || !config.rewriteBasePath) { return; @@ -303,6 +317,11 @@ export class HttpServer { // where some plugin read directly from headers to identify whether a user is authenticated. Object.assign(req.headers, requestHeaders); } + }, + (req, { responseHeaders }) => { + if (responseHeaders) { + this.authResponseHeaders.set(req, responseHeaders); + } } ), })); diff --git a/src/core/server/http/lifecycle/auth.ts b/src/core/server/http/lifecycle/auth.ts index 036ab0211c2ff5..98ccf8b586e3e5 100644 --- a/src/core/server/http/lifecycle/auth.ts +++ b/src/core/server/http/lifecycle/auth.ts @@ -30,6 +30,7 @@ import { /** @public */ export enum AuthResultType { authenticated = 'authenticated', + notHandled = 'notHandled', } /** @public */ @@ -38,10 +39,15 @@ export interface Authenticated extends AuthResultParams { } /** @public */ -export type AuthResult = Authenticated; +export interface AuthNotHandled extends AuthNotHandledResultParams { + type: AuthResultType.notHandled; +} + +/** @public */ +export type AuthResult = Authenticated | AuthNotHandled; const authResult = { - authenticated(data: Partial = {}): AuthResult { + authenticated(data: AuthResultParams = {}): AuthResult { return { type: AuthResultType.authenticated, state: data.state, @@ -49,9 +55,18 @@ const authResult = { responseHeaders: data.responseHeaders, }; }, + notHandled(data: AuthNotHandledResultParams = {}): AuthResult { + return { + type: AuthResultType.notHandled, + responseHeaders: data.responseHeaders, + }; + }, isAuthenticated(result: AuthResult): result is Authenticated { return result && result.type === AuthResultType.authenticated; }, + isNotHandled(result: AuthResult): result is AuthNotHandled { + return result && result.type === AuthResultType.notHandled; + }, }; /** @@ -82,6 +97,14 @@ export interface AuthResultParams { responseHeaders?: AuthHeaders; } +interface AuthNotHandledResultParams { + /** + * Auth specific headers to attach to a response object. + * Used to send back authentication mechanism related headers to a client when needed. + */ + responseHeaders?: AuthHeaders; +} + /** * @public * A tool set defining an outcome of Auth interceptor for incoming request. @@ -89,10 +112,12 @@ export interface AuthResultParams { export interface AuthToolkit { /** Authentication is successful with given credentials, allow request to pass through */ authenticated: (data?: AuthResultParams) => AuthResult; + notHandled: (data?: AuthNotHandledResultParams) => AuthResult; } const toolkit: AuthToolkit = { authenticated: authResult.authenticated, + notHandled: authResult.notHandled, }; /** @@ -109,30 +134,45 @@ export type AuthenticationHandler = ( export function adoptToHapiAuthFormat( fn: AuthenticationHandler, log: Logger, - onSuccess: (req: Request, data: AuthResultParams) => void = () => undefined + onAuth: (request: Request, data: AuthResultParams) => void = () => undefined, + onNotHandled: (request: Request, data: AuthNotHandledResultParams) => void = () => undefined ) { return async function interceptAuth( request: Request, responseToolkit: ResponseToolkit ): Promise { const hapiResponseAdapter = new HapiResponseAdapter(responseToolkit); + const kibanaRequest = KibanaRequest.from(request, undefined, false); + try { - const result = await fn( - KibanaRequest.from(request, undefined, false), - lifecycleResponseFactory, - toolkit - ); + const result = await fn(kibanaRequest, lifecycleResponseFactory, toolkit); + if (isKibanaResponse(result)) { return hapiResponseAdapter.handle(result); } if (authResult.isAuthenticated(result)) { - onSuccess(request, { + onAuth(request, { state: result.state, requestHeaders: result.requestHeaders, responseHeaders: result.responseHeaders, }); return responseToolkit.authenticated({ credentials: result.state || {} }); } + + if (authResult.isNotHandled(result)) { + onNotHandled(request, { + responseHeaders: result.responseHeaders, + }); + if (kibanaRequest.route.options.authRequired === 'optional') { + return responseToolkit.continue; + } + if (kibanaRequest.route.options.authRequired) { + return hapiResponseAdapter.handle(lifecycleResponseFactory.unauthorized()); + } + throw new Error( + `Unexpected route.options.authRequired value in AuthenticationHandler. Expected 'optional' or true, but given: ${result}.` + ); + } throw new Error( `Unexpected result from Authenticate. Expected AuthResult or KibanaResponse, but given: ${result}.` ); diff --git a/src/core/server/http/router/request.ts b/src/core/server/http/router/request.ts index 703571ba53c0a3..903a259853dc7a 100644 --- a/src/core/server/http/router/request.ts +++ b/src/core/server/http/router/request.ts @@ -183,7 +183,7 @@ export class KibanaRequest< const { parse, maxBytes, allow, output } = request.route.settings.payload || {}; const options = ({ - authRequired: request.route.settings.auth !== false, + authRequired: this.getAuthRequired(request), tags: request.route.settings.tags || [], body: ['get', 'options'].includes(method) ? undefined @@ -201,6 +201,31 @@ export class KibanaRequest< options, }; } + + private getAuthRequired(request: Request): boolean | 'optional' { + const authOptions = request.route.settings.auth; + if (typeof authOptions === 'object') { + // 'try' is used in the legacy platform + if (authOptions.mode === 'optional' || authOptions.mode === 'try') { + return 'optional'; + } + if (authOptions.mode === 'required') { + return true; + } + } + + // legacy platform routes + if (authOptions === undefined) { + return true; + } + + if (authOptions === false) return false; + throw new Error( + `unexpected authentication options: ${JSON.stringify(authOptions)} for route: ${ + this.url.href + }` + ); + } } /** diff --git a/src/core/server/http/router/route.ts b/src/core/server/http/router/route.ts index 4439a80b1eac71..be5d47729275d2 100644 --- a/src/core/server/http/router/route.ts +++ b/src/core/server/http/router/route.ts @@ -106,7 +106,7 @@ export interface RouteConfigOptions { * * Enabled by default. */ - authRequired?: boolean; + authRequired?: boolean | 'optional'; /** * Additional metadata tag strings to attach to the route. From 0f3a91568ecadf85dbe700e43cfe6702670d520e Mon Sep 17 00:00:00 2001 From: Mikhail Shustov Date: Wed, 26 Feb 2020 13:06:24 +0100 Subject: [PATCH 02/29] expose auth status via request context --- src/core/server/server.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/core/server/server.ts b/src/core/server/server.ts index db2493b38d6e0a..8482dbdd145240 100644 --- a/src/core/server/server.ts +++ b/src/core/server/server.ts @@ -225,6 +225,9 @@ export class Server { const uiSettingsClient = coreSetup.uiSettings.asScopedToClient(savedObjectsClient); return { + auth: { + isAuthenticated: () => coreSetup.http.auth.isAuthenticated(req), + }, rendering: { render: async (options = {}) => rendering.render(req, uiSettingsClient, { From 05809a480b172e0f6e550069af6b5621f5489358 Mon Sep 17 00:00:00 2001 From: Mikhail Shustov Date: Wed, 26 Feb 2020 13:06:59 +0100 Subject: [PATCH 03/29] update security plugin to use notHandled auth outcome --- .../plugins/security/server/authentication/index.test.ts | 7 ++----- x-pack/plugins/security/server/authentication/index.ts | 4 ++-- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/security/server/authentication/index.test.ts b/x-pack/plugins/security/server/authentication/index.test.ts index 3727b1fc13dac9..84df922c429e70 100644 --- a/x-pack/plugins/security/server/authentication/index.test.ts +++ b/x-pack/plugins/security/server/authentication/index.test.ts @@ -267,16 +267,13 @@ describe('setupAuthentication()', () => { expect(mockResponse.redirected).not.toHaveBeenCalled(); }); - it('returns `unauthorized` when authentication can not be handled', async () => { + it('returns `notHandled` when authentication can not be handled', async () => { const mockResponse = httpServerMock.createLifecycleResponseFactory(); authenticate.mockResolvedValue(AuthenticationResult.notHandled()); await authHandler(httpServerMock.createKibanaRequest(), mockResponse, mockAuthToolkit); - expect(mockResponse.unauthorized).toHaveBeenCalledTimes(1); - const [[response]] = mockResponse.unauthorized.mock.calls; - - expect(response!.body).toBeUndefined(); + expect(mockAuthToolkit.notHandled).toHaveBeenCalledTimes(1); expect(mockAuthToolkit.authenticated).not.toHaveBeenCalled(); expect(mockResponse.redirected).not.toHaveBeenCalled(); diff --git a/x-pack/plugins/security/server/authentication/index.ts b/x-pack/plugins/security/server/authentication/index.ts index 467afe00340259..49f86ae82ef3c8 100644 --- a/x-pack/plugins/security/server/authentication/index.ts +++ b/x-pack/plugins/security/server/authentication/index.ts @@ -153,8 +153,8 @@ export async function setupAuthentication({ } authLogger.debug('Could not handle authentication attempt'); - return response.unauthorized({ - headers: authenticationResult.authResponseHeaders, + return t.notHandled({ + responseHeaders: authenticationResult.authResponseHeaders, }); }); From fc94baa2d649c6fffd731a1a2f7dd52a2b0324ca Mon Sep 17 00:00:00 2001 From: Mikhail Shustov Date: Wed, 26 Feb 2020 13:07:26 +0100 Subject: [PATCH 04/29] capabilities service uses optional auth --- .../capabilities/capabilities_service.tsx | 3 +- .../capabilities/capabilities_service.test.ts | 4 +- src/core/server/capabilities/routes/index.ts | 2 +- .../routes/resolve_capabilities.ts | 44 ++++++++----------- 4 files changed, 23 insertions(+), 30 deletions(-) diff --git a/src/core/public/application/capabilities/capabilities_service.tsx b/src/core/public/application/capabilities/capabilities_service.tsx index 05d718e1073dfa..d602422c146348 100644 --- a/src/core/public/application/capabilities/capabilities_service.tsx +++ b/src/core/public/application/capabilities/capabilities_service.tsx @@ -37,8 +37,7 @@ export interface CapabilitiesStart { */ export class CapabilitiesService { public async start({ appIds, http }: StartDeps): Promise { - const route = http.anonymousPaths.isAnonymous(window.location.pathname) ? '/defaults' : ''; - const capabilities = await http.post(`/api/core/capabilities${route}`, { + const capabilities = await http.post('/api/core/capabilities', { body: JSON.stringify({ applications: appIds }), }); diff --git a/src/core/server/capabilities/capabilities_service.test.ts b/src/core/server/capabilities/capabilities_service.test.ts index aace0b9debf9c0..7d2e7391aa8d4b 100644 --- a/src/core/server/capabilities/capabilities_service.test.ts +++ b/src/core/server/capabilities/capabilities_service.test.ts @@ -41,8 +41,8 @@ describe('CapabilitiesService', () => { }); it('registers the capabilities routes', async () => { - expect(http.createRouter).toHaveBeenCalledWith('/api/core/capabilities'); - expect(router.post).toHaveBeenCalledTimes(2); + expect(http.createRouter).toHaveBeenCalledWith(''); + expect(router.post).toHaveBeenCalledTimes(1); expect(router.post).toHaveBeenCalledWith(expect.any(Object), expect.any(Function)); }); diff --git a/src/core/server/capabilities/routes/index.ts b/src/core/server/capabilities/routes/index.ts index ccaa4621d70035..74c485986a77b2 100644 --- a/src/core/server/capabilities/routes/index.ts +++ b/src/core/server/capabilities/routes/index.ts @@ -22,6 +22,6 @@ import { InternalHttpServiceSetup } from '../../http'; import { registerCapabilitiesRoutes } from './resolve_capabilities'; export function registerRoutes(http: InternalHttpServiceSetup, resolver: CapabilitiesResolver) { - const router = http.createRouter('/api/core/capabilities'); + const router = http.createRouter(''); registerCapabilitiesRoutes(router, resolver); } diff --git a/src/core/server/capabilities/routes/resolve_capabilities.ts b/src/core/server/capabilities/routes/resolve_capabilities.ts index 5e1d49b4b1b7e8..3fb1bb3d13d0bf 100644 --- a/src/core/server/capabilities/routes/resolve_capabilities.ts +++ b/src/core/server/capabilities/routes/resolve_capabilities.ts @@ -22,30 +22,24 @@ import { IRouter } from '../../http'; import { CapabilitiesResolver } from '../resolve_capabilities'; export function registerCapabilitiesRoutes(router: IRouter, resolver: CapabilitiesResolver) { - // Capabilities are fetched on both authenticated and anonymous routes. - // However when `authRequired` is false, authentication is not performed - // and only default capabilities are returned (all disabled), even for authenticated users. - // So we need two endpoints to handle both scenarios. - [true, false].forEach(authRequired => { - router.post( - { - path: authRequired ? '' : '/defaults', - options: { - authRequired, - }, - validate: { - body: schema.object({ - applications: schema.arrayOf(schema.string()), - }), - }, + router.post( + { + path: '/api/core/capabilities', + options: { + authRequired: 'optional', }, - async (ctx, req, res) => { - const { applications } = req.body; - const capabilities = await resolver(req, applications); - return res.ok({ - body: capabilities, - }); - } - ); - }); + validate: { + body: schema.object({ + applications: schema.arrayOf(schema.string()), + }), + }, + }, + async (ctx, req, res) => { + const { applications } = req.body; + const capabilities = await resolver(req, applications); + return res.ok({ + body: capabilities, + }); + } + ); } From de22f7e0f5fa306e42903f0f4497f5880b963d82 Mon Sep 17 00:00:00 2001 From: Mikhail Shustov Date: Wed, 26 Feb 2020 13:07:43 +0100 Subject: [PATCH 05/29] update tests --- src/core/server/http/http_server.test.ts | 4 +- src/core/server/http/http_service.mock.ts | 1 + .../http/integration_tests/lifecycle.test.ts | 40 ++++++- .../http/integration_tests/router.test.ts | 108 ++++++++++++++++++ src/core/server/http/router/request.test.ts | 86 ++++++++++++++ src/core/server/index.ts | 5 +- src/core/server/mocks.ts | 3 + 7 files changed, 243 insertions(+), 4 deletions(-) diff --git a/src/core/server/http/http_server.test.ts b/src/core/server/http/http_server.test.ts index a9fc80c86d878e..296d95e56c42a2 100644 --- a/src/core/server/http/http_server.test.ts +++ b/src/core/server/http/http_server.test.ts @@ -810,7 +810,7 @@ test('exposes route details of incoming request to a route handler', async () => method: 'get', path: '/', options: { - authRequired: true, + authRequired: false, tags: [], }, }); @@ -922,7 +922,7 @@ test('exposes route details of incoming request to a route handler (POST + paylo method: 'post', path: '/', options: { - authRequired: true, + authRequired: false, tags: [], body: { parse: true, // hapi populates the default diff --git a/src/core/server/http/http_service.mock.ts b/src/core/server/http/http_service.mock.ts index 30032ff5da7968..c87b893f5bb932 100644 --- a/src/core/server/http/http_service.mock.ts +++ b/src/core/server/http/http_service.mock.ts @@ -115,6 +115,7 @@ const createOnPostAuthToolkitMock = (): jest.Mocked => ({ const createAuthToolkitMock = (): jest.Mocked => ({ authenticated: jest.fn(), + notHandled: jest.fn(), }); const createOnPreResponseToolkitMock = (): jest.Mocked => ({ diff --git a/src/core/server/http/integration_tests/lifecycle.test.ts b/src/core/server/http/integration_tests/lifecycle.test.ts index 6dc7ece1359df7..caa8c75b9b9d3b 100644 --- a/src/core/server/http/integration_tests/lifecycle.test.ts +++ b/src/core/server/http/integration_tests/lifecycle.test.ts @@ -415,6 +415,23 @@ describe('Auth', () => { .expect(200, { content: 'ok' }); }); + it('blocks access to a resource if credentials are not recognized', async () => { + const { registerAuth, server: innerServer, createRouter } = await server.setup(setupDeps); + const router = createRouter('/'); + + router.get({ path: '/', validate: false }, (context, req, res) => + res.ok({ body: { content: 'ok' } }) + ); + registerAuth((req, res, t) => t.notHandled()); + await server.start(); + + const result = await supertest(innerServer.listener) + .get('/') + .expect(401); + + expect(result.body.message).toBe('Unauthorized'); + }); + it('enables auth for a route by default if registerAuth has been called', async () => { const { registerAuth, server: innerServer, createRouter } = await server.setup(setupDeps); const router = createRouter('/'); @@ -684,6 +701,27 @@ describe('Auth', () => { expect(response.header['www-authenticate']).toBe(authResponseHeader['www-authenticate']); }); + it('attach security header to not handled auth response', async () => { + const { registerAuth, server: innerServer, createRouter } = await server.setup(setupDeps); + const router = createRouter('/'); + + const authResponseHeader = { + 'www-authenticate': 'from auth interceptor', + }; + registerAuth((req, res, toolkit) => { + return toolkit.notHandled({ responseHeaders: authResponseHeader }); + }); + + router.get({ path: '/', validate: false }, (context, req, res) => res.ok()); + await server.start(); + + const response = await supertest(innerServer.listener) + .get('/') + .expect(401); + + expect(response.header['www-authenticate']).toBe(authResponseHeader['www-authenticate']); + }); + it('attach security header to an error response', async () => { const { registerAuth, server: innerServer, createRouter } = await server.setup(setupDeps); const router = createRouter('/'); @@ -865,7 +903,7 @@ describe('Auth', () => { ] `); }); - // eslint-disable-next-line + it(`doesn't share request object between interceptors`, async () => { const { registerOnPostAuth, server: innerServer, createRouter } = await server.setup(setupDeps); const router = createRouter('/'); diff --git a/src/core/server/http/integration_tests/router.test.ts b/src/core/server/http/integration_tests/router.test.ts index a1523781010d47..485844772eeba9 100644 --- a/src/core/server/http/integration_tests/router.test.ts +++ b/src/core/server/http/integration_tests/router.test.ts @@ -46,6 +46,114 @@ afterEach(async () => { await server.stop(); }); +describe('Options', () => { + describe('authRequired', () => { + describe('optional', () => { + it('User has access to a route if auth mechanism not registered', async () => { + const { server: innerServer, createRouter } = await server.setup(setupDeps); + const router = createRouter('/'); + + router.get( + { path: '/', validate: false, options: { authRequired: 'optional' } }, + (context, req, res) => res.ok({ body: 'ok' }) + ); + await server.start(); + + await supertest(innerServer.listener) + .get('/') + .expect(200, 'ok'); + }); + + it('Authenticated user has access to a route', async () => { + const { server: innerServer, createRouter, registerAuth } = await server.setup(setupDeps); + const router = createRouter('/'); + + registerAuth((req, res, toolkit) => { + return toolkit.authenticated(); + }); + router.get( + { path: '/', validate: false, options: { authRequired: 'optional' } }, + (context, req, res) => res.ok({ body: 'ok' }) + ); + await server.start(); + + await supertest(innerServer.listener) + .get('/') + .expect(200, 'ok'); + }); + + it('User with not recognized credentials has access to a route', async () => { + const { server: innerServer, createRouter, registerAuth } = await server.setup(setupDeps); + const router = createRouter('/'); + const authResponseHeader = { + 'www-authenticate': 'from auth interceptor', + }; + + registerAuth((req, res, toolkit) => { + return toolkit.notHandled({ + responseHeaders: authResponseHeader, + }); + }); + router.get( + { path: '/', validate: false, options: { authRequired: 'optional' } }, + (context, req, res) => res.ok({ body: 'ok' }) + ); + await server.start(); + + const response = await supertest(innerServer.listener) + .get('/') + .expect(200, 'ok'); + + expect(response.header['www-authenticate']).toBe(authResponseHeader['www-authenticate']); + }); + }); + describe('true', () => { + it('Authenticated user has access to a route', async () => { + const { server: innerServer, createRouter, registerAuth } = await server.setup(setupDeps); + const router = createRouter('/'); + + registerAuth((req, res, toolkit) => { + return toolkit.authenticated(); + }); + router.get( + { path: '/', validate: false, options: { authRequired: 'optional' } }, + (context, req, res) => res.ok({ body: 'ok' }) + ); + await server.start(); + + await supertest(innerServer.listener) + .get('/') + .expect(200, 'ok'); + }); + + it('User with not recognized credentials has no access to a route', async () => { + const { server: innerServer, createRouter, registerAuth } = await server.setup(setupDeps); + const router = createRouter('/'); + const authResponseHeader = { + 'www-authenticate': 'from auth interceptor', + }; + + registerAuth((req, res, toolkit) => { + return toolkit.notHandled({ + responseHeaders: authResponseHeader, + }); + }); + router.get( + { path: '/', validate: false, options: { authRequired: true } }, + (context, req, res) => res.ok({ body: 'ok' }) + ); + await server.start(); + + const response = await supertest(innerServer.listener) + .get('/') + .expect(401); + + expect(response.header['www-authenticate']).toBe(authResponseHeader['www-authenticate']); + }); + }); + }); +}); + describe('Handler', () => { it("Doesn't expose error details if handler throws", async () => { const { server: innerServer, createRouter } = await server.setup(setupDeps); diff --git a/src/core/server/http/router/request.test.ts b/src/core/server/http/router/request.test.ts index 032027c2344858..e3f67401d7c63d 100644 --- a/src/core/server/http/router/request.test.ts +++ b/src/core/server/http/router/request.test.ts @@ -16,6 +16,7 @@ * specific language governing permissions and limitations * under the License. */ +import { RouteOptions } from 'hapi'; import { KibanaRequest } from './request'; import { httpServerMock } from '../http_server.mocks'; import { schema } from '@kbn/config-schema'; @@ -117,6 +118,91 @@ describe('KibanaRequest', () => { }); }); + describe('route.options.authRequired property', () => { + it('handles required auth: undefined', () => { + const auth: RouteOptions['auth'] = undefined; + const request = httpServerMock.createRawRequest({ + route: { + settings: { + auth, + }, + }, + }); + const kibanaRequest = KibanaRequest.from(request); + + expect(kibanaRequest.route.options.authRequired).toBe(true); + }); + it('handles required auth: false', () => { + const auth: RouteOptions['auth'] = false; + const request = httpServerMock.createRawRequest({ + route: { + settings: { + auth, + }, + }, + }); + const kibanaRequest = KibanaRequest.from(request); + + expect(kibanaRequest.route.options.authRequired).toBe(false); + }); + it('handles required auth: { mode: "required" }', () => { + const auth: RouteOptions['auth'] = { mode: 'required' }; + const request = httpServerMock.createRawRequest({ + route: { + settings: { + auth, + }, + }, + }); + const kibanaRequest = KibanaRequest.from(request); + + expect(kibanaRequest.route.options.authRequired).toBe(true); + }); + it('handles required auth: { mode: "optional" }', () => { + const auth: RouteOptions['auth'] = { mode: 'optional' }; + const request = httpServerMock.createRawRequest({ + route: { + settings: { + auth, + }, + }, + }); + const kibanaRequest = KibanaRequest.from(request); + + expect(kibanaRequest.route.options.authRequired).toBe('optional'); + }); + + it('throws on auth: strategy name', () => { + const auth: RouteOptions['auth'] = 'session'; + const request = httpServerMock.createRawRequest({ + route: { + settings: { + auth, + }, + }, + }); + + expect(() => KibanaRequest.from(request)).toThrowErrorMatchingInlineSnapshot( + `"unexpected authentication options: \\"session\\" for route: /"` + ); + }); + + it('throws on auth: { mode: unexpected mode }', () => { + const auth: RouteOptions['auth'] = { mode: undefined }; + const request = httpServerMock.createRawRequest({ + route: { + settings: { + auth, + }, + }, + }); + + expect(() => KibanaRequest.from(request)).toThrowErrorMatchingInlineSnapshot( + `"unexpected authentication options: {} for route: /"` + ); + }); + }); + describe('RouteSchema type inferring', () => { it('should work with config-schema', () => { const body = Buffer.from('body!'); diff --git a/src/core/server/index.ts b/src/core/server/index.ts index 52827b72ee0cc0..ed3cbb39938e1f 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -45,7 +45,7 @@ import { configSchema as elasticsearchConfigSchema, } from './elasticsearch'; -import { HttpServiceSetup } from './http'; +import { HttpServiceSetup, IsAuthenticated } from './http'; import { IScopedRenderingClient } from './rendering'; import { PluginsServiceSetup, PluginsServiceStart, PluginOpaqueId } from './plugins'; import { ContextSetup } from './context'; @@ -286,6 +286,9 @@ export { */ export interface RequestHandlerContext { core: { + auth: { + isAuthenticated: IsAuthenticated; + }; rendering: IScopedRenderingClient; savedObjects: { client: SavedObjectsClientContract; diff --git a/src/core/server/mocks.ts b/src/core/server/mocks.ts index d6554babab53ea..88eeb10fccef0d 100644 --- a/src/core/server/mocks.ts +++ b/src/core/server/mocks.ts @@ -170,6 +170,9 @@ function createInternalCoreStartMock() { function createCoreRequestHandlerContextMock() { return { + auth: { + isAuthenticated: jest.fn(), + }, rendering: { render: jest.fn(), }, From 006edef0182c17a03ba0cf7915358bd442a5e8e6 Mon Sep 17 00:00:00 2001 From: Mikhail Shustov Date: Wed, 26 Feb 2020 14:47:34 +0100 Subject: [PATCH 06/29] attach security headers only to unauthorised response --- src/core/server/http/integration_tests/router.test.ts | 2 +- src/core/server/http/lifecycle/auth.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/server/http/integration_tests/router.test.ts b/src/core/server/http/integration_tests/router.test.ts index 485844772eeba9..b427a89872f284 100644 --- a/src/core/server/http/integration_tests/router.test.ts +++ b/src/core/server/http/integration_tests/router.test.ts @@ -104,7 +104,7 @@ describe('Options', () => { .get('/') .expect(200, 'ok'); - expect(response.header['www-authenticate']).toBe(authResponseHeader['www-authenticate']); + expect(response.header['www-authenticate']).toBe(undefined); }); }); describe('true', () => { diff --git a/src/core/server/http/lifecycle/auth.ts b/src/core/server/http/lifecycle/auth.ts index 98ccf8b586e3e5..3dfc860685b232 100644 --- a/src/core/server/http/lifecycle/auth.ts +++ b/src/core/server/http/lifecycle/auth.ts @@ -160,13 +160,13 @@ export function adoptToHapiAuthFormat( } if (authResult.isNotHandled(result)) { - onNotHandled(request, { - responseHeaders: result.responseHeaders, - }); if (kibanaRequest.route.options.authRequired === 'optional') { return responseToolkit.continue; } if (kibanaRequest.route.options.authRequired) { + onNotHandled(request, { + responseHeaders: result.responseHeaders, + }); return hapiResponseAdapter.handle(lifecycleResponseFactory.unauthorized()); } throw new Error( From 529eeeed187b1c6eb57cba597c323062c2f00aeb Mon Sep 17 00:00:00 2001 From: Mikhail Shustov Date: Wed, 26 Feb 2020 14:48:17 +0100 Subject: [PATCH 07/29] add isAuthenticated tests for 'optional' auth mode --- .../integration_tests/core_services.test.ts | 39 +++++++++++++++++-- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/src/core/server/http/integration_tests/core_services.test.ts b/src/core/server/http/integration_tests/core_services.test.ts index 425d8cac1893ea..00e54548468e5e 100644 --- a/src/core/server/http/integration_tests/core_services.test.ts +++ b/src/core/server/http/integration_tests/core_services.test.ts @@ -50,7 +50,7 @@ describe('http service', () => { await root.shutdown(); }); describe('#isAuthenticated()', () => { - it('returns true if has been authorized', async () => { + it('returns true if has been authenticated', async () => { const { http } = await root.setup(); const { registerAuth, createRouter, auth } = http; @@ -65,11 +65,11 @@ describe('http service', () => { await kbnTestServer.request.get(root, '/is-auth').expect(200, { isAuthenticated: true }); }); - it('returns false if has not been authorized', async () => { + it('returns false if has not been authenticated', async () => { const { http } = await root.setup(); const { registerAuth, createRouter, auth } = http; - await registerAuth((req, res, toolkit) => toolkit.authenticated()); + registerAuth((req, res, toolkit) => toolkit.authenticated()); const router = createRouter(''); router.get( @@ -81,7 +81,7 @@ describe('http service', () => { await kbnTestServer.request.get(root, '/is-auth').expect(200, { isAuthenticated: false }); }); - it('returns false if no authorization mechanism has been registered', async () => { + it('returns false if no authentication mechanism has been registered', async () => { const { http } = await root.setup(); const { createRouter, auth } = http; @@ -94,6 +94,37 @@ describe('http service', () => { await root.start(); await kbnTestServer.request.get(root, '/is-auth').expect(200, { isAuthenticated: false }); }); + + it('returns true if authenticated on a route with "optional" auth', async () => { + const { http } = await root.setup(); + const { createRouter, auth, registerAuth } = http; + + registerAuth((req, res, toolkit) => toolkit.authenticated()); + const router = createRouter(''); + router.get( + { path: '/is-auth', validate: false, options: { authRequired: 'optional' } }, + (context, req, res) => res.ok({ body: { isAuthenticated: auth.isAuthenticated(req) } }) + ); + + await root.start(); + await kbnTestServer.request.get(root, '/is-auth').expect(200, { isAuthenticated: true }); + }); + + it('returns false if not authenticated on a route with "optional" auth', async () => { + const { http } = await root.setup(); + const { createRouter, auth, registerAuth } = http; + + registerAuth((req, res, toolkit) => toolkit.notHandled()); + + const router = createRouter(''); + router.get( + { path: '/is-auth', validate: false, options: { authRequired: false } }, + (context, req, res) => res.ok({ body: { isAuthenticated: auth.isAuthenticated(req) } }) + ); + + await root.start(); + await kbnTestServer.request.get(root, '/is-auth').expect(200, { isAuthenticated: false }); + }); }); describe('#get()', () => { it('returns authenticated status and allow associate auth state with request', async () => { From 4c9f0bc497398772f162086638bddc725b9b96ea Mon Sep 17 00:00:00 2001 From: Mikhail Shustov Date: Wed, 26 Feb 2020 14:49:15 +0100 Subject: [PATCH 08/29] security plugin relies on http.auth.isAuthenticated to calc capabilities --- x-pack/plugins/security/server/authorization/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/security/server/authorization/index.ts b/x-pack/plugins/security/server/authorization/index.ts index 00a50dd5b8821b..57950940a0f84b 100644 --- a/x-pack/plugins/security/server/authorization/index.ts +++ b/x-pack/plugins/security/server/authorization/index.ts @@ -112,8 +112,8 @@ export function setupAuthorization({ authz ); - // if we're an anonymous route, we disable all ui capabilities - if (request.route.options.authRequired === false) { + // if we're an anonymous route or optional auth, we disable all ui capabilities + if (!http.auth.isAuthenticated(request)) { return disableUICapabilities.all(capabilities); } From a2f254693085039f79693091b52bf45e16625dce Mon Sep 17 00:00:00 2001 From: restrry Date: Wed, 26 Feb 2020 16:11:01 +0100 Subject: [PATCH 09/29] generate docs --- .../kibana-plugin-server.authnothandled.md | 19 +++++++++++++++ ...ibana-plugin-server.authnothandled.type.md | 11 +++++++++ ...lugin-server.authnothandledresultparams.md | 20 ++++++++++++++++ ...hnothandledresultparams.responseheaders.md | 13 +++++++++++ .../server/kibana-plugin-server.authresult.md | 2 +- .../kibana-plugin-server.authresultparams.md | 2 +- .../kibana-plugin-server.authresulttype.md | 1 + .../kibana-plugin-server.authtoolkit.md | 1 + ...na-plugin-server.authtoolkit.nothandled.md | 13 +++++++++++ .../core/server/kibana-plugin-server.md | 4 +++- ...lugin-server.requesthandlercontext.core.md | 3 +++ ...ana-plugin-server.requesthandlercontext.md | 2 +- ...-server.routeconfigoptions.authrequired.md | 6 ++--- ...kibana-plugin-server.routeconfigoptions.md | 2 +- src/core/server/http/index.ts | 2 ++ src/core/server/http/lifecycle/auth.ts | 9 ++++++-- src/core/server/http/router/route.ts | 9 ++++---- src/core/server/index.ts | 2 ++ src/core/server/server.api.md | 23 ++++++++++++++++--- 19 files changed, 127 insertions(+), 17 deletions(-) create mode 100644 docs/development/core/server/kibana-plugin-server.authnothandled.md create mode 100644 docs/development/core/server/kibana-plugin-server.authnothandled.type.md create mode 100644 docs/development/core/server/kibana-plugin-server.authnothandledresultparams.md create mode 100644 docs/development/core/server/kibana-plugin-server.authnothandledresultparams.responseheaders.md create mode 100644 docs/development/core/server/kibana-plugin-server.authtoolkit.nothandled.md diff --git a/docs/development/core/server/kibana-plugin-server.authnothandled.md b/docs/development/core/server/kibana-plugin-server.authnothandled.md new file mode 100644 index 00000000000000..5d4a39a88ea647 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-server.authnothandled.md @@ -0,0 +1,19 @@ + + +[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [AuthNotHandled](./kibana-plugin-server.authnothandled.md) + +## AuthNotHandled interface + + +Signature: + +```typescript +export interface AuthNotHandled extends AuthNotHandledResultParams +``` + +## Properties + +| Property | Type | Description | +| --- | --- | --- | +| [type](./kibana-plugin-server.authnothandled.type.md) | AuthResultType.notHandled | | + diff --git a/docs/development/core/server/kibana-plugin-server.authnothandled.type.md b/docs/development/core/server/kibana-plugin-server.authnothandled.type.md new file mode 100644 index 00000000000000..81543de0ec61b5 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-server.authnothandled.type.md @@ -0,0 +1,11 @@ + + +[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [AuthNotHandled](./kibana-plugin-server.authnothandled.md) > [type](./kibana-plugin-server.authnothandled.type.md) + +## AuthNotHandled.type property + +Signature: + +```typescript +type: AuthResultType.notHandled; +``` diff --git a/docs/development/core/server/kibana-plugin-server.authnothandledresultparams.md b/docs/development/core/server/kibana-plugin-server.authnothandledresultparams.md new file mode 100644 index 00000000000000..90e9f5a771e552 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-server.authnothandledresultparams.md @@ -0,0 +1,20 @@ + + +[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [AuthNotHandledResultParams](./kibana-plugin-server.authnothandledresultparams.md) + +## AuthNotHandledResultParams interface + +Result of unhandled authentication. + +Signature: + +```typescript +export interface AuthNotHandledResultParams +``` + +## Properties + +| Property | Type | Description | +| --- | --- | --- | +| [responseHeaders](./kibana-plugin-server.authnothandledresultparams.responseheaders.md) | AuthHeaders | Auth specific headers to attach to a response object. Used to send back authentication mechanism related headers to a client when needed. | + diff --git a/docs/development/core/server/kibana-plugin-server.authnothandledresultparams.responseheaders.md b/docs/development/core/server/kibana-plugin-server.authnothandledresultparams.responseheaders.md new file mode 100644 index 00000000000000..9e1e733246d16a --- /dev/null +++ b/docs/development/core/server/kibana-plugin-server.authnothandledresultparams.responseheaders.md @@ -0,0 +1,13 @@ + + +[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [AuthNotHandledResultParams](./kibana-plugin-server.authnothandledresultparams.md) > [responseHeaders](./kibana-plugin-server.authnothandledresultparams.responseheaders.md) + +## AuthNotHandledResultParams.responseHeaders property + +Auth specific headers to attach to a response object. Used to send back authentication mechanism related headers to a client when needed. + +Signature: + +```typescript +responseHeaders?: AuthHeaders; +``` diff --git a/docs/development/core/server/kibana-plugin-server.authresult.md b/docs/development/core/server/kibana-plugin-server.authresult.md index 8739c4899bd02a..5dd0e670b682c0 100644 --- a/docs/development/core/server/kibana-plugin-server.authresult.md +++ b/docs/development/core/server/kibana-plugin-server.authresult.md @@ -8,5 +8,5 @@ Signature: ```typescript -export declare type AuthResult = Authenticated; +export declare type AuthResult = Authenticated | AuthNotHandled; ``` diff --git a/docs/development/core/server/kibana-plugin-server.authresultparams.md b/docs/development/core/server/kibana-plugin-server.authresultparams.md index 55b247f21f5a9e..7a725cb340f5bc 100644 --- a/docs/development/core/server/kibana-plugin-server.authresultparams.md +++ b/docs/development/core/server/kibana-plugin-server.authresultparams.md @@ -4,7 +4,7 @@ ## AuthResultParams interface -Result of an incoming request authentication. +Result of successful authentication. Signature: diff --git a/docs/development/core/server/kibana-plugin-server.authresulttype.md b/docs/development/core/server/kibana-plugin-server.authresulttype.md index 61a98ee5e7b110..cf11508bef79d6 100644 --- a/docs/development/core/server/kibana-plugin-server.authresulttype.md +++ b/docs/development/core/server/kibana-plugin-server.authresulttype.md @@ -16,4 +16,5 @@ export declare enum AuthResultType | Member | Value | Description | | --- | --- | --- | | authenticated | "authenticated" | | +| notHandled | "notHandled" | | diff --git a/docs/development/core/server/kibana-plugin-server.authtoolkit.md b/docs/development/core/server/kibana-plugin-server.authtoolkit.md index bc7003c5a68f30..cee59409b79998 100644 --- a/docs/development/core/server/kibana-plugin-server.authtoolkit.md +++ b/docs/development/core/server/kibana-plugin-server.authtoolkit.md @@ -17,4 +17,5 @@ export interface AuthToolkit | Property | Type | Description | | --- | --- | --- | | [authenticated](./kibana-plugin-server.authtoolkit.authenticated.md) | (data?: AuthResultParams) => AuthResult | Authentication is successful with given credentials, allow request to pass through | +| [notHandled](./kibana-plugin-server.authtoolkit.nothandled.md) | (data?: AuthNotHandledResultParams) => AuthResult | User has no credentials | diff --git a/docs/development/core/server/kibana-plugin-server.authtoolkit.nothandled.md b/docs/development/core/server/kibana-plugin-server.authtoolkit.nothandled.md new file mode 100644 index 00000000000000..41c5cb16567d47 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-server.authtoolkit.nothandled.md @@ -0,0 +1,13 @@ + + +[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [AuthToolkit](./kibana-plugin-server.authtoolkit.md) > [notHandled](./kibana-plugin-server.authtoolkit.nothandled.md) + +## AuthToolkit.notHandled property + +User has no credentials + +Signature: + +```typescript +notHandled: (data?: AuthNotHandledResultParams) => AuthResult; +``` diff --git a/docs/development/core/server/kibana-plugin-server.md b/docs/development/core/server/kibana-plugin-server.md index 9ec443d6482e89..119054fd74b875 100644 --- a/docs/development/core/server/kibana-plugin-server.md +++ b/docs/development/core/server/kibana-plugin-server.md @@ -44,7 +44,9 @@ The plugin integrates with the core system via lifecycle events: `setup` | [AssistanceAPIResponse](./kibana-plugin-server.assistanceapiresponse.md) | | | [AssistantAPIClientParams](./kibana-plugin-server.assistantapiclientparams.md) | | | [Authenticated](./kibana-plugin-server.authenticated.md) | | -| [AuthResultParams](./kibana-plugin-server.authresultparams.md) | Result of an incoming request authentication. | +| [AuthNotHandled](./kibana-plugin-server.authnothandled.md) | | +| [AuthNotHandledResultParams](./kibana-plugin-server.authnothandledresultparams.md) | Result of unhandled authentication. | +| [AuthResultParams](./kibana-plugin-server.authresultparams.md) | Result of successful authentication. | | [AuthToolkit](./kibana-plugin-server.authtoolkit.md) | A tool set defining an outcome of Auth interceptor for incoming request. | | [CallAPIOptions](./kibana-plugin-server.callapioptions.md) | The set of options that defines how API call should be made and result be processed. | | [Capabilities](./kibana-plugin-server.capabilities.md) | The read-only set of capabilities available for the current UI session. Capabilities are simple key-value pairs of (string, boolean), where the string denotes the capability ID, and the boolean is a flag indicating if the capability is enabled or disabled. | diff --git a/docs/development/core/server/kibana-plugin-server.requesthandlercontext.core.md b/docs/development/core/server/kibana-plugin-server.requesthandlercontext.core.md index 77bfd85e6e54bb..15e60905dbc308 100644 --- a/docs/development/core/server/kibana-plugin-server.requesthandlercontext.core.md +++ b/docs/development/core/server/kibana-plugin-server.requesthandlercontext.core.md @@ -8,6 +8,9 @@ ```typescript core: { + auth: { + isAuthenticated: IsAuthenticated; + }; rendering: IScopedRenderingClient; savedObjects: { client: SavedObjectsClientContract; diff --git a/docs/development/core/server/kibana-plugin-server.requesthandlercontext.md b/docs/development/core/server/kibana-plugin-server.requesthandlercontext.md index 4d14d890f51a2e..8bd2fc61f9f4e8 100644 --- a/docs/development/core/server/kibana-plugin-server.requesthandlercontext.md +++ b/docs/development/core/server/kibana-plugin-server.requesthandlercontext.md @@ -18,5 +18,5 @@ export interface RequestHandlerContext | Property | Type | Description | | --- | --- | --- | -| [core](./kibana-plugin-server.requesthandlercontext.core.md) | {
rendering: IScopedRenderingClient;
savedObjects: {
client: SavedObjectsClientContract;
};
elasticsearch: {
dataClient: IScopedClusterClient;
adminClient: IScopedClusterClient;
};
uiSettings: {
client: IUiSettingsClient;
};
} | | +| [core](./kibana-plugin-server.requesthandlercontext.core.md) | {
auth: {
isAuthenticated: IsAuthenticated;
};
rendering: IScopedRenderingClient;
savedObjects: {
client: SavedObjectsClientContract;
};
elasticsearch: {
dataClient: IScopedClusterClient;
adminClient: IScopedClusterClient;
};
uiSettings: {
client: IUiSettingsClient;
};
} | | diff --git a/docs/development/core/server/kibana-plugin-server.routeconfigoptions.authrequired.md b/docs/development/core/server/kibana-plugin-server.routeconfigoptions.authrequired.md index e4cbca9c978108..9de93215cd7e9b 100644 --- a/docs/development/core/server/kibana-plugin-server.routeconfigoptions.authrequired.md +++ b/docs/development/core/server/kibana-plugin-server.routeconfigoptions.authrequired.md @@ -4,12 +4,12 @@ ## RouteConfigOptions.authRequired property -A flag shows that authentication for a route: `enabled` when true `disabled` when false +Defines authentication mode for a route: - true. A user has to have valid credentials to access a resource - false. A user can access a resource without any credentials. - 'optional'. A user can access a resource if has valid credentials or no credentials at all. -Enabled by default. +Set to true by default if an auth mechanism is registered. Signature: ```typescript -authRequired?: boolean; +authRequired?: boolean | 'optional'; ``` diff --git a/docs/development/core/server/kibana-plugin-server.routeconfigoptions.md b/docs/development/core/server/kibana-plugin-server.routeconfigoptions.md index 0929e15b6228b7..15d5b5514633c4 100644 --- a/docs/development/core/server/kibana-plugin-server.routeconfigoptions.md +++ b/docs/development/core/server/kibana-plugin-server.routeconfigoptions.md @@ -16,7 +16,7 @@ export interface RouteConfigOptions | Property | Type | Description | | --- | --- | --- | -| [authRequired](./kibana-plugin-server.routeconfigoptions.authrequired.md) | boolean | A flag shows that authentication for a route: enabled when true disabled when falseEnabled by default. | +| [authRequired](./kibana-plugin-server.routeconfigoptions.authrequired.md) | boolean | 'optional' | Defines authentication mode for a route: - true. A user has to have valid credentials to access a resource - false. A user can access a resource without any credentials. - 'optional'. A user can access a resource if has valid credentials or no credentials at all.Set to true by default if an auth mechanism is registered. | | [body](./kibana-plugin-server.routeconfigoptions.body.md) | Method extends 'get' | 'options' ? undefined : RouteConfigOptionsBody | Additional body options [RouteConfigOptionsBody](./kibana-plugin-server.routeconfigoptionsbody.md). | | [tags](./kibana-plugin-server.routeconfigoptions.tags.md) | readonly string[] | Additional metadata tag strings to attach to the route. | diff --git a/src/core/server/http/index.ts b/src/core/server/http/index.ts index d31afe1670e419..85038f6ca6ad0a 100644 --- a/src/core/server/http/index.ts +++ b/src/core/server/http/index.ts @@ -65,9 +65,11 @@ export { AuthenticationHandler, AuthHeaders, AuthResultParams, + AuthNotHandledResultParams, AuthToolkit, AuthResult, Authenticated, + AuthNotHandled, AuthResultType, } from './lifecycle/auth'; export { OnPostAuthHandler, OnPostAuthToolkit } from './lifecycle/on_post_auth'; diff --git a/src/core/server/http/lifecycle/auth.ts b/src/core/server/http/lifecycle/auth.ts index 3dfc860685b232..de8ea7b568e1d0 100644 --- a/src/core/server/http/lifecycle/auth.ts +++ b/src/core/server/http/lifecycle/auth.ts @@ -77,7 +77,7 @@ const authResult = { export type AuthHeaders = Record; /** - * Result of an incoming request authentication. + * Result of successful authentication. * @public */ export interface AuthResultParams { @@ -97,7 +97,11 @@ export interface AuthResultParams { responseHeaders?: AuthHeaders; } -interface AuthNotHandledResultParams { +/** + * Result of unhandled authentication. + * @public + */ +export interface AuthNotHandledResultParams { /** * Auth specific headers to attach to a response object. * Used to send back authentication mechanism related headers to a client when needed. @@ -112,6 +116,7 @@ interface AuthNotHandledResultParams { export interface AuthToolkit { /** Authentication is successful with given credentials, allow request to pass through */ authenticated: (data?: AuthResultParams) => AuthResult; + /** User has no credentials */ notHandled: (data?: AuthNotHandledResultParams) => AuthResult; } diff --git a/src/core/server/http/router/route.ts b/src/core/server/http/router/route.ts index be5d47729275d2..a13d71c3e9e5b3 100644 --- a/src/core/server/http/router/route.ts +++ b/src/core/server/http/router/route.ts @@ -100,11 +100,12 @@ export interface RouteConfigOptionsBody { */ export interface RouteConfigOptions { /** - * A flag shows that authentication for a route: - * `enabled` when true - * `disabled` when false + * Defines authentication mode for a route: + * - true. A user has to have valid credentials to access a resource + * - false. A user can access a resource without any credentials. + * - 'optional'. A user can access a resource if has valid credentials or no credentials at all. * - * Enabled by default. + * Set to true by default if an auth mechanism is registered. */ authRequired?: boolean | 'optional'; diff --git a/src/core/server/index.ts b/src/core/server/index.ts index ed3cbb39938e1f..b0a9da8414cd20 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -97,11 +97,13 @@ export { AuthenticationHandler, AuthHeaders, AuthResultParams, + AuthNotHandledResultParams, AuthStatus, AuthToolkit, AuthResult, AuthResultType, Authenticated, + AuthNotHandled, BasePath, IBasePath, CustomHttpResponseOptions, diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index f717f30fdb0cfd..44afd3a0e816ff 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -419,7 +419,18 @@ export type AuthenticationHandler = (request: KibanaRequest, response: Lifecycle export type AuthHeaders = Record; // @public (undocumented) -export type AuthResult = Authenticated; +export interface AuthNotHandled extends AuthNotHandledResultParams { + // (undocumented) + type: AuthResultType.notHandled; +} + +// @public +export interface AuthNotHandledResultParams { + responseHeaders?: AuthHeaders; +} + +// @public (undocumented) +export type AuthResult = Authenticated | AuthNotHandled; // @public export interface AuthResultParams { @@ -431,7 +442,9 @@ export interface AuthResultParams { // @public (undocumented) export enum AuthResultType { // (undocumented) - authenticated = "authenticated" + authenticated = "authenticated", + // (undocumented) + notHandled = "notHandled" } // @public @@ -444,6 +457,7 @@ export enum AuthStatus { // @public export interface AuthToolkit { authenticated: (data?: AuthResultParams) => AuthResult; + notHandled: (data?: AuthNotHandledResultParams) => AuthResult; } // @public @@ -1346,6 +1360,9 @@ export type RequestHandler

{ // @public export interface RouteConfigOptions { - authRequired?: boolean; + authRequired?: boolean | 'optional'; body?: Method extends 'get' | 'options' ? undefined : RouteConfigOptionsBody; tags?: readonly string[]; } From 9bec57f4af08969173c0eca032d47ddf53bd3be3 Mon Sep 17 00:00:00 2001 From: restrry Date: Wed, 26 Feb 2020 16:14:57 +0100 Subject: [PATCH 10/29] reword test suit names --- src/core/server/http/integration_tests/router.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/server/http/integration_tests/router.test.ts b/src/core/server/http/integration_tests/router.test.ts index b427a89872f284..a323ce97469a3d 100644 --- a/src/core/server/http/integration_tests/router.test.ts +++ b/src/core/server/http/integration_tests/router.test.ts @@ -82,7 +82,7 @@ describe('Options', () => { .expect(200, 'ok'); }); - it('User with not recognized credentials has access to a route', async () => { + it('User with no credentials can access a route', async () => { const { server: innerServer, createRouter, registerAuth } = await server.setup(setupDeps); const router = createRouter('/'); const authResponseHeader = { @@ -126,7 +126,7 @@ describe('Options', () => { .expect(200, 'ok'); }); - it('User with not recognized credentials has no access to a route', async () => { + it('User with no credentials cannot access a route', async () => { const { server: innerServer, createRouter, registerAuth } = await server.setup(setupDeps); const router = createRouter('/'); const authResponseHeader = { From 82835fc1a4d19005044e14d3c864fbccc50c270c Mon Sep 17 00:00:00 2001 From: restrry Date: Wed, 4 Mar 2020 09:14:34 +0100 Subject: [PATCH 11/29] update tests --- src/core/server/http/http_server.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/server/http/http_server.test.ts b/src/core/server/http/http_server.test.ts index 27db79bb94d252..46f9bf3e675bc8 100644 --- a/src/core/server/http/http_server.test.ts +++ b/src/core/server/http/http_server.test.ts @@ -810,7 +810,7 @@ test('exposes route details of incoming request to a route handler', async () => method: 'get', path: '/', options: { - authRequired: true, + authRequired: false, xsrfRequired: false, tags: [], }, @@ -923,7 +923,7 @@ test('exposes route details of incoming request to a route handler (POST + paylo method: 'post', path: '/', options: { - authRequired: true, + authRequired: false, xsrfRequired: true, tags: [], body: { From 46b523220bce61cb611e80f679e9f462eda9d784 Mon Sep 17 00:00:00 2001 From: restrry Date: Wed, 4 Mar 2020 12:20:29 +0100 Subject: [PATCH 12/29] update test checking isAuth on optional auth path --- src/core/server/http/integration_tests/core_services.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/server/http/integration_tests/core_services.test.ts b/src/core/server/http/integration_tests/core_services.test.ts index 00e54548468e5e..7b1630a7de0be0 100644 --- a/src/core/server/http/integration_tests/core_services.test.ts +++ b/src/core/server/http/integration_tests/core_services.test.ts @@ -118,7 +118,7 @@ describe('http service', () => { const router = createRouter(''); router.get( - { path: '/is-auth', validate: false, options: { authRequired: false } }, + { path: '/is-auth', validate: false, options: { authRequired: 'optional' } }, (context, req, res) => res.ok({ body: { isAuthenticated: auth.isAuthenticated(req) } }) ); From 2c2cbe1f5388ec46514ca44b114b31dd7c6eb066 Mon Sep 17 00:00:00 2001 From: restrry Date: Wed, 4 Mar 2020 13:49:10 +0100 Subject: [PATCH 13/29] address Oleg comments --- src/core/server/http/http_server.test.ts | 4 +-- src/core/server/http/http_server.ts | 20 ++++++----- .../http/integration_tests/lifecycle.test.ts | 2 +- .../http/integration_tests/router.test.ts | 36 ++++++++++++++++++- src/core/server/http/lifecycle/auth.ts | 4 +-- 5 files changed, 51 insertions(+), 15 deletions(-) diff --git a/src/core/server/http/http_server.test.ts b/src/core/server/http/http_server.test.ts index 46f9bf3e675bc8..27db79bb94d252 100644 --- a/src/core/server/http/http_server.test.ts +++ b/src/core/server/http/http_server.test.ts @@ -810,7 +810,7 @@ test('exposes route details of incoming request to a route handler', async () => method: 'get', path: '/', options: { - authRequired: false, + authRequired: true, xsrfRequired: false, tags: [], }, @@ -923,7 +923,7 @@ test('exposes route details of incoming request to a route handler (POST + paylo method: 'post', path: '/', options: { - authRequired: false, + authRequired: true, xsrfRequired: true, tags: [], body: { diff --git a/src/core/server/http/http_server.ts b/src/core/server/http/http_server.ts index 0a09c962ec31b1..146c9f264023df 100644 --- a/src/core/server/http/http_server.ts +++ b/src/core/server/http/http_server.ts @@ -196,16 +196,18 @@ export class HttpServer { private getAuthOption( authRequired: RouteConfigOptions['authRequired'] = true - ): false | { mode: 'required' | 'optional' } { - if (this.authRegistered) { - if (authRequired === true) { - return { mode: 'required' }; - } - if (authRequired === 'optional') { - return { mode: 'optional' }; - } + ): undefined | false | { mode: 'required' | 'optional' } { + if (this.authRegistered === false) return undefined; + + if (authRequired === true) { + return { mode: 'required' }; + } + if (authRequired === 'optional') { + return { mode: 'optional' }; + } + if (authRequired === false) { + return false; } - return false; } private setupBasePathRewrite(config: HttpConfig, basePathService: BasePath) { diff --git a/src/core/server/http/integration_tests/lifecycle.test.ts b/src/core/server/http/integration_tests/lifecycle.test.ts index caa8c75b9b9d3b..67e14b9a55db7d 100644 --- a/src/core/server/http/integration_tests/lifecycle.test.ts +++ b/src/core/server/http/integration_tests/lifecycle.test.ts @@ -415,7 +415,7 @@ describe('Auth', () => { .expect(200, { content: 'ok' }); }); - it('blocks access to a resource if credentials are not recognized', async () => { + it('blocks access to a resource if credentials are not provided', async () => { const { registerAuth, server: innerServer, createRouter } = await server.setup(setupDeps); const router = createRouter('/'); diff --git a/src/core/server/http/integration_tests/router.test.ts b/src/core/server/http/integration_tests/router.test.ts index a323ce97469a3d..c955d03f12cf66 100644 --- a/src/core/server/http/integration_tests/router.test.ts +++ b/src/core/server/http/integration_tests/router.test.ts @@ -106,7 +106,25 @@ describe('Options', () => { expect(response.header['www-authenticate']).toBe(undefined); }); + + it('User with invalid credentials cannot access a route', async () => { + const { server: innerServer, createRouter, registerAuth } = await server.setup(setupDeps); + const router = createRouter('/'); + + registerAuth((req, res, toolkit) => res.unauthorized()); + + router.get( + { path: '/', validate: false, options: { authRequired: 'optional' } }, + (context, req, res) => res.ok({ body: 'ok' }) + ); + await server.start(); + + await supertest(innerServer.listener) + .get('/') + .expect(401); + }); }); + describe('true', () => { it('Authenticated user has access to a route', async () => { const { server: innerServer, createRouter, registerAuth } = await server.setup(setupDeps); @@ -116,7 +134,7 @@ describe('Options', () => { return toolkit.authenticated(); }); router.get( - { path: '/', validate: false, options: { authRequired: 'optional' } }, + { path: '/', validate: false, options: { authRequired: true } }, (context, req, res) => res.ok({ body: 'ok' }) ); await server.start(); @@ -150,6 +168,22 @@ describe('Options', () => { expect(response.header['www-authenticate']).toBe(authResponseHeader['www-authenticate']); }); + it('User with invalid credentials cannot access a route', async () => { + const { server: innerServer, createRouter, registerAuth } = await server.setup(setupDeps); + const router = createRouter('/'); + + registerAuth((req, res, toolkit) => res.unauthorized()); + + router.get( + { path: '/', validate: false, options: { authRequired: true } }, + (context, req, res) => res.ok({ body: 'ok' }) + ); + await server.start(); + + await supertest(innerServer.listener) + .get('/') + .expect(401); + }); }); }); }); diff --git a/src/core/server/http/lifecycle/auth.ts b/src/core/server/http/lifecycle/auth.ts index de8ea7b568e1d0..d5f0f1c22bffc5 100644 --- a/src/core/server/http/lifecycle/auth.ts +++ b/src/core/server/http/lifecycle/auth.ts @@ -62,10 +62,10 @@ const authResult = { }; }, isAuthenticated(result: AuthResult): result is Authenticated { - return result && result.type === AuthResultType.authenticated; + return Boolean(result?.type === AuthResultType.authenticated); }, isNotHandled(result: AuthResult): result is AuthNotHandled { - return result && result.type === AuthResultType.notHandled; + return Boolean(result?.type === AuthResultType.notHandled); }, }; From 0801b08468df8eb7c896f41e644cf5df96f34d66 Mon Sep 17 00:00:00 2001 From: restrry Date: Wed, 4 Mar 2020 13:51:36 +0100 Subject: [PATCH 14/29] add test for auth: try --- src/core/server/http/router/request.test.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/core/server/http/router/request.test.ts b/src/core/server/http/router/request.test.ts index e3f67401d7c63d..fb999dc60e39c5 100644 --- a/src/core/server/http/router/request.test.ts +++ b/src/core/server/http/router/request.test.ts @@ -158,6 +158,7 @@ describe('KibanaRequest', () => { expect(kibanaRequest.route.options.authRequired).toBe(true); }); + it('handles required auth: { mode: "optional" }', () => { const auth: RouteOptions['auth'] = { mode: 'optional' }; const request = httpServerMock.createRawRequest({ @@ -172,6 +173,20 @@ describe('KibanaRequest', () => { expect(kibanaRequest.route.options.authRequired).toBe('optional'); }); + it('handles required auth: { mode: "try" } as "optional"', () => { + const auth: RouteOptions['auth'] = { mode: 'try' }; + const request = httpServerMock.createRawRequest({ + route: { + settings: { + auth, + }, + }, + }); + const kibanaRequest = KibanaRequest.from(request); + + expect(kibanaRequest.route.options.authRequired).toBe('optional'); + }); + it('throws on auth: strategy name', () => { const auth: RouteOptions['auth'] = 'session'; const request = httpServerMock.createRawRequest({ From 64ff1ac06b7821219a7300852bf35f6b8cefa442 Mon Sep 17 00:00:00 2001 From: restrry Date: Wed, 4 Mar 2020 14:27:18 +0100 Subject: [PATCH 15/29] fix --- src/core/server/http/lifecycle/auth.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/server/http/lifecycle/auth.ts b/src/core/server/http/lifecycle/auth.ts index d5f0f1c22bffc5..978ad041ccf1a7 100644 --- a/src/core/server/http/lifecycle/auth.ts +++ b/src/core/server/http/lifecycle/auth.ts @@ -62,10 +62,10 @@ const authResult = { }; }, isAuthenticated(result: AuthResult): result is Authenticated { - return Boolean(result?.type === AuthResultType.authenticated); + return result?.type === AuthResultType.authenticated; }, isNotHandled(result: AuthResult): result is AuthNotHandled { - return Boolean(result?.type === AuthResultType.notHandled); + return result?.type === AuthResultType.notHandled; }, }; From afa6d4be0d9dda97c53a4605f0c3b34e105c6619 Mon Sep 17 00:00:00 2001 From: restrry Date: Wed, 4 Mar 2020 14:27:46 +0100 Subject: [PATCH 16/29] pass isAuthenticted as boolean via context --- src/core/server/index.ts | 4 ++-- src/core/server/mocks.ts | 2 +- src/core/server/server.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/server/index.ts b/src/core/server/index.ts index 9072096b9ff096..e17df2f834963a 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -45,7 +45,7 @@ import { configSchema as elasticsearchConfigSchema, } from './elasticsearch'; -import { HttpServiceSetup, IsAuthenticated } from './http'; +import { HttpServiceSetup } from './http'; import { IScopedRenderingClient } from './rendering'; import { PluginsServiceSetup, PluginsServiceStart, PluginOpaqueId } from './plugins'; import { ContextSetup } from './context'; @@ -301,7 +301,7 @@ export { export interface RequestHandlerContext { core: { auth: { - isAuthenticated: IsAuthenticated; + isAuthenticated: boolean; }; rendering: IScopedRenderingClient; savedObjects: { diff --git a/src/core/server/mocks.ts b/src/core/server/mocks.ts index 99ac22ab450dc0..2e9539667e7007 100644 --- a/src/core/server/mocks.ts +++ b/src/core/server/mocks.ts @@ -172,7 +172,7 @@ function createInternalCoreStartMock() { function createCoreRequestHandlerContextMock() { return { auth: { - isAuthenticated: jest.fn(), + isAuthenticated: true, }, rendering: { render: jest.fn(), diff --git a/src/core/server/server.ts b/src/core/server/server.ts index a19c417abb3460..4ac32767aefc5e 100644 --- a/src/core/server/server.ts +++ b/src/core/server/server.ts @@ -234,7 +234,7 @@ export class Server { return { auth: { - isAuthenticated: () => coreSetup.http.auth.isAuthenticated(req), + isAuthenticated: coreSetup.http.auth.isAuthenticated(req), }, rendering: { render: async (options = {}) => From cc3d95cb0345ebfcdf8a646565a1825cc401e1c9 Mon Sep 17 00:00:00 2001 From: restrry Date: Wed, 4 Mar 2020 15:03:00 +0100 Subject: [PATCH 17/29] remove response header from notHandled --- src/core/server/http/http_server.ts | 5 ---- src/core/server/http/index.ts | 1 - .../http/integration_tests/lifecycle.test.ts | 21 --------------- .../http/integration_tests/router.test.ts | 27 ++++--------------- src/core/server/http/lifecycle/auth.ts | 25 +++-------------- src/core/server/index.ts | 1 - .../security/server/authentication/index.ts | 4 +-- 7 files changed, 10 insertions(+), 74 deletions(-) diff --git a/src/core/server/http/http_server.ts b/src/core/server/http/http_server.ts index 146c9f264023df..34035d8decca94 100644 --- a/src/core/server/http/http_server.ts +++ b/src/core/server/http/http_server.ts @@ -323,11 +323,6 @@ export class HttpServer { // where some plugin read directly from headers to identify whether a user is authenticated. Object.assign(req.headers, requestHeaders); } - }, - (req, { responseHeaders }) => { - if (responseHeaders) { - this.authResponseHeaders.set(req, responseHeaders); - } } ), })); diff --git a/src/core/server/http/index.ts b/src/core/server/http/index.ts index b57e2bb0448df2..234455988105c5 100644 --- a/src/core/server/http/index.ts +++ b/src/core/server/http/index.ts @@ -67,7 +67,6 @@ export { AuthenticationHandler, AuthHeaders, AuthResultParams, - AuthNotHandledResultParams, AuthToolkit, AuthResult, Authenticated, diff --git a/src/core/server/http/integration_tests/lifecycle.test.ts b/src/core/server/http/integration_tests/lifecycle.test.ts index 67e14b9a55db7d..23bd355efc19b1 100644 --- a/src/core/server/http/integration_tests/lifecycle.test.ts +++ b/src/core/server/http/integration_tests/lifecycle.test.ts @@ -701,27 +701,6 @@ describe('Auth', () => { expect(response.header['www-authenticate']).toBe(authResponseHeader['www-authenticate']); }); - it('attach security header to not handled auth response', async () => { - const { registerAuth, server: innerServer, createRouter } = await server.setup(setupDeps); - const router = createRouter('/'); - - const authResponseHeader = { - 'www-authenticate': 'from auth interceptor', - }; - registerAuth((req, res, toolkit) => { - return toolkit.notHandled({ responseHeaders: authResponseHeader }); - }); - - router.get({ path: '/', validate: false }, (context, req, res) => res.ok()); - await server.start(); - - const response = await supertest(innerServer.listener) - .get('/') - .expect(401); - - expect(response.header['www-authenticate']).toBe(authResponseHeader['www-authenticate']); - }); - it('attach security header to an error response', async () => { const { registerAuth, server: innerServer, createRouter } = await server.setup(setupDeps); const router = createRouter('/'); diff --git a/src/core/server/http/integration_tests/router.test.ts b/src/core/server/http/integration_tests/router.test.ts index c955d03f12cf66..d3a33710ecfb08 100644 --- a/src/core/server/http/integration_tests/router.test.ts +++ b/src/core/server/http/integration_tests/router.test.ts @@ -85,26 +85,18 @@ describe('Options', () => { it('User with no credentials can access a route', async () => { const { server: innerServer, createRouter, registerAuth } = await server.setup(setupDeps); const router = createRouter('/'); - const authResponseHeader = { - 'www-authenticate': 'from auth interceptor', - }; - registerAuth((req, res, toolkit) => { - return toolkit.notHandled({ - responseHeaders: authResponseHeader, - }); - }); + registerAuth((req, res, toolkit) => toolkit.notHandled()); + router.get( { path: '/', validate: false, options: { authRequired: 'optional' } }, (context, req, res) => res.ok({ body: 'ok' }) ); await server.start(); - const response = await supertest(innerServer.listener) + await supertest(innerServer.listener) .get('/') .expect(200, 'ok'); - - expect(response.header['www-authenticate']).toBe(undefined); }); it('User with invalid credentials cannot access a route', async () => { @@ -147,26 +139,17 @@ describe('Options', () => { it('User with no credentials cannot access a route', async () => { const { server: innerServer, createRouter, registerAuth } = await server.setup(setupDeps); const router = createRouter('/'); - const authResponseHeader = { - 'www-authenticate': 'from auth interceptor', - }; - registerAuth((req, res, toolkit) => { - return toolkit.notHandled({ - responseHeaders: authResponseHeader, - }); - }); + registerAuth((req, res, toolkit) => toolkit.notHandled()); router.get( { path: '/', validate: false, options: { authRequired: true } }, (context, req, res) => res.ok({ body: 'ok' }) ); await server.start(); - const response = await supertest(innerServer.listener) + await supertest(innerServer.listener) .get('/') .expect(401); - - expect(response.header['www-authenticate']).toBe(authResponseHeader['www-authenticate']); }); it('User with invalid credentials cannot access a route', async () => { const { server: innerServer, createRouter, registerAuth } = await server.setup(setupDeps); diff --git a/src/core/server/http/lifecycle/auth.ts b/src/core/server/http/lifecycle/auth.ts index 978ad041ccf1a7..c0893c4f1d529c 100644 --- a/src/core/server/http/lifecycle/auth.ts +++ b/src/core/server/http/lifecycle/auth.ts @@ -39,7 +39,7 @@ export interface Authenticated extends AuthResultParams { } /** @public */ -export interface AuthNotHandled extends AuthNotHandledResultParams { +export interface AuthNotHandled { type: AuthResultType.notHandled; } @@ -55,10 +55,9 @@ const authResult = { responseHeaders: data.responseHeaders, }; }, - notHandled(data: AuthNotHandledResultParams = {}): AuthResult { + notHandled(): AuthResult { return { type: AuthResultType.notHandled, - responseHeaders: data.responseHeaders, }; }, isAuthenticated(result: AuthResult): result is Authenticated { @@ -97,18 +96,6 @@ export interface AuthResultParams { responseHeaders?: AuthHeaders; } -/** - * Result of unhandled authentication. - * @public - */ -export interface AuthNotHandledResultParams { - /** - * Auth specific headers to attach to a response object. - * Used to send back authentication mechanism related headers to a client when needed. - */ - responseHeaders?: AuthHeaders; -} - /** * @public * A tool set defining an outcome of Auth interceptor for incoming request. @@ -117,7 +104,7 @@ export interface AuthToolkit { /** Authentication is successful with given credentials, allow request to pass through */ authenticated: (data?: AuthResultParams) => AuthResult; /** User has no credentials */ - notHandled: (data?: AuthNotHandledResultParams) => AuthResult; + notHandled: () => AuthResult; } const toolkit: AuthToolkit = { @@ -139,8 +126,7 @@ export type AuthenticationHandler = ( export function adoptToHapiAuthFormat( fn: AuthenticationHandler, log: Logger, - onAuth: (request: Request, data: AuthResultParams) => void = () => undefined, - onNotHandled: (request: Request, data: AuthNotHandledResultParams) => void = () => undefined + onAuth: (request: Request, data: AuthResultParams) => void = () => undefined ) { return async function interceptAuth( request: Request, @@ -169,9 +155,6 @@ export function adoptToHapiAuthFormat( return responseToolkit.continue; } if (kibanaRequest.route.options.authRequired) { - onNotHandled(request, { - responseHeaders: result.responseHeaders, - }); return hapiResponseAdapter.handle(lifecycleResponseFactory.unauthorized()); } throw new Error( diff --git a/src/core/server/index.ts b/src/core/server/index.ts index e17df2f834963a..879626005ca458 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -97,7 +97,6 @@ export { AuthenticationHandler, AuthHeaders, AuthResultParams, - AuthNotHandledResultParams, AuthStatus, AuthToolkit, AuthResult, diff --git a/x-pack/plugins/security/server/authentication/index.ts b/x-pack/plugins/security/server/authentication/index.ts index c6646aab9b1159..ae961560697287 100644 --- a/x-pack/plugins/security/server/authentication/index.ts +++ b/x-pack/plugins/security/server/authentication/index.ts @@ -153,9 +153,7 @@ export async function setupAuthentication({ } authLogger.debug('Could not handle authentication attempt'); - return t.notHandled({ - responseHeaders: authenticationResult.authResponseHeaders, - }); + return t.notHandled(); }); authLogger.debug('Successfully registered core authentication handler.'); From e96158bd2940161470fe1e0f9157addf7752bfcb Mon Sep 17 00:00:00 2001 From: restrry Date: Wed, 4 Mar 2020 16:10:58 +0100 Subject: [PATCH 18/29] update docs --- .../kibana-plugin-server.authnothandled.md | 2 +- ...lugin-server.authnothandledresultparams.md | 20 ------------------- ...hnothandledresultparams.responseheaders.md | 13 ------------ .../kibana-plugin-server.authtoolkit.md | 2 +- ...na-plugin-server.authtoolkit.nothandled.md | 2 +- .../core/server/kibana-plugin-server.md | 1 - ...lugin-server.requesthandlercontext.core.md | 2 +- ...ana-plugin-server.requesthandlercontext.md | 2 +- src/core/server/server.api.md | 11 +++------- 9 files changed, 8 insertions(+), 47 deletions(-) delete mode 100644 docs/development/core/server/kibana-plugin-server.authnothandledresultparams.md delete mode 100644 docs/development/core/server/kibana-plugin-server.authnothandledresultparams.responseheaders.md diff --git a/docs/development/core/server/kibana-plugin-server.authnothandled.md b/docs/development/core/server/kibana-plugin-server.authnothandled.md index 5d4a39a88ea647..01e465c266319b 100644 --- a/docs/development/core/server/kibana-plugin-server.authnothandled.md +++ b/docs/development/core/server/kibana-plugin-server.authnothandled.md @@ -8,7 +8,7 @@ Signature: ```typescript -export interface AuthNotHandled extends AuthNotHandledResultParams +export interface AuthNotHandled ``` ## Properties diff --git a/docs/development/core/server/kibana-plugin-server.authnothandledresultparams.md b/docs/development/core/server/kibana-plugin-server.authnothandledresultparams.md deleted file mode 100644 index 90e9f5a771e552..00000000000000 --- a/docs/development/core/server/kibana-plugin-server.authnothandledresultparams.md +++ /dev/null @@ -1,20 +0,0 @@ - - -[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [AuthNotHandledResultParams](./kibana-plugin-server.authnothandledresultparams.md) - -## AuthNotHandledResultParams interface - -Result of unhandled authentication. - -Signature: - -```typescript -export interface AuthNotHandledResultParams -``` - -## Properties - -| Property | Type | Description | -| --- | --- | --- | -| [responseHeaders](./kibana-plugin-server.authnothandledresultparams.responseheaders.md) | AuthHeaders | Auth specific headers to attach to a response object. Used to send back authentication mechanism related headers to a client when needed. | - diff --git a/docs/development/core/server/kibana-plugin-server.authnothandledresultparams.responseheaders.md b/docs/development/core/server/kibana-plugin-server.authnothandledresultparams.responseheaders.md deleted file mode 100644 index 9e1e733246d16a..00000000000000 --- a/docs/development/core/server/kibana-plugin-server.authnothandledresultparams.responseheaders.md +++ /dev/null @@ -1,13 +0,0 @@ - - -[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [AuthNotHandledResultParams](./kibana-plugin-server.authnothandledresultparams.md) > [responseHeaders](./kibana-plugin-server.authnothandledresultparams.responseheaders.md) - -## AuthNotHandledResultParams.responseHeaders property - -Auth specific headers to attach to a response object. Used to send back authentication mechanism related headers to a client when needed. - -Signature: - -```typescript -responseHeaders?: AuthHeaders; -``` diff --git a/docs/development/core/server/kibana-plugin-server.authtoolkit.md b/docs/development/core/server/kibana-plugin-server.authtoolkit.md index cee59409b79998..820f2692186616 100644 --- a/docs/development/core/server/kibana-plugin-server.authtoolkit.md +++ b/docs/development/core/server/kibana-plugin-server.authtoolkit.md @@ -17,5 +17,5 @@ export interface AuthToolkit | Property | Type | Description | | --- | --- | --- | | [authenticated](./kibana-plugin-server.authtoolkit.authenticated.md) | (data?: AuthResultParams) => AuthResult | Authentication is successful with given credentials, allow request to pass through | -| [notHandled](./kibana-plugin-server.authtoolkit.nothandled.md) | (data?: AuthNotHandledResultParams) => AuthResult | User has no credentials | +| [notHandled](./kibana-plugin-server.authtoolkit.nothandled.md) | () => AuthResult | User has no credentials | diff --git a/docs/development/core/server/kibana-plugin-server.authtoolkit.nothandled.md b/docs/development/core/server/kibana-plugin-server.authtoolkit.nothandled.md index 41c5cb16567d47..7a74a4e2800869 100644 --- a/docs/development/core/server/kibana-plugin-server.authtoolkit.nothandled.md +++ b/docs/development/core/server/kibana-plugin-server.authtoolkit.nothandled.md @@ -9,5 +9,5 @@ User has no credentials Signature: ```typescript -notHandled: (data?: AuthNotHandledResultParams) => AuthResult; +notHandled: () => AuthResult; ``` diff --git a/docs/development/core/server/kibana-plugin-server.md b/docs/development/core/server/kibana-plugin-server.md index 08e47a1559e18c..2a6b5df77d4f3c 100644 --- a/docs/development/core/server/kibana-plugin-server.md +++ b/docs/development/core/server/kibana-plugin-server.md @@ -46,7 +46,6 @@ The plugin integrates with the core system via lifecycle events: `setup` | [AssistantAPIClientParams](./kibana-plugin-server.assistantapiclientparams.md) | | | [Authenticated](./kibana-plugin-server.authenticated.md) | | | [AuthNotHandled](./kibana-plugin-server.authnothandled.md) | | -| [AuthNotHandledResultParams](./kibana-plugin-server.authnothandledresultparams.md) | Result of unhandled authentication. | | [AuthResultParams](./kibana-plugin-server.authresultparams.md) | Result of successful authentication. | | [AuthToolkit](./kibana-plugin-server.authtoolkit.md) | A tool set defining an outcome of Auth interceptor for incoming request. | | [CallAPIOptions](./kibana-plugin-server.callapioptions.md) | The set of options that defines how API call should be made and result be processed. | diff --git a/docs/development/core/server/kibana-plugin-server.requesthandlercontext.core.md b/docs/development/core/server/kibana-plugin-server.requesthandlercontext.core.md index 15e60905dbc308..cb0beb20f1c504 100644 --- a/docs/development/core/server/kibana-plugin-server.requesthandlercontext.core.md +++ b/docs/development/core/server/kibana-plugin-server.requesthandlercontext.core.md @@ -9,7 +9,7 @@ ```typescript core: { auth: { - isAuthenticated: IsAuthenticated; + isAuthenticated: boolean; }; rendering: IScopedRenderingClient; savedObjects: { diff --git a/docs/development/core/server/kibana-plugin-server.requesthandlercontext.md b/docs/development/core/server/kibana-plugin-server.requesthandlercontext.md index 8bd2fc61f9f4e8..ddcf6f3e5f973f 100644 --- a/docs/development/core/server/kibana-plugin-server.requesthandlercontext.md +++ b/docs/development/core/server/kibana-plugin-server.requesthandlercontext.md @@ -18,5 +18,5 @@ export interface RequestHandlerContext | Property | Type | Description | | --- | --- | --- | -| [core](./kibana-plugin-server.requesthandlercontext.core.md) | {
auth: {
isAuthenticated: IsAuthenticated;
};
rendering: IScopedRenderingClient;
savedObjects: {
client: SavedObjectsClientContract;
};
elasticsearch: {
dataClient: IScopedClusterClient;
adminClient: IScopedClusterClient;
};
uiSettings: {
client: IUiSettingsClient;
};
} | | +| [core](./kibana-plugin-server.requesthandlercontext.core.md) | {
auth: {
isAuthenticated: boolean;
};
rendering: IScopedRenderingClient;
savedObjects: {
client: SavedObjectsClientContract;
};
elasticsearch: {
dataClient: IScopedClusterClient;
adminClient: IScopedClusterClient;
};
uiSettings: {
client: IUiSettingsClient;
};
} | | diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index 51e1fc5f4f79c7..d8f58bdb62ed50 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -419,16 +419,11 @@ export type AuthenticationHandler = (request: KibanaRequest, response: Lifecycle export type AuthHeaders = Record; // @public (undocumented) -export interface AuthNotHandled extends AuthNotHandledResultParams { +export interface AuthNotHandled { // (undocumented) type: AuthResultType.notHandled; } -// @public -export interface AuthNotHandledResultParams { - responseHeaders?: AuthHeaders; -} - // @public (undocumented) export type AuthResult = Authenticated | AuthNotHandled; @@ -457,7 +452,7 @@ export enum AuthStatus { // @public export interface AuthToolkit { authenticated: (data?: AuthResultParams) => AuthResult; - notHandled: (data?: AuthNotHandledResultParams) => AuthResult; + notHandled: () => AuthResult; } // @public @@ -1429,7 +1424,7 @@ export interface RequestHandlerContext { // (undocumented) core: { auth: { - isAuthenticated: IsAuthenticated; + isAuthenticated: boolean; }; rendering: IScopedRenderingClient; savedObjects: { From 9750ce1f0c8174ce63df0b04b70720d5463f7720 Mon Sep 17 00:00:00 2001 From: Mikhail Shustov Date: Thu, 5 Mar 2020 11:12:02 +0100 Subject: [PATCH 19/29] add redirected for auth interceptor --- src/core/server/http/http_service.mock.ts | 1 + src/core/server/http/index.ts | 2 + .../http/integration_tests/lifecycle.test.ts | 23 +++++-- .../http/integration_tests/router.test.ts | 46 +++++++++++++ src/core/server/http/lifecycle/auth.ts | 64 ++++++++++++++++--- src/core/server/index.ts | 2 + 6 files changed, 124 insertions(+), 14 deletions(-) diff --git a/src/core/server/http/http_service.mock.ts b/src/core/server/http/http_service.mock.ts index c87b893f5bb932..442bc93190d86d 100644 --- a/src/core/server/http/http_service.mock.ts +++ b/src/core/server/http/http_service.mock.ts @@ -116,6 +116,7 @@ const createOnPostAuthToolkitMock = (): jest.Mocked => ({ const createAuthToolkitMock = (): jest.Mocked => ({ authenticated: jest.fn(), notHandled: jest.fn(), + redirected: jest.fn(), }); const createOnPreResponseToolkitMock = (): jest.Mocked => ({ diff --git a/src/core/server/http/index.ts b/src/core/server/http/index.ts index 234455988105c5..a75eb04fa01204 100644 --- a/src/core/server/http/index.ts +++ b/src/core/server/http/index.ts @@ -67,6 +67,8 @@ export { AuthenticationHandler, AuthHeaders, AuthResultParams, + AuthRedirected, + AuthRedirectedParams, AuthToolkit, AuthResult, Authenticated, diff --git a/src/core/server/http/integration_tests/lifecycle.test.ts b/src/core/server/http/integration_tests/lifecycle.test.ts index 23bd355efc19b1..82dcd0cd6d30e2 100644 --- a/src/core/server/http/integration_tests/lifecycle.test.ts +++ b/src/core/server/http/integration_tests/lifecycle.test.ts @@ -57,7 +57,7 @@ interface StorageData { } describe('OnPreAuth', () => { - it('supports registering request inceptors', async () => { + it('supports registering a request interceptor', async () => { const { registerOnPreAuth, server: innerServer, createRouter } = await server.setup(setupDeps); const router = createRouter('/'); @@ -509,11 +509,9 @@ describe('Auth', () => { router.get({ path: '/', validate: false }, (context, req, res) => res.ok()); const redirectTo = '/redirect-url'; - registerAuth((req, res) => - res.redirected({ - headers: { - location: redirectTo, - }, + registerAuth((req, res, t) => + t.redirected({ + location: redirectTo, }) ); await server.start(); @@ -524,6 +522,19 @@ describe('Auth', () => { expect(response.header.location).toBe(redirectTo); }); + it('throws if redirection url is not provided', async () => { + const { registerAuth, server: innerServer, createRouter } = await server.setup(setupDeps); + const router = createRouter('/'); + + router.get({ path: '/', validate: false }, (context, req, res) => res.ok()); + registerAuth((req, res, t) => t.redirected({})); + await server.start(); + + await supertest(innerServer.listener) + .get('/') + .expect(500); + }); + it(`doesn't expose internal error details`, async () => { const { registerAuth, server: innerServer, createRouter } = await server.setup(setupDeps); const router = createRouter('/'); diff --git a/src/core/server/http/integration_tests/router.test.ts b/src/core/server/http/integration_tests/router.test.ts index d3a33710ecfb08..5c22029d73d78c 100644 --- a/src/core/server/http/integration_tests/router.test.ts +++ b/src/core/server/http/integration_tests/router.test.ts @@ -115,6 +115,27 @@ describe('Options', () => { .get('/') .expect(401); }); + + it('does not redirect user and allows access to a resource', async () => { + const { server: innerServer, createRouter, registerAuth } = await server.setup(setupDeps); + const router = createRouter('/'); + + registerAuth((req, res, toolkit) => + toolkit.redirected({ + location: '/redirect-to', + }) + ); + + router.get( + { path: '/', validate: false, options: { authRequired: 'optional' } }, + (context, req, res) => res.ok({ body: 'ok' }) + ); + await server.start(); + + await supertest(innerServer.listener) + .get('/') + .expect(200, 'ok'); + }); }); describe('true', () => { @@ -151,6 +172,7 @@ describe('Options', () => { .get('/') .expect(401); }); + it('User with invalid credentials cannot access a route', async () => { const { server: innerServer, createRouter, registerAuth } = await server.setup(setupDeps); const router = createRouter('/'); @@ -167,6 +189,30 @@ describe('Options', () => { .get('/') .expect(401); }); + + it('allows redirecting an user', async () => { + const { server: innerServer, createRouter, registerAuth } = await server.setup(setupDeps); + const router = createRouter('/'); + const redirectUrl = '/redirect-to'; + + registerAuth((req, res, toolkit) => + toolkit.redirected({ + location: redirectUrl, + }) + ); + + router.get( + { path: '/', validate: false, options: { authRequired: true } }, + (context, req, res) => res.ok({ body: 'ok' }) + ); + await server.start(); + + const result = await supertest(innerServer.listener) + .get('/') + .expect(302); + + expect(result.header.location).toBe(redirectUrl); + }); }); }); }); diff --git a/src/core/server/http/lifecycle/auth.ts b/src/core/server/http/lifecycle/auth.ts index c0893c4f1d529c..637c5a7cb96fdc 100644 --- a/src/core/server/http/lifecycle/auth.ts +++ b/src/core/server/http/lifecycle/auth.ts @@ -25,12 +25,14 @@ import { lifecycleResponseFactory, LifecycleResponseFactory, isKibanaResponse, + ResponseHeaders, } from '../router'; /** @public */ export enum AuthResultType { authenticated = 'authenticated', notHandled = 'notHandled', + redirected = 'redirected', } /** @public */ @@ -44,7 +46,12 @@ export interface AuthNotHandled { } /** @public */ -export type AuthResult = Authenticated | AuthNotHandled; +export interface AuthRedirected extends AuthRedirectedParams { + type: AuthResultType.redirected; +} + +/** @public */ +export type AuthResult = Authenticated | AuthNotHandled | AuthRedirected; const authResult = { authenticated(data: AuthResultParams = {}): AuthResult { @@ -60,12 +67,21 @@ const authResult = { type: AuthResultType.notHandled, }; }, + redirected(headers: ResponseHeaders): AuthResult { + return { + type: AuthResultType.redirected, + headers, + }; + }, isAuthenticated(result: AuthResult): result is Authenticated { return result?.type === AuthResultType.authenticated; }, isNotHandled(result: AuthResult): result is AuthNotHandled { return result?.type === AuthResultType.notHandled; }, + isRedirected(result: AuthResult): result is AuthRedirected { + return result?.type === AuthResultType.redirected; + }, }; /** @@ -96,6 +112,18 @@ export interface AuthResultParams { responseHeaders?: AuthHeaders; } +/** + * Result of auth redirection. + * @public + */ +export interface AuthRedirectedParams { + /** + * Headers to attach for auth redirect. + * Must include "location" header + */ + headers: ResponseHeaders; +} + /** * @public * A tool set defining an outcome of Auth interceptor for incoming request. @@ -103,13 +131,23 @@ export interface AuthResultParams { export interface AuthToolkit { /** Authentication is successful with given credentials, allow request to pass through */ authenticated: (data?: AuthResultParams) => AuthResult; - /** User has no credentials */ + /** + * User has no credentials. + * Allows user to access a resource when authRequired: 'optional' + * Rejects a request when authRequired: true + * */ notHandled: () => AuthResult; + /** + * Redirect user to IdP when authRequired: true + * Allows user to access a resource without redirection when authRequired: 'optional' + * */ + redirected: (headers: ResponseHeaders) => AuthResult; } const toolkit: AuthToolkit = { authenticated: authResult.authenticated, notHandled: authResult.notHandled, + redirected: authResult.redirected, }; /** @@ -141,6 +179,7 @@ export function adoptToHapiAuthFormat( if (isKibanaResponse(result)) { return hapiResponseAdapter.handle(result); } + if (authResult.isAuthenticated(result)) { onAuth(request, { state: result.state, @@ -150,17 +189,26 @@ export function adoptToHapiAuthFormat( return responseToolkit.authenticated({ credentials: result.state || {} }); } - if (authResult.isNotHandled(result)) { + if (authResult.isRedirected(result)) { + // we cannot redirect a user when resources with optional auth requested if (kibanaRequest.route.options.authRequired === 'optional') { return responseToolkit.continue; } - if (kibanaRequest.route.options.authRequired) { - return hapiResponseAdapter.handle(lifecycleResponseFactory.unauthorized()); - } - throw new Error( - `Unexpected route.options.authRequired value in AuthenticationHandler. Expected 'optional' or true, but given: ${result}.` + + return hapiResponseAdapter.handle( + lifecycleResponseFactory.redirected({ + // hapi doesn't accept string[] as a valid header + headers: result.headers as any, + }) ); } + + if (authResult.isNotHandled(result)) { + if (kibanaRequest.route.options.authRequired === 'optional') { + return responseToolkit.continue; + } + return hapiResponseAdapter.handle(lifecycleResponseFactory.unauthorized()); + } throw new Error( `Unexpected result from Authenticate. Expected AuthResult or KibanaResponse, but given: ${result}.` ); diff --git a/src/core/server/index.ts b/src/core/server/index.ts index 879626005ca458..efd7abbab0472c 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -99,6 +99,8 @@ export { AuthResultParams, AuthStatus, AuthToolkit, + AuthRedirected, + AuthRedirectedParams, AuthResult, AuthResultType, Authenticated, From 5efb16a6243e04d659604fe30f29de1f883b06d9 Mon Sep 17 00:00:00 2001 From: Mikhail Shustov Date: Thu, 5 Mar 2020 11:13:07 +0100 Subject: [PATCH 20/29] security plugin uses t.redirected to be compat with auth: optional --- .../server/authentication/index.test.ts | 20 +++++++++---------- .../security/server/authentication/index.ts | 6 ++---- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/x-pack/plugins/security/server/authentication/index.test.ts b/x-pack/plugins/security/server/authentication/index.test.ts index 92a92dda84280e..30929ba98d33bf 100644 --- a/x-pack/plugins/security/server/authentication/index.test.ts +++ b/x-pack/plugins/security/server/authentication/index.test.ts @@ -132,7 +132,7 @@ describe('setupAuthentication()', () => { expect(mockAuthToolkit.authenticated).toHaveBeenCalledTimes(1); expect(mockAuthToolkit.authenticated).toHaveBeenCalledWith(); - expect(mockResponse.redirected).not.toHaveBeenCalled(); + expect(mockAuthToolkit.redirected).not.toHaveBeenCalled(); expect(mockResponse.internalError).not.toHaveBeenCalled(); expect(authenticate).not.toHaveBeenCalled(); @@ -155,7 +155,7 @@ describe('setupAuthentication()', () => { state: mockUser, requestHeaders: mockAuthHeaders, }); - expect(mockResponse.redirected).not.toHaveBeenCalled(); + expect(mockAuthToolkit.redirected).not.toHaveBeenCalled(); expect(mockResponse.internalError).not.toHaveBeenCalled(); expect(authenticate).toHaveBeenCalledTimes(1); @@ -184,7 +184,7 @@ describe('setupAuthentication()', () => { requestHeaders: mockAuthHeaders, responseHeaders: mockAuthResponseHeaders, }); - expect(mockResponse.redirected).not.toHaveBeenCalled(); + expect(mockAuthToolkit.redirected).not.toHaveBeenCalled(); expect(mockResponse.internalError).not.toHaveBeenCalled(); expect(authenticate).toHaveBeenCalledTimes(1); @@ -197,9 +197,9 @@ describe('setupAuthentication()', () => { await authHandler(httpServerMock.createKibanaRequest(), mockResponse, mockAuthToolkit); - expect(mockResponse.redirected).toHaveBeenCalledTimes(1); - expect(mockResponse.redirected).toHaveBeenCalledWith({ - headers: { location: '/some/url' }, + expect(mockAuthToolkit.redirected).toHaveBeenCalledTimes(1); + expect(mockAuthToolkit.redirected).toHaveBeenCalledWith({ + location: '/some/url', }); expect(mockAuthToolkit.authenticated).not.toHaveBeenCalled(); expect(mockResponse.internalError).not.toHaveBeenCalled(); @@ -216,7 +216,7 @@ describe('setupAuthentication()', () => { expect(error).toBeUndefined(); expect(mockAuthToolkit.authenticated).not.toHaveBeenCalled(); - expect(mockResponse.redirected).not.toHaveBeenCalled(); + expect(mockAuthToolkit.redirected).not.toHaveBeenCalled(); expect(loggingServiceMock.collect(mockSetupAuthenticationParams.loggers).error) .toMatchInlineSnapshot(` Array [ @@ -239,7 +239,7 @@ describe('setupAuthentication()', () => { expect(response.body).toBe(esError); expect(mockAuthToolkit.authenticated).not.toHaveBeenCalled(); - expect(mockResponse.redirected).not.toHaveBeenCalled(); + expect(mockAuthToolkit.redirected).not.toHaveBeenCalled(); }); it('includes `WWW-Authenticate` header if `authenticate` fails to authenticate user and provides challenges', async () => { @@ -264,7 +264,7 @@ describe('setupAuthentication()', () => { expect(options!.headers).toEqual({ 'WWW-Authenticate': 'Negotiate' }); expect(mockAuthToolkit.authenticated).not.toHaveBeenCalled(); - expect(mockResponse.redirected).not.toHaveBeenCalled(); + expect(mockAuthToolkit.redirected).not.toHaveBeenCalled(); }); it('returns `notHandled` when authentication can not be handled', async () => { @@ -276,7 +276,7 @@ describe('setupAuthentication()', () => { expect(mockAuthToolkit.notHandled).toHaveBeenCalledTimes(1); expect(mockAuthToolkit.authenticated).not.toHaveBeenCalled(); - expect(mockResponse.redirected).not.toHaveBeenCalled(); + expect(mockAuthToolkit.redirected).not.toHaveBeenCalled(); }); }); diff --git a/x-pack/plugins/security/server/authentication/index.ts b/x-pack/plugins/security/server/authentication/index.ts index ae961560697287..1eed53efc64415 100644 --- a/x-pack/plugins/security/server/authentication/index.ts +++ b/x-pack/plugins/security/server/authentication/index.ts @@ -127,10 +127,8 @@ export async function setupAuthentication({ // authentication (username and password) or arbitrary external page managed by 3rd party // Identity Provider for SSO authentication mechanisms. Authentication provider is the one who // decides what location user should be redirected to. - return response.redirected({ - headers: { - location: authenticationResult.redirectURL!, - }, + return t.redirected({ + location: authenticationResult.redirectURL!, }); } From c5cb1fd1361fe69e33a50036522c52b782c5b8b2 Mon Sep 17 00:00:00 2001 From: Mikhail Shustov Date: Thu, 5 Mar 2020 11:13:47 +0100 Subject: [PATCH 21/29] update docs --- .../kibana-plugin-server.authredirected.md | 19 ++++++++++++++++++ ...ibana-plugin-server.authredirected.type.md | 11 ++++++++++ ...gin-server.authredirectedparams.headers.md | 13 ++++++++++++ ...bana-plugin-server.authredirectedparams.md | 20 +++++++++++++++++++ .../server/kibana-plugin-server.authresult.md | 2 +- .../kibana-plugin-server.authresulttype.md | 1 + .../kibana-plugin-server.authtoolkit.md | 3 ++- ...na-plugin-server.authtoolkit.nothandled.md | 2 +- ...na-plugin-server.authtoolkit.redirected.md | 13 ++++++++++++ .../core/server/kibana-plugin-server.md | 2 ++ src/core/server/server.api.md | 18 +++++++++++++++-- 11 files changed, 99 insertions(+), 5 deletions(-) create mode 100644 docs/development/core/server/kibana-plugin-server.authredirected.md create mode 100644 docs/development/core/server/kibana-plugin-server.authredirected.type.md create mode 100644 docs/development/core/server/kibana-plugin-server.authredirectedparams.headers.md create mode 100644 docs/development/core/server/kibana-plugin-server.authredirectedparams.md create mode 100644 docs/development/core/server/kibana-plugin-server.authtoolkit.redirected.md diff --git a/docs/development/core/server/kibana-plugin-server.authredirected.md b/docs/development/core/server/kibana-plugin-server.authredirected.md new file mode 100644 index 00000000000000..3eb88d6c5a2307 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-server.authredirected.md @@ -0,0 +1,19 @@ + + +[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [AuthRedirected](./kibana-plugin-server.authredirected.md) + +## AuthRedirected interface + + +Signature: + +```typescript +export interface AuthRedirected extends AuthRedirectedParams +``` + +## Properties + +| Property | Type | Description | +| --- | --- | --- | +| [type](./kibana-plugin-server.authredirected.type.md) | AuthResultType.redirected | | + diff --git a/docs/development/core/server/kibana-plugin-server.authredirected.type.md b/docs/development/core/server/kibana-plugin-server.authredirected.type.md new file mode 100644 index 00000000000000..866ed358119e7d --- /dev/null +++ b/docs/development/core/server/kibana-plugin-server.authredirected.type.md @@ -0,0 +1,11 @@ + + +[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [AuthRedirected](./kibana-plugin-server.authredirected.md) > [type](./kibana-plugin-server.authredirected.type.md) + +## AuthRedirected.type property + +Signature: + +```typescript +type: AuthResultType.redirected; +``` diff --git a/docs/development/core/server/kibana-plugin-server.authredirectedparams.headers.md b/docs/development/core/server/kibana-plugin-server.authredirectedparams.headers.md new file mode 100644 index 00000000000000..1d9cdca9807e37 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-server.authredirectedparams.headers.md @@ -0,0 +1,13 @@ + + +[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [AuthRedirectedParams](./kibana-plugin-server.authredirectedparams.md) > [headers](./kibana-plugin-server.authredirectedparams.headers.md) + +## AuthRedirectedParams.headers property + +Headers to attach for auth redirect. Must include "location" header + +Signature: + +```typescript +headers: ResponseHeaders; +``` diff --git a/docs/development/core/server/kibana-plugin-server.authredirectedparams.md b/docs/development/core/server/kibana-plugin-server.authredirectedparams.md new file mode 100644 index 00000000000000..03f7b2797b2543 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-server.authredirectedparams.md @@ -0,0 +1,20 @@ + + +[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [AuthRedirectedParams](./kibana-plugin-server.authredirectedparams.md) + +## AuthRedirectedParams interface + +Result of auth redirection. + +Signature: + +```typescript +export interface AuthRedirectedParams +``` + +## Properties + +| Property | Type | Description | +| --- | --- | --- | +| [headers](./kibana-plugin-server.authredirectedparams.headers.md) | ResponseHeaders | Headers to attach for auth redirect. Must include "location" header | + diff --git a/docs/development/core/server/kibana-plugin-server.authresult.md b/docs/development/core/server/kibana-plugin-server.authresult.md index 5dd0e670b682c0..f540173f34c7ca 100644 --- a/docs/development/core/server/kibana-plugin-server.authresult.md +++ b/docs/development/core/server/kibana-plugin-server.authresult.md @@ -8,5 +8,5 @@ Signature: ```typescript -export declare type AuthResult = Authenticated | AuthNotHandled; +export declare type AuthResult = Authenticated | AuthNotHandled | AuthRedirected; ``` diff --git a/docs/development/core/server/kibana-plugin-server.authresulttype.md b/docs/development/core/server/kibana-plugin-server.authresulttype.md index cf11508bef79d6..48c159a94c23d9 100644 --- a/docs/development/core/server/kibana-plugin-server.authresulttype.md +++ b/docs/development/core/server/kibana-plugin-server.authresulttype.md @@ -17,4 +17,5 @@ export declare enum AuthResultType | --- | --- | --- | | authenticated | "authenticated" | | | notHandled | "notHandled" | | +| redirected | "redirected" | | diff --git a/docs/development/core/server/kibana-plugin-server.authtoolkit.md b/docs/development/core/server/kibana-plugin-server.authtoolkit.md index 820f2692186616..6f1c51be05ffac 100644 --- a/docs/development/core/server/kibana-plugin-server.authtoolkit.md +++ b/docs/development/core/server/kibana-plugin-server.authtoolkit.md @@ -17,5 +17,6 @@ export interface AuthToolkit | Property | Type | Description | | --- | --- | --- | | [authenticated](./kibana-plugin-server.authtoolkit.authenticated.md) | (data?: AuthResultParams) => AuthResult | Authentication is successful with given credentials, allow request to pass through | -| [notHandled](./kibana-plugin-server.authtoolkit.nothandled.md) | () => AuthResult | User has no credentials | +| [notHandled](./kibana-plugin-server.authtoolkit.nothandled.md) | () => AuthResult | User has no credentials. Allows user to access a resource when authRequired: 'optional' Rejects a request when authRequired: true | +| [redirected](./kibana-plugin-server.authtoolkit.redirected.md) | (headers: ResponseHeaders) => AuthResult | Redirect user to IdP when authRequired: true Allows user to access a resource without redirection when authRequired: 'optional' | diff --git a/docs/development/core/server/kibana-plugin-server.authtoolkit.nothandled.md b/docs/development/core/server/kibana-plugin-server.authtoolkit.nothandled.md index 7a74a4e2800869..7de174b3c7bb68 100644 --- a/docs/development/core/server/kibana-plugin-server.authtoolkit.nothandled.md +++ b/docs/development/core/server/kibana-plugin-server.authtoolkit.nothandled.md @@ -4,7 +4,7 @@ ## AuthToolkit.notHandled property -User has no credentials +User has no credentials. Allows user to access a resource when authRequired: 'optional' Rejects a request when authRequired: true Signature: diff --git a/docs/development/core/server/kibana-plugin-server.authtoolkit.redirected.md b/docs/development/core/server/kibana-plugin-server.authtoolkit.redirected.md new file mode 100644 index 00000000000000..a7034870c9c858 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-server.authtoolkit.redirected.md @@ -0,0 +1,13 @@ + + +[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [AuthToolkit](./kibana-plugin-server.authtoolkit.md) > [redirected](./kibana-plugin-server.authtoolkit.redirected.md) + +## AuthToolkit.redirected property + +Redirect user to IdP when authRequired: true Allows user to access a resource without redirection when authRequired: 'optional' + +Signature: + +```typescript +redirected: (headers: ResponseHeaders) => AuthResult; +``` diff --git a/docs/development/core/server/kibana-plugin-server.md b/docs/development/core/server/kibana-plugin-server.md index 2a6b5df77d4f3c..6dc1717d116aa3 100644 --- a/docs/development/core/server/kibana-plugin-server.md +++ b/docs/development/core/server/kibana-plugin-server.md @@ -46,6 +46,8 @@ The plugin integrates with the core system via lifecycle events: `setup` | [AssistantAPIClientParams](./kibana-plugin-server.assistantapiclientparams.md) | | | [Authenticated](./kibana-plugin-server.authenticated.md) | | | [AuthNotHandled](./kibana-plugin-server.authnothandled.md) | | +| [AuthRedirected](./kibana-plugin-server.authredirected.md) | | +| [AuthRedirectedParams](./kibana-plugin-server.authredirectedparams.md) | Result of auth redirection. | | [AuthResultParams](./kibana-plugin-server.authresultparams.md) | Result of successful authentication. | | [AuthToolkit](./kibana-plugin-server.authtoolkit.md) | A tool set defining an outcome of Auth interceptor for incoming request. | | [CallAPIOptions](./kibana-plugin-server.callapioptions.md) | The set of options that defines how API call should be made and result be processed. | diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index d8f58bdb62ed50..6214428b8bd5a3 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -425,7 +425,18 @@ export interface AuthNotHandled { } // @public (undocumented) -export type AuthResult = Authenticated | AuthNotHandled; +export interface AuthRedirected extends AuthRedirectedParams { + // (undocumented) + type: AuthResultType.redirected; +} + +// @public +export interface AuthRedirectedParams { + headers: ResponseHeaders; +} + +// @public (undocumented) +export type AuthResult = Authenticated | AuthNotHandled | AuthRedirected; // @public export interface AuthResultParams { @@ -439,7 +450,9 @@ export enum AuthResultType { // (undocumented) authenticated = "authenticated", // (undocumented) - notHandled = "notHandled" + notHandled = "notHandled", + // (undocumented) + redirected = "redirected" } // @public @@ -453,6 +466,7 @@ export enum AuthStatus { export interface AuthToolkit { authenticated: (data?: AuthResultParams) => AuthResult; notHandled: () => AuthResult; + redirected: (headers: ResponseHeaders) => AuthResult; } // @public From fcb4994b981c0c01e6a476b8abdccf109b2ad91b Mon Sep 17 00:00:00 2001 From: restrry Date: Thu, 5 Mar 2020 11:29:29 +0100 Subject: [PATCH 22/29] require location header in the interface --- .../kibana-plugin-server.authredirectedparams.headers.md | 4 +++- .../server/kibana-plugin-server.authredirectedparams.md | 2 +- .../core/server/kibana-plugin-server.authtoolkit.md | 2 +- .../server/kibana-plugin-server.authtoolkit.redirected.md | 4 +++- src/core/server/http/integration_tests/lifecycle.test.ts | 2 +- src/core/server/http/lifecycle/auth.ts | 6 +++--- src/core/server/server.api.md | 8 ++++++-- 7 files changed, 18 insertions(+), 10 deletions(-) diff --git a/docs/development/core/server/kibana-plugin-server.authredirectedparams.headers.md b/docs/development/core/server/kibana-plugin-server.authredirectedparams.headers.md index 1d9cdca9807e37..c1cf8218e75094 100644 --- a/docs/development/core/server/kibana-plugin-server.authredirectedparams.headers.md +++ b/docs/development/core/server/kibana-plugin-server.authredirectedparams.headers.md @@ -9,5 +9,7 @@ Headers to attach for auth redirect. Must include "location" header Signature: ```typescript -headers: ResponseHeaders; +headers: { + location: string; + } & ResponseHeaders; ``` diff --git a/docs/development/core/server/kibana-plugin-server.authredirectedparams.md b/docs/development/core/server/kibana-plugin-server.authredirectedparams.md index 03f7b2797b2543..3658f88fb64950 100644 --- a/docs/development/core/server/kibana-plugin-server.authredirectedparams.md +++ b/docs/development/core/server/kibana-plugin-server.authredirectedparams.md @@ -16,5 +16,5 @@ export interface AuthRedirectedParams | Property | Type | Description | | --- | --- | --- | -| [headers](./kibana-plugin-server.authredirectedparams.headers.md) | ResponseHeaders | Headers to attach for auth redirect. Must include "location" header | +| [headers](./kibana-plugin-server.authredirectedparams.headers.md) | {
location: string;
} & ResponseHeaders | Headers to attach for auth redirect. Must include "location" header | diff --git a/docs/development/core/server/kibana-plugin-server.authtoolkit.md b/docs/development/core/server/kibana-plugin-server.authtoolkit.md index 6f1c51be05ffac..a6a30dae894ada 100644 --- a/docs/development/core/server/kibana-plugin-server.authtoolkit.md +++ b/docs/development/core/server/kibana-plugin-server.authtoolkit.md @@ -18,5 +18,5 @@ export interface AuthToolkit | --- | --- | --- | | [authenticated](./kibana-plugin-server.authtoolkit.authenticated.md) | (data?: AuthResultParams) => AuthResult | Authentication is successful with given credentials, allow request to pass through | | [notHandled](./kibana-plugin-server.authtoolkit.nothandled.md) | () => AuthResult | User has no credentials. Allows user to access a resource when authRequired: 'optional' Rejects a request when authRequired: true | -| [redirected](./kibana-plugin-server.authtoolkit.redirected.md) | (headers: ResponseHeaders) => AuthResult | Redirect user to IdP when authRequired: true Allows user to access a resource without redirection when authRequired: 'optional' | +| [redirected](./kibana-plugin-server.authtoolkit.redirected.md) | (headers: {
location: string;
} & ResponseHeaders) => AuthResult | Redirect user to IdP when authRequired: true Allows user to access a resource without redirection when authRequired: 'optional' | diff --git a/docs/development/core/server/kibana-plugin-server.authtoolkit.redirected.md b/docs/development/core/server/kibana-plugin-server.authtoolkit.redirected.md index a7034870c9c858..64d1d04a4abc0c 100644 --- a/docs/development/core/server/kibana-plugin-server.authtoolkit.redirected.md +++ b/docs/development/core/server/kibana-plugin-server.authtoolkit.redirected.md @@ -9,5 +9,7 @@ Redirect user to IdP when authRequired: true Allows user to access a resource wi Signature: ```typescript -redirected: (headers: ResponseHeaders) => AuthResult; +redirected: (headers: { + location: string; + } & ResponseHeaders) => AuthResult; ``` diff --git a/src/core/server/http/integration_tests/lifecycle.test.ts b/src/core/server/http/integration_tests/lifecycle.test.ts index 82dcd0cd6d30e2..0f0d54e88daca3 100644 --- a/src/core/server/http/integration_tests/lifecycle.test.ts +++ b/src/core/server/http/integration_tests/lifecycle.test.ts @@ -527,7 +527,7 @@ describe('Auth', () => { const router = createRouter('/'); router.get({ path: '/', validate: false }, (context, req, res) => res.ok()); - registerAuth((req, res, t) => t.redirected({})); + registerAuth((req, res, t) => t.redirected({} as any)); await server.start(); await supertest(innerServer.listener) diff --git a/src/core/server/http/lifecycle/auth.ts b/src/core/server/http/lifecycle/auth.ts index 637c5a7cb96fdc..b5bb9c3dc65a3e 100644 --- a/src/core/server/http/lifecycle/auth.ts +++ b/src/core/server/http/lifecycle/auth.ts @@ -67,7 +67,7 @@ const authResult = { type: AuthResultType.notHandled, }; }, - redirected(headers: ResponseHeaders): AuthResult { + redirected(headers: { location: string } & ResponseHeaders): AuthResult { return { type: AuthResultType.redirected, headers, @@ -121,7 +121,7 @@ export interface AuthRedirectedParams { * Headers to attach for auth redirect. * Must include "location" header */ - headers: ResponseHeaders; + headers: { location: string } & ResponseHeaders; } /** @@ -141,7 +141,7 @@ export interface AuthToolkit { * Redirect user to IdP when authRequired: true * Allows user to access a resource without redirection when authRequired: 'optional' * */ - redirected: (headers: ResponseHeaders) => AuthResult; + redirected: (headers: { location: string } & ResponseHeaders) => AuthResult; } const toolkit: AuthToolkit = { diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index 6214428b8bd5a3..d75a8c2a73d275 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -432,7 +432,9 @@ export interface AuthRedirected extends AuthRedirectedParams { // @public export interface AuthRedirectedParams { - headers: ResponseHeaders; + headers: { + location: string; + } & ResponseHeaders; } // @public (undocumented) @@ -466,7 +468,9 @@ export enum AuthStatus { export interface AuthToolkit { authenticated: (data?: AuthResultParams) => AuthResult; notHandled: () => AuthResult; - redirected: (headers: ResponseHeaders) => AuthResult; + redirected: (headers: { + location: string; + } & ResponseHeaders) => AuthResult; } // @public From f1ecba3028cf5fd6e849db74c2662203149afa4c Mon Sep 17 00:00:00 2001 From: restrry Date: Fri, 6 Mar 2020 08:53:16 +0100 Subject: [PATCH 23/29] address comments #1 --- src/core/server/http/router/route.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/core/server/http/router/route.ts b/src/core/server/http/router/route.ts index b978d98fef39c5..bb0a8616e72222 100644 --- a/src/core/server/http/router/route.ts +++ b/src/core/server/http/router/route.ts @@ -120,8 +120,9 @@ export interface RouteConfigOptions { * - true. A user has to have valid credentials to access a resource * - false. A user can access a resource without any credentials. * - 'optional'. A user can access a resource if has valid credentials or no credentials at all. + * Can be useful when we grant access to a resource but want to identify a user if possible. * - * Set to true by default if an auth mechanism is registered. + * Defaults to `true` if an auth mechanism is registered. */ authRequired?: boolean | 'optional'; From f6dad7f26ea750bc86ec8f9a073f8abaa92efbef Mon Sep 17 00:00:00 2001 From: restrry Date: Fri, 6 Mar 2020 10:08:28 +0100 Subject: [PATCH 24/29] declare isAuthenticated on KibanaRequest --- src/core/server/http/http_server.mocks.ts | 6 + .../http/integration_tests/request.test.ts | 83 +++++++++++ .../http/integration_tests/router.test.ts | 139 ++++++++++++++++-- src/core/server/http/router/request.ts | 8 + 4 files changed, 221 insertions(+), 15 deletions(-) diff --git a/src/core/server/http/http_server.mocks.ts b/src/core/server/http/http_server.mocks.ts index 741c723ca93652..bbef0a105c0896 100644 --- a/src/core/server/http/http_server.mocks.ts +++ b/src/core/server/http/http_server.mocks.ts @@ -36,6 +36,7 @@ import { OnPostAuthToolkit } from './lifecycle/on_post_auth'; import { OnPreAuthToolkit } from './lifecycle/on_pre_auth'; interface RequestFixtureOptions

{ + auth?: { isAuthenticated: boolean }; headers?: Record; params?: Record; body?: Record; @@ -65,11 +66,13 @@ function createKibanaRequestMock

({ routeAuthRequired, validation = {}, kibanaRouteState = { xsrfRequired: true }, + auth = { isAuthenticated: true }, }: RequestFixtureOptions = {}) { const queryString = stringify(query, { sort: false }); return KibanaRequest.from( createRawRequestMock({ + auth, headers, params, query, @@ -113,6 +116,9 @@ function createRawRequestMock(customization: DeepPartial = {}) { {}, { app: { xsrfRequired: true } as any, + auth: { + isAuthenticated: true, + }, headers: {}, path: '/', route: { settings: {} }, diff --git a/src/core/server/http/integration_tests/request.test.ts b/src/core/server/http/integration_tests/request.test.ts index bc1bbc881315ab..85270174fbc048 100644 --- a/src/core/server/http/integration_tests/request.test.ts +++ b/src/core/server/http/integration_tests/request.test.ts @@ -45,6 +45,89 @@ afterEach(async () => { const delay = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)); describe('KibanaRequest', () => { + describe('auth', () => { + describe('isAuthenticated', () => { + it('returns false if no auth interceptor was registered', async () => { + const { server: innerServer, createRouter } = await server.setup(setupDeps); + const router = createRouter('/'); + router.get( + { path: '/', validate: false, options: { authRequired: true } }, + (context, req, res) => res.ok({ body: { isAuthenticated: req.auth.isAuthenticated } }) + ); + await server.start(); + + await supertest(innerServer.listener) + .get('/') + .expect(200, { + isAuthenticated: false, + }); + }); + it('returns false if not authenticated on a route with authRequired: "optional"', async () => { + const { server: innerServer, createRouter, registerAuth } = await server.setup(setupDeps); + const router = createRouter('/'); + registerAuth((req, res, toolkit) => toolkit.notHandled()); + router.get( + { path: '/', validate: false, options: { authRequired: 'optional' } }, + (context, req, res) => res.ok({ body: { isAuthenticated: req.auth.isAuthenticated } }) + ); + await server.start(); + + await supertest(innerServer.listener) + .get('/') + .expect(200, { + isAuthenticated: false, + }); + }); + it('returns false if redirected on a route with authRequired: "optional"', async () => { + const { server: innerServer, createRouter, registerAuth } = await server.setup(setupDeps); + const router = createRouter('/'); + registerAuth((req, res, toolkit) => toolkit.redirected({ location: '/any' })); + router.get( + { path: '/', validate: false, options: { authRequired: 'optional' } }, + (context, req, res) => res.ok({ body: { isAuthenticated: req.auth.isAuthenticated } }) + ); + await server.start(); + + await supertest(innerServer.listener) + .get('/') + .expect(200, { + isAuthenticated: false, + }); + }); + it('returns true if authenticated on a route with authRequired: "optional"', async () => { + const { server: innerServer, createRouter, registerAuth } = await server.setup(setupDeps); + const router = createRouter('/'); + registerAuth((req, res, toolkit) => toolkit.authenticated()); + router.get( + { path: '/', validate: false, options: { authRequired: 'optional' } }, + (context, req, res) => res.ok({ body: { isAuthenticated: req.auth.isAuthenticated } }) + ); + await server.start(); + + await supertest(innerServer.listener) + .get('/') + .expect(200, { + isAuthenticated: true, + }); + }); + it('returns true if authenticated', async () => { + const { server: innerServer, createRouter, registerAuth } = await server.setup(setupDeps); + const router = createRouter('/'); + registerAuth((req, res, toolkit) => toolkit.authenticated()); + router.get( + { path: '/', validate: false, options: { authRequired: true } }, + (context, req, res) => res.ok({ body: { isAuthenticated: req.auth.isAuthenticated } }) + ); + await server.start(); + + await supertest(innerServer.listener) + .get('/') + .expect(200, { + isAuthenticated: true, + }); + }); + }); + }); describe('events', () => { describe('aborted$', () => { it('emits once and completes when request aborted', async done => { diff --git a/src/core/server/http/integration_tests/router.test.ts b/src/core/server/http/integration_tests/router.test.ts index 5c22029d73d78c..ee5b0c50acafb1 100644 --- a/src/core/server/http/integration_tests/router.test.ts +++ b/src/core/server/http/integration_tests/router.test.ts @@ -50,22 +50,33 @@ describe('Options', () => { describe('authRequired', () => { describe('optional', () => { it('User has access to a route if auth mechanism not registered', async () => { - const { server: innerServer, createRouter } = await server.setup(setupDeps); + const { server: innerServer, createRouter, auth } = await server.setup(setupDeps); const router = createRouter('/'); router.get( { path: '/', validate: false, options: { authRequired: 'optional' } }, - (context, req, res) => res.ok({ body: 'ok' }) + (context, req, res) => + res.ok({ + body: { + httpAuthIsAuthenticated: auth.isAuthenticated(req), + requestIsAuthenticated: req.auth.isAuthenticated, + }, + }) ); await server.start(); await supertest(innerServer.listener) .get('/') - .expect(200, 'ok'); + .expect(200, { + httpAuthIsAuthenticated: false, + requestIsAuthenticated: false, + }); }); it('Authenticated user has access to a route', async () => { - const { server: innerServer, createRouter, registerAuth } = await server.setup(setupDeps); + const { server: innerServer, createRouter, registerAuth, auth } = await server.setup( + setupDeps + ); const router = createRouter('/'); registerAuth((req, res, toolkit) => { @@ -73,30 +84,50 @@ describe('Options', () => { }); router.get( { path: '/', validate: false, options: { authRequired: 'optional' } }, - (context, req, res) => res.ok({ body: 'ok' }) + (context, req, res) => + res.ok({ + body: { + httpAuthIsAuthenticated: auth.isAuthenticated(req), + requestIsAuthenticated: req.auth.isAuthenticated, + }, + }) ); await server.start(); await supertest(innerServer.listener) .get('/') - .expect(200, 'ok'); + .expect(200, { + httpAuthIsAuthenticated: true, + requestIsAuthenticated: true, + }); }); it('User with no credentials can access a route', async () => { - const { server: innerServer, createRouter, registerAuth } = await server.setup(setupDeps); + const { server: innerServer, createRouter, registerAuth, auth } = await server.setup( + setupDeps + ); const router = createRouter('/'); registerAuth((req, res, toolkit) => toolkit.notHandled()); router.get( { path: '/', validate: false, options: { authRequired: 'optional' } }, - (context, req, res) => res.ok({ body: 'ok' }) + (context, req, res) => + res.ok({ + body: { + httpAuthIsAuthenticated: auth.isAuthenticated(req), + requestIsAuthenticated: req.auth.isAuthenticated, + }, + }) ); await server.start(); await supertest(innerServer.listener) .get('/') - .expect(200, 'ok'); + .expect(200, { + httpAuthIsAuthenticated: false, + requestIsAuthenticated: false, + }); }); it('User with invalid credentials cannot access a route', async () => { @@ -117,7 +148,9 @@ describe('Options', () => { }); it('does not redirect user and allows access to a resource', async () => { - const { server: innerServer, createRouter, registerAuth } = await server.setup(setupDeps); + const { server: innerServer, createRouter, registerAuth, auth } = await server.setup( + setupDeps + ); const router = createRouter('/'); registerAuth((req, res, toolkit) => @@ -128,19 +161,54 @@ describe('Options', () => { router.get( { path: '/', validate: false, options: { authRequired: 'optional' } }, - (context, req, res) => res.ok({ body: 'ok' }) + (context, req, res) => + res.ok({ + body: { + httpAuthIsAuthenticated: auth.isAuthenticated(req), + requestIsAuthenticated: req.auth.isAuthenticated, + }, + }) ); await server.start(); await supertest(innerServer.listener) .get('/') - .expect(200, 'ok'); + .expect(200, { + httpAuthIsAuthenticated: false, + requestIsAuthenticated: false, + }); }); }); describe('true', () => { + it('User has access to a route if auth interceptor is not registered', async () => { + const { server: innerServer, createRouter, auth } = await server.setup(setupDeps); + const router = createRouter('/'); + + router.get( + { path: '/', validate: false, options: { authRequired: true } }, + (context, req, res) => + res.ok({ + body: { + httpAuthIsAuthenticated: auth.isAuthenticated(req), + requestIsAuthenticated: req.auth.isAuthenticated, + }, + }) + ); + await server.start(); + + await supertest(innerServer.listener) + .get('/') + .expect(200, { + httpAuthIsAuthenticated: false, + requestIsAuthenticated: false, + }); + }); + it('Authenticated user has access to a route', async () => { - const { server: innerServer, createRouter, registerAuth } = await server.setup(setupDeps); + const { server: innerServer, createRouter, registerAuth, auth } = await server.setup( + setupDeps + ); const router = createRouter('/'); registerAuth((req, res, toolkit) => { @@ -148,13 +216,22 @@ describe('Options', () => { }); router.get( { path: '/', validate: false, options: { authRequired: true } }, - (context, req, res) => res.ok({ body: 'ok' }) + (context, req, res) => + res.ok({ + body: { + httpAuthIsAuthenticated: auth.isAuthenticated(req), + requestIsAuthenticated: req.auth.isAuthenticated, + }, + }) ); await server.start(); await supertest(innerServer.listener) .get('/') - .expect(200, 'ok'); + .expect(200, { + httpAuthIsAuthenticated: true, + requestIsAuthenticated: true, + }); }); it('User with no credentials cannot access a route', async () => { @@ -214,6 +291,38 @@ describe('Options', () => { expect(result.header.location).toBe(redirectUrl); }); }); + + describe('false', () => { + it('does not try to authenticate a user', async () => { + const { server: innerServer, createRouter, registerAuth, auth } = await server.setup( + setupDeps + ); + const router = createRouter('/'); + + const authHook = jest.fn(); + registerAuth(authHook); + router.get( + { path: '/', validate: false, options: { authRequired: false } }, + (context, req, res) => + res.ok({ + body: { + httpAuthIsAuthenticated: auth.isAuthenticated(req), + requestIsAuthenticated: req.auth.isAuthenticated, + }, + }) + ); + await server.start(); + + await supertest(innerServer.listener) + .get('/') + .expect(200, { + httpAuthIsAuthenticated: false, + requestIsAuthenticated: false, + }); + + expect(authHook).toHaveBeenCalledTimes(0); + }); + }); }); }); diff --git a/src/core/server/http/router/request.ts b/src/core/server/http/router/request.ts index 024a12ad99051f..3ec31b823a790f 100644 --- a/src/core/server/http/router/request.ts +++ b/src/core/server/http/router/request.ts @@ -143,6 +143,10 @@ export class KibanaRequest< public readonly socket: IKibanaSocket; /** Request events {@link KibanaRequestEvents} */ public readonly events: KibanaRequestEvents; + public readonly auth: { + /* true if the request has been successfully authenticated, otherwise false. */ + isAuthenticated: boolean; + }; /** @internal */ protected readonly [requestSymbol]: Request; @@ -172,6 +176,10 @@ export class KibanaRequest< this.route = deepFreeze(this.getRouteInfo(request)); this.socket = new KibanaSocket(request.raw.req.socket); this.events = this.getEvents(request); + + this.auth = { + isAuthenticated: request.auth.isAuthenticated, + }; } private getEvents(request: Request): KibanaRequestEvents { From d1901a8ad323511959c4e6b4973a97e86ef25f19 Mon Sep 17 00:00:00 2001 From: restrry Date: Fri, 6 Mar 2020 11:22:40 +0100 Subject: [PATCH 25/29] remove auth.isAuthenticated from scope --- src/core/server/index.ts | 3 --- src/core/server/mocks.ts | 3 --- src/core/server/server.ts | 3 --- x-pack/plugins/security/server/authorization/index.ts | 2 +- 4 files changed, 1 insertion(+), 10 deletions(-) diff --git a/src/core/server/index.ts b/src/core/server/index.ts index efd7abbab0472c..63eeb724da0d50 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -301,9 +301,6 @@ export { */ export interface RequestHandlerContext { core: { - auth: { - isAuthenticated: boolean; - }; rendering: IScopedRenderingClient; savedObjects: { client: SavedObjectsClientContract; diff --git a/src/core/server/mocks.ts b/src/core/server/mocks.ts index 2e9539667e7007..037f3bbed67e06 100644 --- a/src/core/server/mocks.ts +++ b/src/core/server/mocks.ts @@ -171,9 +171,6 @@ function createInternalCoreStartMock() { function createCoreRequestHandlerContextMock() { return { - auth: { - isAuthenticated: true, - }, rendering: { render: jest.fn(), }, diff --git a/src/core/server/server.ts b/src/core/server/server.ts index 4ac32767aefc5e..8603f5fba1da87 100644 --- a/src/core/server/server.ts +++ b/src/core/server/server.ts @@ -233,9 +233,6 @@ export class Server { const uiSettingsClient = coreSetup.uiSettings.asScopedToClient(savedObjectsClient); return { - auth: { - isAuthenticated: coreSetup.http.auth.isAuthenticated(req), - }, rendering: { render: async (options = {}) => rendering.render(req, uiSettingsClient, { diff --git a/x-pack/plugins/security/server/authorization/index.ts b/x-pack/plugins/security/server/authorization/index.ts index 57950940a0f84b..507f8a3fc8c0bf 100644 --- a/x-pack/plugins/security/server/authorization/index.ts +++ b/x-pack/plugins/security/server/authorization/index.ts @@ -113,7 +113,7 @@ export function setupAuthorization({ ); // if we're an anonymous route or optional auth, we disable all ui capabilities - if (!http.auth.isAuthenticated(request)) { + if (!request.auth.isAuthenticated) { return disableUICapabilities.all(capabilities); } From 0275e257412575ef54905a320b6a96399ed65146 Mon Sep 17 00:00:00 2001 From: restrry Date: Fri, 6 Mar 2020 11:24:40 +0100 Subject: [PATCH 26/29] update docs --- .../kibana-plugin-server.kibanarequest.auth.md | 13 +++++++++++++ .../server/kibana-plugin-server.kibanarequest.md | 1 + ...bana-plugin-server.requesthandlercontext.core.md | 3 --- .../kibana-plugin-server.requesthandlercontext.md | 2 +- ...plugin-server.routeconfigoptions.authrequired.md | 4 ++-- .../kibana-plugin-server.routeconfigoptions.md | 2 +- src/core/server/server.api.md | 7 ++++--- 7 files changed, 22 insertions(+), 10 deletions(-) create mode 100644 docs/development/core/server/kibana-plugin-server.kibanarequest.auth.md diff --git a/docs/development/core/server/kibana-plugin-server.kibanarequest.auth.md b/docs/development/core/server/kibana-plugin-server.kibanarequest.auth.md new file mode 100644 index 00000000000000..536d6bd04d9379 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-server.kibanarequest.auth.md @@ -0,0 +1,13 @@ + + +[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [KibanaRequest](./kibana-plugin-server.kibanarequest.md) > [auth](./kibana-plugin-server.kibanarequest.auth.md) + +## KibanaRequest.auth property + +Signature: + +```typescript +readonly auth: { + isAuthenticated: boolean; + }; +``` diff --git a/docs/development/core/server/kibana-plugin-server.kibanarequest.md b/docs/development/core/server/kibana-plugin-server.kibanarequest.md index cb6745623e3818..0d520783fd4cf6 100644 --- a/docs/development/core/server/kibana-plugin-server.kibanarequest.md +++ b/docs/development/core/server/kibana-plugin-server.kibanarequest.md @@ -22,6 +22,7 @@ export declare class KibanaRequest{
isAuthenticated: boolean;
} | | | [body](./kibana-plugin-server.kibanarequest.body.md) | | Body | | | [events](./kibana-plugin-server.kibanarequest.events.md) | | KibanaRequestEvents | Request events [KibanaRequestEvents](./kibana-plugin-server.kibanarequestevents.md) | | [headers](./kibana-plugin-server.kibanarequest.headers.md) | | Headers | Readonly copy of incoming request headers. | diff --git a/docs/development/core/server/kibana-plugin-server.requesthandlercontext.core.md b/docs/development/core/server/kibana-plugin-server.requesthandlercontext.core.md index cb0beb20f1c504..77bfd85e6e54bb 100644 --- a/docs/development/core/server/kibana-plugin-server.requesthandlercontext.core.md +++ b/docs/development/core/server/kibana-plugin-server.requesthandlercontext.core.md @@ -8,9 +8,6 @@ ```typescript core: { - auth: { - isAuthenticated: boolean; - }; rendering: IScopedRenderingClient; savedObjects: { client: SavedObjectsClientContract; diff --git a/docs/development/core/server/kibana-plugin-server.requesthandlercontext.md b/docs/development/core/server/kibana-plugin-server.requesthandlercontext.md index ddcf6f3e5f973f..4d14d890f51a2e 100644 --- a/docs/development/core/server/kibana-plugin-server.requesthandlercontext.md +++ b/docs/development/core/server/kibana-plugin-server.requesthandlercontext.md @@ -18,5 +18,5 @@ export interface RequestHandlerContext | Property | Type | Description | | --- | --- | --- | -| [core](./kibana-plugin-server.requesthandlercontext.core.md) | {
auth: {
isAuthenticated: boolean;
};
rendering: IScopedRenderingClient;
savedObjects: {
client: SavedObjectsClientContract;
};
elasticsearch: {
dataClient: IScopedClusterClient;
adminClient: IScopedClusterClient;
};
uiSettings: {
client: IUiSettingsClient;
};
} | | +| [core](./kibana-plugin-server.requesthandlercontext.core.md) | {
rendering: IScopedRenderingClient;
savedObjects: {
client: SavedObjectsClientContract;
};
elasticsearch: {
dataClient: IScopedClusterClient;
adminClient: IScopedClusterClient;
};
uiSettings: {
client: IUiSettingsClient;
};
} | | diff --git a/docs/development/core/server/kibana-plugin-server.routeconfigoptions.authrequired.md b/docs/development/core/server/kibana-plugin-server.routeconfigoptions.authrequired.md index 9de93215cd7e9b..830abd4dde738c 100644 --- a/docs/development/core/server/kibana-plugin-server.routeconfigoptions.authrequired.md +++ b/docs/development/core/server/kibana-plugin-server.routeconfigoptions.authrequired.md @@ -4,9 +4,9 @@ ## RouteConfigOptions.authRequired property -Defines authentication mode for a route: - true. A user has to have valid credentials to access a resource - false. A user can access a resource without any credentials. - 'optional'. A user can access a resource if has valid credentials or no credentials at all. +Defines authentication mode for a route: - true. A user has to have valid credentials to access a resource - false. A user can access a resource without any credentials. - 'optional'. A user can access a resource if has valid credentials or no credentials at all. Can be useful when we grant access to a resource but want to identify a user if possible. -Set to true by default if an auth mechanism is registered. +Defaults to `true` if an auth mechanism is registered. Signature: diff --git a/docs/development/core/server/kibana-plugin-server.routeconfigoptions.md b/docs/development/core/server/kibana-plugin-server.routeconfigoptions.md index d458a04a1f6589..6664a28424a326 100644 --- a/docs/development/core/server/kibana-plugin-server.routeconfigoptions.md +++ b/docs/development/core/server/kibana-plugin-server.routeconfigoptions.md @@ -16,7 +16,7 @@ export interface RouteConfigOptions | Property | Type | Description | | --- | --- | --- | -| [authRequired](./kibana-plugin-server.routeconfigoptions.authrequired.md) | boolean | 'optional' | Defines authentication mode for a route: - true. A user has to have valid credentials to access a resource - false. A user can access a resource without any credentials. - 'optional'. A user can access a resource if has valid credentials or no credentials at all.Set to true by default if an auth mechanism is registered. | +| [authRequired](./kibana-plugin-server.routeconfigoptions.authrequired.md) | boolean | 'optional' | Defines authentication mode for a route: - true. A user has to have valid credentials to access a resource - false. A user can access a resource without any credentials. - 'optional'. A user can access a resource if has valid credentials or no credentials at all. Can be useful when we grant access to a resource but want to identify a user if possible.Defaults to true if an auth mechanism is registered. | | [body](./kibana-plugin-server.routeconfigoptions.body.md) | Method extends 'get' | 'options' ? undefined : RouteConfigOptionsBody | Additional body options [RouteConfigOptionsBody](./kibana-plugin-server.routeconfigoptionsbody.md). | | [tags](./kibana-plugin-server.routeconfigoptions.tags.md) | readonly string[] | Additional metadata tag strings to attach to the route. | | [xsrfRequired](./kibana-plugin-server.routeconfigoptions.xsrfrequired.md) | Method extends 'get' ? never : boolean | Defines xsrf protection requirements for a route: - true. Requires an incoming POST/PUT/DELETE request to contain kbn-xsrf header. - false. Disables xsrf protection.Set to true by default | diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index d75a8c2a73d275..f9db676fcdfe7a 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -989,6 +989,10 @@ export class KibanaRequest Date: Fri, 6 Mar 2020 11:27:17 +0100 Subject: [PATCH 27/29] remove unnecessary comment --- x-pack/plugins/security/server/authorization/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugins/security/server/authorization/index.ts b/x-pack/plugins/security/server/authorization/index.ts index 507f8a3fc8c0bf..4cbc76ecb6be43 100644 --- a/x-pack/plugins/security/server/authorization/index.ts +++ b/x-pack/plugins/security/server/authorization/index.ts @@ -112,7 +112,6 @@ export function setupAuthorization({ authz ); - // if we're an anonymous route or optional auth, we disable all ui capabilities if (!request.auth.isAuthenticated) { return disableUICapabilities.all(capabilities); } From f177d6940962d369d9a43bab6d69a3a6a459d542 Mon Sep 17 00:00:00 2001 From: restrry Date: Fri, 6 Mar 2020 12:39:24 +0100 Subject: [PATCH 28/29] do not fail on FakrRequest --- src/core/server/http/router/request.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/core/server/http/router/request.ts b/src/core/server/http/router/request.ts index 3ec31b823a790f..f266677c1a1727 100644 --- a/src/core/server/http/router/request.ts +++ b/src/core/server/http/router/request.ts @@ -178,7 +178,8 @@ export class KibanaRequest< this.events = this.getEvents(request); this.auth = { - isAuthenticated: request.auth.isAuthenticated, + // missing in fakeRequests, so we cast to false + isAuthenticated: Boolean(request.auth?.isAuthenticated), }; } From 86f46030e966efcb3b694ea9ea35d8e5beb4fdaa Mon Sep 17 00:00:00 2001 From: restrry Date: Fri, 6 Mar 2020 19:09:28 +0100 Subject: [PATCH 29/29] small improvements --- src/core/server/http/http_server.ts | 2 +- src/core/server/http/lifecycle/auth.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/server/http/http_server.ts b/src/core/server/http/http_server.ts index 34035d8decca94..f898ed0ea1a991 100644 --- a/src/core/server/http/http_server.ts +++ b/src/core/server/http/http_server.ts @@ -147,7 +147,7 @@ export class HttpServer { this.log.debug(`registering route handler for [${route.path}]`); // Hapi does not allow payload validation to be specified for 'head' or 'get' requests const validate = isSafeMethod(route.method) ? undefined : { payload: true }; - const { authRequired = true, tags, body = {} } = route.options; + const { authRequired, tags, body = {} } = route.options; const { accepts: allow, maxBytes, output, parse } = body; const kibanaRouteState: KibanaRouteState = { diff --git a/src/core/server/http/lifecycle/auth.ts b/src/core/server/http/lifecycle/auth.ts index b5bb9c3dc65a3e..2eaf7e0f6fbfed 100644 --- a/src/core/server/http/lifecycle/auth.ts +++ b/src/core/server/http/lifecycle/auth.ts @@ -138,7 +138,7 @@ export interface AuthToolkit { * */ notHandled: () => AuthResult; /** - * Redirect user to IdP when authRequired: true + * Redirects user to another location to complete authentication when authRequired: true * Allows user to access a resource without redirection when authRequired: 'optional' * */ redirected: (headers: { location: string } & ResponseHeaders) => AuthResult;