From b9e4241a10c7a97bae405fe4a0a79f9784ee8933 Mon Sep 17 00:00:00 2001 From: Brandon Kobel Date: Fri, 15 Mar 2019 16:39:12 -0700 Subject: [PATCH] Security - authorization mode request cache (#33248) (#33353) * Changing authorization mode to useRbacForRequest and caching response * Removing authorization from the mocha server fixture, it's not used * Switching usages to useRbacForRequest * Using jest's mocking support instead of hand-rolling one * Using the request fixture --- x-pack/plugins/security/index.js | 4 +- .../lib/__tests__/__fixtures__/server.js | 8 -- .../security/server/lib/authorization/mode.js | 10 +- .../server/lib/authorization/mode.test.js | 69 +++++++++--- .../__snapshots__/spaces_client.test.ts.snap | 18 +-- .../spaces/server/lib/spaces_client.test.ts | 104 +++++++++--------- .../spaces/server/lib/spaces_client.ts | 2 +- 7 files changed, 126 insertions(+), 89 deletions(-) diff --git a/x-pack/plugins/security/index.js b/x-pack/plugins/security/index.js index d343bddbcee26..42f8d707922c2 100644 --- a/x-pack/plugins/security/index.js +++ b/x-pack/plugins/security/index.js @@ -140,7 +140,7 @@ export const security = (kibana) => new kibana.Plugin({ const { callWithRequest, callWithInternalUser } = adminCluster; const callCluster = (...args) => callWithRequest(request, ...args); - if (authorization.mode.useRbac()) { + if (authorization.mode.useRbacForRequest(request)) { const internalRepository = savedObjects.getSavedObjectsRepository(callWithInternalUser); return new savedObjects.SavedObjectsClient(internalRepository); } @@ -150,7 +150,7 @@ export const security = (kibana) => new kibana.Plugin({ }); savedObjects.addScopedSavedObjectsClientWrapperFactory(Number.MIN_VALUE, ({ client, request }) => { - if (authorization.mode.useRbac()) { + if (authorization.mode.useRbacForRequest(request)) { const { spaces } = server.plugins; return new SecureSavedObjectsClientWrapper({ 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 c5b06a57dc104..237178b1bc268 100644 --- a/x-pack/plugins/security/server/lib/__tests__/__fixtures__/server.js +++ b/x-pack/plugins/security/server/lib/__tests__/__fixtures__/server.js @@ -37,14 +37,6 @@ export function serverFixture() { getUser: stub(), authenticate: stub(), deauthenticate: stub(), - authorization: { - mode: { - useRbac: stub(), - }, - actions: { - login: 'stub-login-action', - }, - }, }, xpack_main: { diff --git a/x-pack/plugins/security/server/lib/authorization/mode.js b/x-pack/plugins/security/server/lib/authorization/mode.js index ed59a6213afb5..31769609e8b3e 100644 --- a/x-pack/plugins/security/server/lib/authorization/mode.js +++ b/x-pack/plugins/security/server/lib/authorization/mode.js @@ -7,9 +7,15 @@ export function authorizationModeFactory( xpackInfoFeature, ) { + const useRbacForRequestCache = new WeakMap(); + return { - useRbac() { - return xpackInfoFeature.getLicenseCheckResults().allowRbac; + useRbacForRequest(request) { + if (!useRbacForRequestCache.has(request)) { + useRbacForRequestCache.set(request, Boolean(xpackInfoFeature.getLicenseCheckResults().allowRbac)); + } + + return useRbacForRequestCache.get(request); }, }; } diff --git a/x-pack/plugins/security/server/lib/authorization/mode.test.js b/x-pack/plugins/security/server/lib/authorization/mode.test.js index 449792664af6d..5f8f6e1de1c36 100644 --- a/x-pack/plugins/security/server/lib/authorization/mode.test.js +++ b/x-pack/plugins/security/server/lib/authorization/mode.test.js @@ -5,31 +5,70 @@ */ import { authorizationModeFactory } from './mode'; +import { requestFixture } from '../__tests__/__fixtures__/request'; + +class MockXPackInfoFeature { + constructor(allowRbac) { + this.getLicenseCheckResults.mockReturnValue({ allowRbac }); + } + + getLicenseCheckResults = jest.fn(); +} + +describe(`#useRbacForRequest`, () => { + test(`throws an Error if request isn't specified`, async () => { + const mockXpackInfoFeature = new MockXPackInfoFeature(false); + const mode = authorizationModeFactory(mockXpackInfoFeature); + + expect(() => mode.useRbacForRequest()).toThrowErrorMatchingInlineSnapshot( + `"Invalid value used as weak map key"` + ); + }); + + test(`throws an Error if request is "null"`, async () => { + const mockXpackInfoFeature = new MockXPackInfoFeature(false); + const mode = authorizationModeFactory(mockXpackInfoFeature); + + expect(() => mode.useRbacForRequest(null)).toThrowErrorMatchingInlineSnapshot( + `"Invalid value used as weak map key"` + ); + }); -const createMockXpackInfoFeature = (allowRbac) => { - return { - getLicenseCheckResults() { - return { - allowRbac - }; - } - }; -}; - -describe(`#useRbac`, () => { test(`returns false if xpackInfoFeature.getLicenseCheckResults().allowRbac is false`, async () => { - const mockXpackInfoFeature = createMockXpackInfoFeature(false); + const mockXpackInfoFeature = new MockXPackInfoFeature(false); const mode = authorizationModeFactory(mockXpackInfoFeature); + const request = requestFixture(); - const result = mode.useRbac(); + const result = mode.useRbacForRequest(request); expect(result).toBe(false); }); + test(`returns false if xpackInfoFeature.getLicenseCheckResults().allowRbac is initially false, and changes to true`, async () => { + const mockXpackInfoFeature = new MockXPackInfoFeature(false); + const mode = authorizationModeFactory(mockXpackInfoFeature); + const request = requestFixture(); + + expect(mode.useRbacForRequest(request)).toBe(false); + mockXpackInfoFeature.getLicenseCheckResults.mockReturnValue({ allowRbac: true }); + expect(mode.useRbacForRequest(request)).toBe(false); + }); + test(`returns true if xpackInfoFeature.getLicenseCheckResults().allowRbac is true`, async () => { - const mockXpackInfoFeature = createMockXpackInfoFeature(true); + const mockXpackInfoFeature = new MockXPackInfoFeature(true); const mode = authorizationModeFactory(mockXpackInfoFeature); + const request = requestFixture(); - const result = mode.useRbac(); + const result = mode.useRbacForRequest(request); expect(result).toBe(true); }); + + test(`returns true if xpackInfoFeature.getLicenseCheckResults().allowRbac is initially true, and changes to false`, async () => { + const mockXpackInfoFeature = new MockXPackInfoFeature(true); + const mode = authorizationModeFactory(mockXpackInfoFeature); + const request = requestFixture(); + + expect(mode.useRbacForRequest(request)).toBe(true); + mockXpackInfoFeature.getLicenseCheckResults.mockReturnValue({ allowRbac: false }); + expect(mode.useRbacForRequest(request)).toBe(true); + }); }); diff --git a/x-pack/plugins/spaces/server/lib/__snapshots__/spaces_client.test.ts.snap b/x-pack/plugins/spaces/server/lib/__snapshots__/spaces_client.test.ts.snap index 7b2edc7e299c5..3d3049a2c290e 100644 --- a/x-pack/plugins/spaces/server/lib/__snapshots__/spaces_client.test.ts.snap +++ b/x-pack/plugins/spaces/server/lib/__snapshots__/spaces_client.test.ts.snap @@ -2,22 +2,22 @@ exports[`#create authorization is null throws bad request when we are at the maximum number of spaces 1`] = `"Unable to create Space, this exceeds the maximum number of spaces set by the xpack.spaces.maxSpaces setting"`; -exports[`#create authorization.mode.useRbac returns false throws bad request when we're at the maximum number of spaces 1`] = `"Unable to create Space, this exceeds the maximum number of spaces set by the xpack.spaces.maxSpaces setting"`; +exports[`#create authorization.mode.useRbacForRequest returns false throws bad request when we're at the maximum number of spaces 1`] = `"Unable to create Space, this exceeds the maximum number of spaces set by the xpack.spaces.maxSpaces setting"`; -exports[`#create useRbac is true throws Boom.forbidden if the user isn't authorized at space 1`] = `"Unauthorized to create spaces"`; +exports[`#create useRbacForRequest is true throws Boom.forbidden if the user isn't authorized at space 1`] = `"Unauthorized to create spaces"`; -exports[`#create useRbac is true throws bad request when we are at the maximum number of spaces 1`] = `"Unable to create Space, this exceeds the maximum number of spaces set by the xpack.spaces.maxSpaces setting"`; +exports[`#create useRbacForRequest is true throws bad request when we are at the maximum number of spaces 1`] = `"Unable to create Space, this exceeds the maximum number of spaces set by the xpack.spaces.maxSpaces setting"`; exports[`#delete authorization is null throws bad request when the space is reserved 1`] = `"This Space cannot be deleted because it is reserved."`; -exports[`#delete authorization.mode.useRbac returns false throws bad request when the space is reserved 1`] = `"This Space cannot be deleted because it is reserved."`; +exports[`#delete authorization.mode.useRbacForRequest returns false throws bad request when the space is reserved 1`] = `"This Space cannot be deleted because it is reserved."`; -exports[`#delete authorization.mode.useRbac returns true throws Boom.forbidden if the user isn't authorized 1`] = `"Unauthorized to delete spaces"`; +exports[`#delete authorization.mode.useRbacForRequest returns true throws Boom.forbidden if the user isn't authorized 1`] = `"Unauthorized to delete spaces"`; -exports[`#delete authorization.mode.useRbac returns true throws bad request if the user is authorized but the space is reserved 1`] = `"This Space cannot be deleted because it is reserved."`; +exports[`#delete authorization.mode.useRbacForRequest returns true throws bad request if the user is authorized but the space is reserved 1`] = `"This Space cannot be deleted because it is reserved."`; -exports[`#get useRbac is true throws Boom.forbidden if the user isn't authorized at space 1`] = `"Unauthorized to get foo-space space"`; +exports[`#get useRbacForRequest is true throws Boom.forbidden if the user isn't authorized at space 1`] = `"Unauthorized to get foo-space space"`; -exports[`#getAll useRbac is true throws Boom.forbidden when user isn't authorized for any spaces 1`] = `"Forbidden"`; +exports[`#getAll useRbacForRequest is true throws Boom.forbidden when user isn't authorized for any spaces 1`] = `"Forbidden"`; -exports[`#update useRbac is true throws Boom.forbidden when user isn't authorized at space 1`] = `"Unauthorized to update spaces"`; +exports[`#update useRbacForRequest is true throws Boom.forbidden when user isn't authorized at space 1`] = `"Unauthorized to update spaces"`; diff --git a/x-pack/plugins/spaces/server/lib/spaces_client.test.ts b/x-pack/plugins/spaces/server/lib/spaces_client.test.ts index c5b54c4808972..360a51ecdd798 100644 --- a/x-pack/plugins/spaces/server/lib/spaces_client.test.ts +++ b/x-pack/plugins/spaces/server/lib/spaces_client.test.ts @@ -33,7 +33,7 @@ const createMockAuthorization = () => { globally: mockCheckPrivilegesGlobally, })), mode: { - useRbac: jest.fn(), + useRbacForRequest: jest.fn(), }, }; @@ -132,12 +132,12 @@ describe('#getAll', () => { }); }); - describe(`authorization.mode.useRbac returns false`, () => { + describe(`authorization.mode.useRbacForRequest returns false`, () => { test(`finds spaces using callWithRequestRepository`, async () => { const mockAuditLogger = createMockAuditLogger(); const mockDebugLogger = createMockDebugLogger(); const { mockAuthorization } = createMockAuthorization(); - mockAuthorization.mode.useRbac.mockReturnValue(false); + mockAuthorization.mode.useRbacForRequest.mockReturnValue(false); const mockCallWithRequestRepository = { find: jest.fn().mockReturnValue({ saved_objects: savedObjects, @@ -167,19 +167,19 @@ describe('#getAll', () => { perPage: maxSpaces, sortField: 'name.keyword', }); - expect(mockAuthorization.mode.useRbac).toHaveBeenCalled(); + expect(mockAuthorization.mode.useRbacForRequest).toHaveBeenCalledWith(request); expect(mockAuditLogger.spacesAuthorizationFailure).toHaveBeenCalledTimes(0); expect(mockAuditLogger.spacesAuthorizationSuccess).toHaveBeenCalledTimes(0); }); }); - describe('useRbac is true', () => { + describe('useRbacForRequest is true', () => { test(`throws Boom.forbidden when user isn't authorized for any spaces`, async () => { const username = Symbol(); const mockAuditLogger = createMockAuditLogger(); const mockDebugLogger = createMockDebugLogger(); const { mockAuthorization, mockCheckPrivilegesAtSpaces } = createMockAuthorization(); - mockAuthorization.mode.useRbac.mockReturnValue(true); + mockAuthorization.mode.useRbacForRequest.mockReturnValue(true); mockCheckPrivilegesAtSpaces.mockReturnValue({ username, spacePrivileges: { @@ -219,7 +219,7 @@ describe('#getAll', () => { perPage: maxSpaces, sortField: 'name.keyword', }); - expect(mockAuthorization.mode.useRbac).toHaveBeenCalled(); + expect(mockAuthorization.mode.useRbacForRequest).toHaveBeenCalledWith(request); expect(mockAuthorization.checkPrivilegesWithRequest).toHaveBeenCalledWith(request); expect(mockCheckPrivilegesAtSpaces).toHaveBeenCalledWith( savedObjects.map(savedObject => savedObject.id), @@ -234,7 +234,7 @@ describe('#getAll', () => { const mockAuditLogger = createMockAuditLogger(); const mockDebugLogger = createMockDebugLogger(); const { mockAuthorization, mockCheckPrivilegesAtSpaces } = createMockAuthorization(); - mockAuthorization.mode.useRbac.mockReturnValue(true); + mockAuthorization.mode.useRbacForRequest.mockReturnValue(true); mockCheckPrivilegesAtSpaces.mockReturnValue({ username, spacePrivileges: { @@ -275,7 +275,7 @@ describe('#getAll', () => { perPage: maxSpaces, sortField: 'name.keyword', }); - expect(mockAuthorization.mode.useRbac).toHaveBeenCalled(); + expect(mockAuthorization.mode.useRbacForRequest).toHaveBeenCalledWith(request); expect(mockAuthorization.checkPrivilegesWithRequest).toHaveBeenCalledWith(request); expect(mockCheckPrivilegesAtSpaces).toHaveBeenCalledWith( savedObjects.map(savedObject => savedObject.id), @@ -314,12 +314,12 @@ describe('#canEnumerateSpaces', () => { }); }); - describe(`authorization.mode.useRbac is false`, () => { + describe(`authorization.mode.useRbacForRequest is false`, () => { test(`returns true`, async () => { const mockAuditLogger = createMockAuditLogger(); const mockDebugLogger = createMockDebugLogger(); const { mockAuthorization } = createMockAuthorization(); - mockAuthorization.mode.useRbac.mockReturnValue(false); + mockAuthorization.mode.useRbacForRequest.mockReturnValue(false); const request = Symbol(); const client = new SpacesClient( @@ -339,13 +339,13 @@ describe('#canEnumerateSpaces', () => { }); }); - describe('useRbac is true', () => { + describe('useRbacForRequest is true', () => { test(`returns false if user is not authorized to enumerate spaces`, async () => { const username = Symbol(); const mockAuditLogger = createMockAuditLogger(); const mockDebugLogger = createMockDebugLogger(); const { mockAuthorization, mockCheckPrivilegesGlobally } = createMockAuthorization(); - mockAuthorization.mode.useRbac.mockReturnValue(true); + mockAuthorization.mode.useRbacForRequest.mockReturnValue(true); mockCheckPrivilegesGlobally.mockReturnValue({ username, hasAllRequested: false, @@ -379,7 +379,7 @@ describe('#canEnumerateSpaces', () => { const mockAuditLogger = createMockAuditLogger(); const mockDebugLogger = createMockDebugLogger(); const { mockAuthorization, mockCheckPrivilegesGlobally } = createMockAuthorization(); - mockAuthorization.mode.useRbac.mockReturnValue(true); + mockAuthorization.mode.useRbacForRequest.mockReturnValue(true); mockCheckPrivilegesGlobally.mockReturnValue({ username, hasAllRequested: true, @@ -456,12 +456,12 @@ describe('#get', () => { }); }); - describe(`authorization.mode.useRbac returns false`, () => { + describe(`authorization.mode.useRbacForRequest returns false`, () => { test(`gets space using callWithRequestRepository`, async () => { const mockAuditLogger = createMockAuditLogger(); const mockDebugLogger = createMockDebugLogger(); const { mockAuthorization } = createMockAuthorization(); - mockAuthorization.mode.useRbac.mockReturnValue(false); + mockAuthorization.mode.useRbacForRequest.mockReturnValue(false); const mockCallWithRequestRepository = { get: jest.fn().mockReturnValue(savedObject), }; @@ -480,20 +480,20 @@ describe('#get', () => { const actualSpace = await client.get(id); expect(actualSpace).toEqual(expectedSpace); - expect(mockAuthorization.mode.useRbac).toHaveBeenCalled(); + expect(mockAuthorization.mode.useRbacForRequest).toHaveBeenCalledWith(request); expect(mockCallWithRequestRepository.get).toHaveBeenCalledWith('space', id); expect(mockAuditLogger.spacesAuthorizationFailure).toHaveBeenCalledTimes(0); expect(mockAuditLogger.spacesAuthorizationSuccess).toHaveBeenCalledTimes(0); }); }); - describe('useRbac is true', () => { + describe('useRbacForRequest is true', () => { test(`throws Boom.forbidden if the user isn't authorized at space`, async () => { const username = Symbol(); const mockAuditLogger = createMockAuditLogger(); const mockDebugLogger = createMockDebugLogger(); const { mockAuthorization, mockCheckPrivilegesAtSpace } = createMockAuthorization(); - mockAuthorization.mode.useRbac.mockReturnValue(true); + mockAuthorization.mode.useRbacForRequest.mockReturnValue(true); mockCheckPrivilegesAtSpace.mockReturnValue({ username, hasAllRequested: false, @@ -526,7 +526,7 @@ describe('#get', () => { const mockAuditLogger = createMockAuditLogger(); const mockDebugLogger = createMockDebugLogger(); const { mockAuthorization, mockCheckPrivilegesAtSpace } = createMockAuthorization(); - mockAuthorization.mode.useRbac.mockReturnValue(true); + mockAuthorization.mode.useRbacForRequest.mockReturnValue(true); mockCheckPrivilegesAtSpace.mockReturnValue({ username, hasAllRequested: true, @@ -675,13 +675,13 @@ describe('#create', () => { }); }); - describe(`authorization.mode.useRbac returns false`, () => { + describe(`authorization.mode.useRbacForRequest returns false`, () => { test(`creates space using callWithRequestRepository when we're under the max`, async () => { const maxSpaces = 5; const mockAuditLogger = createMockAuditLogger(); const mockDebugLogger = createMockDebugLogger(); const { mockAuthorization } = createMockAuthorization(); - mockAuthorization.mode.useRbac.mockReturnValue(false); + mockAuthorization.mode.useRbacForRequest.mockReturnValue(false); const mockCallWithRequestRepository = { create: jest.fn().mockReturnValue(savedObject), find: jest.fn().mockReturnValue({ @@ -706,7 +706,7 @@ describe('#create', () => { const actualSpace = await client.create(spaceToCreate); expect(actualSpace).toEqual(expectedReturnedSpace); - expect(mockAuthorization.mode.useRbac).toHaveBeenCalled(); + expect(mockAuthorization.mode.useRbacForRequest).toHaveBeenCalledWith(request); expect(mockCallWithRequestRepository.find).toHaveBeenCalledWith({ type: 'space', page: 1, @@ -724,7 +724,7 @@ describe('#create', () => { const mockAuditLogger = createMockAuditLogger(); const mockDebugLogger = createMockDebugLogger(); const { mockAuthorization } = createMockAuthorization(); - mockAuthorization.mode.useRbac.mockReturnValue(false); + mockAuthorization.mode.useRbacForRequest.mockReturnValue(false); const mockCallWithRequestRepository = { create: jest.fn().mockReturnValue(savedObject), find: jest.fn().mockReturnValue({ @@ -748,7 +748,7 @@ describe('#create', () => { await expect(client.create(spaceToCreate)).rejects.toThrowErrorMatchingSnapshot(); - expect(mockAuthorization.mode.useRbac).toHaveBeenCalled(); + expect(mockAuthorization.mode.useRbacForRequest).toHaveBeenCalledWith(request); expect(mockCallWithRequestRepository.find).toHaveBeenCalledWith({ type: 'space', page: 1, @@ -760,13 +760,13 @@ describe('#create', () => { }); }); - describe('useRbac is true', () => { + describe('useRbacForRequest is true', () => { test(`throws Boom.forbidden if the user isn't authorized at space`, async () => { const username = Symbol(); const mockAuditLogger = createMockAuditLogger(); const mockDebugLogger = createMockDebugLogger(); const { mockAuthorization, mockCheckPrivilegesGlobally } = createMockAuthorization(); - mockAuthorization.mode.useRbac.mockReturnValue(true); + mockAuthorization.mode.useRbacForRequest.mockReturnValue(true); mockCheckPrivilegesGlobally.mockReturnValue({ username, hasAllRequested: false, @@ -785,7 +785,7 @@ describe('#create', () => { await expect(client.create(spaceToCreate)).rejects.toThrowErrorMatchingSnapshot(); - expect(mockAuthorization.mode.useRbac).toHaveBeenCalled(); + expect(mockAuthorization.mode.useRbacForRequest).toHaveBeenCalledWith(request); expect(mockAuthorization.checkPrivilegesWithRequest).toHaveBeenCalledWith(request); expect(mockCheckPrivilegesGlobally).toHaveBeenCalledWith( mockAuthorization.actions.manageSpaces @@ -800,7 +800,7 @@ describe('#create', () => { const mockAuditLogger = createMockAuditLogger(); const mockDebugLogger = createMockDebugLogger(); const { mockAuthorization, mockCheckPrivilegesGlobally } = createMockAuthorization(); - mockAuthorization.mode.useRbac.mockReturnValue(true); + mockAuthorization.mode.useRbacForRequest.mockReturnValue(true); mockCheckPrivilegesGlobally.mockReturnValue({ username, hasAllRequested: true, @@ -837,7 +837,7 @@ describe('#create', () => { expect(mockInternalRepository.create).toHaveBeenCalledWith('space', attributes, { id, }); - expect(mockAuthorization.mode.useRbac).toHaveBeenCalled(); + expect(mockAuthorization.mode.useRbacForRequest).toHaveBeenCalledWith(request); expect(mockAuthorization.checkPrivilegesWithRequest).toHaveBeenCalledWith(request); expect(mockCheckPrivilegesGlobally).toHaveBeenCalledWith( mockAuthorization.actions.manageSpaces @@ -852,7 +852,7 @@ describe('#create', () => { const mockAuditLogger = createMockAuditLogger(); const mockDebugLogger = createMockDebugLogger(); const { mockAuthorization, mockCheckPrivilegesGlobally } = createMockAuthorization(); - mockAuthorization.mode.useRbac.mockReturnValue(true); + mockAuthorization.mode.useRbacForRequest.mockReturnValue(true); mockCheckPrivilegesGlobally.mockReturnValue({ username, hasAllRequested: true, @@ -886,7 +886,7 @@ describe('#create', () => { perPage: 0, }); expect(mockInternalRepository.create).not.toHaveBeenCalled(); - expect(mockAuthorization.mode.useRbac).toHaveBeenCalled(); + expect(mockAuthorization.mode.useRbacForRequest).toHaveBeenCalledWith(request); expect(mockAuthorization.checkPrivilegesWithRequest).toHaveBeenCalledWith(request); expect(mockCheckPrivilegesGlobally).toHaveBeenCalledWith( mockAuthorization.actions.manageSpaces @@ -960,12 +960,12 @@ describe('#update', () => { expect(mockAuditLogger.spacesAuthorizationSuccess).toHaveBeenCalledTimes(0); }); }); - describe(`authorization.mode.useRbac returns false`, () => { + describe(`authorization.mode.useRbacForRequest returns false`, () => { test(`updates space using callWithRequestRepository`, async () => { const mockAuditLogger = createMockAuditLogger(); const mockDebugLogger = createMockDebugLogger(); const { mockAuthorization } = createMockAuthorization(); - mockAuthorization.mode.useRbac.mockReturnValue(false); + mockAuthorization.mode.useRbacForRequest.mockReturnValue(false); const mockCallWithRequestRepository = { update: jest.fn(), get: jest.fn().mockReturnValue(savedObject), @@ -985,7 +985,7 @@ describe('#update', () => { const actualSpace = await client.update(id, spaceToUpdate); expect(actualSpace).toEqual(expectedReturnedSpace); - expect(mockAuthorization.mode.useRbac).toHaveBeenCalled(); + expect(mockAuthorization.mode.useRbacForRequest).toHaveBeenCalledWith(request); expect(mockCallWithRequestRepository.update).toHaveBeenCalledWith('space', id, attributes); expect(mockCallWithRequestRepository.get).toHaveBeenCalledWith('space', id); expect(mockAuditLogger.spacesAuthorizationFailure).toHaveBeenCalledTimes(0); @@ -993,7 +993,7 @@ describe('#update', () => { }); }); - describe('useRbac is true', () => { + describe('useRbacForRequest is true', () => { test(`throws Boom.forbidden when user isn't authorized at space`, async () => { const username = Symbol(); const mockAuditLogger = createMockAuditLogger(); @@ -1003,7 +1003,7 @@ describe('#update', () => { hasAllRequested: false, username, }); - mockAuthorization.mode.useRbac.mockReturnValue(true); + mockAuthorization.mode.useRbacForRequest.mockReturnValue(true); const request = Symbol(); const client = new SpacesClient( @@ -1018,7 +1018,7 @@ describe('#update', () => { const id = savedObject.id; await expect(client.update(id, spaceToUpdate)).rejects.toThrowErrorMatchingSnapshot(); - expect(mockAuthorization.mode.useRbac).toHaveBeenCalled(); + expect(mockAuthorization.mode.useRbacForRequest).toHaveBeenCalledWith(request); expect(mockAuthorization.checkPrivilegesWithRequest).toHaveBeenCalledWith(request); expect(mockCheckPrivilegesGlobally).toHaveBeenCalledWith( mockAuthorization.actions.manageSpaces @@ -1036,7 +1036,7 @@ describe('#update', () => { hasAllRequested: true, username, }); - mockAuthorization.mode.useRbac.mockReturnValue(true); + mockAuthorization.mode.useRbacForRequest.mockReturnValue(true); const mockInternalRepository = { update: jest.fn(), get: jest.fn().mockReturnValue(savedObject), @@ -1056,7 +1056,7 @@ describe('#update', () => { const actualSpace = await client.update(id, spaceToUpdate); expect(actualSpace).toEqual(expectedReturnedSpace); - expect(mockAuthorization.mode.useRbac).toHaveBeenCalled(); + expect(mockAuthorization.mode.useRbacForRequest).toHaveBeenCalledWith(request); expect(mockAuthorization.checkPrivilegesWithRequest).toHaveBeenCalledWith(request); expect(mockCheckPrivilegesGlobally).toHaveBeenCalledWith( mockAuthorization.actions.manageSpaces @@ -1150,12 +1150,12 @@ describe('#delete', () => { }); }); - describe(`authorization.mode.useRbac returns false`, () => { + describe(`authorization.mode.useRbacForRequest returns false`, () => { test(`throws bad request when the space is reserved`, async () => { const mockAuditLogger = createMockAuditLogger(); const mockDebugLogger = createMockDebugLogger(); const { mockAuthorization } = createMockAuthorization(); - mockAuthorization.mode.useRbac.mockReturnValue(false); + mockAuthorization.mode.useRbacForRequest.mockReturnValue(false); const mockCallWithRequestRepository = { get: jest.fn().mockReturnValue(reservedSavedObject), }; @@ -1173,7 +1173,7 @@ describe('#delete', () => { await expect(client.delete(id)).rejects.toThrowErrorMatchingSnapshot(); - expect(mockAuthorization.mode.useRbac).toHaveBeenCalled(); + expect(mockAuthorization.mode.useRbacForRequest).toHaveBeenCalledWith(request); expect(mockCallWithRequestRepository.get).toHaveBeenCalledWith('space', id); expect(mockAuditLogger.spacesAuthorizationFailure).toHaveBeenCalledTimes(0); expect(mockAuditLogger.spacesAuthorizationSuccess).toHaveBeenCalledTimes(0); @@ -1183,7 +1183,7 @@ describe('#delete', () => { const mockAuditLogger = createMockAuditLogger(); const mockDebugLogger = createMockDebugLogger(); const { mockAuthorization } = createMockAuthorization(); - mockAuthorization.mode.useRbac.mockReturnValue(false); + mockAuthorization.mode.useRbacForRequest.mockReturnValue(false); const mockCallWithRequestRepository = { get: jest.fn().mockReturnValue(notReservedSavedObject), delete: jest.fn(), @@ -1204,7 +1204,7 @@ describe('#delete', () => { await client.delete(id); - expect(mockAuthorization.mode.useRbac).toHaveBeenCalled(); + expect(mockAuthorization.mode.useRbacForRequest).toHaveBeenCalledWith(request); expect(mockCallWithRequestRepository.get).toHaveBeenCalledWith('space', id); expect(mockCallWithRequestRepository.delete).toHaveBeenCalledWith('space', id); expect(mockCallWithRequestRepository.deleteByNamespace).toHaveBeenCalledWith(id); @@ -1213,13 +1213,13 @@ describe('#delete', () => { }); }); - describe('authorization.mode.useRbac returns true', () => { + describe('authorization.mode.useRbacForRequest returns true', () => { test(`throws Boom.forbidden if the user isn't authorized`, async () => { const username = Symbol(); const mockAuditLogger = createMockAuditLogger(); const mockDebugLogger = createMockDebugLogger(); const { mockAuthorization, mockCheckPrivilegesGlobally } = createMockAuthorization(); - mockAuthorization.mode.useRbac.mockReturnValue(true); + mockAuthorization.mode.useRbacForRequest.mockReturnValue(true); mockCheckPrivilegesGlobally.mockReturnValue({ username, hasAllRequested: false, @@ -1237,7 +1237,7 @@ describe('#delete', () => { await expect(client.delete(id)).rejects.toThrowErrorMatchingSnapshot(); - expect(mockAuthorization.mode.useRbac).toHaveBeenCalled(); + expect(mockAuthorization.mode.useRbacForRequest).toHaveBeenCalledWith(request); expect(mockAuthorization.checkPrivilegesWithRequest).toHaveBeenCalledWith(request); expect(mockCheckPrivilegesGlobally).toHaveBeenCalledWith( mockAuthorization.actions.manageSpaces @@ -1251,7 +1251,7 @@ describe('#delete', () => { const mockAuditLogger = createMockAuditLogger(); const mockDebugLogger = createMockDebugLogger(); const { mockAuthorization, mockCheckPrivilegesGlobally } = createMockAuthorization(); - mockAuthorization.mode.useRbac.mockReturnValue(true); + mockAuthorization.mode.useRbacForRequest.mockReturnValue(true); mockCheckPrivilegesGlobally.mockReturnValue({ username, hasAllRequested: true, @@ -1272,7 +1272,7 @@ describe('#delete', () => { await expect(client.delete(id)).rejects.toThrowErrorMatchingSnapshot(); - expect(mockAuthorization.mode.useRbac).toHaveBeenCalled(); + expect(mockAuthorization.mode.useRbacForRequest).toHaveBeenCalledWith(request); expect(mockAuthorization.checkPrivilegesWithRequest).toHaveBeenCalledWith(request); expect(mockCheckPrivilegesGlobally).toHaveBeenCalledWith( mockAuthorization.actions.manageSpaces @@ -1287,7 +1287,7 @@ describe('#delete', () => { const mockAuditLogger = createMockAuditLogger(); const mockDebugLogger = createMockDebugLogger(); const { mockAuthorization, mockCheckPrivilegesGlobally } = createMockAuthorization(); - mockAuthorization.mode.useRbac.mockReturnValue(true); + mockAuthorization.mode.useRbacForRequest.mockReturnValue(true); mockCheckPrivilegesGlobally.mockReturnValue({ username, hasAllRequested: true, @@ -1311,7 +1311,7 @@ describe('#delete', () => { await client.delete(id); - expect(mockAuthorization.mode.useRbac).toHaveBeenCalled(); + expect(mockAuthorization.mode.useRbacForRequest).toHaveBeenCalledWith(request); expect(mockAuthorization.checkPrivilegesWithRequest).toHaveBeenCalledWith(request); expect(mockCheckPrivilegesGlobally).toHaveBeenCalledWith( mockAuthorization.actions.manageSpaces diff --git a/x-pack/plugins/spaces/server/lib/spaces_client.ts b/x-pack/plugins/spaces/server/lib/spaces_client.ts index fc564004ea3de..52085cf6b2e5a 100644 --- a/x-pack/plugins/spaces/server/lib/spaces_client.ts +++ b/x-pack/plugins/spaces/server/lib/spaces_client.ts @@ -196,7 +196,7 @@ export class SpacesClient { } private useRbac(): boolean { - return this.authorization && this.authorization.mode.useRbac(); + return this.authorization && this.authorization.mode.useRbacForRequest(this.request); } private async ensureAuthorizedGlobally(action: string, method: string, forbiddenMessage: string) {