From 07a6c940c1f90469ce32bd642ea120082137233f Mon Sep 17 00:00:00 2001 From: Thom Heymann Date: Thu, 20 Jan 2022 16:02:52 +0000 Subject: [PATCH] Added suggestions from code review --- .../security/server/audit/audit_service.ts | 14 ++++++- .../security/server/audit/index.mock.ts | 2 + .../spaces/secure_spaces_client_wrapper.ts | 37 ++++++++++--------- 3 files changed, 34 insertions(+), 19 deletions(-) diff --git a/x-pack/plugins/security/server/audit/audit_service.ts b/x-pack/plugins/security/server/audit/audit_service.ts index a29ec221b347..fd39abd3df78 100644 --- a/x-pack/plugins/security/server/audit/audit_service.ts +++ b/x-pack/plugins/security/server/audit/audit_service.ts @@ -49,6 +49,14 @@ export interface AuditLogger { * ``` */ log: (event: AuditEvent | undefined) => void; + + /** + * Boolean to check whether audit logging is enabled or not. + * + * Useful for skipping resource-intense operations that don't need to be performed when audit + * logging is disabled. + */ + readonly enabled: boolean; } export interface AuditServiceSetup { @@ -122,7 +130,8 @@ export class AuditService { ); // Record feature usage at a regular interval if enabled and license allows - if (config.enabled && config.appender) { + const enabled = !!(config.enabled && config.appender); + if (enabled) { license.features$.subscribe((features) => { clearInterval(this.usageIntervalId!); if (features.allowAuditLogging) { @@ -169,6 +178,7 @@ export class AuditService { trace: { id: request.id }, }); }, + enabled, }); http.registerOnPostAuth((request, response, t) => { @@ -180,7 +190,7 @@ export class AuditService { return { asScoped, - withoutRequest: { log }, + withoutRequest: { log, enabled }, }; } diff --git a/x-pack/plugins/security/server/audit/index.mock.ts b/x-pack/plugins/security/server/audit/index.mock.ts index c84faacff014..6ac9108b51a8 100644 --- a/x-pack/plugins/security/server/audit/index.mock.ts +++ b/x-pack/plugins/security/server/audit/index.mock.ts @@ -13,9 +13,11 @@ export const auditServiceMock = { getLogger: jest.fn(), asScoped: jest.fn().mockReturnValue({ log: jest.fn(), + enabled: true, }), withoutRequest: { log: jest.fn(), + enabled: true, }, } as jest.Mocked>; }, 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 342ec4e1b58a..52fc1ea84a66 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 @@ -270,24 +270,27 @@ export class SecureSpacesClientWrapper implements ISpacesClient { } // Fetch saved objects to be removed for audit logging - const finder = this.spacesClient.createSavedObjectFinder(id); - try { - for await (const response of finder.find()) { - response.saved_objects.forEach((savedObject) => { - const isOnlySpace = !savedObject.namespaces || savedObject.namespaces.length === 1; - this.auditLogger.log( - savedObjectEvent({ - action: isOnlySpace - ? SavedObjectAction.DELETE - : SavedObjectAction.UPDATE_OBJECTS_SPACES, - outcome: 'unknown', - savedObject: { type: savedObject.type, id: savedObject.id }, - }) - ); - }); + if (this.auditLogger.enabled) { + const finder = this.spacesClient.createSavedObjectFinder(id); + try { + for await (const response of finder.find()) { + response.saved_objects.forEach((savedObject) => { + const isOnlySpace = !savedObject.namespaces || savedObject.namespaces.length === 1; + this.auditLogger.log( + savedObjectEvent({ + action: isOnlySpace + ? SavedObjectAction.DELETE + : SavedObjectAction.UPDATE_OBJECTS_SPACES, + outcome: 'unknown', + savedObject: { type: savedObject.type, id: savedObject.id }, + deleteFromSpaces: [id], + }) + ); + }); + } + } finally { + await finder.close(); } - } finally { - await finder.close(); } this.auditLogger.log(