From 1db42a4894edd978f6aec50bf13098f590f693c5 Mon Sep 17 00:00:00 2001 From: kobelb Date: Tue, 3 Jul 2018 09:51:07 -0400 Subject: [PATCH 01/13] Logging legacy fallback deprecation on login --- .../lib/authorization/check_privileges.js | 18 +- .../authorization/check_privileges.test.js | 229 +++++++++++++----- .../server/routes/api/v1/authenticate.js | 11 + .../apis/saved_objects/bulk_get.js | 2 +- .../apis/saved_objects/create.js | 8 +- .../apis/saved_objects/delete.js | 12 +- .../apis/saved_objects/find.js | 16 +- .../apis/saved_objects/get.js | 2 +- .../apis/saved_objects/update.js | 12 +- 9 files changed, 209 insertions(+), 101 deletions(-) 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 2ccc0f3e6e0ce..49e4af98c38a7 100644 --- a/x-pack/plugins/security/server/lib/authorization/check_privileges.js +++ b/x-pack/plugins/security/server/lib/authorization/check_privileges.js @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ +import { uniq } from 'lodash'; import { getClient } from '../../../../../server/lib/get_client_shield'; import { DEFAULT_RESOURCE } from '../../../common/constants'; import { getVersionAction, getLoginAction } from '../privileges'; @@ -38,19 +39,19 @@ export function checkPrivilegesWithRequestFactory(server) { } }); - const hasPrivileges = privilegeCheck.application[application][DEFAULT_RESOURCE]; + const privilegeCheckPrivileges = privilegeCheck.application[application][DEFAULT_RESOURCE]; // 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()]) { + if (!privilegeCheckPrivileges[versionAction] && privilegeCheckPrivileges[loginAction]) { throw new Error('Multiple versions of Kibana are running against the same Elasticsearch cluster, unable to authorize user.'); } return { username: privilegeCheck.username, hasAllRequested: privilegeCheck.has_all_requested, - privileges: hasPrivileges + privileges: privilegeCheckPrivileges }; }; @@ -68,15 +69,15 @@ export function checkPrivilegesWithRequestFactory(server) { }; return async function checkPrivileges(privileges) { - const allPrivileges = [versionAction, loginAction, ...privileges]; + const allPrivileges = uniq([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 + // we only return missing privileges that they're specifically checking for const missing = Object.keys(applicationPrivilegesCheck.privileges) - .filter(p => !applicationPrivilegesCheck.privileges[p]) - .filter(p => p !== versionAction); + .filter(privilege => privileges.includes(privilege)) + .filter(privilege => !applicationPrivilegesCheck.privileges[privilege]); if (applicationPrivilegesCheck.hasAllRequested) { return { @@ -87,9 +88,6 @@ export function checkPrivilegesWithRequestFactory(server) { } 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, 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 13299709577f0..33b9818bee369 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 @@ -76,14 +76,6 @@ const createMockCallWithRequest = (responses) => { return mockCallWithRequest; }; -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); -}; - test(`returns authorized if they have all application privileges`, async () => { const privilege = `action:saved_objects/${savedObjectTypes[0]}/get`; const username = 'foo-username'; @@ -107,7 +99,6 @@ test(`returns authorized if they have all application privileges`, async () => { const privileges = [privilege]; const result = await checkPrivileges(privileges); - expectNoDeprecationLogged(mockServer); expect(mockCallWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { body: { applications: [{ @@ -126,7 +117,7 @@ test(`returns authorized if they have all application privileges`, async () => { }); }); -test(`returns unauthorized they have only one application privilege`, async () => { +test(`returns unauthorized if they have only one application action`, async () => { const privilege1 = `action:saved_objects/${savedObjectTypes[0]}/get`; const privilege2 = `action:saved_objects/${savedObjectTypes[0]}/create`; const username = 'foo-username'; @@ -151,7 +142,6 @@ test(`returns unauthorized they have only one application privilege`, async () = const privileges = [privilege1, privilege2]; const result = await checkPrivileges(privileges); - expectNoDeprecationLogged(mockServer); expect(mockCallWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { body: { applications: [{ @@ -188,64 +178,174 @@ test(`throws error if missing version privilege and has login privilege`, async const checkPrivileges = checkPrivilegesWithRequest({}); await expect(checkPrivileges([privilege])).rejects.toThrowErrorMatchingSnapshot(); - expectNoDeprecationLogged(mockServer); }); 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, - }, + describe('they have no privileges on the kibana index', () => { + test(`returns unauthorized and missing application action if checking application action `, 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); + + expect(callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { + body: { + applications: [{ + application: defaultApplication, + resources: [DEFAULT_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, - }), - 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); - - expectNoDeprecationLogged(mockServer); - expect(callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { - body: { - applications: [{ - application: defaultApplication, - resources: [DEFAULT_RESOURCE], - privileges: [ - getVersionAction(defaultVersion), getLoginAction(), ...privileges - ] - }] - } + missing: [...privileges], + }); }); - expect(callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { - body: { - index: [{ - names: [ defaultKibanaIndex ], - privileges: ['create', 'delete', 'read', 'view_index_metadata'] - }] - } + + test(`returns unauthorized and missing login if checking login action`, async () => { + const privilege = getLoginAction(); + const username = 'foo-username'; + const mockServer = createMockServer(); + const callWithRequest = createMockCallWithRequest([ + mockApplicationPrivilegeResponse({ + hasAllRequested: false, + privileges: { + [getVersionAction(defaultVersion)]: false, + [getLoginAction()]: 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([privilege]); + + expect(callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { + body: { + applications: [{ + application: defaultApplication, + resources: [DEFAULT_RESOURCE], + privileges: [ + getVersionAction(defaultVersion), ...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: [...privileges], + }); }); - expect(result).toEqual({ - result: CHECK_PRIVILEGES_RESULT.UNAUTHORIZED, - username, - missing: [getLoginAction(), ...privileges], + + test(`returns unauthorized and missing version if checking version action`, async () => { + const privilege = getVersionAction(defaultVersion); + const username = 'foo-username'; + const mockServer = createMockServer(); + const callWithRequest = createMockCallWithRequest([ + mockApplicationPrivilegeResponse({ + hasAllRequested: false, + privileges: { + [getVersionAction(defaultVersion)]: false, + [getLoginAction()]: 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([privilege]); + + expect(callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { + body: { + applications: [{ + application: defaultApplication, + resources: [DEFAULT_RESOURCE], + privileges: [ + ...privileges, getLoginAction() + ] + }] + } + }); + 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: [...privileges], + }); }); }); @@ -281,7 +381,6 @@ describe('legacy fallback with no application privileges', () => { const privileges = [privilege]; const result = await checkPrivileges(privileges); - expectDeprecationLogged(mockServer); expect(callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { body: { applications: [{ @@ -304,7 +403,7 @@ describe('legacy fallback with no application privileges', () => { expect(result).toEqual({ result: CHECK_PRIVILEGES_RESULT.LEGACY, username, - missing: [getLoginAction(), ...privileges], + missing: [...privileges], }); }); }); 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 9697774bfe891..90b3284e5842e 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,13 @@ 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 { checkPrivilegesWithRequestFactory, CHECK_PRIVILEGES_RESULT } from '../../../../server/lib/authorization/check_privileges'; +import { getLoginAction } from '../../../../server/lib/privileges'; export function initAuthenticateApi(server) { + + const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(server); + server.route({ method: 'POST', path: '/api/security/v1/login', @@ -35,6 +40,12 @@ export function initAuthenticateApi(server) { return reply(Boom.unauthorized(authenticationResult.error)); } + const privilegeCheck = await checkPrivilegesWithRequest(request)([getLoginAction()]); + 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/test/rbac_api_integration/apis/saved_objects/bulk_get.js b/x-pack/test/rbac_api_integration/apis/saved_objects/bulk_get.js index 6089aec5d89d0..a89f2b23f8f72 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 cff5da3502838..0a37bf5a47a38 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 b75ab5342c6ba..f1f693046f74b 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 0498021e5daae..26e43bba21cf0 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 ac7cc6b70f50a..23c3c0b5aaa35 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 edcec1ffb6124..4b50600ba60c1 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, }, } }); From 27211ffa9e7654b4ed99c77d971fe16b7bbceff4 Mon Sep 17 00:00:00 2001 From: kobelb Date: Tue, 3 Jul 2018 10:14:51 -0400 Subject: [PATCH 02/13] Consolidation the privileges/authorization folder --- x-pack/plugins/security/index.js | 3 +-- .../security/server/lib/authorization/check_privileges.js | 2 +- .../security/server/lib/authorization/check_privileges.test.js | 2 +- .../lib/{privileges => authorization}/equivalent_privileges.js | 0 .../security/server/lib/{privileges => authorization}/index.js | 3 ++- .../server/lib/{privileges => authorization}/privileges.js | 0 .../register_privileges_with_cluster.js} | 0 .../register_privileges_with_cluster.test.js} | 2 +- x-pack/plugins/security/server/routes/api/v1/authenticate.js | 3 +-- x-pack/plugins/security/server/routes/api/v1/privileges.js | 2 +- 10 files changed, 8 insertions(+), 9 deletions(-) rename x-pack/plugins/security/server/lib/{privileges => authorization}/equivalent_privileges.js (100%) rename x-pack/plugins/security/server/lib/{privileges => authorization}/index.js (64%) rename x-pack/plugins/security/server/lib/{privileges => authorization}/privileges.js (100%) rename x-pack/plugins/security/server/lib/{privileges/privilege_action_registry.js => authorization/register_privileges_with_cluster.js} (100%) rename x-pack/plugins/security/server/lib/{privileges/privilege_action_registry.test.js => authorization/register_privileges_with_cluster.test.js} (99%) diff --git a/x-pack/plugins/security/index.js b/x-pack/plugins/security/index.js index 6bcaf4e7aec46..90a2aa17019f1 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 { checkPrivilegesWithRequestFactory, registerPrivilegesWithCluster } from './server/lib/authorization'; import { watchStatusAndLicenseToInitialize } from './server/lib/watch_status_and_license_to_initialize'; export const security = (kibana) => new kibana.Plugin({ 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 49e4af98c38a7..925e3fb98e972 100644 --- a/x-pack/plugins/security/server/lib/authorization/check_privileges.js +++ b/x-pack/plugins/security/server/lib/authorization/check_privileges.js @@ -7,7 +7,7 @@ import { uniq } from 'lodash'; import { getClient } from '../../../../../server/lib/get_client_shield'; import { DEFAULT_RESOURCE } from '../../../common/constants'; -import { getVersionAction, getLoginAction } from '../privileges'; +import { getVersionAction, getLoginAction } from './privileges'; export const CHECK_PRIVILEGES_RESULT = { UNAUTHORIZED: Symbol(), 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 33b9818bee369..9d41b5c4519f7 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 @@ -7,7 +7,7 @@ import { checkPrivilegesWithRequestFactory, CHECK_PRIVILEGES_RESULT } from './check_privileges'; import { getClient } from '../../../../../server/lib/get_client_shield'; import { DEFAULT_RESOURCE } from '../../../common/constants'; -import { getLoginAction, getVersionAction } from '../privileges'; +import { getLoginAction, getVersionAction } from './privileges'; jest.mock('../../../../../server/lib/get_client_shield', () => ({ getClient: jest.fn() diff --git a/x-pack/plugins/security/server/lib/privileges/equivalent_privileges.js b/x-pack/plugins/security/server/lib/authorization/equivalent_privileges.js similarity index 100% rename from x-pack/plugins/security/server/lib/privileges/equivalent_privileges.js rename to x-pack/plugins/security/server/lib/authorization/equivalent_privileges.js diff --git a/x-pack/plugins/security/server/lib/privileges/index.js b/x-pack/plugins/security/server/lib/authorization/index.js similarity index 64% rename from x-pack/plugins/security/server/lib/privileges/index.js rename to x-pack/plugins/security/server/lib/authorization/index.js index f888dffa922dd..8304ba8ccf400 100644 --- a/x-pack/plugins/security/server/lib/privileges/index.js +++ b/x-pack/plugins/security/server/lib/authorization/index.js @@ -4,5 +4,6 @@ * you may not use this file except in compliance with the Elastic License. */ -export { registerPrivilegesWithCluster } from './privilege_action_registry'; +export { checkPrivilegesWithRequestFactory, CHECK_PRIVILEGES_RESULT } from './check_privileges'; +export { registerPrivilegesWithCluster } from './register_privileges_with_cluster'; 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/authorization/privileges.js similarity index 100% rename from x-pack/plugins/security/server/lib/privileges/privileges.js rename to x-pack/plugins/security/server/lib/authorization/privileges.js 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 100% 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 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 99% 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 59b354fc24771..b47d42b10d980 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,7 +4,7 @@ * 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'; jest.mock('../../../../../server/lib/get_client_shield', () => ({ 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 90b3284e5842e..2885e0bb050ef 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,7 @@ 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 { checkPrivilegesWithRequestFactory, CHECK_PRIVILEGES_RESULT } from '../../../../server/lib/authorization/check_privileges'; -import { getLoginAction } from '../../../../server/lib/privileges'; +import { checkPrivilegesWithRequestFactory, CHECK_PRIVILEGES_RESULT, getLoginAction } from '../../../../server/lib/authorization'; export function initAuthenticateApi(server) { 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 e3c830bf17688..9c86b28144bb2 100644 --- a/x-pack/plugins/security/server/routes/api/v1/privileges.js +++ b/x-pack/plugins/security/server/routes/api/v1/privileges.js @@ -4,7 +4,7 @@ * 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(); From a1d65fa95f327a5aebce3a449fbfcc704ed2bcf5 Mon Sep 17 00:00:00 2001 From: kobelb Date: Tue, 3 Jul 2018 11:27:56 -0400 Subject: [PATCH 03/13] Exposing rudimentary authorization service and fixing authenticate tests --- x-pack/plugins/security/index.js | 4 ++ .../lib/__tests__/__fixtures__/server.js | 5 +- .../routes/api/v1/__tests__/authenticate.js | 68 ++++++++++++++++--- .../server/routes/api/v1/authenticate.js | 7 +- 4 files changed, 70 insertions(+), 14 deletions(-) diff --git a/x-pack/plugins/security/index.js b/x-pack/plugins/security/index.js index 90a2aa17019f1..dc3615490c270 100644 --- a/x-pack/plugins/security/index.js +++ b/x-pack/plugins/security/index.js @@ -114,6 +114,10 @@ export const security = (kibana) => new kibana.Plugin({ const auditLogger = new SecurityAuditLogger(server.config(), new AuditLogger(server, 'security')); const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(server); + server.expose('authorization', { + checkPrivilegesWithRequest, + }); + const { savedObjects } = server; savedObjects.setScopedSavedObjectsClientFactory(({ 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 acc211eb0647e..d092da118fbe5 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,10 @@ export function serverFixture() { security: { getUser: stub(), authenticate: stub(), - deauthenticate: stub() + deauthenticate: stub(), + authorization: { + checkPrivilegesWithRequest: stub() + } }, xpack_main: { 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 0e4bcbf2f6a06..adb7c84dc4ac3 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, getLoginAction } 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, [getLoginAction()]); + 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, [getLoginAction()]); + 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 drepcation 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, [getLoginAction()]); + 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 2885e0bb050ef..8ee9e6677bf61 100644 --- a/x-pack/plugins/security/server/routes/api/v1/authenticate.js +++ b/x-pack/plugins/security/server/routes/api/v1/authenticate.js @@ -9,12 +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 { checkPrivilegesWithRequestFactory, CHECK_PRIVILEGES_RESULT, getLoginAction } from '../../../../server/lib/authorization'; +import { CHECK_PRIVILEGES_RESULT, getLoginAction } from '../../../../server/lib/authorization'; export function initAuthenticateApi(server) { - const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(server); - server.route({ method: 'POST', path: '/api/security/v1/login', @@ -39,7 +37,8 @@ export function initAuthenticateApi(server) { return reply(Boom.unauthorized(authenticationResult.error)); } - const privilegeCheck = await checkPrivilegesWithRequest(request)([getLoginAction()]); + const checkPrivileges = server.plugins.security.authorization.checkPrivilegesWithRequest(request); + const privilegeCheck = await checkPrivileges([getLoginAction()]); 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); From 3f4a948d2652853b3a73c2f95c810d0fc6c6f220 Mon Sep 17 00:00:00 2001 From: kobelb Date: Tue, 3 Jul 2018 12:16:42 -0400 Subject: [PATCH 04/13] Moving authorization services configuration to initAuthorization --- x-pack/plugins/security/index.js | 11 +++----- .../server/lib/authorization/index.js | 3 ++- .../security/server/lib/authorization/init.js | 12 +++++++++ .../server/lib/authorization/init.test.js | 26 +++++++++++++++++++ 4 files changed, 43 insertions(+), 9 deletions(-) create mode 100644 x-pack/plugins/security/server/lib/authorization/init.js create mode 100644 x-pack/plugins/security/server/lib/authorization/init.test.js diff --git a/x-pack/plugins/security/index.js b/x-pack/plugins/security/index.js index dc3615490c270..f0495e2974ade 100644 --- a/x-pack/plugins/security/index.js +++ b/x-pack/plugins/security/index.js @@ -20,7 +20,7 @@ import { initPrivilegesApi } from './server/routes/api/v1/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 { checkPrivilegesWithRequestFactory, registerPrivilegesWithCluster } from './server/lib/authorization'; +import { initAuthorization, registerPrivilegesWithCluster } from './server/lib/authorization'; import { watchStatusAndLicenseToInitialize } from './server/lib/watch_status_and_license_to_initialize'; export const security = (kibana) => new kibana.Plugin({ @@ -113,13 +113,8 @@ 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); - server.expose('authorization', { - checkPrivilegesWithRequest, - }); - + initAuthorization(server); const { savedObjects } = server; - savedObjects.setScopedSavedObjectsClientFactory(({ request, }) => { @@ -133,7 +128,7 @@ export const security = (kibana) => new kibana.Plugin({ return new savedObjects.SavedObjectsClient(callWithRequestRepository); } - const checkPrivileges = checkPrivilegesWithRequest(request); + const checkPrivileges = server.plugins.security.authorization.checkPrivilegesWithRequest(request); const internalRepository = savedObjects.getSavedObjectsRepository(callWithInternalUser); return new SecureSavedObjectsClient({ diff --git a/x-pack/plugins/security/server/lib/authorization/index.js b/x-pack/plugins/security/server/lib/authorization/index.js index 8304ba8ccf400..3c71cbf0feafb 100644 --- a/x-pack/plugins/security/server/lib/authorization/index.js +++ b/x-pack/plugins/security/server/lib/authorization/index.js @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -export { checkPrivilegesWithRequestFactory, CHECK_PRIVILEGES_RESULT } from './check_privileges'; +export { CHECK_PRIVILEGES_RESULT } from './check_privileges'; export { registerPrivilegesWithCluster } from './register_privileges_with_cluster'; export { buildPrivilegeMap, getLoginAction, getVersionAction } from './privileges'; +export { initAuthorization } 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 0000000000000..6d49da4e412e8 --- /dev/null +++ b/x-pack/plugins/security/server/lib/authorization/init.js @@ -0,0 +1,12 @@ +/* + * 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 { checkPrivilegesWithRequestFactory } from './check_privileges'; + +export function initAuthorization(server) { + server.expose('authorization', { + checkPrivilegesWithRequest: checkPrivilegesWithRequestFactory(server), + }); +} 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 0000000000000..043b314e77d65 --- /dev/null +++ b/x-pack/plugins/security/server/lib/authorization/init.test.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 { initAuthorization } from './init'; +import { checkPrivilegesWithRequestFactory } from './check_privileges'; + +jest.mock('./check_privileges', () => ({ + checkPrivilegesWithRequestFactory: jest.fn(), +})); + +test(`calls server.expose with exposed services`, () => { + const mockServer = { + expose: jest.fn(), + }; + const checkPrivilegesWithRequest = Symbol(); + checkPrivilegesWithRequestFactory.mockReturnValue(checkPrivilegesWithRequest); + + initAuthorization(mockServer); + + expect(mockServer.expose).toHaveBeenCalledWith('authorization', { + checkPrivilegesWithRequest + }); +}); From 9723082600a132b161cb65e30def18dfade50f5a Mon Sep 17 00:00:00 2001 From: kobelb Date: Thu, 5 Jul 2018 12:19:06 -0400 Subject: [PATCH 05/13] Adding "actions" service exposed by the authorization --- x-pack/plugins/security/index.js | 7 +- .../lib/__tests__/__fixtures__/server.js | 7 +- .../server/lib/authorization/actions.js | 18 ++ .../server/lib/authorization/actions.test.js | 51 +++ .../lib/authorization/check_privileges.js | 17 +- .../authorization/check_privileges.test.js | 164 +++++----- .../security/server/lib/authorization/init.js | 11 +- .../server/lib/authorization/init.test.js | 26 +- .../server/lib/authorization/privileges.js | 31 +- .../register_privileges_with_cluster.js | 6 +- .../register_privileges_with_cluster.test.js | 17 +- .../secure_saved_objects_client.js | 18 +- .../secure_saved_objects_client.test.js | 296 +++++++++--------- .../routes/api/v1/__tests__/authenticate.js | 8 +- .../server/routes/api/v1/authenticate.js | 7 +- .../server/routes/api/v1/privileges.js | 4 +- 16 files changed, 396 insertions(+), 292 deletions(-) create mode 100644 x-pack/plugins/security/server/lib/authorization/actions.js create mode 100644 x-pack/plugins/security/server/lib/authorization/actions.test.js diff --git a/x-pack/plugins/security/index.js b/x-pack/plugins/security/index.js index f0495e2974ade..8a3be468427a1 100644 --- a/x-pack/plugins/security/index.js +++ b/x-pack/plugins/security/index.js @@ -113,7 +113,10 @@ export const security = (kibana) => new kibana.Plugin({ server.auth.strategy('session', 'login', 'required'); const auditLogger = new SecurityAuditLogger(server.config(), new AuditLogger(server, 'security')); + + // exposes server.plugins.security.authorization initAuthorization(server); + const { savedObjects } = server; savedObjects.setScopedSavedObjectsClientFactory(({ request, @@ -128,7 +131,8 @@ export const security = (kibana) => new kibana.Plugin({ return new savedObjects.SavedObjectsClient(callWithRequestRepository); } - const checkPrivileges = server.plugins.security.authorization.checkPrivilegesWithRequest(request); + const { authorization } = server.plugins.security; + const checkPrivileges = authorization.checkPrivilegesWithRequest(request); const internalRepository = savedObjects.getSavedObjectsRepository(callWithInternalUser); return new SecureSavedObjectsClient({ @@ -138,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 d092da118fbe5..78a015bc14105 100644 --- a/x-pack/plugins/security/server/lib/__tests__/__fixtures__/server.js +++ b/x-pack/plugins/security/server/lib/__tests__/__fixtures__/server.js @@ -37,8 +37,11 @@ export function serverFixture() { authenticate: stub(), deauthenticate: stub(), authorization: { - checkPrivilegesWithRequest: stub() - } + checkPrivilegesWithRequest: stub(), + actions: { + login: 'stub-login-action', + }, + }, }, xpack_main: { 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 0000000000000..6851f57719eb4 --- /dev/null +++ b/x-pack/plugins/security/server/lib/authorization/actions.js @@ -0,0 +1,18 @@ +/* + * 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 actionsFactory(config) { + const kibanaVersion = config.get('pkg.version'); + + return { + getSavedObjectAction(type, action) { + 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 0000000000000..b6aa7d2f2113e --- /dev/null +++ b/x-pack/plugins/security/server/lib/authorization/actions.test.js @@ -0,0 +1,51 @@ +/* + * 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}`); + }); +}); 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 925e3fb98e972..5d33de44dfb7b 100644 --- a/x-pack/plugins/security/server/lib/authorization/check_privileges.js +++ b/x-pack/plugins/security/server/lib/authorization/check_privileges.js @@ -5,9 +5,7 @@ */ import { uniq } from 'lodash'; -import { getClient } from '../../../../../server/lib/get_client_shield'; import { DEFAULT_RESOURCE } from '../../../common/constants'; -import { getVersionAction, getLoginAction } from './privileges'; export const CHECK_PRIVILEGES_RESULT = { UNAUTHORIZED: Symbol(), @@ -15,17 +13,12 @@ export const CHECK_PRIVILEGES_RESULT = { LEGACY: Symbol(), }; -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); - return function checkPrivilegesWithRequest(request) { const checkApplicationPrivileges = async (privileges) => { @@ -44,7 +37,7 @@ export function checkPrivilegesWithRequestFactory(server) { // 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 (!privilegeCheckPrivileges[versionAction] && privilegeCheckPrivileges[loginAction]) { + if (!privilegeCheckPrivileges[actions.version] && privilegeCheckPrivileges[actions.login]) { throw new Error('Multiple versions of Kibana are running against the same Elasticsearch cluster, unable to authorize user.'); } @@ -69,7 +62,7 @@ export function checkPrivilegesWithRequestFactory(server) { }; return async function checkPrivileges(privileges) { - const allPrivileges = uniq([versionAction, loginAction, ...privileges]); + const allPrivileges = uniq([actions.version, actions.login, ...privileges]); const applicationPrivilegesCheck = await checkApplicationPrivileges(allPrivileges); const username = applicationPrivilegesCheck.username; @@ -87,7 +80,7 @@ export function checkPrivilegesWithRequestFactory(server) { }; } - if (!applicationPrivilegesCheck.privileges[loginAction] && await hasPrivilegesOnKibanaIndex()) { + if (!applicationPrivilegesCheck.privileges[actions.login] && await hasPrivilegesOnKibanaIndex()) { return { result: CHECK_PRIVILEGES_RESULT.LEGACY, username, 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 9d41b5c4519f7..a8ee476fb6efd 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 @@ -5,25 +5,23 @@ */ import { checkPrivilegesWithRequestFactory, CHECK_PRIVILEGES_RESULT } from './check_privileges'; -import { getClient } from '../../../../../server/lib/get_client_shield'; import { DEFAULT_RESOURCE } from '../../../common/constants'; -import { getLoginAction, getVersionAction } from './privileges'; - -jest.mock('../../../../../server/lib/get_client_shield', () => ({ - getClient: jest.fn() -})); 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 createMockActions = () => { + return { + login: 'mock-action:login', + version: 'mock-action:version', + }; +}; + +const createMockConfig = ({ settings = {} } = {}) => { + const mockConfig = { + get: jest.fn() }; const defaultSettings = { @@ -32,15 +30,12 @@ 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 = '' }) =>{ @@ -63,29 +58,29 @@ const mockKibanaIndexPrivilegesResponse = ({ privileges, index = defaultKibanaIn }; }; -const createMockCallWithRequest = (responses) => { +const createMockShieldClient = (responses) => { const mockCallWithRequest = jest.fn(); - getClient.mockReturnValue({ - callWithRequest: mockCallWithRequest - }); for (const response of responses) { mockCallWithRequest.mockImplementationOnce(async () => response); } - return mockCallWithRequest; + return { + callWithRequest: mockCallWithRequest, + }; }; 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([ + const mockConfig = createMockConfig(); + const mockActions = createMockActions(); + const mockShieldClient = createMockShieldClient([ mockApplicationPrivilegeResponse({ hasAllRequested: true, privileges: { - [getVersionAction(defaultVersion)]: true, - [getLoginAction()]: true, + [mockActions.version]: true, + [mockActions.login]: true, [privilege]: true, }, application: defaultApplication, @@ -93,19 +88,19 @@ test(`returns authorized if they have all application privileges`, async () => { }) ]); - const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(mockServer); + const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(mockShieldClient, mockConfig, mockActions); const request = Symbol(); const checkPrivileges = checkPrivilegesWithRequest(request); const privileges = [privilege]; const result = await checkPrivileges(privileges); - expect(mockCallWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { + expect(mockShieldClient.callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { body: { applications: [{ application: defaultApplication, resources: [DEFAULT_RESOURCE], privileges: [ - getVersionAction(defaultVersion), getLoginAction(), ...privileges + mockActions.version, mockActions.login, ...privileges ] }] } @@ -121,13 +116,14 @@ test(`returns unauthorized if they have only one application action`, 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([ + const mockConfig = createMockConfig(); + const mockActions = createMockActions(); + const mockShieldClient = createMockShieldClient([ mockApplicationPrivilegeResponse({ hasAllRequested: false, privileges: { - [getVersionAction(defaultVersion)]: true, - [getLoginAction()]: true, + [mockActions.version]: true, + [mockActions.login]: true, [privilege1]: true, [privilege2]: false, }, @@ -136,19 +132,19 @@ test(`returns unauthorized if they have only one application action`, async () = }) ]); - const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(mockServer); + const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(mockShieldClient, mockConfig, mockActions); const request = Symbol(); const checkPrivileges = checkPrivilegesWithRequest(request); const privileges = [privilege1, privilege2]; const result = await checkPrivileges(privileges); - expect(mockCallWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { + expect(mockShieldClient.callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { body: { applications: [{ application: defaultApplication, resources: [DEFAULT_RESOURCE], privileges: [ - getVersionAction(defaultVersion), getLoginAction(), ...privileges + mockActions.version, mockActions.login, ...privileges ] }] } @@ -162,19 +158,20 @@ test(`returns unauthorized if they have only one application action`, async () = test(`throws error if missing version privilege and has login privilege`, async () => { const privilege = `action:saved_objects/${savedObjectTypes[0]}/get`; - const mockServer = createMockServer(); - createMockCallWithRequest([ + const mockConfig = createMockConfig(); + const mockActions = createMockActions(); + const mockShieldClient = createMockShieldClient([ mockApplicationPrivilegeResponse({ hasAllRequested: false, privileges: { - [getVersionAction(defaultVersion)]: false, - [getLoginAction()]: true, + [mockActions.version]: false, + [mockActions.login]: true, [privilege]: true, } }) ]); - const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(mockServer); + const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(mockShieldClient, mockConfig, mockActions); const checkPrivileges = checkPrivilegesWithRequest({}); await expect(checkPrivileges([privilege])).rejects.toThrowErrorMatchingSnapshot(); @@ -182,16 +179,17 @@ test(`throws error if missing version privilege and has login privilege`, async describe('legacy fallback with no application privileges', () => { describe('they have no privileges on the kibana index', () => { - test(`returns unauthorized and missing application action if checking application action `, async () => { + test(`returns unauthorized and missing application action when checking application action `, async () => { const privilege = `action:saved_objects/${savedObjectTypes[0]}/get`; const username = 'foo-username'; - const mockServer = createMockServer(); - const callWithRequest = createMockCallWithRequest([ + const mockConfig = createMockConfig(); + const mockActions = createMockActions(); + const mockShieldClient = createMockShieldClient([ mockApplicationPrivilegeResponse({ hasAllRequested: false, privileges: { - [getVersionAction(defaultVersion)]: false, - [getLoginAction()]: false, + [mockActions.version]: false, + [mockActions.login]: false, [privilege]: false, }, username, @@ -206,24 +204,24 @@ describe('legacy fallback with no application privileges', () => { }) ]); - const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(mockServer); + const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(mockShieldClient, mockConfig, mockActions); const request = Symbol(); const checkPrivileges = checkPrivilegesWithRequest(request); const privileges = [privilege]; const result = await checkPrivileges(privileges); - expect(callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { + expect(mockShieldClient.callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { body: { applications: [{ application: defaultApplication, resources: [DEFAULT_RESOURCE], privileges: [ - getVersionAction(defaultVersion), getLoginAction(), ...privileges + mockActions.version, mockActions.login, ...privileges ] }] } }); - expect(callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { + expect(mockShieldClient.callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { body: { index: [{ names: [ defaultKibanaIndex ], @@ -238,16 +236,16 @@ describe('legacy fallback with no application privileges', () => { }); }); - test(`returns unauthorized and missing login if checking login action`, async () => { - const privilege = getLoginAction(); + test(`returns unauthorized and missing login when checking login action`, async () => { const username = 'foo-username'; - const mockServer = createMockServer(); - const callWithRequest = createMockCallWithRequest([ + const mockConfig = createMockConfig(); + const mockActions = createMockActions(); + const mockShieldClient = createMockShieldClient([ mockApplicationPrivilegeResponse({ hasAllRequested: false, privileges: { - [getVersionAction(defaultVersion)]: false, - [getLoginAction()]: false, + [mockActions.version]: false, + [mockActions.login]: false, }, username, }), @@ -261,24 +259,23 @@ describe('legacy fallback with no application privileges', () => { }) ]); - const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(mockServer); + const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(mockShieldClient, mockConfig, mockActions); const request = Symbol(); const checkPrivileges = checkPrivilegesWithRequest(request); - const privileges = [privilege]; - const result = await checkPrivileges([privilege]); + const result = await checkPrivileges([mockActions.login]); - expect(callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { + expect(mockShieldClient.callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { body: { applications: [{ application: defaultApplication, resources: [DEFAULT_RESOURCE], privileges: [ - getVersionAction(defaultVersion), ...privileges + mockActions.version, mockActions.login ] }] } }); - expect(callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { + expect(mockShieldClient.callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { body: { index: [{ names: [ defaultKibanaIndex ], @@ -289,20 +286,20 @@ describe('legacy fallback with no application privileges', () => { expect(result).toEqual({ result: CHECK_PRIVILEGES_RESULT.UNAUTHORIZED, username, - missing: [...privileges], + missing: [mockActions.login], }); }); test(`returns unauthorized and missing version if checking version action`, async () => { - const privilege = getVersionAction(defaultVersion); const username = 'foo-username'; - const mockServer = createMockServer(); - const callWithRequest = createMockCallWithRequest([ + const mockConfig = createMockConfig(); + const mockActions = createMockActions(); + const mockShieldClient = createMockShieldClient([ mockApplicationPrivilegeResponse({ hasAllRequested: false, privileges: { - [getVersionAction(defaultVersion)]: false, - [getLoginAction()]: false, + [mockActions.version]: false, + [mockActions.login]: false, }, username, }), @@ -316,24 +313,24 @@ describe('legacy fallback with no application privileges', () => { }) ]); - const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(mockServer); + const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(mockShieldClient, mockConfig, mockActions); const request = Symbol(); const checkPrivileges = checkPrivilegesWithRequest(request); - const privileges = [privilege]; - const result = await checkPrivileges([privilege]); - expect(callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { + const result = await checkPrivileges([mockActions.version]); + + expect(mockShieldClient.callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { body: { applications: [{ application: defaultApplication, resources: [DEFAULT_RESOURCE], privileges: [ - ...privileges, getLoginAction() + mockActions.version, mockActions.login ] }] } }); - expect(callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { + expect(mockShieldClient.callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { body: { index: [{ names: [ defaultKibanaIndex ], @@ -344,7 +341,7 @@ describe('legacy fallback with no application privileges', () => { expect(result).toEqual({ result: CHECK_PRIVILEGES_RESULT.UNAUTHORIZED, username, - missing: [...privileges], + missing: [mockActions.version], }); }); }); @@ -353,13 +350,14 @@ describe('legacy fallback with no application privileges', () => { 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([ + const mockConfig = createMockConfig(); + const mockActions = createMockActions(); + const mockShieldClient = createMockShieldClient([ mockApplicationPrivilegeResponse({ hasAllRequested: false, privileges: { - [getVersionAction(defaultVersion)]: false, - [getLoginAction()]: false, + [mockActions.version]: false, + [mockActions.login]: false, [privilege]: false, }, username, @@ -375,24 +373,24 @@ describe('legacy fallback with no application privileges', () => { }) ]); - const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(mockServer); + const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(mockShieldClient, mockConfig, mockActions); const request = Symbol(); const checkPrivileges = checkPrivilegesWithRequest(request); const privileges = [privilege]; const result = await checkPrivileges(privileges); - expect(callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { + expect(mockShieldClient.callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { body: { applications: [{ application: defaultApplication, resources: [DEFAULT_RESOURCE], privileges: [ - getVersionAction(defaultVersion), getLoginAction(), ...privileges + mockActions.version, mockActions.login, ...privileges ] }] } }); - expect(callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { + expect(mockShieldClient.callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { body: { index: [{ names: [ defaultKibanaIndex ], diff --git a/x-pack/plugins/security/server/lib/authorization/init.js b/x-pack/plugins/security/server/lib/authorization/init.js index 6d49da4e412e8..61746550f6e45 100644 --- a/x-pack/plugins/security/server/lib/authorization/init.js +++ b/x-pack/plugins/security/server/lib/authorization/init.js @@ -3,10 +3,19 @@ * 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 { getClient } from '../../../../../server/lib/get_client_shield'; export function initAuthorization(server) { + const shieldClient = getClient(server); + const config = server.config(); + + const actions = actionsFactory(config); + server.expose('authorization', { - checkPrivilegesWithRequest: checkPrivilegesWithRequestFactory(server), + 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 index 043b314e77d65..37e90273f7727 100644 --- a/x-pack/plugins/security/server/lib/authorization/init.test.js +++ b/x-pack/plugins/security/server/lib/authorization/init.test.js @@ -5,22 +5,42 @@ */ import { initAuthorization } 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 checkPrivilegesWithRequest = Symbol(); - checkPrivilegesWithRequestFactory.mockReturnValue(checkPrivilegesWithRequest); + const mockShieldClient = Symbol(); + getClient.mockReturnValue(mockShieldClient); + const mockCheckPrivilegesWithRequest = Symbol(); + checkPrivilegesWithRequestFactory.mockReturnValue(mockCheckPrivilegesWithRequest); + const mockActions = Symbol(); + actionsFactory.mockReturnValue(mockActions); initAuthorization(mockServer); + expect(getClient).toHaveBeenCalledWith(mockServer); + expect(actionsFactory).toHaveBeenCalledWith(mockConfig); + expect(checkPrivilegesWithRequestFactory).toHaveBeenCalledWith(mockShieldClient, mockConfig, mockActions); expect(mockServer.expose).toHaveBeenCalledWith('authorization', { - checkPrivilegesWithRequest + checkPrivilegesWithRequest: mockCheckPrivilegesWithRequest, + actions: mockActions, }); }); diff --git a/x-pack/plugins/security/server/lib/authorization/privileges.js b/x-pack/plugins/security/server/lib/authorization/privileges.js index ea22f6e276594..251b47245e3e9 100644 --- a/x-pack/plugins/security/server/lib/authorization/privileges.js +++ b/x-pack/plugins/security/server/lib/authorization/privileges.js @@ -4,40 +4,25 @@ * 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); +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], []); + }; return { all: { application, name: 'all', - actions: [getVersionAction(kibanaVersion), 'action:*'], + actions: [actions.version, 'action:*'], metadata: {} }, read: { application, name: 'read', - actions: [getVersionAction(kibanaVersion), getLoginAction(), ...readSavedObjectsActions], + actions: [actions.version, actions.login, ...buildSavedObjectsActions(['get', 'bulk_get', 'find'])], 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/authorization/register_privileges_with_cluster.js b/x-pack/plugins/security/server/lib/authorization/register_privileges_with_cluster.js index 3112265a9b928..06636e966b6f7 100644 --- a/x-pack/plugins/security/server/lib/authorization/register_privileges_with_cluster.js +++ b/x-pack/plugins/security/server/lib/authorization/register_privileges_with_cluster.js @@ -7,15 +7,17 @@ import { buildPrivilegeMap } from './privileges'; import { getClient } from '../../../../../server/lib/get_client_shield'; import { equivalentPrivileges } from './equivalent_privileges'; +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; 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/authorization/register_privileges_with_cluster.test.js b/x-pack/plugins/security/server/lib/authorization/register_privileges_with_cluster.test.js index b47d42b10d980..b016859720520 100644 --- a/x-pack/plugins/security/server/lib/authorization/register_privileges_with_cluster.test.js +++ b/x-pack/plugins/security/server/lib/authorization/register_privileges_with_cluster.test.js @@ -7,16 +7,21 @@ 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, @@ -136,6 +141,7 @@ const registerPrivilegesWithClusterTest = (description, { } }); + actionsFactory.mockReturnValue(actions); buildPrivilegeMap.mockReturnValue(expectedPrivileges); let error; @@ -156,7 +162,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', @@ -165,8 +171,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/saved_objects_client/secure_saved_objects_client.js b/x-pack/plugins/security/server/lib/saved_objects_client/secure_saved_objects_client.js index 419a379f7d023..eb19f59c29693 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 5e1b1f7116524..99656772c0df7 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 adb7c84dc4ac3..f48206037cb10 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,7 +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, getLoginAction } from '../../../../lib/authorization'; +import { CHECK_PRIVILEGES_RESULT } from '../../../../lib/authorization'; describe('Authentication routes', () => { let serverStub; @@ -142,7 +142,7 @@ describe('Authentication routes', () => { await loginRoute.handler(request, replyStub); sinon.assert.calledWithExactly(checkPrivilegesWithRequestStub, request); - sinon.assert.calledWithExactly(checkPrivilegesStub, [getLoginAction()]); + 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); @@ -160,7 +160,7 @@ describe('Authentication routes', () => { await loginRoute.handler(request, replyStub); sinon.assert.calledWithExactly(checkPrivilegesWithRequestStub, request); - sinon.assert.calledWithExactly(checkPrivilegesStub, [getLoginAction()]); + 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); @@ -178,7 +178,7 @@ describe('Authentication routes', () => { await loginRoute.handler(request, replyStub); sinon.assert.calledWithExactly(checkPrivilegesWithRequestStub, request); - sinon.assert.calledWithExactly(checkPrivilegesStub, [getLoginAction()]); + 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); 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 8ee9e6677bf61..4b5d847b724b2 100644 --- a/x-pack/plugins/security/server/routes/api/v1/authenticate.js +++ b/x-pack/plugins/security/server/routes/api/v1/authenticate.js @@ -9,7 +9,7 @@ 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, getLoginAction } from '../../../../server/lib/authorization'; +import { CHECK_PRIVILEGES_RESULT } from '../../../../server/lib/authorization'; export function initAuthenticateApi(server) { @@ -37,8 +37,9 @@ export function initAuthenticateApi(server) { return reply(Boom.unauthorized(authenticationResult.error)); } - const checkPrivileges = server.plugins.security.authorization.checkPrivilegesWithRequest(request); - const privilegeCheck = await checkPrivileges([getLoginAction()]); + 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); 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 9c86b28144bb2..375f687997270 100644 --- a/x-pack/plugins/security/server/routes/api/v1/privileges.js +++ b/x-pack/plugins/security/server/routes/api/v1/privileges.js @@ -8,7 +8,7 @@ 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)); } }); From 0e63c7206deaa9fb218b1a821c7d32951d8144bf Mon Sep 17 00:00:00 2001 From: kobelb Date: Thu, 5 Jul 2018 13:00:46 -0400 Subject: [PATCH 06/13] Fixing misspelling --- .../security/server/routes/api/v1/__tests__/authenticate.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 f48206037cb10..7604f4eff7b85 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 @@ -167,7 +167,7 @@ describe('Authentication routes', () => { sinon.assert.calledWithExactly(replyStub.continue, { credentials: user }); }); - it(`returns user data and doesn't log drepcation warning if checkPrivileges result is unauthorized.`, async () => { + 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)) From d9732c0477a5393bca6b2ca92fc7fc422b1ede4b Mon Sep 17 00:00:00 2001 From: kobelb Date: Thu, 5 Jul 2018 15:21:49 -0400 Subject: [PATCH 07/13] Removing invalid and unused exports --- x-pack/plugins/security/server/lib/authorization/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/security/server/lib/authorization/index.js b/x-pack/plugins/security/server/lib/authorization/index.js index 3c71cbf0feafb..1f1a2038aeee9 100644 --- a/x-pack/plugins/security/server/lib/authorization/index.js +++ b/x-pack/plugins/security/server/lib/authorization/index.js @@ -6,5 +6,5 @@ export { CHECK_PRIVILEGES_RESULT } from './check_privileges'; export { registerPrivilegesWithCluster } from './register_privileges_with_cluster'; -export { buildPrivilegeMap, getLoginAction, getVersionAction } from './privileges'; +export { buildPrivilegeMap } from './privileges'; export { initAuthorization } from './init'; From 84680185ad5447d4d4faf492d1b5ba3fa96eea47 Mon Sep 17 00:00:00 2001 From: kobelb Date: Thu, 5 Jul 2018 15:27:07 -0400 Subject: [PATCH 08/13] Adding note about only adding privileges --- x-pack/plugins/security/server/lib/authorization/privileges.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x-pack/plugins/security/server/lib/authorization/privileges.js b/x-pack/plugins/security/server/lib/authorization/privileges.js index 251b47245e3e9..9ded71662ed51 100644 --- a/x-pack/plugins/security/server/lib/authorization/privileges.js +++ b/x-pack/plugins/security/server/lib/authorization/privileges.js @@ -11,6 +11,8 @@ export function buildPrivilegeMap(savedObjectTypes, application, actions) { .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, From 7a5eeb15d8e3de0d5ed87f942eeedca66080618f Mon Sep 17 00:00:00 2001 From: kobelb Date: Thu, 5 Jul 2018 16:49:09 -0400 Subject: [PATCH 09/13] Calling it initAuthorizationService --- x-pack/plugins/security/index.js | 4 ++-- x-pack/plugins/security/server/lib/authorization/index.js | 2 +- x-pack/plugins/security/server/lib/authorization/init.js | 2 +- x-pack/plugins/security/server/lib/authorization/init.test.js | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/security/index.js b/x-pack/plugins/security/index.js index 8a3be468427a1..0e521589ac141 100644 --- a/x-pack/plugins/security/index.js +++ b/x-pack/plugins/security/index.js @@ -20,7 +20,7 @@ import { initPrivilegesApi } from './server/routes/api/v1/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 { initAuthorization, registerPrivilegesWithCluster } from './server/lib/authorization'; +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({ @@ -115,7 +115,7 @@ export const security = (kibana) => new kibana.Plugin({ const auditLogger = new SecurityAuditLogger(server.config(), new AuditLogger(server, 'security')); // exposes server.plugins.security.authorization - initAuthorization(server); + initAuthorizationService(server); const { savedObjects } = server; savedObjects.setScopedSavedObjectsClientFactory(({ diff --git a/x-pack/plugins/security/server/lib/authorization/index.js b/x-pack/plugins/security/server/lib/authorization/index.js index 1f1a2038aeee9..e0029a3caeafd 100644 --- a/x-pack/plugins/security/server/lib/authorization/index.js +++ b/x-pack/plugins/security/server/lib/authorization/index.js @@ -7,4 +7,4 @@ export { CHECK_PRIVILEGES_RESULT } from './check_privileges'; export { registerPrivilegesWithCluster } from './register_privileges_with_cluster'; export { buildPrivilegeMap } from './privileges'; -export { initAuthorization } from './init'; +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 index 61746550f6e45..be1ab2fc7348a 100644 --- a/x-pack/plugins/security/server/lib/authorization/init.js +++ b/x-pack/plugins/security/server/lib/authorization/init.js @@ -8,7 +8,7 @@ import { actionsFactory } from './actions'; import { checkPrivilegesWithRequestFactory } from './check_privileges'; import { getClient } from '../../../../../server/lib/get_client_shield'; -export function initAuthorization(server) { +export function initAuthorizationService(server) { const shieldClient = getClient(server); const config = server.config(); diff --git a/x-pack/plugins/security/server/lib/authorization/init.test.js b/x-pack/plugins/security/server/lib/authorization/init.test.js index 37e90273f7727..5e96d224bcb28 100644 --- a/x-pack/plugins/security/server/lib/authorization/init.test.js +++ b/x-pack/plugins/security/server/lib/authorization/init.test.js @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { initAuthorization } from './init'; +import { initAuthorizationService } from './init'; import { actionsFactory } from './actions'; import { checkPrivilegesWithRequestFactory } from './check_privileges'; import { getClient } from '../../../../../server/lib/get_client_shield'; @@ -34,7 +34,7 @@ test(`calls server.expose with exposed services`, () => { const mockActions = Symbol(); actionsFactory.mockReturnValue(mockActions); - initAuthorization(mockServer); + initAuthorizationService(mockServer); expect(getClient).toHaveBeenCalledWith(mockServer); expect(actionsFactory).toHaveBeenCalledWith(mockConfig); From 2c003b4cb6f7153ba487e7e02178cf473a5622a4 Mon Sep 17 00:00:00 2001 From: kobelb Date: Thu, 5 Jul 2018 17:00:37 -0400 Subject: [PATCH 10/13] Throwing explicit validation error in actions.getSavedObjectAction --- .../__snapshots__/actions.test.js.snap | 25 +++++++++++++++++++ .../server/lib/authorization/actions.js | 10 +++++++- .../server/lib/authorization/actions.test.js | 18 +++++++++++++ 3 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 x-pack/plugins/security/server/lib/authorization/__snapshots__/actions.test.js.snap 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 0000000000000..d65733feb37d4 --- /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/actions.js b/x-pack/plugins/security/server/lib/authorization/actions.js index 6851f57719eb4..432698a003cb3 100644 --- a/x-pack/plugins/security/server/lib/authorization/actions.js +++ b/x-pack/plugins/security/server/lib/authorization/actions.js @@ -3,13 +3,21 @@ * 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`, diff --git a/x-pack/plugins/security/server/lib/authorization/actions.test.js b/x-pack/plugins/security/server/lib/authorization/actions.test.js index b6aa7d2f2113e..17834438e1781 100644 --- a/x-pack/plugins/security/server/lib/authorization/actions.test.js +++ b/x-pack/plugins/security/server/lib/authorization/actions.test.js @@ -48,4 +48,22 @@ describe('#getSavedObjectAction()', () => { 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(); + }); + }); }); From bf79ddae9ee8891960635409e60f1eee46ae7e9f Mon Sep 17 00:00:00 2001 From: kobelb Date: Thu, 5 Jul 2018 17:22:42 -0400 Subject: [PATCH 11/13] Deep freezing authorization service --- .../__snapshots__/init.test.js.snap | 7 +++++++ .../server/lib/authorization/deep_freeze.js | 20 +++++++++++++++++++ .../security/server/lib/authorization/init.js | 5 +++-- .../server/lib/authorization/init.test.js | 18 +++++++++++++++++ 4 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 x-pack/plugins/security/server/lib/authorization/__snapshots__/init.test.js.snap create mode 100644 x-pack/plugins/security/server/lib/authorization/deep_freeze.js 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 0000000000000..fd944032d2930 --- /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/deep_freeze.js b/x-pack/plugins/security/server/lib/authorization/deep_freeze.js new file mode 100644 index 0000000000000..0f9363cb410f6 --- /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/init.js b/x-pack/plugins/security/server/lib/authorization/init.js index be1ab2fc7348a..27ca3f7e84b55 100644 --- a/x-pack/plugins/security/server/lib/authorization/init.js +++ b/x-pack/plugins/security/server/lib/authorization/init.js @@ -6,6 +6,7 @@ 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) { @@ -14,8 +15,8 @@ export function initAuthorizationService(server) { const actions = actionsFactory(config); - server.expose('authorization', { + 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 index 5e96d224bcb28..b72813809896f 100644 --- a/x-pack/plugins/security/server/lib/authorization/init.test.js +++ b/x-pack/plugins/security/server/lib/authorization/init.test.js @@ -44,3 +44,21 @@ test(`calls server.expose with exposed services`, () => { 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(); +}); From dc2c9846e359ad941f424ece7c3b838ed3179f46 Mon Sep 17 00:00:00 2001 From: kobelb Date: Fri, 6 Jul 2018 07:57:34 -0400 Subject: [PATCH 12/13] Adding deepFreeze tests --- .../lib/authorization/deep_freeze.test.js | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 x-pack/plugins/security/server/lib/authorization/deep_freeze.test.js 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 0000000000000..dd227fa6269bf --- /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]); +}); From c8f828ee91587cbed6d5c5f74068b32ee5e7e2c2 Mon Sep 17 00:00:00 2001 From: kobelb Date: Fri, 6 Jul 2018 11:03:30 -0400 Subject: [PATCH 13/13] Checking privileges in one call and cleaning up tests --- .../check_privileges.test.js.snap | 4 +- .../lib/authorization/check_privileges.js | 108 ++-- .../authorization/check_privileges.test.js | 606 ++++++++---------- 3 files changed, 315 insertions(+), 403 deletions(-) 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 e191d4b14b6f0..4c0c88a8f8313 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/check_privileges.js b/x-pack/plugins/security/server/lib/authorization/check_privileges.js index 6453d3f22eb17..df8ee33ec2902 100644 --- a/x-pack/plugins/security/server/lib/authorization/check_privileges.js +++ b/x-pack/plugins/security/server/lib/authorization/check_privileges.js @@ -8,9 +8,9 @@ import { uniq } from 'lodash'; import { ALL_RESOURCE } from '../../../common/constants'; export const CHECK_PRIVILEGES_RESULT = { - UNAUTHORIZED: Symbol(), - AUTHORIZED: Symbol(), - LEGACY: Symbol(), + UNAUTHORIZED: Symbol('Unauthorized'), + AUTHORIZED: Symbol('Authorized'), + LEGACY: Symbol('Legacy'), }; export function checkPrivilegesWithRequestFactory(shieldClient, config, actions) { @@ -19,79 +19,67 @@ export function checkPrivilegesWithRequestFactory(shieldClient, config, actions) const application = config.get('xpack.security.rbac.application'); const kibanaIndex = config.get('kibana.index'); - return function checkPrivilegesWithRequest(request) { + const hasIncompatibileVersion = (applicationPrivilegesResponse) => { + return !applicationPrivilegesResponse[actions.version] && applicationPrivilegesResponse[actions.login]; + }; - const checkApplicationPrivileges = async (privileges) => { - const privilegeCheck = await callWithRequest(request, 'shield.hasPrivileges', { - body: { - applications: [{ - application, - resources: [ALL_RESOURCE], - privileges - }] - } - }); + const hasAllApplicationPrivileges = (applicationPrivilegesResponse) => { + return Object.values(applicationPrivilegesResponse).every(val => val === true); + }; - const privilegeCheckPrivileges = privilegeCheck.application[application][ALL_RESOURCE]; + const hasNoApplicationPrivileges = (applicationPrivilegesResponse) => { + return Object.values(applicationPrivilegesResponse).every(val => val === false); + }; - // 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 (!privilegeCheckPrivileges[actions.version] && privilegeCheckPrivileges[actions.login]) { - throw new Error('Multiple versions of Kibana are running against the same Elasticsearch cluster, unable to authorize user.'); - } + const hasLegacyPrivileges = (indexPrivilegesResponse) => { + return Object.values(indexPrivilegesResponse).includes(true); + }; - return { - username: privilegeCheck.username, - hasAllRequested: privilegeCheck.has_all_requested, - privileges: privilegeCheckPrivileges - }; - }; + const determineResult = (applicationPrivilegesResponse, indexPrivilegesResponse) => { + if (hasAllApplicationPrivileges(applicationPrivilegesResponse)) { + return CHECK_PRIVILEGES_RESULT.AUTHORIZED; + } - const hasPrivilegesOnKibanaIndex = async () => { - const privilegeCheck = await callWithRequest(request, 'shield.hasPrivileges', { + if (hasNoApplicationPrivileges(applicationPrivilegesResponse) && hasLegacyPrivileges(indexPrivilegesResponse)) { + return CHECK_PRIVILEGES_RESULT.LEGACY; + } + + 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); - }; + const applicationPrivilegesResponse = hasPrivilegesResponse.application[application][ALL_RESOURCE]; + const indexPrivilegesResponse = hasPrivilegesResponse.index[kibanaIndex]; - return async function checkPrivileges(privileges) { - const allPrivileges = uniq([actions.version, actions.login, ...privileges]); - const applicationPrivilegesCheck = await checkApplicationPrivileges(allPrivileges); - - const username = applicationPrivilegesCheck.username; - - // we only return missing privileges that they're specifically checking for - const missing = Object.keys(applicationPrivilegesCheck.privileges) - .filter(privilege => privileges.includes(privilege)) - .filter(privilege => !applicationPrivilegesCheck.privileges[privilege]); - - if (applicationPrivilegesCheck.hasAllRequested) { - return { - result: CHECK_PRIVILEGES_RESULT.AUTHORIZED, - username, - missing, - }; - } - - if (!applicationPrivilegesCheck.privileges[actions.login] && await hasPrivilegesOnKibanaIndex()) { - 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 c9da17e653d5e..3c414665c8258 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,6 +4,7 @@ * 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 { ALL_RESOURCE } from '../../../common/constants'; @@ -13,11 +14,9 @@ const defaultApplication = 'default-application'; const defaultKibanaIndex = 'default-index'; const savedObjectTypes = ['foo-type', 'bar-type']; -const createMockActions = () => { - return { - login: 'mock-action:login', - version: 'mock-action:version', - }; +const mockActions = { + login: 'mock-action:login', + version: 'mock-action:version', }; const createMockConfig = ({ settings = {} } = {}) => { @@ -39,371 +38,294 @@ const createMockConfig = ({ settings = {} } = {}) => { 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 createMockShieldClient = (responses) => { - const mockCallWithRequest = jest.fn(); - 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 { - callWithRequest: mockCallWithRequest, - }; + if (expectedResult) { + expect(errorThrown).toBeNull(); + expect(actualResult).toEqual(expectedResult); + } + + 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 mockConfig = createMockConfig(); - const mockActions = createMockActions(); - const mockShieldClient = createMockShieldClient([ - mockApplicationPrivilegeResponse({ - hasAllRequested: true, - privileges: { - [mockActions.version]: true, - [mockActions.login]: true, - [privilege]: true, - }, - application: defaultApplication, - username, - }) - ]); - - const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(mockShieldClient, mockConfig, mockActions); - const request = Symbol(); - const checkPrivileges = checkPrivilegesWithRequest(request); - const privileges = [privilege]; - const result = await checkPrivileges(privileges); - - expect(mockShieldClient.callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { - body: { - applications: [{ - application: defaultApplication, - resources: [ALL_RESOURCE], - privileges: [ - mockActions.version, mockActions.login, ...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 if they have only one application action`, async () => { - const privilege1 = `action:saved_objects/${savedObjectTypes[0]}/get`; - const privilege2 = `action:saved_objects/${savedObjectTypes[0]}/create`; - const username = 'foo-username'; - const mockConfig = createMockConfig(); - const mockActions = createMockActions(); - const mockShieldClient = createMockShieldClient([ - mockApplicationPrivilegeResponse({ - hasAllRequested: false, - privileges: { - [mockActions.version]: true, - [mockActions.login]: true, - [privilege1]: true, - [privilege2]: false, - }, - application: defaultApplication, - username, - }) - ]); - - const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(mockShieldClient, mockConfig, mockActions); - const request = Symbol(); - const checkPrivileges = checkPrivilegesWithRequest(request); - const privileges = [privilege1, privilege2]; - const result = await checkPrivileges(privileges); - - expect(mockShieldClient.callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { - body: { - applications: [{ - application: defaultApplication, - resources: [ALL_RESOURCE], - privileges: [ - mockActions.version, mockActions.login, ...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], } }); - expect(result).toEqual({ - result: CHECK_PRIVILEGES_RESULT.UNAUTHORIZED, - username, - missing: [privilege2], + + 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], + } + }); + + 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 mockConfig = createMockConfig(); - const mockActions = createMockActions(); - const mockShieldClient = createMockShieldClient([ - mockApplicationPrivilegeResponse({ - hasAllRequested: false, - privileges: { - [mockActions.version]: false, - [mockActions.login]: true, - [privilege]: true, - } - }) - ]); +describe(`with index privileges`, () => { + const indexPrivilegesResponse = { + create: true, + delete: true, + read: true, + view_index_metadata: true, + }; - const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(mockShieldClient, mockConfig, mockActions); - 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(); -}); + 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', () => { - describe('they have no privileges on the kibana index', () => { - test(`returns unauthorized and missing application action when checking application action `, async () => { - const privilege = `action:saved_objects/${savedObjectTypes[0]}/get`; - const username = 'foo-username'; - const mockConfig = createMockConfig(); - const mockActions = createMockActions(); - const mockShieldClient = createMockShieldClient([ - mockApplicationPrivilegeResponse({ - hasAllRequested: false, - privileges: { - [mockActions.version]: false, - [mockActions.login]: false, - [privilege]: false, - }, - username, - }), - mockKibanaIndexPrivilegesResponse({ - privileges: { - create: false, - delete: false, - read: false, - view_index_metadata: false, - }, - }) - ]); - - const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(mockShieldClient, mockConfig, mockActions); - const request = Symbol(); - const checkPrivileges = checkPrivilegesWithRequest(request); - const privileges = [privilege]; - const result = await checkPrivileges(privileges); - - expect(mockShieldClient.callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { - body: { - applications: [{ - application: defaultApplication, - resources: [ALL_RESOURCE], - privileges: [ - mockActions.version, mockActions.login, ...privileges - ] - }] - } - }); - expect(mockShieldClient.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: [...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], + } + }); - test(`returns unauthorized and missing login when checking login action`, async () => { - const username = 'foo-username'; - const mockConfig = createMockConfig(); - const mockActions = createMockActions(); - const mockShieldClient = createMockShieldClient([ - mockApplicationPrivilegeResponse({ - hasAllRequested: false, - privileges: { - [mockActions.version]: false, - [mockActions.login]: false, - }, - username, - }), - mockKibanaIndexPrivilegesResponse({ - privileges: { - create: false, - delete: false, - read: false, - view_index_metadata: false, - }, - }) - ]); - - const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(mockShieldClient, mockConfig, mockActions); - const request = Symbol(); - const checkPrivileges = checkPrivilegesWithRequest(request); - const result = await checkPrivileges([mockActions.login]); - - expect(mockShieldClient.callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { - body: { - applications: [{ - application: defaultApplication, - resources: [ALL_RESOURCE], - privileges: [ - mockActions.version, mockActions.login - ] - }] - } - }); - expect(mockShieldClient.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: [mockActions.login], - }); - }); + 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], + } + }); - test(`returns unauthorized and missing version if checking version action`, async () => { - const username = 'foo-username'; - const mockConfig = createMockConfig(); - const mockActions = createMockActions(); - const mockShieldClient = createMockShieldClient([ - mockApplicationPrivilegeResponse({ - hasAllRequested: false, - privileges: { - [mockActions.version]: false, - [mockActions.login]: false, - }, - username, - }), - mockKibanaIndexPrivilegesResponse({ - privileges: { - create: false, - delete: false, - read: false, - view_index_metadata: false, - }, - }) - ]); - - const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(mockShieldClient, mockConfig, mockActions); - const request = Symbol(); - const checkPrivileges = checkPrivilegesWithRequest(request); - - const result = await checkPrivileges([mockActions.version]); - - expect(mockShieldClient.callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { - body: { - applications: [{ - application: defaultApplication, - resources: [ALL_RESOURCE], - privileges: [ - mockActions.version, mockActions.login - ] - }] - } - }); - expect(mockShieldClient.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: [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 mockConfig = createMockConfig(); - const mockActions = createMockActions(); - const mockShieldClient = createMockShieldClient([ - mockApplicationPrivilegeResponse({ - hasAllRequested: false, - privileges: { - [mockActions.version]: false, - [mockActions.login]: false, - [privilege]: false, - }, - username, - }), - mockKibanaIndexPrivilegesResponse({ - privileges: { - create: false, - delete: false, - read: false, - view_index_metadata: false, - [indexPrivilege]: true - }, - }) - ]); - - const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(mockShieldClient, mockConfig, mockActions); - const request = Symbol(); - const checkPrivileges = checkPrivilegesWithRequest(request); - const privileges = [privilege]; - const result = await checkPrivileges(privileges); - - expect(mockShieldClient.callWithRequest).toHaveBeenCalledWith(request, 'shield.hasPrivileges', { - body: { - applications: [{ - application: defaultApplication, - resources: [ALL_RESOURCE], - privileges: [ - mockActions.version, mockActions.login, ...privileges - ] - }] - } - }); - expect(mockShieldClient.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: [...privileges], - }); + username: 'foo-username', + missing: [`action:saved_objects/${savedObjectTypes[0]}/get`], + } }); }); });