From 89124e7346add29a28c42342df2519b0f670c0ec Mon Sep 17 00:00:00 2001 From: Larry Gregory Date: Wed, 29 May 2019 11:01:35 -0400 Subject: [PATCH] Security - remove auth scope provider (#36998) * remove auth scope provider * handle missing roles * guard for unauthenticated calls * update functional tests to not expect a scope property * there's always money in the banana stand * revert interceptor optimizations * protect against missing roles * address pr feedback * remove scope as expected property on kerberos auth response --- .../dashboard_mode/common/constants.js | 1 - x-pack/plugins/dashboard_mode/index.js | 4 - .../__tests__/dashboard_mode_auth_scope.js | 98 ------------- .../dashboard_mode_request_interceptor.js | 35 +++-- .../server/dashboard_mode_auth_scope.js | 39 ----- .../dashboard_mode_request_interceptor.js | 30 +++- x-pack/plugins/dashboard_mode/server/index.js | 1 - .../server/lib/auth_scope_service.test.ts | 135 ------------------ .../security/server/lib/auth_scope_service.ts | 71 --------- .../lib/authentication/authenticator.test.ts | 30 +--- .../lib/authentication/authenticator.ts | 20 +-- .../on_post_auth_interceptor.ts | 2 +- .../apis/security/basic_login.js | 2 - .../apis/security/kerberos_login.ts | 1 - .../apis/security/oidc_initiate_auth.js | 2 - .../apis/security/saml_login.js | 2 - 16 files changed, 53 insertions(+), 420 deletions(-) delete mode 100644 x-pack/plugins/dashboard_mode/server/__tests__/dashboard_mode_auth_scope.js delete mode 100644 x-pack/plugins/dashboard_mode/server/dashboard_mode_auth_scope.js delete mode 100644 x-pack/plugins/security/server/lib/auth_scope_service.test.ts delete mode 100644 x-pack/plugins/security/server/lib/auth_scope_service.ts diff --git a/x-pack/plugins/dashboard_mode/common/constants.js b/x-pack/plugins/dashboard_mode/common/constants.js index ac16aae957d0b..c9a2378ac5d82 100644 --- a/x-pack/plugins/dashboard_mode/common/constants.js +++ b/x-pack/plugins/dashboard_mode/common/constants.js @@ -5,4 +5,3 @@ */ export const CONFIG_DASHBOARD_ONLY_MODE_ROLES = 'xpackDashboardMode:roles'; -export const AUTH_SCOPE_DASHBORD_ONLY_MODE = 'xpack:dashboardMode'; diff --git a/x-pack/plugins/dashboard_mode/index.js b/x-pack/plugins/dashboard_mode/index.js index c0c6b3ebc1dbd..6cb3f3f886bc7 100644 --- a/x-pack/plugins/dashboard_mode/index.js +++ b/x-pack/plugins/dashboard_mode/index.js @@ -11,7 +11,6 @@ import { } from './common'; import { - getDashboardModeAuthScope, createDashboardModeRequestInterceptor, } from './server'; @@ -80,9 +79,6 @@ export function dashboardMode(kibana) { )); if (server.plugins.security) { - // register auth getter with security plugin - server.plugins.security.registerAuthScopeGetter(getDashboardModeAuthScope); - // extend the server to intercept requests const dashboardViewerApp = server.getHiddenUiAppById('dashboardViewer'); server.ext(createDashboardModeRequestInterceptor(dashboardViewerApp)); diff --git a/x-pack/plugins/dashboard_mode/server/__tests__/dashboard_mode_auth_scope.js b/x-pack/plugins/dashboard_mode/server/__tests__/dashboard_mode_auth_scope.js deleted file mode 100644 index 644d006de400e..0000000000000 --- a/x-pack/plugins/dashboard_mode/server/__tests__/dashboard_mode_auth_scope.js +++ /dev/null @@ -1,98 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -import expect from '@kbn/expect'; - -import { - CONFIG_DASHBOARD_ONLY_MODE_ROLES, - AUTH_SCOPE_DASHBORD_ONLY_MODE, -} from '../../common'; - -import { - getDashboardModeAuthScope -} from '../dashboard_mode_auth_scope'; - -describe('getDashboardModeAuthScope()', () => { - async function test({ userRoles, dashRoles, expected }) { - const request = { - getUiSettingsService() { - return { - async get(key) { - expect(key).to.equal(CONFIG_DASHBOARD_ONLY_MODE_ROLES); - return dashRoles; - } - }; - } - }; - - const user = { - roles: userRoles - }; - - const result = await getDashboardModeAuthScope(request, user); - expect(result).to.eql(expected); - } - - describe('returns tag when', () => { - it('all roles are dashboardOnlyModeRoles', async () => { - await test({ - userRoles: ['dash-only-role', 'another-limited-role'], - dashRoles: ['dash-only-role', 'another-limited-role'], - expected: [AUTH_SCOPE_DASHBORD_ONLY_MODE] - }); - }); - - it('one role is not in dashboard mode', async () => { - await test({ - userRoles: ['dash-only-role', 'another-limited-role', 'a-super-role'], - dashRoles: ['dash-only-role', 'another-limited-role'], - expected: [AUTH_SCOPE_DASHBORD_ONLY_MODE] - }); - }); - }); - - describe('does not return tag when', () => { - it('no roles are in dashboard mode', async () => { - await test({ - userRoles: ['super-role', 'a-super-role'], - dashRoles: ['dash-only-role', 'another-limited-role'], - expected: undefined - }); - }); - - it('one role is in dashboard mode and the user has superuser privileges', async () => { - await test({ - userRoles: ['dash-only-role', 'another-limited-role', 'superuser'], - dashRoles: ['dash-only-role', 'another-limited-role'], - expected: undefined - }); - }); - - it('the user has no roles', async () => { - await test({ - userRoles: [], - dashRoles: ['dash-only-role'], - expected: undefined - }); - }); - - it('no dash roles', async () => { - await test({ - userRoles: ['dash-only-role'], - dashRoles: [], - expected: undefined - }); - }); - - it('when no roles are in dashboard mode and user has no roles', async () => { - await test({ - userRoles: [], - dashRoles: [], - expected: undefined - }); - }); - }); -}); diff --git a/x-pack/plugins/dashboard_mode/server/__tests__/dashboard_mode_request_interceptor.js b/x-pack/plugins/dashboard_mode/server/__tests__/dashboard_mode_request_interceptor.js index 7250293fd50b0..5eba1c93973d8 100644 --- a/x-pack/plugins/dashboard_mode/server/__tests__/dashboard_mode_request_interceptor.js +++ b/x-pack/plugins/dashboard_mode/server/__tests__/dashboard_mode_request_interceptor.js @@ -6,27 +6,24 @@ import expect from '@kbn/expect'; import Hapi from 'hapi'; -import Chance from 'chance'; import { createDashboardModeRequestInterceptor } from '../dashboard_mode_request_interceptor'; -import * as constantsNS from '../../common/constants'; -const chance = new Chance(); +const DASHBOARD_ONLY_MODE_ROLE = 'test_dashboard_only_mode_role'; function setup() { - // randomize AUTH_SCOPE_DASHBORD_ONLY_MODE "constant" to ensure it's being used everywhere - const authScope = chance.word({ length: 12 }); - Object.defineProperty(constantsNS, 'AUTH_SCOPE_DASHBORD_ONLY_MODE', { - value: authScope, - configurable: true, - }); - const dashboardViewerApp = { name: 'dashboardViewerApp' }; const server = new Hapi.Server(); + server.decorate('request', 'getUiSettingsService', () => { + return { + get: () => Promise.resolve([DASHBOARD_ONLY_MODE_ROLE]) + }; + }); + // attach the extension server.ext(createDashboardModeRequestInterceptor(dashboardViewerApp)); @@ -53,7 +50,7 @@ function setup() { } }); - return { server, authScope }; + return { server }; } describe('DashboardOnlyModeRequestInterceptor', () => { @@ -65,14 +62,14 @@ describe('DashboardOnlyModeRequestInterceptor', () => { }); }); - describe('request does not have `xpack:dashboardMode` scope', () => { + describe('request is not for dashboad-only user', () => { describe('app route', () => { it('lets the route render as normal', async () => { const { server } = setup(); const response = await server.inject({ url: '/app/kibana', credentials: { - scope: ['foo', 'bar'] + roles: ['foo', 'bar'] } }); @@ -91,7 +88,7 @@ describe('DashboardOnlyModeRequestInterceptor', () => { const response = await server.inject({ url: '/foo/bar', credentials: { - scope: ['foo', 'bar'] + roles: ['foo', 'bar'] } }); @@ -105,14 +102,14 @@ describe('DashboardOnlyModeRequestInterceptor', () => { }); }); - describe('request has correct auth scope scope', () => { + describe('request for dashboard-only user', () => { describe('non-kibana app route', () => { it('responds with 404', async () => { - const { server, authScope } = setup(); + const { server } = setup(); const response = await server.inject({ url: '/app/foo', credentials: { - scope: [authScope] + roles: [DASHBOARD_ONLY_MODE_ROLE] } }); @@ -124,11 +121,11 @@ describe('DashboardOnlyModeRequestInterceptor', () => { function testRendersDashboardViewerApp(url) { describe(`requests to url:"${url}"`, () => { it('renders the dashboardViewerApp instead', async () => { - const { server, authScope } = setup(); + const { server } = setup(); const response = await server.inject({ url: '/app/kibana', credentials: { - scope: [authScope] + roles: [DASHBOARD_ONLY_MODE_ROLE] } }); diff --git a/x-pack/plugins/dashboard_mode/server/dashboard_mode_auth_scope.js b/x-pack/plugins/dashboard_mode/server/dashboard_mode_auth_scope.js deleted file mode 100644 index 36551069b8069..0000000000000 --- a/x-pack/plugins/dashboard_mode/server/dashboard_mode_auth_scope.js +++ /dev/null @@ -1,39 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -import { - CONFIG_DASHBOARD_ONLY_MODE_ROLES, - AUTH_SCOPE_DASHBORD_ONLY_MODE, -} from '../common'; - -const superuserRole = 'superuser'; - -/** - * Registered with the security plugin to extend the auth scopes to - * include "xpack:dashboardMode" when the request should be in - * dashboard only mode. - * - * @type {XpackSecurity.ScopeExtender} - * @param {Hapi.Request} request - * @param {Object} user user object from the security api - * @return {Promise|void>} - */ -export async function getDashboardModeAuthScope(request, user) { - const uiSettings = request.getUiSettingsService(); - const dashboardOnlyModeRoles = await uiSettings.get(CONFIG_DASHBOARD_ONLY_MODE_ROLES); - if (!dashboardOnlyModeRoles || user.roles.length === 0) { - return; - } - - const isDashboardOnlyModeRole = role => dashboardOnlyModeRoles.includes(role); - const isSuperUserRole = role => role === superuserRole; - - const isDashboardOnlyModeUser = user.roles.find(isDashboardOnlyModeRole); - const isSuperUser = user.roles.find(isSuperUserRole); - if (isDashboardOnlyModeUser && !isSuperUser) { - return [AUTH_SCOPE_DASHBORD_ONLY_MODE]; - } -} diff --git a/x-pack/plugins/dashboard_mode/server/dashboard_mode_request_interceptor.js b/x-pack/plugins/dashboard_mode/server/dashboard_mode_request_interceptor.js index 275922deb3d60..14710bf80e296 100644 --- a/x-pack/plugins/dashboard_mode/server/dashboard_mode_request_interceptor.js +++ b/x-pack/plugins/dashboard_mode/server/dashboard_mode_request_interceptor.js @@ -7,12 +7,14 @@ import Boom from 'boom'; import { - AUTH_SCOPE_DASHBORD_ONLY_MODE + CONFIG_DASHBOARD_ONLY_MODE_ROLES, } from '../common'; +const superuserRole = 'superuser'; + /** * Intercept all requests after auth has completed and apply filtering - * logic to enforce `xpack:dashboardMode` scope + * logic to enforce dashboard only mode. * * @type {Hapi.RequestExtension} */ @@ -25,14 +27,34 @@ export function createDashboardModeRequestInterceptor(dashboardViewerApp) { type: 'onPostAuth', async method(request, h) { const { auth, url } = request; + const user = auth.credentials; + const roles = user ? user.roles : []; + + if (!user) { + return h.continue; + } + const isAppRequest = url.path.startsWith('/app/'); - if (isAppRequest && auth.credentials.scope && auth.credentials.scope.includes(AUTH_SCOPE_DASHBORD_ONLY_MODE)) { + // The act of retrieving this setting ends up creating the config document if it doesn't already exist. + // Various functional tests have come to indirectly rely on this behavior, so changing this is non-trivial. + // This will be addressed once dashboard-only-mode is removed altogether. + const uiSettings = request.getUiSettingsService(); + const dashboardOnlyModeRoles = await uiSettings.get(CONFIG_DASHBOARD_ONLY_MODE_ROLES); + + if (!isAppRequest || !dashboardOnlyModeRoles || !roles || roles.length === 0) { + return h.continue; + } + + const isDashboardOnlyModeUser = user.roles.find(role => dashboardOnlyModeRoles.includes(role)); + const isSuperUser = user.roles.find(role => role === superuserRole); + + const enforceDashboardOnlyMode = isDashboardOnlyModeUser && !isSuperUser; + if (enforceDashboardOnlyMode) { if (url.path.startsWith('/app/kibana')) { // If the user is in "Dashboard only mode" they should only be allowed to see // that app and none others. Here we are intercepting all other routing and ensuring the viewer // app is the only one ever rendered. - // Read more about Dashboard Only Mode here: https://github.com/elastic/x-pack-kibana/issues/180 const response = await h.renderApp(dashboardViewerApp); return response.takeover(); } diff --git a/x-pack/plugins/dashboard_mode/server/index.js b/x-pack/plugins/dashboard_mode/server/index.js index a011422a15b06..a7193bb560ca1 100644 --- a/x-pack/plugins/dashboard_mode/server/index.js +++ b/x-pack/plugins/dashboard_mode/server/index.js @@ -4,5 +4,4 @@ * you may not use this file except in compliance with the Elastic License. */ -export { getDashboardModeAuthScope } from './dashboard_mode_auth_scope'; export { createDashboardModeRequestInterceptor } from './dashboard_mode_request_interceptor'; diff --git a/x-pack/plugins/security/server/lib/auth_scope_service.test.ts b/x-pack/plugins/security/server/lib/auth_scope_service.test.ts deleted file mode 100644 index 55d0cafc954d2..0000000000000 --- a/x-pack/plugins/security/server/lib/auth_scope_service.test.ts +++ /dev/null @@ -1,135 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -import sinon from 'sinon'; -import { AuthenticatedUser } from '../../common/model'; -import { requestFixture } from './__tests__/__fixtures__/request'; - -import { AuthScopeService } from './auth_scope_service'; - -describe('getCredentialsScope()', function() { - describe('basics', () => { - it('does not take any arguments', () => { - const authScope = new AuthScopeService(); - expect(authScope).toBeInstanceOf(AuthScopeService); - }); - }); - - describe('registerGetter()', () => { - it('throws when not passed a function', () => { - const authScope = new AuthScopeService(); - expect(() => authScope.registerGetter(undefined as any)).toThrowError( - 'Expected `getterFunction` to be a function' - ); - expect(() => authScope.registerGetter(null as any)).toThrowError( - 'Expected `getterFunction` to be a function' - ); - expect(() => authScope.registerGetter(0 as any)).toThrowError( - 'Expected `getterFunction` to be a function' - ); - expect(() => authScope.registerGetter('' as any)).toThrowError( - 'Expected `getterFunction` to be a function' - ); - expect(() => authScope.registerGetter([] as any)).toThrowError( - 'Expected `getterFunction` to be a function' - ); - expect(() => authScope.registerGetter({} as any)).toThrowError( - 'Expected `getterFunction` to be a function' - ); - }); - - it('only calls the passed function when #getForRequestAndUser() is called', async () => { - const authScope = new AuthScopeService(); - - const stub = sinon.stub(); - authScope.registerGetter(stub); - sinon.assert.notCalled(stub); - - const request = requestFixture(); - const user = {} as AuthenticatedUser; - await authScope.getForRequestAndUser(request, user); - sinon.assert.calledOnce(stub); - sinon.assert.calledWithExactly(stub, request, user); - }); - }); - - describe('#getForRequestAndUser()', () => { - it('throws when request and user are not objects', async () => { - const authScope = new AuthScopeService(); - - await expect(authScope.getForRequestAndUser(null as any, {} as any)).rejects.toThrowError( - 'getCredentialsScope() requires a request object' - ); - await expect(authScope.getForRequestAndUser(1 as any, {} as any)).rejects.toThrowError( - 'getCredentialsScope() requires a request object' - ); - await expect(authScope.getForRequestAndUser('abc' as any, {} as any)).rejects.toThrowError( - 'getCredentialsScope() requires a request object' - ); - - await expect(authScope.getForRequestAndUser({} as any, null as any)).rejects.toThrowError( - 'getCredentialsScope() requires a user object' - ); - await expect(authScope.getForRequestAndUser({} as any, 1 as any)).rejects.toThrowError( - 'getCredentialsScope() requires a user object' - ); - await expect(authScope.getForRequestAndUser({} as any, 'abc' as any)).rejects.toThrowError( - 'getCredentialsScope() requires a user object' - ); - }); - - it('returns a promise for an empty array by default', async () => { - const authScope = new AuthScopeService(); - const scope = await authScope.getForRequestAndUser(requestFixture(), {} as any); - expect(scope).toEqual([]); - }); - - it('calls each registered getter once each call', async () => { - const authScope = new AuthScopeService(); - const user = {} as any; - const request = requestFixture(); - const getter1 = sinon.stub(); - const getter2 = sinon.stub(); - const getter3 = sinon.stub(); - - authScope.registerGetter(getter1); - authScope.registerGetter(getter2); - authScope.registerGetter(getter3); - - await authScope.getForRequestAndUser(request, user); - sinon.assert.calledOnce(getter1); - sinon.assert.calledOnce(getter2); - sinon.assert.calledOnce(getter3); - - await authScope.getForRequestAndUser(request, user); - sinon.assert.calledTwice(getter1); - sinon.assert.calledTwice(getter2); - sinon.assert.calledTwice(getter3); - - await authScope.getForRequestAndUser(request, user); - sinon.assert.calledThrice(getter1); - sinon.assert.calledThrice(getter2); - sinon.assert.calledThrice(getter3); - }); - - it('casts the return value of the getters to an produce a flat, unique array', async () => { - const authScope = new AuthScopeService(); - authScope.registerGetter(() => undefined as any); - authScope.registerGetter(() => null as any); - authScope.registerGetter(() => ['foo', undefined, 'bar'] as any); - authScope.registerGetter(() => ['foo', 'foo1', 'foo2']); - authScope.registerGetter(() => 'bar2'); - - expect(await authScope.getForRequestAndUser(requestFixture(), {} as any)).toEqual([ - 'foo', - 'bar', - 'foo1', - 'foo2', - 'bar2', - ]); - }); - }); -}); diff --git a/x-pack/plugins/security/server/lib/auth_scope_service.ts b/x-pack/plugins/security/server/lib/auth_scope_service.ts deleted file mode 100644 index cf7e70f622af8..0000000000000 --- a/x-pack/plugins/security/server/lib/auth_scope_service.ts +++ /dev/null @@ -1,71 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -import { Legacy } from 'kibana'; -import { uniq, flattenDeep } from 'lodash'; -import { AuthenticatedUser } from '../../common/model'; - -export type ScopesGetter = (request: Legacy.Request, user: AuthenticatedUser) => string[] | string; - -/** - * Manages the creation of the scopes attached to the credentials which result - * from authentication. - * - * These scopes are used by plugins like dashboard_mode to tag relevant - * requests as `xpack:dashboardMode`, which it then uses to determine - * how it's filter should behave. - * - * This service's primary reason for existing it to track the list of functions - * that have been provided by plugins to which should be used to create the scopes - * for requests. - */ -export class AuthScopeService { - private readonly getterFunctions: ScopesGetter[] = []; - - /** - * Add a function that will be used to determine the list of scopes for a - * request+user combo. - * - * The getterFunction should take two arguments: a `Hapi.Request` object - * and the `user` object returned by the es Security API. - * - * The function should return either an array of tags (strings) or a - * promise that resolves to an array of tags. - * - * @param getterFunction Auth scope getter function. - */ - registerGetter(getterFunction: ScopesGetter) { - if (typeof getterFunction !== 'function') { - throw new TypeError('Expected `getterFunction` to be a function'); - } - - this.getterFunctions.push(getterFunction); - } - - /** - * Determine the scope for a specific user/request. Hapi credentials (the - * result of a hapi auth scheme) can have a `scope` property that lists scope - * "tags" which can then be required by routes using the - * `route.config.auth.access.scope` property, or accessed in pre-functions, - * extensions, or route handlers as `request.auth.credentials.scope`. - * - * @param request Request instance. - * @param user User object from the security API - */ - async getForRequestAndUser(request: Legacy.Request, user: AuthenticatedUser): Promise { - if (!request || typeof request !== 'object') { - throw new TypeError('getCredentialsScope() requires a request object'); - } - - if (!user || typeof user !== 'object') { - throw new TypeError('getCredentialsScope() requires a user object'); - } - - const getterResults = await Promise.all(this.getterFunctions.map(fn => fn(request, user))); - - return uniq(flattenDeep(getterResults).filter(Boolean)); - } -} diff --git a/x-pack/plugins/security/server/lib/authentication/authenticator.test.ts b/x-pack/plugins/security/server/lib/authentication/authenticator.test.ts index 69b864c61a27b..315675a0ed575 100644 --- a/x-pack/plugins/security/server/lib/authentication/authenticator.test.ts +++ b/x-pack/plugins/security/server/lib/authentication/authenticator.test.ts @@ -13,7 +13,6 @@ import { requestFixture } from '../__tests__/__fixtures__/request'; import { AuthenticationResult } from './authentication_result'; import { DeauthenticationResult } from './deauthentication_result'; import { Session } from './session'; -import { AuthScopeService } from '../auth_scope_service'; import { LoginAttempt } from './login_attempt'; import { initAuthenticator } from './authenticator'; import * as ClientShield from '../../../../../server/lib/get_client_shield'; @@ -46,7 +45,6 @@ describe('Authenticator', () => { .stub(Session, 'create') .withArgs(server as any) .resolves(session as any); - sandbox.stub(AuthScopeService.prototype, 'getForRequestAndUser').resolves([]); sandbox.useFakeTimers(); }); @@ -109,7 +107,7 @@ describe('Authenticator', () => { const authenticationResult = await authenticate(request); expect(authenticationResult.succeeded()).toBe(true); - expect(authenticationResult.user).toEqual({ ...user, scope: [] }); + expect(authenticationResult.user).toEqual(user); }); it('creates session whenever authentication provider returns state for system API requests', async () => { @@ -126,7 +124,7 @@ describe('Authenticator', () => { const systemAPIAuthenticationResult = await authenticate(request); expect(systemAPIAuthenticationResult.succeeded()).toBe(true); - expect(systemAPIAuthenticationResult.user).toEqual({ ...user, scope: [] }); + expect(systemAPIAuthenticationResult.user).toEqual(user); sinon.assert.calledOnce(session.set); sinon.assert.calledWithExactly(session.set, request, { state: { authorization }, @@ -148,7 +146,7 @@ describe('Authenticator', () => { const notSystemAPIAuthenticationResult = await authenticate(request); expect(notSystemAPIAuthenticationResult.succeeded()).toBe(true); - expect(notSystemAPIAuthenticationResult.user).toEqual({ ...user, scope: [] }); + expect(notSystemAPIAuthenticationResult.user).toEqual(user); sinon.assert.calledOnce(session.set); sinon.assert.calledWithExactly(session.set, request, { state: { authorization }, @@ -185,12 +183,12 @@ describe('Authenticator', () => { const systemAPIAuthenticationResult = await authenticate(systemAPIRequest); expect(systemAPIAuthenticationResult.succeeded()).toBe(true); - expect(systemAPIAuthenticationResult.user).toEqual({ ...user, scope: [] }); + expect(systemAPIAuthenticationResult.user).toEqual(user); sinon.assert.notCalled(session.set); const notSystemAPIAuthenticationResult = await authenticate(notSystemAPIRequest); expect(notSystemAPIAuthenticationResult.succeeded()).toBe(true); - expect(notSystemAPIAuthenticationResult.user).toEqual({ ...user, scope: [] }); + expect(notSystemAPIAuthenticationResult.user).toEqual(user); sinon.assert.calledOnce(session.set); sinon.assert.calledWithExactly(session.set, notSystemAPIRequest, { state: { authorization: 'Basic yyy' }, @@ -253,7 +251,7 @@ describe('Authenticator', () => { const authenticationResult = await authenticate(request); expect(authenticationResult.succeeded()).toBe(true); - expect(authenticationResult.user).toEqual({ ...user, scope: [] }); + expect(authenticationResult.user).toEqual(user); sinon.assert.calledOnce(session.set); sinon.assert.calledWithExactly(session.set, request, { state: { authorization }, @@ -280,7 +278,7 @@ describe('Authenticator', () => { const authenticationResult = await authenticate(request); expect(authenticationResult.succeeded()).toBe(true); - expect(authenticationResult.user).toEqual({ ...user, scope: [] }); + expect(authenticationResult.user).toEqual(user); sinon.assert.calledOnce(session.set); sinon.assert.calledWithExactly(session.set, request, { state: { authorization }, @@ -467,20 +465,6 @@ describe('Authenticator', () => { expect(notSystemAPIAuthenticationResult.notHandled()).toBe(true); sinon.assert.calledTwice(session.clear); }); - - it('complements user with `scope` property.', async () => { - const user = { username: 'user' }; - const request = requestFixture({ headers: { authorization: 'Basic ***' } }); - - cluster.callWithRequest.withArgs(request).resolves(user); - (AuthScopeService.prototype.getForRequestAndUser as sinon.SinonStub) - .withArgs(request, user) - .resolves(['foo', 'bar']); - - const authenticationResult = await authenticate(request); - expect(authenticationResult.succeeded()).toBe(true); - expect(authenticationResult.user).toEqual({ ...user, scope: ['foo', 'bar'] }); - }); }); describe('`deauthenticate` method', () => { diff --git a/x-pack/plugins/security/server/lib/authentication/authenticator.ts b/x-pack/plugins/security/server/lib/authentication/authenticator.ts index 232ba41d2a707..d238979e2e8e5 100644 --- a/x-pack/plugins/security/server/lib/authentication/authenticator.ts +++ b/x-pack/plugins/security/server/lib/authentication/authenticator.ts @@ -6,7 +6,6 @@ import { Legacy } from 'kibana'; import { getClient } from '../../../../../server/lib/get_client_shield'; -import { AuthScopeService, ScopesGetter } from '../auth_scope_service'; import { getErrorStatusCode } from '../errors'; import { AuthenticationProviderOptions, @@ -133,14 +132,9 @@ class Authenticator { /** * Instantiates Authenticator and bootstrap configured providers. * @param server Server instance. - * @param authScope AuthScopeService instance. * @param session Session instance. */ - constructor( - private readonly server: Legacy.Server, - private readonly authScope: AuthScopeService, - private readonly session: Session - ) { + constructor(private readonly server: Legacy.Server, private readonly session: Session) { const config = this.server.config(); const authProviders = config.get('xpack.security.authProviders'); if (authProviders.length === 0) { @@ -213,11 +207,7 @@ class Authenticator { } if (authenticationResult.succeeded()) { - return AuthenticationResult.succeeded({ - ...authenticationResult.user, - // Complement user returned from the provider with scopes. - scope: await this.authScope.getForRequestAndUser(request, authenticationResult.user!), - } as any); + return AuthenticationResult.succeeded(authenticationResult.user!); } else if (authenticationResult.redirected()) { return authenticationResult; } @@ -298,8 +288,7 @@ class Authenticator { export async function initAuthenticator(server: Legacy.Server) { const session = await Session.create(server); - const authScope = new AuthScopeService(); - const authenticator = new Authenticator(server, authScope, session); + const authenticator = new Authenticator(server, session); const loginAttempts = new WeakMap(); server.decorate('request', 'loginAttempt', function(this: Legacy.Request) { @@ -316,9 +305,6 @@ export async function initAuthenticator(server: Legacy.Server) { server.expose('deauthenticate', (request: RequestWithLoginAttempt) => authenticator.deauthenticate(request) ); - server.expose('registerAuthScopeGetter', (scopeExtender: ScopesGetter) => - authScope.registerGetter(scopeExtender) - ); server.expose('isAuthenticated', async (request: Legacy.Request) => { try { diff --git a/x-pack/plugins/spaces/server/lib/request_inteceptors/on_post_auth_interceptor.ts b/x-pack/plugins/spaces/server/lib/request_inteceptors/on_post_auth_interceptor.ts index c5aaded074c34..35e110643f5ff 100644 --- a/x-pack/plugins/spaces/server/lib/request_inteceptors/on_post_auth_interceptor.ts +++ b/x-pack/plugins/spaces/server/lib/request_inteceptors/on_post_auth_interceptor.ts @@ -26,7 +26,7 @@ export function initSpacesOnPostAuthRequestInterceptor(server: KbnServer) { const isRequestingApplication = path.startsWith('/app'); // if requesting the application root, then show the Space Selector UI to allow the user to choose which space - // they wish to visit. This is done "onPostAuth" to allow the Saved Objects Client to use the request's auth scope, + // they wish to visit. This is done "onPostAuth" to allow the Saved Objects Client to use the request's auth credentials, // which is not available at the time of "onRequest". if (isRequestingKibanaRoot) { try { diff --git a/x-pack/test/api_integration/apis/security/basic_login.js b/x-pack/test/api_integration/apis/security/basic_login.js index 39c6c0ef0a135..34161a22fe76e 100644 --- a/x-pack/test/api_integration/apis/security/basic_login.js +++ b/x-pack/test/api_integration/apis/security/basic_login.js @@ -98,7 +98,6 @@ export default function ({ getService }) { 'full_name', 'email', 'roles', - 'scope', 'metadata', 'enabled', 'authentication_realm', @@ -137,7 +136,6 @@ export default function ({ getService }) { 'full_name', 'email', 'roles', - 'scope', 'metadata', 'enabled', 'authentication_realm', diff --git a/x-pack/test/kerberos_api_integration/apis/security/kerberos_login.ts b/x-pack/test/kerberos_api_integration/apis/security/kerberos_login.ts index 7610314dbadba..c422a8b514c87 100644 --- a/x-pack/test/kerberos_api_integration/apis/security/kerberos_login.ts +++ b/x-pack/test/kerberos_api_integration/apis/security/kerberos_login.ts @@ -122,7 +122,6 @@ export default function({ getService }: KibanaFunctionalTestDefaultProviders) { .expect(200, { username: 'tester@TEST.ELASTIC.CO', roles: ['krb5-user'], - scope: [], full_name: null, email: null, metadata: { diff --git a/x-pack/test/oidc_api_integration/apis/security/oidc_initiate_auth.js b/x-pack/test/oidc_api_integration/apis/security/oidc_initiate_auth.js index e6d33ebbc742c..b99cf494fe0a5 100644 --- a/x-pack/test/oidc_api_integration/apis/security/oidc_initiate_auth.js +++ b/x-pack/test/oidc_api_integration/apis/security/oidc_initiate_auth.js @@ -148,7 +148,6 @@ export default function ({ getService }) { 'full_name', 'email', 'roles', - 'scope', 'metadata', 'enabled', 'authentication_realm', @@ -196,7 +195,6 @@ export default function ({ getService }) { 'full_name', 'email', 'roles', - 'scope', 'metadata', 'enabled', 'authentication_realm', diff --git a/x-pack/test/saml_api_integration/apis/security/saml_login.js b/x-pack/test/saml_api_integration/apis/security/saml_login.js index 351bb4d162c63..401ffdea0418d 100644 --- a/x-pack/test/saml_api_integration/apis/security/saml_login.js +++ b/x-pack/test/saml_api_integration/apis/security/saml_login.js @@ -151,7 +151,6 @@ export default function ({ getService }) { 'full_name', 'email', 'roles', - 'scope', 'metadata', 'enabled', 'authentication_realm', @@ -192,7 +191,6 @@ export default function ({ getService }) { 'full_name', 'email', 'roles', - 'scope', 'metadata', 'enabled', 'authentication_realm',