diff --git a/x-pack/plugins/security/index.js b/x-pack/plugins/security/index.js index 6bcaf4e7aec46f..0e521589ac1410 100644 --- a/x-pack/plugins/security/index.js +++ b/x-pack/plugins/security/index.js @@ -17,11 +17,10 @@ import { authenticateFactory } from './server/lib/auth_redirect'; import { checkLicense } from './server/lib/check_license'; import { initAuthenticator } from './server/lib/authentication/authenticator'; import { initPrivilegesApi } from './server/routes/api/v1/privileges'; -import { checkPrivilegesWithRequestFactory } from './server/lib/authorization/check_privileges'; import { SecurityAuditLogger } from './server/lib/audit_logger'; import { AuditLogger } from '../../server/lib/audit_logger'; import { SecureSavedObjectsClient } from './server/lib/saved_objects_client/secure_saved_objects_client'; -import { registerPrivilegesWithCluster } from './server/lib/privileges'; +import { initAuthorizationService, registerPrivilegesWithCluster } from './server/lib/authorization'; import { watchStatusAndLicenseToInitialize } from './server/lib/watch_status_and_license_to_initialize'; export const security = (kibana) => new kibana.Plugin({ @@ -114,9 +113,11 @@ export const security = (kibana) => new kibana.Plugin({ server.auth.strategy('session', 'login', 'required'); const auditLogger = new SecurityAuditLogger(server.config(), new AuditLogger(server, 'security')); - const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(server); - const { savedObjects } = server; + // exposes server.plugins.security.authorization + initAuthorizationService(server); + + const { savedObjects } = server; savedObjects.setScopedSavedObjectsClientFactory(({ request, }) => { @@ -130,7 +131,8 @@ export const security = (kibana) => new kibana.Plugin({ return new savedObjects.SavedObjectsClient(callWithRequestRepository); } - const checkPrivileges = checkPrivilegesWithRequest(request); + const { authorization } = server.plugins.security; + const checkPrivileges = authorization.checkPrivilegesWithRequest(request); const internalRepository = savedObjects.getSavedObjectsRepository(callWithInternalUser); return new SecureSavedObjectsClient({ @@ -140,6 +142,7 @@ export const security = (kibana) => new kibana.Plugin({ checkPrivileges, auditLogger, savedObjectTypes: savedObjects.types, + actions: authorization.actions, }); }); diff --git a/x-pack/plugins/security/server/lib/__tests__/__fixtures__/server.js b/x-pack/plugins/security/server/lib/__tests__/__fixtures__/server.js index acc211eb0647ea..78a015bc141058 100644 --- a/x-pack/plugins/security/server/lib/__tests__/__fixtures__/server.js +++ b/x-pack/plugins/security/server/lib/__tests__/__fixtures__/server.js @@ -35,7 +35,13 @@ export function serverFixture() { security: { getUser: stub(), authenticate: stub(), - deauthenticate: stub() + deauthenticate: stub(), + authorization: { + checkPrivilegesWithRequest: stub(), + actions: { + login: 'stub-login-action', + }, + }, }, xpack_main: { diff --git a/x-pack/plugins/security/server/lib/authorization/__snapshots__/actions.test.js.snap b/x-pack/plugins/security/server/lib/authorization/__snapshots__/actions.test.js.snap new file mode 100644 index 00000000000000..d65733feb37d46 --- /dev/null +++ b/x-pack/plugins/security/server/lib/authorization/__snapshots__/actions.test.js.snap @@ -0,0 +1,25 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`#getSavedObjectAction() action of "" throws error 1`] = `"action is required and must be a string"`; + +exports[`#getSavedObjectAction() action of {} throws error 1`] = `"action is required and must be a string"`; + +exports[`#getSavedObjectAction() action of 1 throws error 1`] = `"action is required and must be a string"`; + +exports[`#getSavedObjectAction() action of null throws error 1`] = `"action is required and must be a string"`; + +exports[`#getSavedObjectAction() action of true throws error 1`] = `"action is required and must be a string"`; + +exports[`#getSavedObjectAction() action of undefined throws error 1`] = `"action is required and must be a string"`; + +exports[`#getSavedObjectAction() type of "" throws error 1`] = `"type is required and must be a string"`; + +exports[`#getSavedObjectAction() type of {} throws error 1`] = `"type is required and must be a string"`; + +exports[`#getSavedObjectAction() type of 1 throws error 1`] = `"type is required and must be a string"`; + +exports[`#getSavedObjectAction() type of null throws error 1`] = `"type is required and must be a string"`; + +exports[`#getSavedObjectAction() type of true throws error 1`] = `"type is required and must be a string"`; + +exports[`#getSavedObjectAction() type of undefined throws error 1`] = `"type is required and must be a string"`; diff --git a/x-pack/plugins/security/server/lib/authorization/__snapshots__/check_privileges.test.js.snap b/x-pack/plugins/security/server/lib/authorization/__snapshots__/check_privileges.test.js.snap index e191d4b14b6f00..4c0c88a8f8313c 100644 --- a/x-pack/plugins/security/server/lib/authorization/__snapshots__/check_privileges.test.js.snap +++ b/x-pack/plugins/security/server/lib/authorization/__snapshots__/check_privileges.test.js.snap @@ -1,3 +1,5 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`throws error if missing version privilege and has login privilege 1`] = `"Multiple versions of Kibana are running against the same Elasticsearch cluster, unable to authorize user."`; +exports[`with index privileges throws error if missing version privilege and has login privilege 1`] = `[Error: Multiple versions of Kibana are running against the same Elasticsearch cluster, unable to authorize user.]`; + +exports[`with no index privileges throws error if missing version privilege and has login privilege 1`] = `[Error: Multiple versions of Kibana are running against the same Elasticsearch cluster, unable to authorize user.]`; diff --git a/x-pack/plugins/security/server/lib/authorization/__snapshots__/init.test.js.snap b/x-pack/plugins/security/server/lib/authorization/__snapshots__/init.test.js.snap new file mode 100644 index 00000000000000..fd944032d2930f --- /dev/null +++ b/x-pack/plugins/security/server/lib/authorization/__snapshots__/init.test.js.snap @@ -0,0 +1,7 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`deep freezes exposed service 1`] = `"Cannot delete property 'checkPrivilegesWithRequest' of #"`; + +exports[`deep freezes exposed service 2`] = `"Cannot add property foo, object is not extensible"`; + +exports[`deep freezes exposed service 3`] = `"Cannot assign to read only property 'login' of object '#'"`; diff --git a/x-pack/plugins/security/server/lib/authorization/actions.js b/x-pack/plugins/security/server/lib/authorization/actions.js new file mode 100644 index 00000000000000..432698a003cb35 --- /dev/null +++ b/x-pack/plugins/security/server/lib/authorization/actions.js @@ -0,0 +1,26 @@ +/* + * 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 { isString } from 'lodash'; + +export function actionsFactory(config) { + const kibanaVersion = config.get('pkg.version'); + + return { + getSavedObjectAction(type, action) { + if (!type || !isString(type)) { + throw new Error('type is required and must be a string'); + } + + if (!action || !isString(action)) { + throw new Error('action is required and must be a string'); + } + + return `action:saved_objects/${type}/${action}`; + }, + login: `action:login`, + version: `version:${kibanaVersion}`, + }; +} diff --git a/x-pack/plugins/security/server/lib/authorization/actions.test.js b/x-pack/plugins/security/server/lib/authorization/actions.test.js new file mode 100644 index 00000000000000..17834438e17814 --- /dev/null +++ b/x-pack/plugins/security/server/lib/authorization/actions.test.js @@ -0,0 +1,69 @@ +/* + * 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 { actionsFactory } from './actions'; + +const createMockConfig = (settings = {}) => { + const mockConfig = { + get: jest.fn() + }; + + mockConfig.get.mockImplementation(key => settings[key]); + + return mockConfig; +}; + +describe('#login', () => { + test('returns action:login', () => { + const mockConfig = createMockConfig(); + + const actions = actionsFactory(mockConfig); + + expect(actions.login).toEqual('action:login'); + }); +}); + +describe('#version', () => { + test(`returns version:\${config.get('pkg.version')}`, () => { + const version = 'mock-version'; + const mockConfig = createMockConfig({ 'pkg.version': version }); + + const actions = actionsFactory(mockConfig); + + expect(actions.version).toEqual(`version:${version}`); + }); +}); + +describe('#getSavedObjectAction()', () => { + test('uses type and action to build action', () => { + const mockConfig = createMockConfig(); + const actions = actionsFactory(mockConfig); + const type = 'saved-object-type'; + const action = 'saved-object-action'; + + const result = actions.getSavedObjectAction(type, action); + + expect(result).toEqual(`action:saved_objects/${type}/${action}`); + }); + + [null, undefined, '', 1, true, {}].forEach(type => { + test(`type of ${JSON.stringify(type)} throws error`, () => { + const mockConfig = createMockConfig(); + const actions = actionsFactory(mockConfig); + + expect(() => actions.getSavedObjectAction(type, 'saved-object-action')).toThrowErrorMatchingSnapshot(); + }); + }); + + [null, undefined, '', 1, true, {}].forEach(action => { + test(`action of ${JSON.stringify(action)} throws error`, () => { + const mockConfig = createMockConfig(); + const actions = actionsFactory(mockConfig); + + expect(() => actions.getSavedObjectAction('saved-object-type', action)).toThrowErrorMatchingSnapshot(); + }); + }); +}); diff --git a/x-pack/plugins/security/server/lib/authorization/check_privileges.js b/x-pack/plugins/security/server/lib/authorization/check_privileges.js index a84b1cecdac792..df8ee33ec29029 100644 --- a/x-pack/plugins/security/server/lib/authorization/check_privileges.js +++ b/x-pack/plugins/security/server/lib/authorization/check_privileges.js @@ -4,103 +4,82 @@ * you may not use this file except in compliance with the Elastic License. */ -import { getClient } from '../../../../../server/lib/get_client_shield'; +import { uniq } from 'lodash'; import { ALL_RESOURCE } from '../../../common/constants'; -import { getVersionAction, getLoginAction } from '../privileges'; export const CHECK_PRIVILEGES_RESULT = { - UNAUTHORIZED: Symbol(), - AUTHORIZED: Symbol(), - LEGACY: Symbol(), + UNAUTHORIZED: Symbol('Unauthorized'), + AUTHORIZED: Symbol('Authorized'), + LEGACY: Symbol('Legacy'), }; -export function checkPrivilegesWithRequestFactory(server) { - const callWithRequest = getClient(server).callWithRequest; +export function checkPrivilegesWithRequestFactory(shieldClient, config, actions) { + const { callWithRequest } = shieldClient; - const config = server.config(); - const kibanaVersion = config.get('pkg.version'); const application = config.get('xpack.security.rbac.application'); const kibanaIndex = config.get('kibana.index'); - const loginAction = getLoginAction(); - const versionAction = getVersionAction(kibanaVersion); + const hasIncompatibileVersion = (applicationPrivilegesResponse) => { + return !applicationPrivilegesResponse[actions.version] && applicationPrivilegesResponse[actions.login]; + }; - return function checkPrivilegesWithRequest(request) { + const hasAllApplicationPrivileges = (applicationPrivilegesResponse) => { + return Object.values(applicationPrivilegesResponse).every(val => val === true); + }; - const checkApplicationPrivileges = async (privileges) => { - const privilegeCheck = await callWithRequest(request, 'shield.hasPrivileges', { - body: { - applications: [{ - application, - resources: [ALL_RESOURCE], - privileges - }] - } - }); + const hasNoApplicationPrivileges = (applicationPrivilegesResponse) => { + return Object.values(applicationPrivilegesResponse).every(val => val === false); + }; - const hasPrivileges = privilegeCheck.application[application][ALL_RESOURCE]; + const hasLegacyPrivileges = (indexPrivilegesResponse) => { + return Object.values(indexPrivilegesResponse).includes(true); + }; - // We include the login action in all privileges, so the existence of it and not the version privilege - // lets us know that we're running in an incorrect configuration. Without the login privilege check, we wouldn't - // know whether the user just wasn't authorized for this instance of Kibana in general - if (!hasPrivileges[getVersionAction(kibanaVersion)] && hasPrivileges[getLoginAction()]) { - throw new Error('Multiple versions of Kibana are running against the same Elasticsearch cluster, unable to authorize user.'); - } + const determineResult = (applicationPrivilegesResponse, indexPrivilegesResponse) => { + if (hasAllApplicationPrivileges(applicationPrivilegesResponse)) { + return CHECK_PRIVILEGES_RESULT.AUTHORIZED; + } - return { - username: privilegeCheck.username, - hasAllRequested: privilegeCheck.has_all_requested, - privileges: hasPrivileges - }; - }; + if (hasNoApplicationPrivileges(applicationPrivilegesResponse) && hasLegacyPrivileges(indexPrivilegesResponse)) { + return CHECK_PRIVILEGES_RESULT.LEGACY; + } - const hasPrivilegesOnKibanaIndex = async () => { - const privilegeCheck = await callWithRequest(request, 'shield.hasPrivileges', { + return CHECK_PRIVILEGES_RESULT.UNAUTHORIZED; + }; + + return function checkPrivilegesWithRequest(request) { + + return async function checkPrivileges(privileges) { + const allApplicationPrivileges = uniq([actions.version, actions.login, ...privileges]); + const hasPrivilegesResponse = await callWithRequest(request, 'shield.hasPrivileges', { body: { + applications: [{ + application, + resources: [ALL_RESOURCE], + privileges: allApplicationPrivileges + }], index: [{ names: [kibanaIndex], privileges: ['create', 'delete', 'read', 'view_index_metadata'] - }] + }], } }); - return Object.values(privilegeCheck.index[kibanaIndex]).includes(true); - }; - - return async function checkPrivileges(privileges) { - const allPrivileges = [versionAction, loginAction, ...privileges]; - const applicationPrivilegesCheck = await checkApplicationPrivileges(allPrivileges); - - const username = applicationPrivilegesCheck.username; - - // We don't want to expose the version privilege to consumers, as it's an implementation detail only to detect version mismatch - const missing = Object.keys(applicationPrivilegesCheck.privileges) - .filter(p => !applicationPrivilegesCheck.privileges[p]) - .filter(p => p !== versionAction); - - if (applicationPrivilegesCheck.hasAllRequested) { - return { - result: CHECK_PRIVILEGES_RESULT.AUTHORIZED, - username, - missing, - }; - } + const applicationPrivilegesResponse = hasPrivilegesResponse.application[application][ALL_RESOURCE]; + const indexPrivilegesResponse = hasPrivilegesResponse.index[kibanaIndex]; - if (!applicationPrivilegesCheck.privileges[loginAction] && await hasPrivilegesOnKibanaIndex()) { - const msg = 'Relying on index privileges is deprecated and will be removed in Kibana 7.0'; - server.log(['warning', 'deprecated', 'security'], msg); - - return { - result: CHECK_PRIVILEGES_RESULT.LEGACY, - username, - missing, - }; + if (hasIncompatibileVersion(applicationPrivilegesResponse)) { + throw new Error('Multiple versions of Kibana are running against the same Elasticsearch cluster, unable to authorize user.'); } return { - result: CHECK_PRIVILEGES_RESULT.UNAUTHORIZED, - username, - missing, + result: determineResult(applicationPrivilegesResponse, indexPrivilegesResponse), + username: hasPrivilegesResponse.username, + + // we only return missing privileges that they're specifically checking for + missing: Object.keys(applicationPrivilegesResponse) + .filter(privilege => privileges.includes(privilege)) + .filter(privilege => !applicationPrivilegesResponse[privilege]) }; }; }; diff --git a/x-pack/plugins/security/server/lib/authorization/check_privileges.test.js b/x-pack/plugins/security/server/lib/authorization/check_privileges.test.js index 80d7ea6b2b5fd3..3c414665c8258d 100644 --- a/x-pack/plugins/security/server/lib/authorization/check_privileges.test.js +++ b/x-pack/plugins/security/server/lib/authorization/check_privileges.test.js @@ -4,26 +4,24 @@ * you may not use this file except in compliance with the Elastic License. */ +import { uniq } from 'lodash'; import { checkPrivilegesWithRequestFactory, CHECK_PRIVILEGES_RESULT } from './check_privileges'; -import { getClient } from '../../../../../server/lib/get_client_shield'; -import { ALL_RESOURCE } from '../../../common/constants'; -import { getLoginAction, getVersionAction } from '../privileges'; -jest.mock('../../../../../server/lib/get_client_shield', () => ({ - getClient: jest.fn() -})); +import { ALL_RESOURCE } from '../../../common/constants'; const defaultVersion = 'default-version'; const defaultApplication = 'default-application'; const defaultKibanaIndex = 'default-index'; const savedObjectTypes = ['foo-type', 'bar-type']; -const createMockServer = ({ settings = {} } = {}) => { - const mockServer = { - config: jest.fn().mockReturnValue({ - get: jest.fn() - }), - log: jest.fn() +const mockActions = { + login: 'mock-action:login', + version: 'mock-action:version', +}; + +const createMockConfig = ({ settings = {} } = {}) => { + const mockConfig = { + get: jest.fn() }; const defaultSettings = { @@ -32,280 +30,302 @@ const createMockServer = ({ settings = {} } = {}) => { 'kibana.index': defaultKibanaIndex, }; - mockServer.config().get.mockImplementation(key => { + mockConfig.get.mockImplementation(key => { return key in settings ? settings[key] : defaultSettings[key]; }); - mockServer.savedObjects = { - types: savedObjectTypes - }; - return mockServer; + return mockConfig; }; -const mockApplicationPrivilegeResponse = ({ hasAllRequested, privileges, application = defaultApplication, username = '' }) =>{ +const createMockShieldClient = (response) => { + const mockCallWithRequest = jest.fn(); + + mockCallWithRequest.mockImplementationOnce(async () => response); + return { - username: username, - has_all_requested: hasAllRequested, - application: { - [application]: { - [ALL_RESOURCE]: privileges - } - } + callWithRequest: mockCallWithRequest, }; }; -const mockKibanaIndexPrivilegesResponse = ({ privileges, index = defaultKibanaIndex }) => { - return { - index: { - [index]: privileges +const checkPrivilegesTest = ( + description, { + privileges, + applicationPrivilegesResponse, + indexPrivilegesResponse, + expectedResult, + expectErrorThrown, + }) => { + + test(description, async () => { + const username = 'foo-username'; + const mockConfig = createMockConfig(); + const mockShieldClient = createMockShieldClient({ + username, + application: { + [defaultApplication]: { + [ALL_RESOURCE]: applicationPrivilegesResponse + } + }, + index: { + [defaultKibanaIndex]: indexPrivilegesResponse + }, + }); + + const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(mockShieldClient, mockConfig, mockActions); + const request = Symbol(); + const checkPrivileges = checkPrivilegesWithRequest(request); + + let actualResult; + let errorThrown = null; + try { + actualResult = await checkPrivileges(privileges); + } catch (err) { + errorThrown = err; } - }; -}; -const createMockCallWithRequest = (responses) => { - const mockCallWithRequest = jest.fn(); - getClient.mockReturnValue({ - callWithRequest: mockCallWithRequest - }); - for (const response of responses) { - mockCallWithRequest.mockImplementationOnce(async () => response); - } + expect(mockShieldClient.callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { + body: { + applications: [{ + application: defaultApplication, + resources: [ALL_RESOURCE], + privileges: uniq([ + mockActions.version, mockActions.login, ...privileges + ]) + }], + index: [{ + names: [ defaultKibanaIndex ], + privileges: ['create', 'delete', 'read', 'view_index_metadata'] + }], + } + }); - return mockCallWithRequest; -}; + if (expectedResult) { + expect(errorThrown).toBeNull(); + expect(actualResult).toEqual(expectedResult); + } -const deprecationMessage = 'Relying on index privileges is deprecated and will be removed in Kibana 7.0'; -const expectNoDeprecationLogged = (mockServer) => { - expect(mockServer.log).not.toHaveBeenCalledWith(['warning', 'deprecated', 'security'], deprecationMessage); -}; -const expectDeprecationLogged = (mockServer) => { - expect(mockServer.log).toHaveBeenCalledWith(['warning', 'deprecated', 'security'], deprecationMessage); + if (expectErrorThrown) { + expect(errorThrown).toMatchSnapshot(); + } + }); }; -test(`returns authorized if they have all application privileges`, async () => { - const privilege = `action:saved_objects/${savedObjectTypes[0]}/get`; - const username = 'foo-username'; - const mockServer = createMockServer(); - const mockCallWithRequest = createMockCallWithRequest([ - mockApplicationPrivilegeResponse({ - hasAllRequested: true, - privileges: { - [getVersionAction(defaultVersion)]: true, - [getLoginAction()]: true, - [privilege]: true, - }, - application: defaultApplication, - username, - }) - ]); - - const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(mockServer); - const request = Symbol(); - const checkPrivileges = checkPrivilegesWithRequest(request); - const privileges = [privilege]; - const result = await checkPrivileges(privileges); - - expectNoDeprecationLogged(mockServer); - expect(mockCallWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { - body: { - applications: [{ - application: defaultApplication, - resources: [ALL_RESOURCE], - privileges: [ - getVersionAction(defaultVersion), getLoginAction(), ...privileges - ] - }] +describe(`with no index privileges`, () => { + const indexPrivilegesResponse = { + create: false, + delete: false, + read: false, + view_index_metadata: false, + }; + + checkPrivilegesTest('returns authorized if they have all application privileges', { + username: 'foo-username', + privileges: [ + `action:saved_objects/${savedObjectTypes[0]}/get` + ], + applicationPrivilegesResponse: { + [mockActions.version]: true, + [mockActions.login]: true, + [`action:saved_objects/${savedObjectTypes[0]}/get`]: true, + }, + indexPrivilegesResponse, + expectedResult: { + result: CHECK_PRIVILEGES_RESULT.AUTHORIZED, + username: 'foo-username', + missing: [], } }); - expect(result).toEqual({ - result: CHECK_PRIVILEGES_RESULT.AUTHORIZED, - username, - missing: [], + + checkPrivilegesTest('returns unauthorized and missing application action when checking missing application action', { + username: 'foo-username', + privileges: [ + `action:saved_objects/${savedObjectTypes[0]}/get`, + `action:saved_objects/${savedObjectTypes[0]}/create`, + ], + applicationPrivilegesResponse: { + [mockActions.version]: true, + [mockActions.login]: true, + [`action:saved_objects/${savedObjectTypes[0]}/get`]: true, + [`action:saved_objects/${savedObjectTypes[0]}/create`]: false, + }, + indexPrivilegesResponse, + expectedResult: { + result: CHECK_PRIVILEGES_RESULT.UNAUTHORIZED, + username: 'foo-username', + missing: [`action:saved_objects/${savedObjectTypes[0]}/create`], + } }); -}); -test(`returns unauthorized they have only one application privilege`, async () => { - const privilege1 = `action:saved_objects/${savedObjectTypes[0]}/get`; - const privilege2 = `action:saved_objects/${savedObjectTypes[0]}/create`; - const username = 'foo-username'; - const mockServer = createMockServer(); - const mockCallWithRequest = createMockCallWithRequest([ - mockApplicationPrivilegeResponse({ - hasAllRequested: false, - privileges: { - [getVersionAction(defaultVersion)]: true, - [getLoginAction()]: true, - [privilege1]: true, - [privilege2]: false, - }, - application: defaultApplication, - username, - }) - ]); - - const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(mockServer); - const request = Symbol(); - const checkPrivileges = checkPrivilegesWithRequest(request); - const privileges = [privilege1, privilege2]; - const result = await checkPrivileges(privileges); - - expectNoDeprecationLogged(mockServer); - expect(mockCallWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { - body: { - applications: [{ - application: defaultApplication, - resources: [ALL_RESOURCE], - privileges: [ - getVersionAction(defaultVersion), getLoginAction(), ...privileges - ] - }] + checkPrivilegesTest('returns unauthorized and missing login when checking missing login action', { + username: 'foo-username', + privileges: [ + mockActions.login + ], + applicationPrivilegesResponse: { + [mockActions.login]: false, + [mockActions.version]: false, + }, + indexPrivilegesResponse, + expectedResult: { + result: CHECK_PRIVILEGES_RESULT.UNAUTHORIZED, + username: 'foo-username', + missing: [mockActions.login], + } + }); + + checkPrivilegesTest('returns unauthorized and missing version if checking missing version action', { + username: 'foo-username', + privileges: [ + mockActions.version + ], + applicationPrivilegesResponse: { + [mockActions.login]: false, + [mockActions.version]: false, + }, + indexPrivilegesResponse, + expectedResult: { + result: CHECK_PRIVILEGES_RESULT.UNAUTHORIZED, + username: 'foo-username', + missing: [mockActions.version], } }); - expect(result).toEqual({ - result: CHECK_PRIVILEGES_RESULT.UNAUTHORIZED, - username, - missing: [privilege2], + + checkPrivilegesTest('throws error if missing version privilege and has login privilege', { + username: 'foo-username', + privileges: [ + `action:saved_objects/${savedObjectTypes[0]}/get` + ], + applicationPrivilegesResponse: { + [mockActions.login]: true, + [mockActions.version]: false, + [`action:saved_objects/${savedObjectTypes[0]}/get`]: true, + }, + indexPrivilegesResponse, + expectErrorThrown: true }); }); -test(`throws error if missing version privilege and has login privilege`, async () => { - const privilege = `action:saved_objects/${savedObjectTypes[0]}/get`; - const mockServer = createMockServer(); - createMockCallWithRequest([ - mockApplicationPrivilegeResponse({ - hasAllRequested: false, - privileges: { - [getVersionAction(defaultVersion)]: false, - [getLoginAction()]: true, - [privilege]: true, - } - }) - ]); +describe(`with index privileges`, () => { + const indexPrivilegesResponse = { + create: true, + delete: true, + read: true, + view_index_metadata: true, + }; - const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(mockServer); - const checkPrivileges = checkPrivilegesWithRequest({}); + checkPrivilegesTest('returns authorized if they have all application privileges', { + username: 'foo-username', + privileges: [ + `action:saved_objects/${savedObjectTypes[0]}/get` + ], + applicationPrivilegesResponse: { + [mockActions.version]: true, + [mockActions.login]: true, + [`action:saved_objects/${savedObjectTypes[0]}/get`]: true, + }, + indexPrivilegesResponse, + expectedResult: { + result: CHECK_PRIVILEGES_RESULT.AUTHORIZED, + username: 'foo-username', + missing: [], + } + }); - await expect(checkPrivileges([privilege])).rejects.toThrowErrorMatchingSnapshot(); - expectNoDeprecationLogged(mockServer); -}); + checkPrivilegesTest('returns unauthorized and missing application action when checking missing application action', { + username: 'foo-username', + privileges: [ + `action:saved_objects/${savedObjectTypes[0]}/get`, + `action:saved_objects/${savedObjectTypes[0]}/create`, + ], + applicationPrivilegesResponse: { + [mockActions.version]: true, + [mockActions.login]: true, + [`action:saved_objects/${savedObjectTypes[0]}/get`]: true, + [`action:saved_objects/${savedObjectTypes[0]}/create`]: false, + }, + indexPrivilegesResponse, + expectedResult: { + result: CHECK_PRIVILEGES_RESULT.UNAUTHORIZED, + username: 'foo-username', + missing: [`action:saved_objects/${savedObjectTypes[0]}/create`], + } + }); -describe('legacy fallback with no application privileges', () => { - test(`returns unauthorized if they have no privileges on the kibana index`, async () => { - const privilege = `action:saved_objects/${savedObjectTypes[0]}/get`; - const username = 'foo-username'; - const mockServer = createMockServer(); - const callWithRequest = createMockCallWithRequest([ - mockApplicationPrivilegeResponse({ - hasAllRequested: false, - privileges: { - [getVersionAction(defaultVersion)]: false, - [getLoginAction()]: false, - [privilege]: false, - }, - username, - }), - mockKibanaIndexPrivilegesResponse({ - privileges: { - create: false, - delete: false, - read: false, - view_index_metadata: false, - }, - }) - ]); - - const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(mockServer); - const request = Symbol(); - const checkPrivileges = checkPrivilegesWithRequest(request); - const privileges = [privilege]; - const result = await checkPrivileges(privileges); + checkPrivilegesTest('returns legacy and missing login when checking missing login action', { + username: 'foo-username', + privileges: [ + mockActions.login + ], + applicationPrivilegesResponse: { + [mockActions.login]: false, + [mockActions.version]: false, + }, + indexPrivilegesResponse, + expectedResult: { + result: CHECK_PRIVILEGES_RESULT.LEGACY, + username: 'foo-username', + missing: [mockActions.login], + } + }); - expectNoDeprecationLogged(mockServer); - expect(callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { - body: { - applications: [{ - application: defaultApplication, - resources: [ALL_RESOURCE], - privileges: [ - getVersionAction(defaultVersion), getLoginAction(), ...privileges - ] - }] - } - }); - expect(callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { - body: { - index: [{ - names: [ defaultKibanaIndex ], - privileges: ['create', 'delete', 'read', 'view_index_metadata'] - }] - } - }); - expect(result).toEqual({ - result: CHECK_PRIVILEGES_RESULT.UNAUTHORIZED, - username, - missing: [getLoginAction(), ...privileges], - }); + checkPrivilegesTest('returns legacy and missing version if checking missing version action', { + username: 'foo-username', + privileges: [ + mockActions.version + ], + applicationPrivilegesResponse: { + [mockActions.login]: false, + [mockActions.version]: false, + }, + indexPrivilegesResponse, + expectedResult: { + result: CHECK_PRIVILEGES_RESULT.LEGACY, + username: 'foo-username', + missing: [mockActions.version], + } }); + checkPrivilegesTest('throws error if missing version privilege and has login privilege', { + username: 'foo-username', + privileges: [ + `action:saved_objects/${savedObjectTypes[0]}/get` + ], + applicationPrivilegesResponse: { + [mockActions.login]: true, + [mockActions.version]: false, + [`action:saved_objects/${savedObjectTypes[0]}/get`]: true, + }, + indexPrivilegesResponse, + expectErrorThrown: true + }); +}); + +describe('with no application privileges', () => { ['create', 'delete', 'read', 'view_index_metadata'].forEach(indexPrivilege => { - test(`returns legacy if they have ${indexPrivilege} privilege on the kibana index`, async () => { - const privilege = `action:saved_objects/${savedObjectTypes[0]}/get`; - const username = 'foo-username'; - const mockServer = createMockServer(); - const callWithRequest = createMockCallWithRequest([ - mockApplicationPrivilegeResponse({ - hasAllRequested: false, - privileges: { - [getVersionAction(defaultVersion)]: false, - [getLoginAction()]: false, - [privilege]: false, - }, - username, - }), - mockKibanaIndexPrivilegesResponse({ - privileges: { - create: false, - delete: false, - read: false, - view_index_metadata: false, - [indexPrivilege]: true - }, - }) - ]); - - const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(mockServer); - const request = Symbol(); - const checkPrivileges = checkPrivilegesWithRequest(request); - const privileges = [privilege]; - const result = await checkPrivileges(privileges); - - expectDeprecationLogged(mockServer); - expect(callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { - body: { - applications: [{ - application: defaultApplication, - resources: [ALL_RESOURCE], - privileges: [ - getVersionAction(defaultVersion), getLoginAction(), ...privileges - ] - }] - } - }); - expect(callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { - body: { - index: [{ - names: [ defaultKibanaIndex ], - privileges: ['create', 'delete', 'read', 'view_index_metadata'] - }] - } - }); - expect(result).toEqual({ + checkPrivilegesTest(`returns legacy if they have ${indexPrivilege} privilege on the kibana index`, { + username: 'foo-username', + privileges: [ + `action:saved_objects/${savedObjectTypes[0]}/get` + ], + applicationPrivilegesResponse: { + [mockActions.version]: false, + [mockActions.login]: false, + [`action:saved_objects/${savedObjectTypes[0]}/get`]: false, + }, + indexPrivilegesResponse: { + create: false, + delete: false, + read: false, + view_index_metadata: false, + [indexPrivilege]: true + }, + expectedResult: { result: CHECK_PRIVILEGES_RESULT.LEGACY, - username, - missing: [getLoginAction(), ...privileges], - }); + username: 'foo-username', + missing: [`action:saved_objects/${savedObjectTypes[0]}/get`], + } }); }); }); diff --git a/x-pack/plugins/security/server/lib/authorization/deep_freeze.js b/x-pack/plugins/security/server/lib/authorization/deep_freeze.js new file mode 100644 index 00000000000000..0f9363cb410f6c --- /dev/null +++ b/x-pack/plugins/security/server/lib/authorization/deep_freeze.js @@ -0,0 +1,20 @@ +/* + * 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 { isObject } from 'lodash'; + +export function deepFreeze(object) { + // for any properties that reference an object, makes sure that object is + // recursively frozen as well + Object.keys(object).forEach(key => { + const value = object[key]; + if (isObject(value)) { + deepFreeze(value); + } + }); + + return Object.freeze(object); +} diff --git a/x-pack/plugins/security/server/lib/authorization/deep_freeze.test.js b/x-pack/plugins/security/server/lib/authorization/deep_freeze.test.js new file mode 100644 index 00000000000000..dd227fa6269bff --- /dev/null +++ b/x-pack/plugins/security/server/lib/authorization/deep_freeze.test.js @@ -0,0 +1,97 @@ +/* + * 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 { deepFreeze } from './deep_freeze'; + +test(`freezes result and input`, () => { + const input = {}; + const result = deepFreeze(input); + + Object.isFrozen(input); + Object.isFrozen(result); +}); + +test(`freezes top-level properties that are objects`, () => { + const result = deepFreeze({ + object: {}, + array: [], + fn: () => {}, + number: 1, + string: '', + }); + + Object.isFrozen(result.object); + Object.isFrozen(result.array); + Object.isFrozen(result.fn); + Object.isFrozen(result.number); + Object.isFrozen(result.string); +}); + +test(`freezes child properties that are objects`, () => { + const result = deepFreeze({ + object: { + object: { + }, + array: [], + fn: () => {}, + number: 1, + string: '', + }, + array: [ + {}, + [], + () => {}, + 1, + '', + ], + }); + + Object.isFrozen(result.object.object); + Object.isFrozen(result.object.array); + Object.isFrozen(result.object.fn); + Object.isFrozen(result.object.number); + Object.isFrozen(result.object.string); + Object.isFrozen(result.array[0]); + Object.isFrozen(result.array[1]); + Object.isFrozen(result.array[2]); + Object.isFrozen(result.array[3]); + Object.isFrozen(result.array[4]); +}); + +test(`freezes grand-child properties that are objects`, () => { + const result = deepFreeze({ + object: { + object: { + object: { + }, + array: [], + fn: () => {}, + number: 1, + string: '', + }, + }, + array: [ + [ + {}, + [], + () => {}, + 1, + '', + ], + ], + }); + + Object.isFrozen(result.object.object.object); + Object.isFrozen(result.object.object.array); + Object.isFrozen(result.object.object.fn); + Object.isFrozen(result.object.object.number); + Object.isFrozen(result.object.object.string); + Object.isFrozen(result.array[0][0]); + Object.isFrozen(result.array[0][1]); + Object.isFrozen(result.array[0][2]); + Object.isFrozen(result.array[0][3]); + Object.isFrozen(result.array[0][4]); +}); diff --git a/x-pack/plugins/security/server/lib/authorization/index.js b/x-pack/plugins/security/server/lib/authorization/index.js new file mode 100644 index 00000000000000..e0029a3caeafd1 --- /dev/null +++ b/x-pack/plugins/security/server/lib/authorization/index.js @@ -0,0 +1,10 @@ +/* + * 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. + */ + +export { CHECK_PRIVILEGES_RESULT } from './check_privileges'; +export { registerPrivilegesWithCluster } from './register_privileges_with_cluster'; +export { buildPrivilegeMap } from './privileges'; +export { initAuthorizationService } from './init'; diff --git a/x-pack/plugins/security/server/lib/authorization/init.js b/x-pack/plugins/security/server/lib/authorization/init.js new file mode 100644 index 00000000000000..27ca3f7e84b556 --- /dev/null +++ b/x-pack/plugins/security/server/lib/authorization/init.js @@ -0,0 +1,22 @@ +/* + * 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 { actionsFactory } from './actions'; +import { checkPrivilegesWithRequestFactory } from './check_privileges'; +import { deepFreeze } from './deep_freeze'; +import { getClient } from '../../../../../server/lib/get_client_shield'; + +export function initAuthorizationService(server) { + const shieldClient = getClient(server); + const config = server.config(); + + const actions = actionsFactory(config); + + server.expose('authorization', deepFreeze({ + checkPrivilegesWithRequest: checkPrivilegesWithRequestFactory(shieldClient, config, actions), + actions + })); +} diff --git a/x-pack/plugins/security/server/lib/authorization/init.test.js b/x-pack/plugins/security/server/lib/authorization/init.test.js new file mode 100644 index 00000000000000..b72813809896f0 --- /dev/null +++ b/x-pack/plugins/security/server/lib/authorization/init.test.js @@ -0,0 +1,64 @@ +/* + * 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 { initAuthorizationService } from './init'; +import { actionsFactory } from './actions'; +import { checkPrivilegesWithRequestFactory } from './check_privileges'; +import { getClient } from '../../../../../server/lib/get_client_shield'; + +jest.mock('./check_privileges', () => ({ + checkPrivilegesWithRequestFactory: jest.fn(), +})); + +jest.mock('../../../../../server/lib/get_client_shield', () => ({ + getClient: jest.fn(), +})); + +jest.mock('./actions', () => ({ + actionsFactory: jest.fn(), +})); + +test(`calls server.expose with exposed services`, () => { + const mockConfig = Symbol(); + const mockServer = { + expose: jest.fn(), + config: jest.fn().mockReturnValue(mockConfig) + }; + const mockShieldClient = Symbol(); + getClient.mockReturnValue(mockShieldClient); + const mockCheckPrivilegesWithRequest = Symbol(); + checkPrivilegesWithRequestFactory.mockReturnValue(mockCheckPrivilegesWithRequest); + const mockActions = Symbol(); + actionsFactory.mockReturnValue(mockActions); + + initAuthorizationService(mockServer); + + expect(getClient).toHaveBeenCalledWith(mockServer); + expect(actionsFactory).toHaveBeenCalledWith(mockConfig); + expect(checkPrivilegesWithRequestFactory).toHaveBeenCalledWith(mockShieldClient, mockConfig, mockActions); + expect(mockServer.expose).toHaveBeenCalledWith('authorization', { + checkPrivilegesWithRequest: mockCheckPrivilegesWithRequest, + actions: mockActions, + }); +}); + +test(`deep freezes exposed service`, () => { + const mockConfig = Symbol(); + const mockServer = { + expose: jest.fn(), + config: jest.fn().mockReturnValue(mockConfig) + }; + actionsFactory.mockReturnValue({ + login: 'login', + }); + + initAuthorizationService(mockServer); + + const exposed = mockServer.expose.mock.calls[0][1]; + expect(() => delete exposed.checkPrivilegesWithRequest).toThrowErrorMatchingSnapshot(); + expect(() => exposed.foo = 'bar').toThrowErrorMatchingSnapshot(); + expect(() => exposed.actions.login = 'not-login').toThrowErrorMatchingSnapshot(); +}); diff --git a/x-pack/plugins/security/server/lib/authorization/privileges.js b/x-pack/plugins/security/server/lib/authorization/privileges.js new file mode 100644 index 00000000000000..9ded71662ed51c --- /dev/null +++ b/x-pack/plugins/security/server/lib/authorization/privileges.js @@ -0,0 +1,30 @@ +/* + * 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. + */ + +export function buildPrivilegeMap(savedObjectTypes, application, actions) { + const buildSavedObjectsActions = (savedObjectActions) => { + return savedObjectTypes + .map(type => savedObjectActions.map(savedObjectAction => actions.getSavedObjectAction(type, savedObjectAction))) + .reduce((acc, types) => [...acc, ...types], []); + }; + + // the following list of privileges should only be added to, you can safely remove actions, but not privileges as + // it's a backwards compatibility issue and we'll have to at least adjust registerPrivilegesWithCluster to support it + return { + all: { + application, + name: 'all', + actions: [actions.version, 'action:*'], + metadata: {} + }, + read: { + application, + name: 'read', + actions: [actions.version, actions.login, ...buildSavedObjectsActions(['get', 'bulk_get', 'find'])], + metadata: {} + } + }; +} diff --git a/x-pack/plugins/security/server/lib/privileges/privilege_action_registry.js b/x-pack/plugins/security/server/lib/authorization/register_privileges_with_cluster.js similarity index 96% rename from x-pack/plugins/security/server/lib/privileges/privilege_action_registry.js rename to x-pack/plugins/security/server/lib/authorization/register_privileges_with_cluster.js index 93166fe28dd096..432d4647dc1ee7 100644 --- a/x-pack/plugins/security/server/lib/privileges/privilege_action_registry.js +++ b/x-pack/plugins/security/server/lib/authorization/register_privileges_with_cluster.js @@ -7,12 +7,14 @@ import { difference, isEmpty, isEqual } from 'lodash'; import { buildPrivilegeMap } from './privileges'; import { getClient } from '../../../../../server/lib/get_client_shield'; +import { actionsFactory } from './actions'; export async function registerPrivilegesWithCluster(server) { const config = server.config(); - const kibanaVersion = config.get('pkg.version'); + + const actions = actionsFactory(config); const application = config.get('xpack.security.rbac.application'); const savedObjectTypes = server.savedObjects.types; @@ -25,7 +27,7 @@ export async function registerPrivilegesWithCluster(server) { }; const expectedPrivileges = { - [application]: buildPrivilegeMap(savedObjectTypes, application, kibanaVersion) + [application]: buildPrivilegeMap(savedObjectTypes, application, actions) }; server.log(['security', 'debug'], `Registering Kibana Privileges with Elasticsearch for ${application}`); diff --git a/x-pack/plugins/security/server/lib/privileges/privilege_action_registry.test.js b/x-pack/plugins/security/server/lib/authorization/register_privileges_with_cluster.test.js similarity index 94% rename from x-pack/plugins/security/server/lib/privileges/privilege_action_registry.test.js rename to x-pack/plugins/security/server/lib/authorization/register_privileges_with_cluster.test.js index c6e3428c563d01..9e4f2fa237e7a8 100644 --- a/x-pack/plugins/security/server/lib/privileges/privilege_action_registry.test.js +++ b/x-pack/plugins/security/server/lib/authorization/register_privileges_with_cluster.test.js @@ -4,19 +4,24 @@ * you may not use this file except in compliance with the Elastic License. */ -import { registerPrivilegesWithCluster } from './privilege_action_registry'; +import { registerPrivilegesWithCluster } from './register_privileges_with_cluster'; import { getClient } from '../../../../../server/lib/get_client_shield'; import { buildPrivilegeMap } from './privileges'; +import { actionsFactory } from './actions'; jest.mock('../../../../../server/lib/get_client_shield', () => ({ getClient: jest.fn(), })); jest.mock('./privileges', () => ({ buildPrivilegeMap: jest.fn(), })); +jest.mock('./actions', () => ({ + actionsFactory: jest.fn(), +})); const registerPrivilegesWithClusterTest = (description, { settings = {}, savedObjectTypes, + actions, expectedPrivileges, existingPrivileges, throwErrorWhenGettingPrivileges, @@ -143,6 +148,7 @@ const registerPrivilegesWithClusterTest = (description, { } }); + actionsFactory.mockReturnValue(actions); buildPrivilegeMap.mockReturnValue(expectedPrivileges); let error; @@ -163,7 +169,7 @@ const registerPrivilegesWithClusterTest = (description, { }); }; -registerPrivilegesWithClusterTest(`passes saved object types, application and kibanaVersion to buildPrivilegeMap`, { +registerPrivilegesWithClusterTest(`passes saved object types, application and actions to buildPrivilegeMap`, { settings: { 'pkg.version': 'foo-version', 'xpack.security.rbac.application': 'foo-application', @@ -172,8 +178,15 @@ registerPrivilegesWithClusterTest(`passes saved object types, application and ki 'foo-type', 'bar-type', ], + actions: { + login: 'mock-action:login', + version: 'mock-action:version', + }, assert: ({ mocks }) => { - expect(mocks.buildPrivilegeMap).toHaveBeenCalledWith(['foo-type', 'bar-type'], 'foo-application', 'foo-version'); + expect(mocks.buildPrivilegeMap).toHaveBeenCalledWith(['foo-type', 'bar-type'], 'foo-application', { + login: 'mock-action:login', + version: 'mock-action:version', + }); }, }); diff --git a/x-pack/plugins/security/server/lib/privileges/index.js b/x-pack/plugins/security/server/lib/privileges/index.js deleted file mode 100644 index f888dffa922dd6..00000000000000 --- a/x-pack/plugins/security/server/lib/privileges/index.js +++ /dev/null @@ -1,8 +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. - */ - -export { registerPrivilegesWithCluster } from './privilege_action_registry'; -export { buildPrivilegeMap, getLoginAction, getVersionAction } from './privileges'; diff --git a/x-pack/plugins/security/server/lib/privileges/privileges.js b/x-pack/plugins/security/server/lib/privileges/privileges.js deleted file mode 100644 index ea22f6e276594f..00000000000000 --- a/x-pack/plugins/security/server/lib/privileges/privileges.js +++ /dev/null @@ -1,43 +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. - */ - -export function getVersionAction(kibanaVersion) { - return `version:${kibanaVersion}`; -} - -export function getLoginAction() { - return `action:login`; -} - -export function buildPrivilegeMap(savedObjectTypes, application, kibanaVersion) { - const readSavedObjectsActions = buildSavedObjectsReadActions(savedObjectTypes); - - return { - all: { - application, - name: 'all', - actions: [getVersionAction(kibanaVersion), 'action:*'], - metadata: {} - }, - read: { - application, - name: 'read', - actions: [getVersionAction(kibanaVersion), getLoginAction(), ...readSavedObjectsActions], - metadata: {} - } - }; -} - -function buildSavedObjectsReadActions(savedObjectTypes) { - const readActions = ['get', 'bulk_get', 'find']; - return buildSavedObjectsActions(savedObjectTypes, readActions); -} - -function buildSavedObjectsActions(savedObjectTypes, actions) { - return savedObjectTypes - .map(type => actions.map(action => `action:saved_objects/${type}/${action}`)) - .reduce((acc, types) => [...acc, ...types], []); -} diff --git a/x-pack/plugins/security/server/lib/saved_objects_client/secure_saved_objects_client.js b/x-pack/plugins/security/server/lib/saved_objects_client/secure_saved_objects_client.js index 419a379f7d0239..eb19f59c296932 100644 --- a/x-pack/plugins/security/server/lib/saved_objects_client/secure_saved_objects_client.js +++ b/x-pack/plugins/security/server/lib/saved_objects_client/secure_saved_objects_client.js @@ -7,10 +7,6 @@ import { get, uniq } from 'lodash'; import { CHECK_PRIVILEGES_RESULT } from '../authorization/check_privileges'; -const getPrivilege = (type, action) => { - return `action:saved_objects/${type}/${action}`; -}; - export class SecureSavedObjectsClient { constructor(options) { const { @@ -20,6 +16,7 @@ export class SecureSavedObjectsClient { checkPrivileges, auditLogger, savedObjectTypes, + actions, } = options; this.errors = errors; @@ -28,6 +25,7 @@ export class SecureSavedObjectsClient { this._checkPrivileges = checkPrivileges; this._auditLogger = auditLogger; this._savedObjectTypes = savedObjectTypes; + this._actions = actions; } async create(type, attributes = {}, options = {}) { @@ -94,9 +92,9 @@ export class SecureSavedObjectsClient { ); } - async _checkSavedObjectPrivileges(privileges) { + async _checkSavedObjectPrivileges(actions) { try { - return await this._checkPrivileges(privileges); + return await this._checkPrivileges(actions); } catch(error) { const { reason } = get(error, 'body.error', {}); throw this.errors.decorateGeneralError(error, reason); @@ -105,8 +103,8 @@ export class SecureSavedObjectsClient { async _execute(typeOrTypes, action, args, fn) { const types = Array.isArray(typeOrTypes) ? typeOrTypes : [typeOrTypes]; - const privileges = types.map(type => getPrivilege(type, action)); - const { result, username, missing } = await this._checkSavedObjectPrivileges(privileges); + const actions = types.map(type => this._actions.getSavedObjectAction(type, action)); + const { result, username, missing } = await this._checkSavedObjectPrivileges(actions); switch (result) { case CHECK_PRIVILEGES_RESULT.AUTHORIZED: @@ -116,7 +114,7 @@ export class SecureSavedObjectsClient { return await fn(this._callWithRequestRepository); case CHECK_PRIVILEGES_RESULT.UNAUTHORIZED: this._auditLogger.savedObjectsAuthorizationFailure(username, action, types, missing, args); - const msg = `Unable to ${action} ${types.sort().join(',')}, missing ${missing.sort().join(',')}`; + const msg = `Unable to ${action} ${[...types].sort().join(',')}, missing ${[...missing].sort().join(',')}`; throw this.errors.decorateForbiddenError(new Error(msg)); default: throw new Error('Unexpected result from hasPrivileges'); @@ -128,7 +126,7 @@ export class SecureSavedObjectsClient { // we have to filter for only their authorized types const types = this._savedObjectTypes; - const typesToPrivilegesMap = new Map(types.map(type => [type, getPrivilege(type, action)])); + const typesToPrivilegesMap = new Map(types.map(type => [type, this._actions.getSavedObjectAction(type, action)])); const { result, username, missing } = await this._checkSavedObjectPrivileges(Array.from(typesToPrivilegesMap.values())); if (result === CHECK_PRIVILEGES_RESULT.LEGACY) { diff --git a/x-pack/plugins/security/server/lib/saved_objects_client/secure_saved_objects_client.test.js b/x-pack/plugins/security/server/lib/saved_objects_client/secure_saved_objects_client.test.js index 5e1b1f71165246..99656772c0df79 100644 --- a/x-pack/plugins/security/server/lib/saved_objects_client/secure_saved_objects_client.test.js +++ b/x-pack/plugins/security/server/lib/saved_objects_client/secure_saved_objects_client.test.js @@ -26,6 +26,14 @@ const createMockAuditLogger = () => { }; }; +const createMockActions = () => { + return { + getSavedObjectAction(type, action) { + return `mock-action:saved_objects/${type}/${action}`; + } + }; +}; + describe('#errors', () => { test(`assigns errors from constructor to .errors`, () => { const errors = Symbol(); @@ -44,15 +52,17 @@ describe('#create', () => { throw new Error(); }); const mockAuditLogger = createMockAuditLogger(); + const mockActions = createMockActions(); const client = new SecureSavedObjectsClient({ errors: mockErrors, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: mockActions, }); await expect(client.create(type)).rejects.toThrowError(mockErrors.generalError); - expect(mockCheckPrivileges).toHaveBeenCalledWith([`action:saved_objects/${type}/create`]); + expect(mockCheckPrivileges).toHaveBeenCalledWith([mockActions.getSavedObjectAction(type, 'create')]); expect(mockErrors.decorateGeneralError).toHaveBeenCalledTimes(1); expect(mockAuditLogger.savedObjectsAuthorizationFailure).not.toHaveBeenCalled(); expect(mockAuditLogger.savedObjectsAuthorizationSuccess).not.toHaveBeenCalled(); @@ -62,31 +72,31 @@ describe('#create', () => { const type = 'foo'; const username = Symbol(); const mockErrors = createMockErrors(); - const mockCheckPrivileges = jest.fn().mockImplementation(async () => ({ + const mockCheckPrivileges = jest.fn().mockImplementation(async privileges => ({ result: CHECK_PRIVILEGES_RESULT.UNAUTHORIZED, username, - missing: [ - `action:saved_objects/${type}/create` - ], + missing: privileges, })); const mockAuditLogger = createMockAuditLogger(); + const mockActions = createMockActions(); const client = new SecureSavedObjectsClient({ errors: mockErrors, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: mockActions, }); const attributes = Symbol(); const options = Symbol(); await expect(client.create(type, attributes, options)).rejects.toThrowError(mockErrors.forbiddenError); - expect(mockCheckPrivileges).toHaveBeenCalledWith([`action:saved_objects/${type}/create`]); + expect(mockCheckPrivileges).toHaveBeenCalledWith([mockActions.getSavedObjectAction(type, 'create')]); expect(mockErrors.decorateForbiddenError).toHaveBeenCalledTimes(1); expect(mockAuditLogger.savedObjectsAuthorizationFailure).toHaveBeenCalledWith( username, 'create', [type], - [`action:saved_objects/${type}/create`], + [mockActions.getSavedObjectAction(type, 'create')], { type, attributes, @@ -112,6 +122,7 @@ describe('#create', () => { internalRepository: mockRepository, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: createMockActions(), }); const attributes = Symbol(); const options = Symbol(); @@ -135,18 +146,17 @@ describe('#create', () => { const mockRepository = { create: jest.fn().mockReturnValue(returnValue) }; - const mockCheckPrivileges = jest.fn().mockImplementation(async () => ({ + const mockCheckPrivileges = jest.fn().mockImplementation(async privileges => ({ result: CHECK_PRIVILEGES_RESULT.LEGACY, username, - missing: [ - `action:saved_objects/${type}/create` - ], + missing: privileges, })); const mockAuditLogger = createMockAuditLogger(); const client = new SecureSavedObjectsClient({ callWithRequestRepository: mockRepository, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: createMockActions(), }); const attributes = Symbol(); const options = Symbol(); @@ -168,15 +178,17 @@ describe('#bulkCreate', () => { throw new Error(); }); const mockAuditLogger = createMockAuditLogger(); + const mockActions = createMockActions(); const client = new SecureSavedObjectsClient({ errors: mockErrors, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: mockActions, }); await expect(client.bulkCreate([{ type }])).rejects.toThrowError(mockErrors.generalError); - expect(mockCheckPrivileges).toHaveBeenCalledWith([`action:saved_objects/${type}/bulk_create`]); + expect(mockCheckPrivileges).toHaveBeenCalledWith([mockActions.getSavedObjectAction(type, 'bulk_create')]); expect(mockErrors.decorateGeneralError).toHaveBeenCalledTimes(1); expect(mockAuditLogger.savedObjectsAuthorizationFailure).not.toHaveBeenCalled(); expect(mockAuditLogger.savedObjectsAuthorizationSuccess).not.toHaveBeenCalled(); @@ -187,18 +199,20 @@ describe('#bulkCreate', () => { const type2 = 'bar'; const username = Symbol(); const mockErrors = createMockErrors(); - const mockCheckPrivileges = jest.fn().mockImplementation(async () => ({ + const mockCheckPrivileges = jest.fn().mockImplementation(async privileges => ({ result: CHECK_PRIVILEGES_RESULT.UNAUTHORIZED, username, missing: [ - `action:saved_objects/${type1}/bulk_create` + privileges[0] ], })); const mockAuditLogger = createMockAuditLogger(); + const mockActions = createMockActions(); const client = new SecureSavedObjectsClient({ errors: mockErrors, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: mockActions, }); const objects = [ { type: type1 }, @@ -210,15 +224,15 @@ describe('#bulkCreate', () => { await expect(client.bulkCreate(objects, options)).rejects.toThrowError(mockErrors.forbiddenError); expect(mockCheckPrivileges).toHaveBeenCalledWith([ - `action:saved_objects/${type1}/bulk_create`, - `action:saved_objects/${type2}/bulk_create` + mockActions.getSavedObjectAction(type1, 'bulk_create'), + mockActions.getSavedObjectAction(type2, 'bulk_create'), ]); expect(mockErrors.decorateForbiddenError).toHaveBeenCalledTimes(1); expect(mockAuditLogger.savedObjectsAuthorizationFailure).toHaveBeenCalledWith( username, 'bulk_create', - [type2, type1], - [`action:saved_objects/${type1}/bulk_create`], + [type1, type2], + [mockActions.getSavedObjectAction(type1, 'bulk_create')], { objects, options, @@ -243,6 +257,7 @@ describe('#bulkCreate', () => { internalRepository: mockRepository, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: createMockActions(), }); const objects = [ { type: type1, otherThing: 'sup' }, @@ -269,19 +284,17 @@ describe('#bulkCreate', () => { const mockRepository = { bulkCreate: jest.fn().mockReturnValue(returnValue) }; - const mockCheckPrivileges = jest.fn().mockImplementation(async () => ({ + const mockCheckPrivileges = jest.fn().mockImplementation(async privileges => ({ result: CHECK_PRIVILEGES_RESULT.LEGACY, username, - missing: [ - `action:saved_objects/${type1}/bulk_create`, - `action:saved_objects/${type2}/bulk_create`, - ], + missing: privileges, })); const mockAuditLogger = createMockAuditLogger(); const client = new SecureSavedObjectsClient({ callWithRequestRepository: mockRepository, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: createMockActions(), }); const objects = [ { type: type1, otherThing: 'sup' }, @@ -306,15 +319,17 @@ describe('#delete', () => { throw new Error(); }); const mockAuditLogger = createMockAuditLogger(); + const mockActions = createMockActions(); const client = new SecureSavedObjectsClient({ errors: mockErrors, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: mockActions, }); await expect(client.delete(type)).rejects.toThrowError(mockErrors.generalError); - expect(mockCheckPrivileges).toHaveBeenCalledWith([`action:saved_objects/${type}/delete`]); + expect(mockCheckPrivileges).toHaveBeenCalledWith([mockActions.getSavedObjectAction(type, 'delete')]); expect(mockErrors.decorateGeneralError).toHaveBeenCalledTimes(1); expect(mockAuditLogger.savedObjectsAuthorizationFailure).not.toHaveBeenCalled(); expect(mockAuditLogger.savedObjectsAuthorizationSuccess).not.toHaveBeenCalled(); @@ -324,30 +339,30 @@ describe('#delete', () => { const type = 'foo'; const username = Symbol(); const mockErrors = createMockErrors(); - const mockCheckPrivileges = jest.fn().mockImplementation(async () => ({ + const mockCheckPrivileges = jest.fn().mockImplementation(async privileges => ({ result: CHECK_PRIVILEGES_RESULT.UNAUTHORIZED, username, - missing: [ - `action:saved_objects/${type}/delete` - ], + missing: privileges, })); const mockAuditLogger = createMockAuditLogger(); + const mockActions = createMockActions(); const client = new SecureSavedObjectsClient({ errors: mockErrors, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: mockActions, }); const id = Symbol(); await expect(client.delete(type, id)).rejects.toThrowError(mockErrors.forbiddenError); - expect(mockCheckPrivileges).toHaveBeenCalledWith([`action:saved_objects/${type}/delete`]); + expect(mockCheckPrivileges).toHaveBeenCalledWith([mockActions.getSavedObjectAction(type, 'delete')]); expect(mockErrors.decorateForbiddenError).toHaveBeenCalledTimes(1); expect(mockAuditLogger.savedObjectsAuthorizationFailure).toHaveBeenCalledWith( username, 'delete', [type], - [`action:saved_objects/${type}/delete`], + [mockActions.getSavedObjectAction(type, 'delete')], { type, id, @@ -372,6 +387,7 @@ describe('#delete', () => { internalRepository: mockRepository, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: createMockActions(), }); const id = Symbol(); @@ -393,18 +409,17 @@ describe('#delete', () => { const mockRepository = { delete: jest.fn().mockReturnValue(returnValue) }; - const mockCheckPrivileges = jest.fn().mockImplementation(async () => ({ + const mockCheckPrivileges = jest.fn().mockImplementation(async privileges => ({ result: CHECK_PRIVILEGES_RESULT.LEGACY, username, - missing: [ - `action:saved_objects/${type}/delete` - ], + missing: privileges, })); const mockAuditLogger = createMockAuditLogger(); const client = new SecureSavedObjectsClient({ callWithRequestRepository: mockRepository, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: createMockActions(), }); const id = Symbol(); @@ -426,15 +441,17 @@ describe('#find', () => { throw new Error(); }); const mockAuditLogger = createMockAuditLogger(); + const mockActions = createMockActions(); const client = new SecureSavedObjectsClient({ errors: mockErrors, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: mockActions, }); await expect(client.find({ type })).rejects.toThrowError(mockErrors.generalError); - expect(mockCheckPrivileges).toHaveBeenCalledWith([`action:saved_objects/${type}/find`]); + expect(mockCheckPrivileges).toHaveBeenCalledWith([mockActions.getSavedObjectAction(type, 'find')]); expect(mockErrors.decorateGeneralError).toHaveBeenCalledTimes(1); expect(mockAuditLogger.savedObjectsAuthorizationFailure).not.toHaveBeenCalled(); expect(mockAuditLogger.savedObjectsAuthorizationSuccess).not.toHaveBeenCalled(); @@ -445,31 +462,31 @@ describe('#find', () => { const username = Symbol(); const mockRepository = {}; const mockErrors = createMockErrors(); - const mockCheckPrivileges = jest.fn().mockImplementation(async () => ({ + const mockCheckPrivileges = jest.fn().mockImplementation(async privileges => ({ result: CHECK_PRIVILEGES_RESULT.UNAUTHORIZED, username, - missing: [ - `action:saved_objects/${type}/find` - ], + missing: privileges, })); const mockAuditLogger = createMockAuditLogger(); + const mockActions = createMockActions(); const client = new SecureSavedObjectsClient({ errors: mockErrors, internalRepository: mockRepository, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: mockActions, }); const options = { type }; await expect(client.find(options)).rejects.toThrowError(mockErrors.forbiddenError); - expect(mockCheckPrivileges).toHaveBeenCalledWith([`action:saved_objects/${type}/find`]); + expect(mockCheckPrivileges).toHaveBeenCalledWith([mockActions.getSavedObjectAction(type, 'find')]); expect(mockErrors.decorateForbiddenError).toHaveBeenCalledTimes(1); expect(mockAuditLogger.savedObjectsAuthorizationFailure).toHaveBeenCalledWith( username, 'find', [type], - [`action:saved_objects/${type}/find`], + [mockActions.getSavedObjectAction(type, 'find')], { options } @@ -482,69 +499,37 @@ describe('#find', () => { const type2 = 'bar'; const username = Symbol(); const mockErrors = createMockErrors(); - const mockCheckPrivileges = jest.fn().mockImplementation(async () => ({ - result: CHECK_PRIVILEGES_RESULT.UNAUTHORIZED, - username, - missing: [ - `action:saved_objects/${type1}/find` - ], - })); - const mockAuditLogger = createMockAuditLogger(); - const client = new SecureSavedObjectsClient({ - errors: mockErrors, - checkPrivileges: mockCheckPrivileges, - auditLogger: mockAuditLogger, + const mockCheckPrivileges = jest.fn().mockImplementation(async privileges => { + return { + result: CHECK_PRIVILEGES_RESULT.UNAUTHORIZED, + username, + missing: [ + privileges[0] + ], + }; }); - const options = { type: [ type1, type2 ] }; - - await expect(client.find(options)).rejects.toThrowError(mockErrors.forbiddenError); - - expect(mockCheckPrivileges).toHaveBeenCalledWith([`action:saved_objects/${type1}/find`, `action:saved_objects/${type2}/find`]); - expect(mockErrors.decorateForbiddenError).toHaveBeenCalledTimes(1); - expect(mockAuditLogger.savedObjectsAuthorizationFailure).toHaveBeenCalledWith( - username, - 'find', - [type2, type1], - [`action:saved_objects/${type1}/find`], - { - options - } - ); - expect(mockAuditLogger.savedObjectsAuthorizationSuccess).not.toHaveBeenCalled(); - }); - - test(`throws decorated ForbiddenError when type's an array and unauthorized`, async () => { - const type1 = 'foo'; - const type2 = 'bar'; - const username = Symbol(); - const mockRepository = {}; - const mockErrors = createMockErrors(); - const mockCheckPrivileges = jest.fn().mockImplementation(async () => ({ - result: CHECK_PRIVILEGES_RESULT.UNAUTHORIZED, - username, - missing: [ - `action:saved_objects/${type1}/find`, - `action:saved_objects/${type2}/find` - ], - })); const mockAuditLogger = createMockAuditLogger(); + const mockActions = createMockActions(); const client = new SecureSavedObjectsClient({ errors: mockErrors, - repository: mockRepository, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: mockActions, }); const options = { type: [ type1, type2 ] }; await expect(client.find(options)).rejects.toThrowError(mockErrors.forbiddenError); - expect(mockCheckPrivileges).toHaveBeenCalledWith([`action:saved_objects/${type1}/find`, `action:saved_objects/${type2}/find`]); + expect(mockCheckPrivileges).toHaveBeenCalledWith([ + mockActions.getSavedObjectAction(type1, 'find'), + mockActions.getSavedObjectAction(type2, 'find') + ]); expect(mockErrors.decorateForbiddenError).toHaveBeenCalledTimes(1); expect(mockAuditLogger.savedObjectsAuthorizationFailure).toHaveBeenCalledWith( username, 'find', - [type2, type1], - [`action:saved_objects/${type2}/find`, `action:saved_objects/${type1}/find`], + [type1, type2], + [mockActions.getSavedObjectAction(type1, 'find')], { options } @@ -568,6 +553,7 @@ describe('#find', () => { internalRepository: mockRepository, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: createMockActions(), }); const options = { type }; @@ -588,18 +574,17 @@ describe('#find', () => { const mockRepository = { find: jest.fn().mockReturnValue(returnValue) }; - const mockCheckPrivileges = jest.fn().mockImplementation(async () => ({ + const mockCheckPrivileges = jest.fn().mockImplementation(async privileges => ({ result: CHECK_PRIVILEGES_RESULT.LEGACY, username, - missing: [ - `action:saved_objects/${type}/find` - ], + missing: privileges, })); const mockAuditLogger = createMockAuditLogger(); const client = new SecureSavedObjectsClient({ callWithRequestRepository: mockRepository, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: createMockActions(), }); const options = { type }; @@ -622,17 +607,22 @@ describe('#find', () => { throw new Error(); }); const mockAuditLogger = createMockAuditLogger(); + const mockActions = createMockActions(); const client = new SecureSavedObjectsClient({ errors: mockErrors, repository: mockRepository, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, - savedObjectTypes: [type1, type2] + savedObjectTypes: [type1, type2], + actions: mockActions, }); await expect(client.find()).rejects.toThrowError(mockErrors.generalError); - expect(mockCheckPrivileges).toHaveBeenCalledWith([`action:saved_objects/${type1}/find`, `action:saved_objects/${type2}/find`]); + expect(mockCheckPrivileges).toHaveBeenCalledWith([ + mockActions.getSavedObjectAction(type1, 'find'), + mockActions.getSavedObjectAction(type2, 'find'), + ]); expect(mockErrors.decorateGeneralError).toHaveBeenCalledTimes(1); expect(mockAuditLogger.savedObjectsAuthorizationFailure).not.toHaveBeenCalled(); expect(mockAuditLogger.savedObjectsAuthorizationSuccess).not.toHaveBeenCalled(); @@ -643,32 +633,34 @@ describe('#find', () => { const username = Symbol(); const mockRepository = {}; const mockErrors = createMockErrors(); - const mockCheckPrivileges = jest.fn().mockImplementation(async () => ({ + const mockCheckPrivileges = jest.fn().mockImplementation(async privileges => ({ result: CHECK_PRIVILEGES_RESULT.UNAUTHORIZED, username, missing: [ - `action:saved_objects/${type}/find` + privileges[0] ], })); const mockAuditLogger = createMockAuditLogger(); + const mockActions = createMockActions(); const client = new SecureSavedObjectsClient({ errors: mockErrors, repository: mockRepository, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, - savedObjectTypes: [type] + savedObjectTypes: [type], + actions: mockActions, }); const options = Symbol(); await expect(client.find(options)).rejects.toThrowError(mockErrors.forbiddenError); - expect(mockCheckPrivileges).toHaveBeenCalledWith([`action:saved_objects/${type}/find`]); + expect(mockCheckPrivileges).toHaveBeenCalledWith([mockActions.getSavedObjectAction(type, 'find')]); expect(mockErrors.decorateForbiddenError).toHaveBeenCalledTimes(1); expect(mockAuditLogger.savedObjectsAuthorizationFailure).toHaveBeenCalledWith( username, 'find', [type], - [`action:saved_objects/${type}/find`], + [mockActions.getSavedObjectAction(type, 'find')], { options } @@ -684,27 +676,27 @@ describe('#find', () => { find: jest.fn().mockReturnValue(returnValue) }; const mockErrors = createMockErrors(); - const mockCheckPrivileges = jest.fn().mockImplementation(async () => ({ + const mockCheckPrivileges = jest.fn().mockImplementation(async privileges => ({ result: CHECK_PRIVILEGES_RESULT.LEGACY, username, - missing: [ - `action:saved_objects/${type}/find` - ], + missing: privileges, })); const mockAuditLogger = createMockAuditLogger(); + const mockActions = createMockActions(); const client = new SecureSavedObjectsClient({ errors: mockErrors, callWithRequestRepository: mockRepository, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, - savedObjectTypes: [type] + savedObjectTypes: [type], + actions: mockActions, }); const options = Symbol(); const result = await client.find(options); expect(result).toBe(returnValue); - expect(mockCheckPrivileges).toHaveBeenCalledWith([`action:saved_objects/${type}/find`]); + expect(mockCheckPrivileges).toHaveBeenCalledWith([mockActions.getSavedObjectAction(type, 'find')]); expect(mockRepository.find).toHaveBeenCalledWith(options); expect(mockAuditLogger.savedObjectsAuthorizationFailure).not.toHaveBeenCalled(); expect(mockAuditLogger.savedObjectsAuthorizationSuccess).not.toHaveBeenCalled(); @@ -717,24 +709,29 @@ describe('#find', () => { find: jest.fn(), }; const mockErrors = createMockErrors(); - const mockCheckPrivileges = jest.fn().mockImplementation(async () => ({ + const mockCheckPrivileges = jest.fn().mockImplementation(async privileges => ({ result: CHECK_PRIVILEGES_RESULT.UNAUTHORIZED, missing: [ - `action:saved_objects/${type1}/find` + privileges[0] ] })); const mockAuditLogger = createMockAuditLogger(); + const mockActions = createMockActions(); const client = new SecureSavedObjectsClient({ errors: mockErrors, internalRepository: mockRepository, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, - savedObjectTypes: [type1, type2] + savedObjectTypes: [type1, type2], + actions: mockActions, }); await client.find(); - expect(mockCheckPrivileges).toHaveBeenCalledWith([`action:saved_objects/${type1}/find`, `action:saved_objects/${type2}/find`]); + expect(mockCheckPrivileges).toHaveBeenCalledWith([ + mockActions.getSavedObjectAction(type1, 'find'), + mockActions.getSavedObjectAction(type2, 'find'), + ]); expect(mockRepository.find).toHaveBeenCalledWith(expect.objectContaining({ type: [type2] })); @@ -757,7 +754,8 @@ describe('#find', () => { internalRepository: mockRepository, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, - savedObjectTypes: [type] + savedObjectTypes: [type], + actions: createMockActions(), }); const options = Symbol(); @@ -781,15 +779,17 @@ describe('#bulkGet', () => { throw new Error(); }); const mockAuditLogger = createMockAuditLogger(); + const mockActions = createMockActions(); const client = new SecureSavedObjectsClient({ errors: mockErrors, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: mockActions, }); await expect(client.bulkGet([{ type }])).rejects.toThrowError(mockErrors.generalError); - expect(mockCheckPrivileges).toHaveBeenCalledWith(['action:saved_objects/foo/bulk_get']); + expect(mockCheckPrivileges).toHaveBeenCalledWith([mockActions.getSavedObjectAction(type, 'bulk_get')]); expect(mockErrors.decorateGeneralError).toHaveBeenCalledTimes(1); expect(mockAuditLogger.savedObjectsAuthorizationFailure).not.toHaveBeenCalled(); expect(mockAuditLogger.savedObjectsAuthorizationSuccess).not.toHaveBeenCalled(); @@ -800,18 +800,20 @@ describe('#bulkGet', () => { const type2 = 'bar'; const username = Symbol(); const mockErrors = createMockErrors(); - const mockCheckPrivileges = jest.fn().mockImplementation(async () => ({ + const mockCheckPrivileges = jest.fn().mockImplementation(async privileges => ({ result: CHECK_PRIVILEGES_RESULT.UNAUTHORIZED, username, missing: [ - `action:saved_objects/${type1}/bulk_get` + privileges[0] ], })); const mockAuditLogger = createMockAuditLogger(); + const mockActions = createMockActions(); const client = new SecureSavedObjectsClient({ errors: mockErrors, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: mockActions, }); const objects = [ { type: type1 }, @@ -821,13 +823,16 @@ describe('#bulkGet', () => { await expect(client.bulkGet(objects)).rejects.toThrowError(mockErrors.forbiddenError); - expect(mockCheckPrivileges).toHaveBeenCalledWith([`action:saved_objects/${type1}/bulk_get`, `action:saved_objects/${type2}/bulk_get`]); + expect(mockCheckPrivileges).toHaveBeenCalledWith([ + mockActions.getSavedObjectAction(type1, 'bulk_get'), + mockActions.getSavedObjectAction(type2, 'bulk_get'), + ]); expect(mockErrors.decorateForbiddenError).toHaveBeenCalledTimes(1); expect(mockAuditLogger.savedObjectsAuthorizationFailure).toHaveBeenCalledWith( username, 'bulk_get', - [type2, type1], - [`action:saved_objects/${type1}/bulk_get`], + [type1, type2], + [mockActions.getSavedObjectAction(type1, 'bulk_get')], { objects } @@ -852,6 +857,7 @@ describe('#bulkGet', () => { internalRepository: mockRepository, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: createMockActions(), }); const objects = [ { type: type1, id: 'foo-id' }, @@ -876,19 +882,17 @@ describe('#bulkGet', () => { const mockRepository = { bulkGet: jest.fn().mockReturnValue(returnValue) }; - const mockCheckPrivileges = jest.fn().mockImplementation(async () => ({ + const mockCheckPrivileges = jest.fn().mockImplementation(async privileges => ({ result: CHECK_PRIVILEGES_RESULT.LEGACY, username, - missing: [ - `action:saved_objects/${type1}/bulk_get`, - `action:saved_objects/${type2}/bulk_get` - ], + missing: privileges, })); const mockAuditLogger = createMockAuditLogger(); const client = new SecureSavedObjectsClient({ callWithRequestRepository: mockRepository, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: createMockActions(), }); const objects = [ { type: type1, id: 'foo-id' }, @@ -912,15 +916,17 @@ describe('#get', () => { throw new Error(); }); const mockAuditLogger = createMockAuditLogger(); + const mockActions = createMockActions(); const client = new SecureSavedObjectsClient({ errors: mockErrors, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: mockActions, }); await expect(client.get(type)).rejects.toThrowError(mockErrors.generalError); - expect(mockCheckPrivileges).toHaveBeenCalledWith([`action:saved_objects/${type}/get`]); + expect(mockCheckPrivileges).toHaveBeenCalledWith([mockActions.getSavedObjectAction(type, 'get')]); expect(mockErrors.decorateGeneralError).toHaveBeenCalledTimes(1); expect(mockAuditLogger.savedObjectsAuthorizationFailure).not.toHaveBeenCalled(); expect(mockAuditLogger.savedObjectsAuthorizationSuccess).not.toHaveBeenCalled(); @@ -930,30 +936,30 @@ describe('#get', () => { const type = 'foo'; const username = Symbol(); const mockErrors = createMockErrors(); - const mockCheckPrivileges = jest.fn().mockImplementation(async () => ({ + const mockCheckPrivileges = jest.fn().mockImplementation(async privileges => ({ result: CHECK_PRIVILEGES_RESULT.UNAUTHORIZED, username, - missing: [ - `action:saved_objects/${type}/get` - ], + missing: privileges, })); const mockAuditLogger = createMockAuditLogger(); + const mockActions = createMockActions(); const client = new SecureSavedObjectsClient({ errors: mockErrors, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: mockActions, }); const id = Symbol(); await expect(client.get(type, id)).rejects.toThrowError(mockErrors.forbiddenError); - expect(mockCheckPrivileges).toHaveBeenCalledWith([`action:saved_objects/${type}/get`]); + expect(mockCheckPrivileges).toHaveBeenCalledWith([mockActions.getSavedObjectAction(type, 'get')]); expect(mockErrors.decorateForbiddenError).toHaveBeenCalledTimes(1); expect(mockAuditLogger.savedObjectsAuthorizationFailure).toHaveBeenCalledWith( username, 'get', [type], - [`action:saved_objects/${type}/get`], + [mockActions.getSavedObjectAction(type, 'get')], { type, id, @@ -978,6 +984,7 @@ describe('#get', () => { internalRepository: mockRepository, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: createMockActions(), }); const id = Symbol(); @@ -999,18 +1006,17 @@ describe('#get', () => { const mockRepository = { get: jest.fn().mockReturnValue(returnValue) }; - const mockCheckPrivileges = jest.fn().mockImplementation(async () => ({ + const mockCheckPrivileges = jest.fn().mockImplementation(async privileges => ({ result: CHECK_PRIVILEGES_RESULT.LEGACY, username, - missing: [ - `action:saved_objects/${type}/get` - ], + missing: privileges, })); const mockAuditLogger = createMockAuditLogger(); const client = new SecureSavedObjectsClient({ callWithRequestRepository: mockRepository, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: createMockActions(), }); const id = Symbol(); @@ -1031,15 +1037,17 @@ describe('#update', () => { throw new Error(); }); const mockAuditLogger = createMockAuditLogger(); + const mockActions = createMockActions(); const client = new SecureSavedObjectsClient({ errors: mockErrors, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: mockActions, }); await expect(client.update(type)).rejects.toThrowError(mockErrors.generalError); - expect(mockCheckPrivileges).toHaveBeenCalledWith([`action:saved_objects/${type}/update`]); + expect(mockCheckPrivileges).toHaveBeenCalledWith([mockActions.getSavedObjectAction(type, 'update')]); expect(mockErrors.decorateGeneralError).toHaveBeenCalledTimes(1); expect(mockAuditLogger.savedObjectsAuthorizationFailure).not.toHaveBeenCalled(); expect(mockAuditLogger.savedObjectsAuthorizationSuccess).not.toHaveBeenCalled(); @@ -1049,18 +1057,18 @@ describe('#update', () => { const type = 'foo'; const username = Symbol(); const mockErrors = createMockErrors(); - const mockCheckPrivileges = jest.fn().mockImplementation(async () => ({ + const mockCheckPrivileges = jest.fn().mockImplementation(async privileges => ({ result: CHECK_PRIVILEGES_RESULT.UNAUTHORIZED, username, - missing: [ - 'action:saved_objects/foo/update' - ], + missing: privileges, })); const mockAuditLogger = createMockAuditLogger(); + const mockActions = createMockActions(); const client = new SecureSavedObjectsClient({ errors: mockErrors, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: mockActions, }); const id = Symbol(); const attributes = Symbol(); @@ -1068,13 +1076,13 @@ describe('#update', () => { await expect(client.update(type, id, attributes, options)).rejects.toThrowError(mockErrors.forbiddenError); - expect(mockCheckPrivileges).toHaveBeenCalledWith([`action:saved_objects/${type}/update`]); + expect(mockCheckPrivileges).toHaveBeenCalledWith([mockActions.getSavedObjectAction(type, 'update')]); expect(mockErrors.decorateForbiddenError).toHaveBeenCalledTimes(1); expect(mockAuditLogger.savedObjectsAuthorizationFailure).toHaveBeenCalledWith( username, 'update', [type], - [`action:saved_objects/${type}/update`], + [mockActions.getSavedObjectAction(type, 'update')], { type, id, @@ -1101,6 +1109,7 @@ describe('#update', () => { internalRepository: mockRepository, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: createMockActions(), }); const id = Symbol(); const attributes = Symbol(); @@ -1126,18 +1135,17 @@ describe('#update', () => { const mockRepository = { update: jest.fn().mockReturnValue(returnValue) }; - const mockCheckPrivileges = jest.fn().mockImplementation(async () => ({ + const mockCheckPrivileges = jest.fn().mockImplementation(async privileges => ({ result: CHECK_PRIVILEGES_RESULT.LEGACY, username, - missing: [ - 'action:saved_objects/foo/update' - ], + missing: privileges, })); const mockAuditLogger = createMockAuditLogger(); const client = new SecureSavedObjectsClient({ callWithRequestRepository: mockRepository, checkPrivileges: mockCheckPrivileges, auditLogger: mockAuditLogger, + actions: createMockActions(), }); const id = Symbol(); const attributes = Symbol(); diff --git a/x-pack/plugins/security/server/routes/api/v1/__tests__/authenticate.js b/x-pack/plugins/security/server/routes/api/v1/__tests__/authenticate.js index 0e4bcbf2f6a066..7604f4eff7b85d 100644 --- a/x-pack/plugins/security/server/routes/api/v1/__tests__/authenticate.js +++ b/x-pack/plugins/security/server/routes/api/v1/__tests__/authenticate.js @@ -15,6 +15,7 @@ import { AuthenticationResult } from '../../../../../server/lib/authentication/a import { BasicCredentials } from '../../../../../server/lib/authentication/providers/basic'; import { initAuthenticateApi } from '../authenticate'; import { DeauthenticationResult } from '../../../../lib/authentication/deauthentication_result'; +import { CHECK_PRIVILEGES_RESULT } from '../../../../lib/authorization'; describe('Authentication routes', () => { let serverStub; @@ -33,6 +34,7 @@ describe('Authentication routes', () => { let loginRoute; let request; let authenticateStub; + let checkPrivilegesWithRequestStub; beforeEach(() => { loginRoute = serverStub.route @@ -48,6 +50,7 @@ describe('Authentication routes', () => { authenticateStub = serverStub.plugins.security.authenticate.withArgs( sinon.match(BasicCredentials.decorateRequest({ headers: {} }, 'user', 'password')) ); + checkPrivilegesWithRequestStub = serverStub.plugins.security.authorization.checkPrivilegesWithRequest; }); it('correctly defines route.', async () => { @@ -124,18 +127,65 @@ describe('Authentication routes', () => { ); }); - it('returns user data if authentication succeed.', async () => { - const user = { username: 'user' }; - authenticateStub.returns( - Promise.resolve(AuthenticationResult.succeeded(user)) - ); + describe('authentication succeeds', () => { + const getDeprecationMessage = username => + `${username} relies on index privileges on the Kibana index. This is deprecated and will be removed in Kibana 7.0`; - await loginRoute.handler(request, replyStub); + it(`returns user data and doesn't log deprecation warning if checkPrivileges result is authorized.`, async () => { + const user = { username: 'user' }; + authenticateStub.returns( + Promise.resolve(AuthenticationResult.succeeded(user)) + ); + const checkPrivilegesStub = sinon.stub().returns({ result: CHECK_PRIVILEGES_RESULT.AUTHORIZED }); + checkPrivilegesWithRequestStub.returns(checkPrivilegesStub); - sinon.assert.notCalled(replyStub); - sinon.assert.calledOnce(replyStub.continue); - sinon.assert.calledWithExactly(replyStub.continue, { credentials: user }); + await loginRoute.handler(request, replyStub); + + sinon.assert.calledWithExactly(checkPrivilegesWithRequestStub, request); + sinon.assert.calledWithExactly(checkPrivilegesStub, [serverStub.plugins.security.authorization.actions.login]); + sinon.assert.neverCalledWith(serverStub.log, ['warning', 'deprecated', 'security'], getDeprecationMessage(user.username)); + sinon.assert.notCalled(replyStub); + sinon.assert.calledOnce(replyStub.continue); + sinon.assert.calledWithExactly(replyStub.continue, { credentials: user }); + }); + + it(`returns user data and logs deprecation warning if checkPrivileges result is legacy.`, async () => { + const user = { username: 'user' }; + authenticateStub.returns( + Promise.resolve(AuthenticationResult.succeeded(user)) + ); + const checkPrivilegesStub = sinon.stub().returns({ result: CHECK_PRIVILEGES_RESULT.LEGACY }); + checkPrivilegesWithRequestStub.returns(checkPrivilegesStub); + + await loginRoute.handler(request, replyStub); + + sinon.assert.calledWithExactly(checkPrivilegesWithRequestStub, request); + sinon.assert.calledWithExactly(checkPrivilegesStub, [serverStub.plugins.security.authorization.actions.login]); + sinon.assert.calledWith(serverStub.log, ['warning', 'deprecated', 'security'], getDeprecationMessage(user.username)); + sinon.assert.notCalled(replyStub); + sinon.assert.calledOnce(replyStub.continue); + sinon.assert.calledWithExactly(replyStub.continue, { credentials: user }); + }); + + it(`returns user data and doesn't log deprecation warning if checkPrivileges result is unauthorized.`, async () => { + const user = { username: 'user' }; + authenticateStub.returns( + Promise.resolve(AuthenticationResult.succeeded(user)) + ); + const checkPrivilegesStub = sinon.stub().returns({ result: CHECK_PRIVILEGES_RESULT.UNAUTHORIZED }); + checkPrivilegesWithRequestStub.returns(checkPrivilegesStub); + + await loginRoute.handler(request, replyStub); + + sinon.assert.calledWithExactly(checkPrivilegesWithRequestStub, request); + sinon.assert.calledWithExactly(checkPrivilegesStub, [serverStub.plugins.security.authorization.actions.login]); + sinon.assert.neverCalledWith(serverStub.log, ['warning', 'deprecated', 'security'], getDeprecationMessage(user.username)); + sinon.assert.notCalled(replyStub); + sinon.assert.calledOnce(replyStub.continue); + sinon.assert.calledWithExactly(replyStub.continue, { credentials: user }); + }); }); + }); describe('logout', () => { diff --git a/x-pack/plugins/security/server/routes/api/v1/authenticate.js b/x-pack/plugins/security/server/routes/api/v1/authenticate.js index 9697774bfe8919..4b5d847b724b21 100644 --- a/x-pack/plugins/security/server/routes/api/v1/authenticate.js +++ b/x-pack/plugins/security/server/routes/api/v1/authenticate.js @@ -9,8 +9,10 @@ import Joi from 'joi'; import { wrapError } from '../../../lib/errors'; import { BasicCredentials } from '../../../../server/lib/authentication/providers/basic'; import { canRedirectRequest } from '../../../lib/can_redirect_request'; +import { CHECK_PRIVILEGES_RESULT } from '../../../../server/lib/authorization'; export function initAuthenticateApi(server) { + server.route({ method: 'POST', path: '/api/security/v1/login', @@ -35,6 +37,14 @@ export function initAuthenticateApi(server) { return reply(Boom.unauthorized(authenticationResult.error)); } + const { authorization } = server.plugins.security; + const checkPrivileges = authorization.checkPrivilegesWithRequest(request); + const privilegeCheck = await checkPrivileges([authorization.actions.login]); + if (privilegeCheck.result === CHECK_PRIVILEGES_RESULT.LEGACY) { + const msg = `${username} relies on index privileges on the Kibana index. This is deprecated and will be removed in Kibana 7.0`; + server.log(['warning', 'deprecated', 'security'], msg); + } + return reply.continue({ credentials: authenticationResult.user }); } catch(err) { return reply(wrapError(err)); diff --git a/x-pack/plugins/security/server/routes/api/v1/privileges.js b/x-pack/plugins/security/server/routes/api/v1/privileges.js index e3c830bf176885..375f6879972701 100644 --- a/x-pack/plugins/security/server/routes/api/v1/privileges.js +++ b/x-pack/plugins/security/server/routes/api/v1/privileges.js @@ -4,11 +4,11 @@ * you may not use this file except in compliance with the Elastic License. */ -import { buildPrivilegeMap } from '../../../lib/privileges/privileges'; +import { buildPrivilegeMap } from '../../../lib/authorization'; export function initPrivilegesApi(server) { const config = server.config(); - const kibanaVersion = config.get('pkg.version'); + const { authorization } = server.plugins.security; const application = config.get('xpack.security.rbac.application'); const savedObjectTypes = server.savedObjects.types; @@ -20,7 +20,7 @@ export function initPrivilegesApi(server) { // in Elasticsearch because our current thinking is that we'll associate additional structure/metadata // with our view of them to allow users to more efficiently edit privileges for roles, and serialize it // into a different structure for enforcement within Elasticsearch - const privileges = buildPrivilegeMap(savedObjectTypes, application, kibanaVersion); + const privileges = buildPrivilegeMap(savedObjectTypes, application, authorization.actions); reply(Object.values(privileges)); } }); diff --git a/x-pack/test/rbac_api_integration/apis/saved_objects/bulk_get.js b/x-pack/test/rbac_api_integration/apis/saved_objects/bulk_get.js index 6089aec5d89d02..a89f2b23f8f72e 100644 --- a/x-pack/test/rbac_api_integration/apis/saved_objects/bulk_get.js +++ b/x-pack/test/rbac_api_integration/apis/saved_objects/bulk_get.js @@ -70,7 +70,7 @@ export default function ({ getService }) { const expectRbacForbidden = resp => { //eslint-disable-next-line max-len - const missingActions = `action:login,action:saved_objects/config/bulk_get,action:saved_objects/dashboard/bulk_get,action:saved_objects/visualization/bulk_get`; + const missingActions = `action:saved_objects/config/bulk_get,action:saved_objects/dashboard/bulk_get,action:saved_objects/visualization/bulk_get`; expect(resp.body).to.eql({ statusCode: 403, error: 'Forbidden', diff --git a/x-pack/test/rbac_api_integration/apis/saved_objects/create.js b/x-pack/test/rbac_api_integration/apis/saved_objects/create.js index cff5da3502838c..0a37bf5a47a382 100644 --- a/x-pack/test/rbac_api_integration/apis/saved_objects/create.js +++ b/x-pack/test/rbac_api_integration/apis/saved_objects/create.js @@ -29,11 +29,11 @@ export default function ({ getService }) { }); }; - const createExpectRbacForbidden = canLogin => resp => { + const expectRbacForbidden = resp => { expect(resp.body).to.eql({ statusCode: 403, error: 'Forbidden', - message: `Unable to create visualization, missing ${canLogin ? '' : 'action:login,'}action:saved_objects/visualization/create` + message: `Unable to create visualization, missing action:saved_objects/visualization/create` }); }; @@ -73,7 +73,7 @@ export default function ({ getService }) { tests: { default: { statusCode: 403, - response: createExpectRbacForbidden(false), + response: expectRbacForbidden, }, } }); @@ -138,7 +138,7 @@ export default function ({ getService }) { tests: { default: { statusCode: 403, - response: createExpectRbacForbidden(true), + response: expectRbacForbidden, }, } }); diff --git a/x-pack/test/rbac_api_integration/apis/saved_objects/delete.js b/x-pack/test/rbac_api_integration/apis/saved_objects/delete.js index b75ab5342c6baa..f1f693046f74be 100644 --- a/x-pack/test/rbac_api_integration/apis/saved_objects/delete.js +++ b/x-pack/test/rbac_api_integration/apis/saved_objects/delete.js @@ -25,11 +25,11 @@ export default function ({ getService }) { }); }; - const createExpectRbacForbidden = canLogin => resp => { + const expectRbacForbidden = resp => { expect(resp.body).to.eql({ statusCode: 403, error: 'Forbidden', - message: `Unable to delete dashboard, missing ${canLogin ? '' : 'action:login,'}action:saved_objects/dashboard/delete` + message: `Unable to delete dashboard, missing action:saved_objects/dashboard/delete` }); }; @@ -73,11 +73,11 @@ export default function ({ getService }) { tests: { actualId: { statusCode: 403, - response: createExpectRbacForbidden(false), + response: expectRbacForbidden, }, invalidId: { statusCode: 403, - response: createExpectRbacForbidden(false), + response: expectRbacForbidden, } } }); @@ -158,11 +158,11 @@ export default function ({ getService }) { tests: { actualId: { statusCode: 403, - response: createExpectRbacForbidden(true), + response: expectRbacForbidden, }, invalidId: { statusCode: 403, - response: createExpectRbacForbidden(true), + response: expectRbacForbidden, } } }); diff --git a/x-pack/test/rbac_api_integration/apis/saved_objects/find.js b/x-pack/test/rbac_api_integration/apis/saved_objects/find.js index 0498021e5daae2..26e43bba21cf05 100644 --- a/x-pack/test/rbac_api_integration/apis/saved_objects/find.js +++ b/x-pack/test/rbac_api_integration/apis/saved_objects/find.js @@ -122,11 +122,11 @@ export default function ({ getService }) { }); }; - const createExpectRbacForbidden = (canLogin, type) => resp => { + const createExpectRbacForbidden = (type) => resp => { expect(resp.body).to.eql({ statusCode: 403, error: 'Forbidden', - message: `Unable to find ${type}, missing ${canLogin ? '' : 'action:login,'}action:saved_objects/${type}/find` + message: `Unable to find ${type}, missing action:saved_objects/${type}/find` }); }; @@ -202,22 +202,22 @@ export default function ({ getService }) { normal: { description: 'forbidden login and find visualization message', statusCode: 403, - response: createExpectRbacForbidden(false, 'visualization'), + response: createExpectRbacForbidden('visualization'), }, unknownType: { description: 'forbidden login and find wigwags message', statusCode: 403, - response: createExpectRbacForbidden(false, 'wigwags'), + response: createExpectRbacForbidden('wigwags'), }, pageBeyondTotal: { description: 'forbidden login and find visualization message', statusCode: 403, - response: createExpectRbacForbidden(false, 'visualization'), + response: createExpectRbacForbidden('visualization'), }, unknownSearchField: { description: 'forbidden login and find wigwags message', statusCode: 403, - response: createExpectRbacForbidden(false, 'wigwags'), + response: createExpectRbacForbidden('wigwags'), }, noType: { description: `forbidded can't find any types`, @@ -377,7 +377,7 @@ export default function ({ getService }) { unknownType: { description: 'forbidden find wigwags message', statusCode: 403, - response: createExpectRbacForbidden(true, 'wigwags'), + response: createExpectRbacForbidden('wigwags'), }, pageBeyondTotal: { description: 'empty result', @@ -387,7 +387,7 @@ export default function ({ getService }) { unknownSearchField: { description: 'forbidden find wigwags message', statusCode: 403, - response: createExpectRbacForbidden(true, 'wigwags'), + response: createExpectRbacForbidden('wigwags'), }, noType: { description: 'all objects', diff --git a/x-pack/test/rbac_api_integration/apis/saved_objects/get.js b/x-pack/test/rbac_api_integration/apis/saved_objects/get.js index ac7cc6b70f50a8..23c3c0b5aaa35c 100644 --- a/x-pack/test/rbac_api_integration/apis/saved_objects/get.js +++ b/x-pack/test/rbac_api_integration/apis/saved_objects/get.js @@ -43,7 +43,7 @@ export default function ({ getService }) { expect(resp.body).to.eql({ statusCode: 403, error: 'Forbidden', - message: `Unable to get visualization, missing action:login,action:saved_objects/visualization/get` + message: `Unable to get visualization, missing action:saved_objects/visualization/get` }); }; diff --git a/x-pack/test/rbac_api_integration/apis/saved_objects/update.js b/x-pack/test/rbac_api_integration/apis/saved_objects/update.js index edcec1ffb61246..4b50600ba60c16 100644 --- a/x-pack/test/rbac_api_integration/apis/saved_objects/update.js +++ b/x-pack/test/rbac_api_integration/apis/saved_objects/update.js @@ -38,11 +38,11 @@ export default function ({ getService }) { }); }; - const createExpectRbacForbidden = canLogin => resp => { + const expectRbacForbidden = resp => { expect(resp.body).to.eql({ statusCode: 403, error: 'Forbidden', - message: `Unable to update visualization, missing ${canLogin ? '' : 'action:login,'}action:saved_objects/visualization/update` + message: `Unable to update visualization, missing action:saved_objects/visualization/update` }); }; @@ -97,11 +97,11 @@ export default function ({ getService }) { tests: { exists: { statusCode: 403, - response: createExpectRbacForbidden(false), + response: expectRbacForbidden, }, doesntExist: { statusCode: 403, - response: createExpectRbacForbidden(false), + response: expectRbacForbidden, }, } }); @@ -182,11 +182,11 @@ export default function ({ getService }) { tests: { exists: { statusCode: 403, - response: createExpectRbacForbidden(true), + response: expectRbacForbidden, }, doesntExist: { statusCode: 403, - response: createExpectRbacForbidden(true), + response: expectRbacForbidden, }, } });