Revert "Add audit logging to space deletion (#123378)"

This reverts commit 5819cfb1bf.

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

@@ -32,7 +32,6 @@ 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,7 +24,6 @@ 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,7 +46,6 @@ 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,7 +25,6 @@ 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,7 +24,6 @@ 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,7 +25,6 @@ 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,7 +24,6 @@ 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,6 +218,8 @@ 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',
@@ -234,6 +236,8 @@ 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',
@@ -268,6 +272,8 @@ 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,7 +68,6 @@ describe('#setup', () => {
       Object {
         "asScoped": [Function],
         "withoutRequest": Object {
-          "enabled": true,
           "log": [Function],
         },
       }

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

@@ -49,14 +49,6 @@ 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 {
@@ -130,8 +122,7 @@ export class AuditService {
     );
 
     // Record feature usage at a regular interval if enabled and license allows
-    const enabled = !!(config.enabled && config.appender);
-    if (enabled) {
+    if (config.enabled && config.appender) {
       license.features$.subscribe((features) => {
         clearInterval(this.usageIntervalId!);
         if (features.allowAuditLogging) {
@@ -178,7 +169,6 @@ export class AuditService {
           trace: { id: request.id },
         });
       },
-      enabled,
     });
 
     http.registerOnPostAuth((request, response, t) => {
@@ -190,7 +180,7 @@ export class AuditService {
 
     return {
       asScoped,
-      withoutRequest: { log, enabled },
+      withoutRequest: { log },
     };
   }
 

x-pack/plugins/security/server/audit/index.mock.ts

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

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

@@ -68,7 +68,6 @@ 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,11 +8,7 @@
 import { mockEnsureAuthorized } from './secure_spaces_client_wrapper.test.mocks';
 
 import { deepFreeze } from '@kbn/std';
-import type {
-  EcsEventOutcome,
-  SavedObjectsClientContract,
-  SavedObjectsFindResponse,
-} from 'src/core/server';
+import type { EcsEventOutcome, SavedObjectsClientContract } from 'src/core/server';
 import { SavedObjectsErrorHelpers } from 'src/core/server';
 import { httpServerMock } from 'src/core/server/mocks';
 
@@ -67,31 +63,6 @@ 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',
@@ -631,7 +602,7 @@ describe('SecureSpacesClientWrapper', () => {
       });
     });
 
-    it(`throws a forbidden error when unauthorized`, async () => {
+    test(`throws a forbidden error when unauthorized`, async () => {
       const username = 'some_user';
 
       const { wrapper, baseClient, authorization, auditLogger, request } = setup({
@@ -666,7 +637,7 @@ describe('SecureSpacesClientWrapper', () => {
       });
     });
 
-    it('deletes the space with all saved objects when authorized', async () => {
+    it('deletes the space when authorized', async () => {
       const username = 'some_user';
 
       const { wrapper, baseClient, authorization, auditLogger, request } = setup({
@@ -698,14 +669,6 @@ 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,7 +17,6 @@ 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';
@@ -247,10 +246,6 @@ 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 {
@@ -270,35 +265,6 @@ 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,15 +5,12 @@
  * 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 repositoryMock = savedObjectsRepositoryMock.create();
-  return {
+const createSpacesClientMock = () =>
+  ({
     getAll: jest.fn().mockResolvedValue([
       {
         id: DEFAULT_SPACE_ID,
@@ -31,11 +28,10 @@ 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,10 +90,7 @@ describe('#getAll', () => {
   });
 
   test(`throws Boom.badRequest when an invalid purpose is provided'`, async () => {
-    const mockDebugLogger = createMockDebugLogger();
-    const mockCallWithRequestRepository = savedObjectsRepositoryMock.create();
-    const mockConfig = createMockConfig();
-    const client = new SpacesClient(mockDebugLogger, mockConfig, mockCallWithRequestRepository, []);
+    const client = new SpacesClient(null as any, null as any, null as any);
     await expect(
       client.getAll({ purpose: 'invalid_purpose' as GetAllSpacesPurpose })
     ).rejects.toThrowErrorMatchingInlineSnapshot(`"unsupported space purpose: invalid_purpose"`);
@@ -125,7 +122,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);
 
@@ -184,7 +181,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);
 
@@ -210,7 +207,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"`
@@ -270,7 +267,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);
 
@@ -312,7 +309,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."`
@@ -327,7 +324,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);
 
@@ -342,12 +339,7 @@ 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,11 +8,7 @@
 import Boom from '@hapi/boom';
 import { omit } from 'lodash';
 
-import type {
-  ISavedObjectsPointInTimeFinder,
-  ISavedObjectsRepository,
-  SavedObject,
-} from 'src/core/server';
+import type { ISavedObjectsRepository, SavedObject } from 'src/core/server';
 
 import type {
   GetAllSpacesOptions,
@@ -62,13 +58,6 @@ 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.
@@ -89,8 +78,7 @@ export class SpacesClient implements ISpacesClient {
   constructor(
     private readonly debugLogger: (message: string) => void,
     private readonly config: ConfigType,
-    private readonly repository: ISavedObjectsRepository,
-    private readonly nonGlobalTypeNames: string[]
+    private readonly repository: ISavedObjectsRepository
   ) {}
 
   public async getAll(options: GetAllSpacesOptions = {}): Promise<GetSpaceResult[]> {
@@ -148,13 +136,6 @@ 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,28 +96,20 @@ 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, [...hiddenTypeNames, 'space']);
+        savedObjectsStart.createScopedRepository(request, ['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),
-          nonGlobalTypeNames
+          this.repositoryFactory!(request, coreStart.savedObjects)
         );
         if (this.clientWrapper) {
           return this.clientWrapper(request, baseClient);