audit space deletion (#124145)

* Add audit logging to space deletion

* Fix outcome

* Delete all non-global saved objects

* Added suggestions from code review

* Fix tests

* Fix wording

* Fix alert type errors

* updated snapshot

* update mocks

* Added suggestions from code review

* fix type error

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(() => {
@@ -1831,6 +1833,7 @@ describe('Authenticator', () => {
     let mockSessVal: SessionValue;
     const auditLogger = {
       log: jest.fn(),
+      enabled: true,
     };
 
     beforeEach(() => {
@@ -2009,6 +2012,7 @@ describe('Authenticator', () => {
     let mockSessVal: SessionValue;
     const auditLogger = {
       log: jest.fn(),
+      enabled: true,
     };
 
     beforeEach(() => {
@@ -2145,6 +2149,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,
 };

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

@@ -77,7 +77,7 @@ describe('#getAll', () => {
     } as any);
     const mockConfig = createMockConfig({ maxSpaces: 1234 });
 
-    const client = new SpacesClient(mockDebugLogger, mockConfig, mockCallWithRequestRepository);
+    const client = new SpacesClient(mockDebugLogger, mockConfig, mockCallWithRequestRepository, []);
     const actualSpaces = await client.getAll();
 
     expect(actualSpaces).toEqual(expectedSpaces);
@@ -90,7 +90,10 @@ describe('#getAll', () => {
   });
 
   test(`throws Boom.badRequest when an invalid purpose is provided'`, async () => {
-    const client = new SpacesClient(null as any, null as any, null as any);
+    const mockDebugLogger = createMockDebugLogger();
+    const mockCallWithRequestRepository = savedObjectsRepositoryMock.create();
+    const mockConfig = createMockConfig();
+    const client = new SpacesClient(mockDebugLogger, mockConfig, mockCallWithRequestRepository, []);
     await expect(
       client.getAll({ purpose: 'invalid_purpose' as GetAllSpacesPurpose })
     ).rejects.toThrowErrorMatchingInlineSnapshot(`"unsupported space purpose: invalid_purpose"`);
@@ -122,7 +125,7 @@ describe('#get', () => {
     const mockCallWithRequestRepository = savedObjectsRepositoryMock.create();
     mockCallWithRequestRepository.get.mockResolvedValue(savedObject);
 
-    const client = new SpacesClient(mockDebugLogger, mockConfig, mockCallWithRequestRepository);
+    const client = new SpacesClient(mockDebugLogger, mockConfig, mockCallWithRequestRepository, []);
     const id = savedObject.id;
     const actualSpace = await client.get(id);
 
@@ -181,7 +184,7 @@ describe('#create', () => {
 
     const mockConfig = createMockConfig({ maxSpaces });
 
-    const client = new SpacesClient(mockDebugLogger, mockConfig, mockCallWithRequestRepository);
+    const client = new SpacesClient(mockDebugLogger, mockConfig, mockCallWithRequestRepository, []);
 
     const actualSpace = await client.create(spaceToCreate);
 
@@ -207,7 +210,7 @@ describe('#create', () => {
 
     const mockConfig = createMockConfig({ maxSpaces });
 
-    const client = new SpacesClient(mockDebugLogger, mockConfig, mockCallWithRequestRepository);
+    const client = new SpacesClient(mockDebugLogger, mockConfig, mockCallWithRequestRepository, []);
 
     expect(client.create(spaceToCreate)).rejects.toThrowErrorMatchingInlineSnapshot(
       `"Unable to create Space, this exceeds the maximum number of spaces set by the xpack.spaces.maxSpaces setting"`
@@ -267,7 +270,7 @@ describe('#update', () => {
     const mockCallWithRequestRepository = savedObjectsRepositoryMock.create();
     mockCallWithRequestRepository.get.mockResolvedValue(savedObject);
 
-    const client = new SpacesClient(mockDebugLogger, mockConfig, mockCallWithRequestRepository);
+    const client = new SpacesClient(mockDebugLogger, mockConfig, mockCallWithRequestRepository, []);
     const id = savedObject.id;
     const actualSpace = await client.update(id, spaceToUpdate);
 
@@ -309,7 +312,7 @@ describe('#delete', () => {
     const mockCallWithRequestRepository = savedObjectsRepositoryMock.create();
     mockCallWithRequestRepository.get.mockResolvedValue(reservedSavedObject);
 
-    const client = new SpacesClient(mockDebugLogger, mockConfig, mockCallWithRequestRepository);
+    const client = new SpacesClient(mockDebugLogger, mockConfig, mockCallWithRequestRepository, []);
 
     expect(client.delete(id)).rejects.toThrowErrorMatchingInlineSnapshot(
       `"The foo space cannot be deleted because it is reserved."`
@@ -324,7 +327,7 @@ describe('#delete', () => {
     const mockCallWithRequestRepository = savedObjectsRepositoryMock.create();
     mockCallWithRequestRepository.get.mockResolvedValue(notReservedSavedObject);
 
-    const client = new SpacesClient(mockDebugLogger, mockConfig, mockCallWithRequestRepository);
+    const client = new SpacesClient(mockDebugLogger, mockConfig, mockCallWithRequestRepository, []);
 
     await client.delete(id);
 
@@ -339,7 +342,12 @@ describe('#delete', () => {
       const mockConfig = createMockConfig();
       const mockCallWithRequestRepository = savedObjectsRepositoryMock.create();
 
-      const client = new SpacesClient(mockDebugLogger, mockConfig, mockCallWithRequestRepository);
+      const client = new SpacesClient(
+        mockDebugLogger,
+        mockConfig,
+        mockCallWithRequestRepository,
+        []
+      );
       const aliases = [
         { targetSpace: 'space1', targetType: 'foo', sourceId: '123' },
         { targetSpace: 'space2', targetType: 'bar', sourceId: '456' },

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

@@ -8,7 +8,11 @@
 import Boom from '@hapi/boom';
 import { omit } from 'lodash';
 
-import type { ISavedObjectsRepository, SavedObject } from 'src/core/server';
+import type {
+  ISavedObjectsPointInTimeFinder,
+  ISavedObjectsRepository,
+  SavedObject,
+} from 'src/core/server';
 
 import type {
   GetAllSpacesOptions,
@@ -58,6 +62,13 @@ export interface ISpacesClient {
    */
   update(id: string, space: Space): Promise<Space>;
 
+  /**
+   * Returns a {@link ISavedObjectsPointInTimeFinder} to help page through
+   * saved objects within the specified space.
+   * @param id the id of the space to search.
+   */
+  createSavedObjectFinder(id: string): ISavedObjectsPointInTimeFinder<unknown, unknown>;
+
   /**
    * Deletes a space, and all saved objects belonging to that space.
    * @param id the id of the space to delete.
@@ -78,7 +89,8 @@ export class SpacesClient implements ISpacesClient {
   constructor(
     private readonly debugLogger: (message: string) => void,
     private readonly config: ConfigType,
-    private readonly repository: ISavedObjectsRepository
+    private readonly repository: ISavedObjectsRepository,
+    private readonly nonGlobalTypeNames: string[]
   ) {}
 
   public async getAll(options: GetAllSpacesOptions = {}): Promise<GetSpaceResult[]> {
@@ -136,6 +148,13 @@ export class SpacesClient implements ISpacesClient {
     return this.transformSavedObjectToSpace(updatedSavedObject);
   }
 
+  public createSavedObjectFinder(id: string) {
+    return this.repository.createPointInTimeFinder({
+      type: this.nonGlobalTypeNames,
+      namespaces: [id],
+    });
+  }
+
   public async delete(id: string) {
     const existingSavedObject = await this.repository.get('space', id);
     if (isReservedSpace(this.transformSavedObjectToSpace(existingSavedObject))) {

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

@@ -96,20 +96,28 @@ export class SpacesClientService {
   }
 
   public start(coreStart: CoreStart): SpacesClientServiceStart {
+    const nonGlobalTypes = coreStart.savedObjects
+      .getTypeRegistry()
+      .getAllTypes()
+      .filter((x) => x.namespaceType !== 'agnostic');
+    const nonGlobalTypeNames = nonGlobalTypes.map((x) => x.name);
+
     if (!this.repositoryFactory) {
+      const hiddenTypeNames = nonGlobalTypes.filter((x) => x.hidden).map((x) => x.name);
       this.repositoryFactory = (request, savedObjectsStart) =>
-        savedObjectsStart.createScopedRepository(request, ['space']);
+        savedObjectsStart.createScopedRepository(request, [...hiddenTypeNames, 'space']);
     }
+
     return {
       createSpacesClient: (request: KibanaRequest) => {
         if (!this.config) {
           throw new Error('Initialization error: spaces config is not available');
         }
-
         const baseClient = new SpacesClient(
           this.debugLogger,
           this.config,
-          this.repositoryFactory!(request, coreStart.savedObjects)
+          this.repositoryFactory!(request, coreStart.savedObjects),
+          nonGlobalTypeNames
         );
         if (this.clientWrapper) {
           return this.clientWrapper(request, baseClient);