From 20d9ae2c50396625ca89cbaf6cdcbe188a0acc78 Mon Sep 17 00:00:00 2001 From: Thom Heymann <190132+thomheymann@users.noreply.github.com> Date: Mon, 21 Dec 2020 12:36:24 +0000 Subject: [PATCH] Add audit events for spaces (#86343) (#86620) * Add audit events for spaces * fix eslint error --- docs/user/security/audit-logging.asciidoc | 58 ++++-- .../server/audit/audit_events.test.ts | 87 +++++++++ .../security/server/audit/audit_events.ts | 74 ++++++- x-pack/plugins/security/server/audit/index.ts | 2 + .../secure_spaces_client_wrapper.test.ts | 183 +++++++++++++++--- .../spaces/secure_spaces_client_wrapper.ts | 152 ++++++++++++--- .../server/spaces/setup_spaces_client.test.ts | 18 ++ .../server/spaces/setup_spaces_client.ts | 8 +- 8 files changed, 501 insertions(+), 81 deletions(-) diff --git a/docs/user/security/audit-logging.asciidoc b/docs/user/security/audit-logging.asciidoc index 4b3512ae3056b..7facde28e956f 100644 --- a/docs/user/security/audit-logging.asciidoc +++ b/docs/user/security/audit-logging.asciidoc @@ -58,7 +58,6 @@ authorization checks have passed, but before the response from {es} is received. Refer to the corresponding {es} logs for potential write errors. ============================================================================ - [cols="3*<"] |====== 3+a| @@ -89,9 +88,12 @@ Refer to the corresponding {es} logs for potential write errors. | `failure` | User is not authorized to create a connector. .2+| `alert_create` -| `unknown` | User is creating an alert rule. -| `failure` | User is not authorized to create an alert rule. +| `unknown` | User is creating an alert. +| `failure` | User is not authorized to create an alert. +.2+| `space_create` +| `unknown` | User is creating a space. +| `failure` | User is not authorized to create a space. 3+a| ====== Type: change @@ -121,28 +123,28 @@ Refer to the corresponding {es} logs for potential write errors. | `failure` | User is not authorized to update a connector. .2+| `alert_update` -| `unknown` | User is updating an alert rule. -| `failure` | User is not authorized to update an alert rule. +| `unknown` | User is updating an alert. +| `failure` | User is not authorized to update an alert. .2+| `alert_update_api_key` -| `unknown` | User is updating the API key of an alert rule. -| `failure` | User is not authorized to update the API key of an alert rule. +| `unknown` | User is updating the API key of an alert. +| `failure` | User is not authorized to update the API key of an alert. .2+| `alert_enable` -| `unknown` | User is enabling an alert rule. -| `failure` | User is not authorized to enable an alert rule. +| `unknown` | User is enabling an alert. +| `failure` | User is not authorized to enable an alert. .2+| `alert_disable` -| `unknown` | User is disabling an alert rule. -| `failure` | User is not authorized to disable an alert rule. +| `unknown` | User is disabling an alert. +| `failure` | User is not authorized to disable an alert. .2+| `alert_mute` -| `unknown` | User is muting an alert rule. -| `failure` | User is not authorized to mute an alert rule. +| `unknown` | User is muting an alert. +| `failure` | User is not authorized to mute an alert. .2+| `alert_unmute` -| `unknown` | User is unmuting an alert rule. -| `failure` | User is not authorized to unmute an alert rule. +| `unknown` | User is unmuting an alert. +| `failure` | User is not authorized to unmute an alert. .2+| `alert_instance_mute` | `unknown` | User is muting an alert instance. @@ -152,6 +154,9 @@ Refer to the corresponding {es} logs for potential write errors. | `unknown` | User is unmuting an alert instance. | `failure` | User is not authorized to unmute an alert instance. +.2+| `space_update` +| `unknown` | User is updating a space. +| `failure` | User is not authorized to update a space. 3+a| ====== Type: deletion @@ -169,8 +174,12 @@ Refer to the corresponding {es} logs for potential write errors. | `failure` | User is not authorized to delete a connector. .2+| `alert_delete` -| `unknown` | User is deleting an alert rule. -| `failure` | User is not authorized to delete an alert rule. +| `unknown` | User is deleting an alert. +| `failure` | User is not authorized to delete an alert. + +.2+| `space_delete` +| `unknown` | User is deleting a space. +| `failure` | User is not authorized to delete a space. 3+a| ====== Type: access @@ -196,13 +205,20 @@ Refer to the corresponding {es} logs for potential write errors. | `failure` | User is not authorized to search for connectors. .2+| `alert_get` -| `success` | User has accessed an alert rule. -| `failure` | User is not authorized to access an alert rule. +| `success` | User has accessed an alert. +| `failure` | User is not authorized to access an alert. .2+| `alert_find` -| `success` | User has accessed an alert rule as part of a search operation. -| `failure` | User is not authorized to search for alert rules. +| `success` | User has accessed an alert as part of a search operation. +| `failure` | User is not authorized to search for alerts. + +.2+| `space_get` +| `success` | User has accessed a space. +| `failure` | User is not authorized to access a space. +.2+| `space_find` +| `success` | User has accessed a space as part of a search operation. +| `failure` | User is not authorized to search for spaces. 3+a| ===== Category: web diff --git a/x-pack/plugins/security/server/audit/audit_events.test.ts b/x-pack/plugins/security/server/audit/audit_events.test.ts index c826bb1d33f99..9bda628df66dc 100644 --- a/x-pack/plugins/security/server/audit/audit_events.test.ts +++ b/x-pack/plugins/security/server/audit/audit_events.test.ts @@ -11,6 +11,8 @@ import { savedObjectEvent, userLoginEvent, httpRequestEvent, + spaceAuditEvent, + SpaceAuditAction, } from './audit_events'; import { AuthenticationResult } from '../authentication'; import { mockAuthenticatedUser } from '../../common/model/authenticated_user.mock'; @@ -325,3 +327,88 @@ describe('#httpRequestEvent', () => { `); }); }); + +describe('#spaceAuditEvent', () => { + test('creates event with `unknown` outcome', () => { + expect( + spaceAuditEvent({ + action: SpaceAuditAction.CREATE, + outcome: EventOutcome.UNKNOWN, + savedObject: { type: 'space', id: 'SPACE_ID' }, + }) + ).toMatchInlineSnapshot(` + Object { + "error": undefined, + "event": Object { + "action": "space_create", + "category": "database", + "outcome": "unknown", + "type": "creation", + }, + "kibana": Object { + "saved_object": Object { + "id": "SPACE_ID", + "type": "space", + }, + }, + "message": "User is creating space [id=SPACE_ID]", + } + `); + }); + + test('creates event with `success` outcome', () => { + expect( + spaceAuditEvent({ + action: SpaceAuditAction.CREATE, + savedObject: { type: 'space', id: 'SPACE_ID' }, + }) + ).toMatchInlineSnapshot(` + Object { + "error": undefined, + "event": Object { + "action": "space_create", + "category": "database", + "outcome": "success", + "type": "creation", + }, + "kibana": Object { + "saved_object": Object { + "id": "SPACE_ID", + "type": "space", + }, + }, + "message": "User has created space [id=SPACE_ID]", + } + `); + }); + + test('creates event with `failure` outcome', () => { + expect( + spaceAuditEvent({ + action: SpaceAuditAction.CREATE, + savedObject: { type: 'space', id: 'SPACE_ID' }, + error: new Error('ERROR_MESSAGE'), + }) + ).toMatchInlineSnapshot(` + Object { + "error": Object { + "code": "Error", + "message": "ERROR_MESSAGE", + }, + "event": Object { + "action": "space_create", + "category": "database", + "outcome": "failure", + "type": "creation", + }, + "kibana": Object { + "saved_object": Object { + "id": "SPACE_ID", + "type": "space", + }, + }, + "message": "Failed attempt to create space [id=SPACE_ID]", + } + `); + }); +}); diff --git a/x-pack/plugins/security/server/audit/audit_events.ts b/x-pack/plugins/security/server/audit/audit_events.ts index 59184562b67ff..7f0dd39162adf 100644 --- a/x-pack/plugins/security/server/audit/audit_events.ts +++ b/x-pack/plugins/security/server/audit/audit_events.ts @@ -20,7 +20,7 @@ export interface AuditEvent { * Human readable message describing action, outcome and user. * * @example - * User [jdoe] logged in using basic provider [name=basic1] + * Failed attempt to login using basic provider [name=basic1] */ message: string; event: { @@ -208,7 +208,7 @@ export enum SavedObjectAction { type VerbsTuple = [string, string, string]; -const eventVerbs: Record = { +const savedObjectAuditVerbs: Record = { saved_object_create: ['create', 'creating', 'created'], saved_object_get: ['access', 'accessing', 'accessed'], saved_object_update: ['update', 'updating', 'updated'], @@ -223,7 +223,7 @@ const eventVerbs: Record = { ], }; -const eventTypes: Record = { +const savedObjectAuditTypes: Record = { saved_object_create: EventType.CREATION, saved_object_get: EventType.ACCESS, saved_object_update: EventType.CHANGE, @@ -252,13 +252,13 @@ export function savedObjectEvent({ error, }: SavedObjectEventParams): AuditEvent | undefined { const doc = savedObject ? `${savedObject.type} [id=${savedObject.id}]` : 'saved objects'; - const [present, progressive, past] = eventVerbs[action]; + const [present, progressive, past] = savedObjectAuditVerbs[action]; const message = error ? `Failed attempt to ${present} ${doc}` : outcome === EventOutcome.UNKNOWN ? `User is ${progressive} ${doc}` : `User has ${past} ${doc}`; - const type = eventTypes[action]; + const type = savedObjectAuditTypes[action]; if ( type === EventType.ACCESS && @@ -287,3 +287,67 @@ export function savedObjectEvent({ }, }; } + +export enum SpaceAuditAction { + CREATE = 'space_create', + GET = 'space_get', + UPDATE = 'space_update', + DELETE = 'space_delete', + FIND = 'space_find', +} + +const spaceAuditVerbs: Record = { + space_create: ['create', 'creating', 'created'], + space_get: ['access', 'accessing', 'accessed'], + space_update: ['update', 'updating', 'updated'], + space_delete: ['delete', 'deleting', 'deleted'], + space_find: ['access', 'accessing', 'accessed'], +}; + +const spaceAuditTypes: Record = { + space_create: EventType.CREATION, + space_get: EventType.ACCESS, + space_update: EventType.CHANGE, + space_delete: EventType.DELETION, + space_find: EventType.ACCESS, +}; + +export interface SpacesAuditEventParams { + action: SpaceAuditAction; + outcome?: EventOutcome; + savedObject?: NonNullable['saved_object']; + error?: Error; +} + +export function spaceAuditEvent({ + action, + savedObject, + outcome, + error, +}: SpacesAuditEventParams): AuditEvent { + const doc = savedObject ? `space [id=${savedObject.id}]` : 'spaces'; + const [present, progressive, past] = spaceAuditVerbs[action]; + const message = error + ? `Failed attempt to ${present} ${doc}` + : outcome === EventOutcome.UNKNOWN + ? `User is ${progressive} ${doc}` + : `User has ${past} ${doc}`; + const type = spaceAuditTypes[action]; + + return { + message, + event: { + action, + category: EventCategory.DATABASE, + type, + outcome: outcome ?? (error ? EventOutcome.FAILURE : EventOutcome.SUCCESS), + }, + kibana: { + saved_object: savedObject, + }, + error: error && { + code: error.name, + message: error.message, + }, + }; +} diff --git a/x-pack/plugins/security/server/audit/index.ts b/x-pack/plugins/security/server/audit/index.ts index 09f3df8b310e7..2c79c312beacc 100644 --- a/x-pack/plugins/security/server/audit/index.ts +++ b/x-pack/plugins/security/server/audit/index.ts @@ -13,6 +13,8 @@ export { userLoginEvent, httpRequestEvent, savedObjectEvent, + spaceAuditEvent, SavedObjectAction, + SpaceAuditAction, } from './audit_events'; export { SecurityAuditLogger } from './security_audit_logger'; diff --git a/x-pack/plugins/security/server/spaces/secure_spaces_client_wrapper.test.ts b/x-pack/plugins/security/server/spaces/secure_spaces_client_wrapper.test.ts index 90ee95f518089..24f26c3827056 100644 --- a/x-pack/plugins/security/server/spaces/secure_spaces_client_wrapper.test.ts +++ b/x-pack/plugins/security/server/spaces/secure_spaces_client_wrapper.test.ts @@ -9,6 +9,7 @@ import { httpServerMock } from '../../../../../src/core/server/mocks'; import { SecureSpacesClientWrapper } from './secure_spaces_client_wrapper'; import { spacesClientMock } from '../../../spaces/server/mocks'; +import { auditServiceMock } from '../audit/index.mock'; import { deepFreeze } from '@kbn/std'; import { Space } from '../../../spaces/server'; import { authorizationMock } from '../authorization/index.mock'; @@ -17,6 +18,7 @@ import { GetAllSpacesPurpose } from '../../../spaces/common/model/types'; import { CheckPrivilegesResponse } from '../authorization/types'; import { LegacySpacesAuditLogger } from './legacy_audit_logger'; import { SavedObjectsErrorHelpers } from 'src/core/server'; +import { AuditLogger, AuditEvent, EventOutcome, SpaceAuditAction } from '../audit'; interface Opts { securityEnabled?: boolean; @@ -62,12 +64,14 @@ const setup = ({ securityEnabled = false }: Opts = {}) => { spacesAuthorizationFailure: jest.fn(), spacesAuthorizationSuccess: jest.fn(), } as unknown) as jest.Mocked; + const auditLogger = auditServiceMock.create().asScoped(httpServerMock.createKibanaRequest()); const request = httpServerMock.createKibanaRequest(); const wrapper = new SecureSpacesClientWrapper( baseClient, request, authorization, + auditLogger, legacyAuditLogger ); return { @@ -75,6 +79,7 @@ const setup = ({ securityEnabled = false }: Opts = {}) => { wrapper, request, baseClient, + auditLogger, legacyAuditLogger, }; }; @@ -128,6 +133,27 @@ const expectSuccessAuditLogging = ( expect(auditLogger.spacesAuthorizationFailure).not.toHaveBeenCalled(); }; +const expectAuditEvent = ( + auditLogger: AuditLogger, + action: AuditEvent['event']['action'], + outcome: AuditEvent['event']['outcome'], + savedObject?: Required['kibana']['saved_object'] +) => { + expect(auditLogger.log).toHaveBeenCalledWith( + expect.objectContaining({ + event: expect.objectContaining({ + action, + outcome, + }), + kibana: savedObject + ? expect.objectContaining({ + saved_object: savedObject, + }) + : expect.anything(), + }) + ); +}; + describe('SecureSpacesClientWrapper', () => { describe('#getAll', () => { const savedObjects = [ @@ -158,7 +184,7 @@ describe('SecureSpacesClientWrapper', () => { ]; it('delegates to base client when security is not enabled', async () => { - const { wrapper, baseClient, authorization, legacyAuditLogger } = setup({ + const { wrapper, baseClient, authorization, auditLogger, legacyAuditLogger } = setup({ securityEnabled: false, }); @@ -168,6 +194,18 @@ describe('SecureSpacesClientWrapper', () => { expect(response).toEqual(spaces); expectNoAuthorizationCheck(authorization); expectNoAuditLogging(legacyAuditLogger); + expectAuditEvent(auditLogger, SpaceAuditAction.FIND, EventOutcome.SUCCESS, { + type: 'space', + id: spaces[0].id, + }); + expectAuditEvent(auditLogger, SpaceAuditAction.FIND, EventOutcome.SUCCESS, { + type: 'space', + id: spaces[1].id, + }); + expectAuditEvent(auditLogger, SpaceAuditAction.FIND, EventOutcome.SUCCESS, { + type: 'space', + id: spaces[2].id, + }); }); [ @@ -206,7 +244,14 @@ describe('SecureSpacesClientWrapper', () => { describe(`with purpose='${scenario.purpose}'`, () => { test(`throws Boom.forbidden when user isn't authorized for any spaces`, async () => { const username = 'some-user'; - const { authorization, wrapper, baseClient, request, legacyAuditLogger } = setup({ + const { + authorization, + wrapper, + baseClient, + request, + auditLogger, + legacyAuditLogger, + } = setup({ securityEnabled: true, }); @@ -240,11 +285,19 @@ describe('SecureSpacesClientWrapper', () => { ); expectForbiddenAuditLogging(legacyAuditLogger, username, 'getAll'); + expectAuditEvent(auditLogger, SpaceAuditAction.FIND, EventOutcome.FAILURE); }); test(`returns spaces that the user is authorized for`, async () => { const username = 'some-user'; - const { authorization, wrapper, baseClient, request, legacyAuditLogger } = setup({ + const { + authorization, + wrapper, + baseClient, + request, + auditLogger, + legacyAuditLogger, + } = setup({ securityEnabled: true, }); @@ -277,6 +330,10 @@ describe('SecureSpacesClientWrapper', () => { ); expectSuccessAuditLogging(legacyAuditLogger, username, 'getAll', [spaces[0].id]); + expectAuditEvent(auditLogger, SpaceAuditAction.FIND, EventOutcome.SUCCESS, { + type: 'space', + id: spaces[0].id, + }); }); }); }); @@ -284,7 +341,7 @@ describe('SecureSpacesClientWrapper', () => { describe('#get', () => { it('delegates to base client when security is not enabled', async () => { - const { wrapper, baseClient, authorization, legacyAuditLogger } = setup({ + const { wrapper, baseClient, authorization, auditLogger, legacyAuditLogger } = setup({ securityEnabled: false, }); @@ -294,15 +351,21 @@ describe('SecureSpacesClientWrapper', () => { expect(response).toEqual(spaces[0]); expectNoAuthorizationCheck(authorization); expectNoAuditLogging(legacyAuditLogger); + expectAuditEvent(auditLogger, SpaceAuditAction.GET, EventOutcome.SUCCESS, { + type: 'space', + id: spaces[0].id, + }); }); test(`throws a forbidden error when unauthorized`, async () => { const username = 'some_user'; const spaceId = 'default'; - const { wrapper, baseClient, authorization, legacyAuditLogger, request } = setup({ - securityEnabled: true, - }); + const { wrapper, baseClient, authorization, auditLogger, legacyAuditLogger, request } = setup( + { + securityEnabled: true, + } + ); const checkPrivileges = jest.fn().mockResolvedValue({ username, @@ -329,15 +392,21 @@ describe('SecureSpacesClientWrapper', () => { }); expectForbiddenAuditLogging(legacyAuditLogger, username, 'get', spaceId); + expectAuditEvent(auditLogger, SpaceAuditAction.GET, EventOutcome.FAILURE, { + type: 'space', + id: spaces[0].id, + }); }); it('returns the space when authorized', async () => { const username = 'some_user'; const spaceId = 'default'; - const { wrapper, baseClient, authorization, legacyAuditLogger, request } = setup({ - securityEnabled: true, - }); + const { wrapper, baseClient, authorization, auditLogger, legacyAuditLogger, request } = setup( + { + securityEnabled: true, + } + ); const checkPrivileges = jest.fn().mockResolvedValue({ username, @@ -363,6 +432,10 @@ describe('SecureSpacesClientWrapper', () => { }); expectSuccessAuditLogging(legacyAuditLogger, username, 'get', [spaceId]); + expectAuditEvent(auditLogger, SpaceAuditAction.GET, EventOutcome.SUCCESS, { + type: 'space', + id: spaceId, + }); }); }); @@ -374,7 +447,7 @@ describe('SecureSpacesClientWrapper', () => { }); it('delegates to base client when security is not enabled', async () => { - const { wrapper, baseClient, authorization, legacyAuditLogger } = setup({ + const { wrapper, baseClient, authorization, auditLogger, legacyAuditLogger } = setup({ securityEnabled: false, }); @@ -384,14 +457,20 @@ describe('SecureSpacesClientWrapper', () => { expect(response).toEqual(space); expectNoAuthorizationCheck(authorization); expectNoAuditLogging(legacyAuditLogger); + expectAuditEvent(auditLogger, SpaceAuditAction.CREATE, EventOutcome.UNKNOWN, { + type: 'space', + id: space.id, + }); }); test(`throws a forbidden error when unauthorized`, async () => { const username = 'some_user'; - const { wrapper, baseClient, authorization, legacyAuditLogger, request } = setup({ - securityEnabled: true, - }); + const { wrapper, baseClient, authorization, auditLogger, legacyAuditLogger, request } = setup( + { + securityEnabled: true, + } + ); const checkPrivileges = jest.fn().mockResolvedValue({ username, @@ -416,14 +495,20 @@ describe('SecureSpacesClientWrapper', () => { }); expectForbiddenAuditLogging(legacyAuditLogger, username, 'create'); + expectAuditEvent(auditLogger, SpaceAuditAction.CREATE, EventOutcome.FAILURE, { + type: 'space', + id: space.id, + }); }); it('creates the space when authorized', async () => { const username = 'some_user'; - const { wrapper, baseClient, authorization, legacyAuditLogger, request } = setup({ - securityEnabled: true, - }); + const { wrapper, baseClient, authorization, auditLogger, legacyAuditLogger, request } = setup( + { + securityEnabled: true, + } + ); const checkPrivileges = jest.fn().mockResolvedValue({ username, @@ -449,6 +534,10 @@ describe('SecureSpacesClientWrapper', () => { }); expectSuccessAuditLogging(legacyAuditLogger, username, 'create'); + expectAuditEvent(auditLogger, SpaceAuditAction.CREATE, EventOutcome.UNKNOWN, { + type: 'space', + id: space.id, + }); }); }); @@ -460,7 +549,7 @@ describe('SecureSpacesClientWrapper', () => { }); it('delegates to base client when security is not enabled', async () => { - const { wrapper, baseClient, authorization, legacyAuditLogger } = setup({ + const { wrapper, baseClient, authorization, auditLogger, legacyAuditLogger } = setup({ securityEnabled: false, }); @@ -470,14 +559,20 @@ describe('SecureSpacesClientWrapper', () => { expect(response).toEqual(space.id); expectNoAuthorizationCheck(authorization); expectNoAuditLogging(legacyAuditLogger); + expectAuditEvent(auditLogger, SpaceAuditAction.UPDATE, EventOutcome.UNKNOWN, { + type: 'space', + id: space.id, + }); }); test(`throws a forbidden error when unauthorized`, async () => { const username = 'some_user'; - const { wrapper, baseClient, authorization, legacyAuditLogger, request } = setup({ - securityEnabled: true, - }); + const { wrapper, baseClient, authorization, auditLogger, legacyAuditLogger, request } = setup( + { + securityEnabled: true, + } + ); const checkPrivileges = jest.fn().mockResolvedValue({ username, @@ -502,14 +597,20 @@ describe('SecureSpacesClientWrapper', () => { }); expectForbiddenAuditLogging(legacyAuditLogger, username, 'update'); + expectAuditEvent(auditLogger, SpaceAuditAction.UPDATE, EventOutcome.FAILURE, { + type: 'space', + id: space.id, + }); }); it('updates the space when authorized', async () => { const username = 'some_user'; - const { wrapper, baseClient, authorization, legacyAuditLogger, request } = setup({ - securityEnabled: true, - }); + const { wrapper, baseClient, authorization, auditLogger, legacyAuditLogger, request } = setup( + { + securityEnabled: true, + } + ); const checkPrivileges = jest.fn().mockResolvedValue({ username, @@ -535,6 +636,10 @@ describe('SecureSpacesClientWrapper', () => { }); expectSuccessAuditLogging(legacyAuditLogger, username, 'update'); + expectAuditEvent(auditLogger, SpaceAuditAction.UPDATE, EventOutcome.UNKNOWN, { + type: 'space', + id: space.id, + }); }); }); @@ -546,7 +651,7 @@ describe('SecureSpacesClientWrapper', () => { }); it('delegates to base client when security is not enabled', async () => { - const { wrapper, baseClient, authorization, legacyAuditLogger } = setup({ + const { wrapper, baseClient, authorization, auditLogger, legacyAuditLogger } = setup({ securityEnabled: false, }); @@ -555,14 +660,20 @@ describe('SecureSpacesClientWrapper', () => { expect(baseClient.delete).toHaveBeenCalledWith(space.id); expectNoAuthorizationCheck(authorization); expectNoAuditLogging(legacyAuditLogger); + expectAuditEvent(auditLogger, SpaceAuditAction.DELETE, EventOutcome.UNKNOWN, { + type: 'space', + id: space.id, + }); }); test(`throws a forbidden error when unauthorized`, async () => { const username = 'some_user'; - const { wrapper, baseClient, authorization, legacyAuditLogger, request } = setup({ - securityEnabled: true, - }); + const { wrapper, baseClient, authorization, auditLogger, legacyAuditLogger, request } = setup( + { + securityEnabled: true, + } + ); const checkPrivileges = jest.fn().mockResolvedValue({ username, @@ -587,14 +698,20 @@ describe('SecureSpacesClientWrapper', () => { }); expectForbiddenAuditLogging(legacyAuditLogger, username, 'delete'); + expectAuditEvent(auditLogger, SpaceAuditAction.DELETE, EventOutcome.FAILURE, { + type: 'space', + id: space.id, + }); }); it('deletes the space when authorized', async () => { const username = 'some_user'; - const { wrapper, baseClient, authorization, legacyAuditLogger, request } = setup({ - securityEnabled: true, - }); + const { wrapper, baseClient, authorization, auditLogger, legacyAuditLogger, request } = setup( + { + securityEnabled: true, + } + ); const checkPrivileges = jest.fn().mockResolvedValue({ username, @@ -618,6 +735,10 @@ describe('SecureSpacesClientWrapper', () => { }); expectSuccessAuditLogging(legacyAuditLogger, username, 'delete'); + expectAuditEvent(auditLogger, SpaceAuditAction.DELETE, EventOutcome.UNKNOWN, { + type: 'space', + id: space.id, + }); }); }); }); diff --git a/x-pack/plugins/security/server/spaces/secure_spaces_client_wrapper.ts b/x-pack/plugins/security/server/spaces/secure_spaces_client_wrapper.ts index bd65673422fc1..a0b174d979a8d 100644 --- a/x-pack/plugins/security/server/spaces/secure_spaces_client_wrapper.ts +++ b/x-pack/plugins/security/server/spaces/secure_spaces_client_wrapper.ts @@ -10,6 +10,7 @@ import { GetAllSpacesPurpose, GetSpaceResult } from '../../../spaces/common/mode import { Space, ISpacesClient } from '../../../spaces/server'; import { LegacySpacesAuditLogger } from './legacy_audit_logger'; import { AuthorizationServiceSetup } from '../authorization'; +import { AuditLogger, EventOutcome, SpaceAuditAction, spaceAuditEvent } from '../audit'; import { SecurityPluginSetup } from '..'; const PURPOSE_PRIVILEGE_MAP: Record< @@ -40,6 +41,7 @@ export class SecureSpacesClientWrapper implements ISpacesClient { private readonly spacesClient: ISpacesClient, private readonly request: KibanaRequest, private readonly authorization: AuthorizationServiceSetup, + private readonly auditLogger: AuditLogger, private readonly legacyAuditLogger: LegacySpacesAuditLogger ) {} @@ -50,6 +52,15 @@ export class SecureSpacesClientWrapper implements ISpacesClient { const allSpaces = await this.spacesClient.getAll({ purpose, includeAuthorizedPurposes }); if (!this.useRbac) { + allSpaces.forEach(({ id }) => + this.auditLogger.log( + spaceAuditEvent({ + action: SpaceAuditAction.FIND, + savedObject: { type: 'space', id }, + }) + ) + ); + return allSpaces; } @@ -108,62 +119,157 @@ export class SecureSpacesClientWrapper implements ISpacesClient { .filter(this.filterUnauthorizedSpaceResults); if (authorizedSpaces.length === 0) { + const error = Boom.forbidden(); + this.legacyAuditLogger.spacesAuthorizationFailure(username, 'getAll'); - throw Boom.forbidden(); // Note: there is a catch for this in `SpacesSavedObjectsClient.find`; if we get rid of this error, remove that too + this.auditLogger.log( + spaceAuditEvent({ + action: SpaceAuditAction.FIND, + error, + }) + ); + + throw error; // Note: there is a catch for this in `SpacesSavedObjectsClient.find`; if we get rid of this error, remove that too } const authorizedSpaceIds = authorizedSpaces.map((space) => space.id); + this.legacyAuditLogger.spacesAuthorizationSuccess(username, 'getAll', authorizedSpaceIds); + authorizedSpaces.forEach(({ id }) => + this.auditLogger.log( + spaceAuditEvent({ + action: SpaceAuditAction.FIND, + savedObject: { type: 'space', id }, + }) + ) + ); return authorizedSpaces; } public async get(id: string) { if (this.useRbac) { - await this.ensureAuthorizedAtSpace( - id, - this.authorization.actions.login, - 'get', - `Unauthorized to get ${id} space` - ); + try { + await this.ensureAuthorizedAtSpace( + id, + this.authorization.actions.login, + 'get', + `Unauthorized to get ${id} space` + ); + } catch (error) { + this.auditLogger.log( + spaceAuditEvent({ + action: SpaceAuditAction.GET, + savedObject: { type: 'space', id }, + error, + }) + ); + throw error; + } } - return this.spacesClient.get(id); + const space = this.spacesClient.get(id); + + this.auditLogger.log( + spaceAuditEvent({ + action: SpaceAuditAction.GET, + savedObject: { type: 'space', id }, + }) + ); + + return space; } public async create(space: Space) { if (this.useRbac) { - await this.ensureAuthorizedGlobally( - this.authorization.actions.space.manage, - 'create', - 'Unauthorized to create spaces' - ); + try { + await this.ensureAuthorizedGlobally( + this.authorization.actions.space.manage, + 'create', + 'Unauthorized to create spaces' + ); + } catch (error) { + this.auditLogger.log( + spaceAuditEvent({ + action: SpaceAuditAction.CREATE, + savedObject: { type: 'space', id: space.id }, + error, + }) + ); + throw error; + } } + this.auditLogger.log( + spaceAuditEvent({ + action: SpaceAuditAction.CREATE, + outcome: EventOutcome.UNKNOWN, + savedObject: { type: 'space', id: space.id }, + }) + ); + return this.spacesClient.create(space); } public async update(id: string, space: Space) { if (this.useRbac) { - await this.ensureAuthorizedGlobally( - this.authorization.actions.space.manage, - 'update', - 'Unauthorized to update spaces' - ); + try { + await this.ensureAuthorizedGlobally( + this.authorization.actions.space.manage, + 'update', + 'Unauthorized to update spaces' + ); + } catch (error) { + this.auditLogger.log( + spaceAuditEvent({ + action: SpaceAuditAction.UPDATE, + savedObject: { type: 'space', id }, + error, + }) + ); + throw error; + } } + this.auditLogger.log( + spaceAuditEvent({ + action: SpaceAuditAction.UPDATE, + outcome: EventOutcome.UNKNOWN, + savedObject: { type: 'space', id }, + }) + ); + return this.spacesClient.update(id, space); } public async delete(id: string) { if (this.useRbac) { - await this.ensureAuthorizedGlobally( - this.authorization.actions.space.manage, - 'delete', - 'Unauthorized to delete spaces' - ); + try { + await this.ensureAuthorizedGlobally( + this.authorization.actions.space.manage, + 'delete', + 'Unauthorized to delete spaces' + ); + } catch (error) { + this.auditLogger.log( + spaceAuditEvent({ + action: SpaceAuditAction.DELETE, + savedObject: { type: 'space', id }, + error, + }) + ); + throw error; + } } + this.auditLogger.log( + spaceAuditEvent({ + action: SpaceAuditAction.DELETE, + outcome: EventOutcome.UNKNOWN, + savedObject: { type: 'space', id }, + }) + ); + return this.spacesClient.delete(id); } diff --git a/x-pack/plugins/security/server/spaces/setup_spaces_client.test.ts b/x-pack/plugins/security/server/spaces/setup_spaces_client.test.ts index ee17f366583ba..6f0e41a162e58 100644 --- a/x-pack/plugins/security/server/spaces/setup_spaces_client.test.ts +++ b/x-pack/plugins/security/server/spaces/setup_spaces_client.test.ts @@ -77,4 +77,22 @@ describe('setupSpacesClient', () => { expect(savedObjects.createScopedRepository).toHaveBeenCalledTimes(1); expect(savedObjects.createScopedRepository).toHaveBeenCalledWith(request, ['space']); }); + + it('registers a spaces client wrapper with scoped audit logger', () => { + const authz = authorizationMock.create(); + const audit = auditServiceMock.create(); + const spaces = spacesMock.createSetup(); + + setupSpacesClient({ authz, audit, spaces }); + + expect(spaces.spacesClient.registerClientWrapper).toHaveBeenCalledTimes(1); + const [wrapper] = spaces.spacesClient.registerClientWrapper.mock.calls[0]; + + const request = httpServerMock.createKibanaRequest(); + + wrapper(request, {} as any); + + expect(audit.asScoped).toHaveBeenCalledTimes(1); + expect(audit.asScoped).toHaveBeenCalledWith(request); + }); }); diff --git a/x-pack/plugins/security/server/spaces/setup_spaces_client.ts b/x-pack/plugins/security/server/spaces/setup_spaces_client.ts index f9b105d630516..b285bdb4568af 100644 --- a/x-pack/plugins/security/server/spaces/setup_spaces_client.ts +++ b/x-pack/plugins/security/server/spaces/setup_spaces_client.ts @@ -33,6 +33,12 @@ export const setupSpacesClient = ({ audit, authz, spaces }: Deps) => { spacesClient.registerClientWrapper( (request, baseClient) => - new SecureSpacesClientWrapper(baseClient, request, authz, spacesAuditLogger) + new SecureSpacesClientWrapper( + baseClient, + request, + authz, + audit.asScoped(request), + spacesAuditLogger + ) ); };