Add audit logging to space deletion (elastic#123378)

* Add audit logging to space deletion

* Fix outcome

* Delete all non-global saved objects

* Added suggestions from code review

* Fix tests

x-pack/plugins/cases/server/authorization/audit_logger.test.ts

@@ -32,6 +32,7 @@ describe('audit_logger', () => {
   describe('log function', () => {
     const mockLogger: jest.Mocked<AuditLogger> = {
       log: jest.fn(),
+      enabled: true,
     };
 
     let logger: AuthorizationAuditLogger;

x-pack/plugins/cases/server/authorization/authorization.test.ts

@@ -24,6 +24,7 @@ describe('authorization', () => {
     request = httpServerMock.createKibanaRequest();
     mockLogger = {
       log: jest.fn(),
+      enabled: true,
     };
   });
 

x-pack/plugins/rule_registry/server/alert_data_client/alerts_client_factory.test.ts

@@ -46,6 +46,7 @@ const fakeRequest = {
 
 const auditLogger = {
   log: jest.fn(),
+  enabled: true,
 } as jest.Mocked<AuditLogger>;
 
 describe('AlertsClientFactory', () => {

x-pack/plugins/rule_registry/server/alert_data_client/tests/bulk_update.test.ts

@@ -25,6 +25,7 @@ const alertingAuthMock = alertingAuthorizationMock.create();
 const esClientMock = elasticsearchClientMock.createElasticsearchClient();
 const auditLogger = {
   log: jest.fn(),
+  enabled: true,
 } as jest.Mocked<AuditLogger>;
 
 const alertsClientParams: jest.Mocked<ConstructorOptions> = {

x-pack/plugins/rule_registry/server/alert_data_client/tests/find_alerts.test.ts

@@ -24,6 +24,7 @@ const alertingAuthMock = alertingAuthorizationMock.create();
 const esClientMock = elasticsearchClientMock.createElasticsearchClient();
 const auditLogger = {
   log: jest.fn(),
+  enabled: true,
 } as jest.Mocked<AuditLogger>;
 
 const alertsClientParams: jest.Mocked<ConstructorOptions> = {

x-pack/plugins/rule_registry/server/alert_data_client/tests/get.test.ts

@@ -25,6 +25,7 @@ const alertingAuthMock = alertingAuthorizationMock.create();
 const esClientMock = elasticsearchClientMock.createElasticsearchClient();
 const auditLogger = {
   log: jest.fn(),
+  enabled: true,
 } as jest.Mocked<AuditLogger>;
 
 const alertsClientParams: jest.Mocked<ConstructorOptions> = {

x-pack/plugins/rule_registry/server/alert_data_client/tests/update.test.ts

@@ -24,6 +24,7 @@ const alertingAuthMock = alertingAuthorizationMock.create();
 const esClientMock = elasticsearchClientMock.createElasticsearchClient();
 const auditLogger = {
   log: jest.fn(),
+  enabled: true,
 } as jest.Mocked<AuditLogger>;
 
 const alertsClientParams: jest.Mocked<ConstructorOptions> = {

x-pack/plugins/security/server/audit/audit_events.ts

@@ -218,8 +218,6 @@ export enum SavedObjectAction {
   UPDATE = 'saved_object_update',
   DELETE = 'saved_object_delete',
   FIND = 'saved_object_find',
-  ADD_TO_SPACES = 'saved_object_add_to_spaces',
-  DELETE_FROM_SPACES = 'saved_object_delete_from_spaces',
   REMOVE_REFERENCES = 'saved_object_remove_references',
   OPEN_POINT_IN_TIME = 'saved_object_open_point_in_time',
   CLOSE_POINT_IN_TIME = 'saved_object_close_point_in_time',
@@ -236,8 +234,6 @@ const savedObjectAuditVerbs: Record<SavedObjectAction, VerbsTuple> = {
   saved_object_update: ['update', 'updating', 'updated'],
   saved_object_delete: ['delete', 'deleting', 'deleted'],
   saved_object_find: ['access', 'accessing', 'accessed'],
-  saved_object_add_to_spaces: ['update', 'updating', 'updated'],
-  saved_object_delete_from_spaces: ['update', 'updating', 'updated'],
   saved_object_open_point_in_time: [
     'open point-in-time',
     'opening point-in-time',
@@ -272,8 +268,6 @@ const savedObjectAuditTypes: Record<SavedObjectAction, EcsEventType> = {
   saved_object_update: 'change',
   saved_object_delete: 'deletion',
   saved_object_find: 'access',
-  saved_object_add_to_spaces: 'change',
-  saved_object_delete_from_spaces: 'change',
   saved_object_open_point_in_time: 'creation',
   saved_object_close_point_in_time: 'deletion',
   saved_object_remove_references: 'change',

x-pack/plugins/security/server/audit/audit_service.test.ts

@@ -68,6 +68,7 @@ describe('#setup', () => {
       Object {
         "asScoped": [Function],
         "withoutRequest": Object {
+          "enabled": true,
           "log": [Function],
         },
       }

x-pack/plugins/security/server/audit/audit_service.ts

@@ -49,6 +49,14 @@ export interface AuditLogger {
    * ```
    */
   log: (event: AuditEvent | undefined) => void;
+
+  /**
+   * Indicates 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 },
     };
   }
 

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<ReturnType<AuditService['setup']>>;
   },

x-pack/plugins/security/server/authentication/authenticator.test.ts

@@ -264,6 +264,7 @@ describe('Authenticator', () => {
     let mockSessVal: SessionValue;
     const auditLogger = {
       log: jest.fn(),
+      enabled: true,
     };
 
     beforeEach(() => {
@@ -1094,6 +1095,7 @@ describe('Authenticator', () => {
     let mockSessVal: SessionValue;
     const auditLogger = {
       log: jest.fn(),
+      enabled: true,
     };
 
     beforeEach(() => {
@@ -2009,6 +2011,7 @@ describe('Authenticator', () => {
     let mockSessVal: SessionValue;
     const auditLogger = {
       log: jest.fn(),
+      enabled: true,
     };
 
     beforeEach(() => {
@@ -2145,6 +2148,7 @@ describe('Authenticator', () => {
     let mockSessionValue: SessionValue;
     const auditLogger = {
       log: jest.fn(),
+      enabled: true,
     };
 
     beforeEach(() => {

x-pack/plugins/security/server/plugin.test.ts

@@ -68,6 +68,7 @@ describe('Security Plugin', () => {
           "audit": Object {
             "asScoped": [Function],
             "withoutRequest": Object {
+              "enabled": false,
               "log": [Function],
             },
           },

x-pack/plugins/security/server/spaces/secure_spaces_client_wrapper.test.ts

@@ -8,7 +8,11 @@
 import { mockEnsureAuthorized } from './secure_spaces_client_wrapper.test.mocks';
 
 import { deepFreeze } from '@kbn/std';
-import type { EcsEventOutcome, SavedObjectsClientContract } from 'src/core/server';
+import type {
+  EcsEventOutcome,
+  SavedObjectsClientContract,
+  SavedObjectsFindResponse,
+} from 'src/core/server';
 import { SavedObjectsErrorHelpers } from 'src/core/server';
 import { httpServerMock } from 'src/core/server/mocks';
 
@@ -63,6 +67,31 @@ const setup = ({ securityEnabled = false }: Opts = {}) => {
     return space;
   });
 
+  baseClient.createSavedObjectFinder.mockImplementation(() => ({
+    async *find() {
+      yield {
+        saved_objects: [
+          {
+            namespaces: ['*'],
+            type: 'dashboard',
+            id: '1',
+          },
+          {
+            namespaces: ['existing_space'],
+            type: 'dashboard',
+            id: '2',
+          },
+          {
+            namespaces: ['default', 'existing_space'],
+            type: 'dashboard',
+            id: '3',
+          },
+        ],
+      } as SavedObjectsFindResponse<unknown, unknown>;
+    },
+    async close() {},
+  }));
+
   const authorization = authorizationMock.create({
     version: 'unit-test',
     applicationName: 'kibana',
@@ -602,7 +631,7 @@ describe('SecureSpacesClientWrapper', () => {
       });
     });
 
-    test(`throws a forbidden error when unauthorized`, async () => {
+    it(`throws a forbidden error when unauthorized`, async () => {
       const username = 'some_user';
 
       const { wrapper, baseClient, authorization, auditLogger, request } = setup({
@@ -637,7 +666,7 @@ describe('SecureSpacesClientWrapper', () => {
       });
     });
 
-    it('deletes the space when authorized', async () => {
+    it('deletes the space with all saved objects when authorized', async () => {
       const username = 'some_user';
 
       const { wrapper, baseClient, authorization, auditLogger, request } = setup({
@@ -669,6 +698,14 @@ describe('SecureSpacesClientWrapper', () => {
         type: 'space',
         id: space.id,
       });
+      expectAuditEvent(auditLogger, SavedObjectAction.DELETE, 'unknown', {
+        type: 'dashboard',
+        id: '2',
+      });
+      expectAuditEvent(auditLogger, SavedObjectAction.UPDATE_OBJECTS_SPACES, 'unknown', {
+        type: 'dashboard',
+        id: '3',
+      });
     });
   });
 

x-pack/plugins/security/server/spaces/secure_spaces_client_wrapper.ts

@@ -17,6 +17,7 @@ import type {
   LegacyUrlAliasTarget,
   Space,
 } from '../../../spaces/server';
+import { ALL_SPACES_ID } from '../../common/constants';
 import type { AuditLogger } from '../audit';
 import { SavedObjectAction, savedObjectEvent, SpaceAuditAction, spaceAuditEvent } from '../audit';
 import type { AuthorizationServiceSetup } from '../authorization';
@@ -246,6 +247,10 @@ export class SecureSpacesClientWrapper implements ISpacesClient {
     return this.spacesClient.update(id, space);
   }
 
+  public createSavedObjectFinder(id: string) {
+    return this.spacesClient.createSavedObjectFinder(id);
+  }
+
   public async delete(id: string) {
     if (this.useRbac) {
       try {
@@ -265,6 +270,35 @@ export class SecureSpacesClientWrapper implements ISpacesClient {
       }
     }
 
+    // Fetch saved objects to be removed for audit logging
+    if (this.auditLogger.enabled) {
+      const finder = this.spacesClient.createSavedObjectFinder(id);
+      try {
+        for await (const response of finder.find()) {
+          response.saved_objects.forEach((savedObject) => {
+            const { namespaces = [] } = savedObject;
+            const isOnlySpace = namespaces.length === 1; // We can always rely on the `namespaces` field having >=1 element
+            if (namespaces.includes(ALL_SPACES_ID) && !namespaces.includes(id)) {
+              // This object exists in All Spaces and its `namespaces` field isn't going to change; there's nothing to audit
+              return;
+            }
+            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();
+      }
+    }
+
     this.auditLogger.log(
       spaceAuditEvent({
         action: SpaceAuditAction.DELETE,

x-pack/plugins/spaces/server/spaces_client/spaces_client.mock.ts

@@ -5,12 +5,15 @@
  * 2.0.
  */
 
+import { savedObjectsRepositoryMock } from 'src/core/server/mocks';
+
 import type { Space } from '../../common';
 import { DEFAULT_SPACE_ID } from '../../common/constants';
 import type { SpacesClient } from './spaces_client';
 
-const createSpacesClientMock = () =>
-  ({
+const createSpacesClientMock = () => {
+  const repositoryMock = savedObjectsRepositoryMock.create();
+  return {
     getAll: jest.fn().mockResolvedValue([
       {
         id: DEFAULT_SPACE_ID,
@@ -28,10 +31,11 @@ const createSpacesClientMock = () =>
     }),
     create: jest.fn().mockImplementation((space: Space) => Promise.resolve(space)),
     update: jest.fn().mockImplementation((space: Space) => Promise.resolve(space)),
+    createSavedObjectFinder: repositoryMock.createPointInTimeFinder,
     delete: jest.fn(),
     disableLegacyUrlAliases: jest.fn(),
-  } as unknown as jest.Mocked<SpacesClient>);
-
+  } as unknown as jest.Mocked<SpacesClient>;
+};
 export const spacesClientMock = {
   create: createSpacesClientMock,
 };