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 025ab2bf56ac2..860e8feaa9c72 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 036ab0211c2ff..98ccf8b586e3e 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 703571ba53c0a..903a259853dc7 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 4439a80b1eac7..be5d47729275d 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 db2493b38d6e0..8482dbdd14524 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 3727b1fc13dac..84df922c429e7 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 467afe0034025..49f86ae82ef3c 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 05d718e1073df..d602422c14634 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 aace0b9debf9c..7d2e7391aa8d4 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 ccaa4621d7003..74c485986a77b 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 5e1d49b4b1b7e..3fb1bb3d13d0b 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 a9fc80c86d878..296d95e56c42a 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 30032ff5da796..c87b893f5bb93 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 6dc7ece1359df..caa8c75b9b9d3 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 a1523781010d4..485844772eeba 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 032027c234485..e3f67401d7c63 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 52827b72ee0cc..ed3cbb39938e1 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 d6554babab53e..88eeb10fccef0 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 485844772eeba..b427a89872f28 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 98ccf8b586e3e..3dfc860685b23 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 425d8cac1893e..00e54548468e5 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 00a50dd5b8821..57950940a0f84 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 0000000000000..5d4a39a88ea64 --- /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 0000000000000..81543de0ec61b --- /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 0000000000000..90e9f5a771e55 --- /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 0000000000000..9e1e733246d16 --- /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 8739c4899bd02..5dd0e670b682c 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 55b247f21f5a9..7a725cb340f5b 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 61a98ee5e7b11..cf11508bef79d 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 bc7003c5a68f3..cee59409b7999 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 0000000000000..41c5cb16567d4 --- /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 9ec443d6482e8..119054fd74b87 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 77bfd85e6e54b..15e60905dbc30 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 4d14d890f51a2..8bd2fc61f9f4e 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 e4cbca9c97810..9de93215cd7e9 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 0929e15b6228b..15d5b5514633c 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 d31afe1670e41..85038f6ca6ad0 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 3dfc860685b23..de8ea7b568e1d 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 be5d47729275d..a13d71c3e9e5b 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 ed3cbb39938e1..b0a9da8414cd2 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 f717f30fdb0cf..44afd3a0e816f 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 b427a89872f28..a323ce97469a3 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 27db79bb94d25..46f9bf3e675bc 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 00e54548468e5..7b1630a7de0be 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 46f9bf3e675bc..27db79bb94d25 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 0a09c962ec31b..146c9f264023d 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 caa8c75b9b9d3..67e14b9a55db7 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 a323ce97469a3..c955d03f12cf6 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 de8ea7b568e1d..d5f0f1c22bffc 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 e3f67401d7c63..fb999dc60e39c 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 d5f0f1c22bffc..978ad041ccf1a 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 9072096b9ff09..e17df2f834963 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 99ac22ab450dc..2e9539667e700 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 a19c417abb346..4ac32767aefc5 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 146c9f264023d..34035d8decca9 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 b57e2bb0448df..234455988105c 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 67e14b9a55db7..23bd355efc19b 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 c955d03f12cf6..d3a33710ecfb0 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 978ad041ccf1a..c0893c4f1d529 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 e17df2f834963..879626005ca45 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 c6646aab9b115..ae96156069728 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 5d4a39a88ea64..01e465c266319 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 90e9f5a771e55..0000000000000 --- 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 9e1e733246d16..0000000000000 --- 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 cee59409b7999..820f269218661 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 41c5cb16567d4..7a74a4e280086 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 08e47a1559e18..2a6b5df77d4f3 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 15e60905dbc30..cb0beb20f1c50 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 8bd2fc61f9f4e..ddcf6f3e5f973 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 51e1fc5f4f79c..d8f58bdb62ed5 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 c87b893f5bb93..442bc93190d86 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 234455988105c..a75eb04fa0120 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 23bd355efc19b..82dcd0cd6d30e 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 d3a33710ecfb0..5c22029d73d78 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 c0893c4f1d529..637c5a7cb96fd 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 879626005ca45..efd7abbab0472 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 92a92dda84280..30929ba98d33b 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 ae96156069728..1eed53efc6441 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 0000000000000..3eb88d6c5a230 --- /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 0000000000000..866ed358119e7 --- /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 0000000000000..1d9cdca9807e3 --- /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 0000000000000..03f7b2797b254 --- /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 5dd0e670b682c..f540173f34c7c 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 cf11508bef79d..48c159a94c23d 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 820f269218661..6f1c51be05ffa 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 7a74a4e280086..7de174b3c7bb6 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 0000000000000..a7034870c9c85 --- /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 2a6b5df77d4f3..6dc1717d116aa 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 d8f58bdb62ed5..6214428b8bd5a 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 1d9cdca9807e3..c1cf8218e7509 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 03f7b2797b254..3658f88fb6495 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 6f1c51be05ffa..a6a30dae894ad 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 a7034870c9c85..64d1d04a4abc0 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 82dcd0cd6d30e..0f0d54e88daca 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 637c5a7cb96fd..b5bb9c3dc65a3 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 6214428b8bd5a..d75a8c2a73d27 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 b978d98fef39c..bb0a8616e7222 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 741c723ca9365..bbef0a105c089 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 bc1bbc881315a..85270174fbc04 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 5c22029d73d78..ee5b0c50acafb 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 024a12ad99051..3ec31b823a790 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 efd7abbab0472..63eeb724da0d5 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 2e9539667e700..037f3bbed67e0 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 4ac32767aefc5..8603f5fba1da8 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 57950940a0f84..507f8a3fc8c0b 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 0000000000000..536d6bd04d937 --- /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 cb6745623e381..0d520783fd4cf 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 cb0beb20f1c50..77bfd85e6e54b 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 ddcf6f3e5f973..4d14d890f51a2 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 9de93215cd7e9..830abd4dde738 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 d458a04a1f658..6664a28424a32 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 d75a8c2a73d27..f9db676fcdfe7 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 507f8a3fc8c0b..4cbc76ecb6be4 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 3ec31b823a790..f266677c1a172 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 34035d8decca9..f898ed0ea1a99 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 b5bb9c3dc65a3..2eaf7e0f6fbfe 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;