[Security Solution][Detections]Alerts migrations can be finalized/cleaned up in all spaces (#93809)

* Retrieve SOs by ID in a space-aware manner by using bulkGet

We were previously using a manual invocation of find(), which was a)
tied to the current implementation of how SOs generate their _ids, and
b) didn't respect spaces.

By replacing this with a call to bulkGet, which automatically respects
the space of the current request, and which abstracts away the building
of the actual _id based on the SO ID and type, we address the issues
above.

* Surface SO errors to the finalize/delete APIs

Now that we're using bulkGet, we receive an object with errors if the
object is not found, which by default breaks our subsequent validation.
In order to provider better UX, we re-raise the first of these errors
that we find, if present, and return that to the user.

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

x-pack/plugins/security_solution/server/lib/detection_engine/migrations/get_migration_saved_objects_by_id.test.ts

@@ -0,0 +1,65 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import { savedObjectsClientMock } from 'src/core/server/mocks';
+import {
+  getSignalsMigrationSavedObjectMock,
+  getSignalsMigrationSavedObjectErrorMock,
+} from './saved_objects_schema.mock';
+import { getMigrationSavedObjectsById } from './get_migration_saved_objects_by_id';
+
+describe('getMigrationSavedObjectsById', () => {
+  let ids: string[];
+  let soClient: ReturnType<typeof savedObjectsClientMock.create>;
+
+  beforeEach(() => {
+    ids = ['id1'];
+    soClient = savedObjectsClientMock.create();
+  });
+
+  it('resolves an array of objects, if valid', async () => {
+    // @ts-expect-error stubbing our SO call
+    soClient.bulkGet.mockResolvedValue({ saved_objects: [getSignalsMigrationSavedObjectMock()] });
+    const result = await getMigrationSavedObjectsById({
+      ids,
+      soClient,
+    });
+
+    expect(result).toEqual([getSignalsMigrationSavedObjectMock()]);
+  });
+
+  it('rejects if SO client throws', () => {
+    const error = new Error('whoops');
+    soClient.bulkGet.mockRejectedValue(error);
+
+    return expect(getMigrationSavedObjectsById({ ids, soClient })).rejects.toThrow(error);
+  });
+
+  it('throws a 404 error if the response includes a 404', async () => {
+    soClient.bulkGet.mockResolvedValue({
+      saved_objects: [
+        // @ts-expect-error stubbing our SO call
+        getSignalsMigrationSavedObjectErrorMock({ statusCode: 404, message: 'not found' }),
+      ],
+    });
+
+    return expect(getMigrationSavedObjectsById({ ids, soClient })).rejects.toThrow(
+      expect.objectContaining({ statusCode: 404 })
+    );
+  });
+
+  it('rejects if response is invalid', () => {
+    // @ts-expect-error intentionally breaking the type
+    const badSavedObject = getSignalsMigrationSavedObjectMock({ destinationIndex: 4 });
+    // @ts-expect-error stubbing our SO call
+    soClient.bulkGet.mockResolvedValue({ saved_objects: [badSavedObject] });
+
+    return expect(() => getMigrationSavedObjectsById({ ids, soClient })).rejects.toThrow(
+      'Invalid value "4" supplied to "attributes,destinationIndex"'
+    );
+  });
+});

x-pack/plugins/security_solution/server/lib/detection_engine/migrations/get_migration_saved_objects_by_id.ts

@@ -5,10 +5,21 @@
  * 2.0.
  */
 
+import { fold } from 'fp-ts/lib/Either';
+import { pipe } from 'fp-ts/lib/pipeable';
+
 import { SavedObjectsClientContract } from 'src/core/server';
-import { findMigrationSavedObjects } from './find_migration_saved_objects';
-import { signalsMigrationType } from './saved_objects';
-import { SignalsMigrationSO } from './saved_objects_schema';
+import { validateEither } from '../../../../common/validate';
+import { signalsMigrationSOClient } from './saved_objects_client';
+import { SignalsMigrationSO, signalsMigrationSOs } from './saved_objects_schema';
+
+class MigrationResponseError extends Error {
+  public readonly statusCode: number;
+  constructor(message: string, statusCode: number) {
+    super(message);
+    this.statusCode = statusCode;
+  }
+}
 
 /**
  * Retrieves a list of migrations SOs by their ID
@@ -26,13 +37,22 @@ export const getMigrationSavedObjectsById = async ({
 }: {
   ids: string[];
   soClient: SavedObjectsClientContract;
-}): Promise<SignalsMigrationSO[]> =>
-  findMigrationSavedObjects({
-    soClient,
-    options: {
-      search: ids.map((id) => `${signalsMigrationType}:${id}`).join(' OR '),
-      rootSearchFields: ['_id'],
-      sortField: 'updated',
-      sortOrder: 'desc',
-    },
-  });
+}): Promise<SignalsMigrationSO[]> => {
+  const client = signalsMigrationSOClient(soClient);
+  const objects = ids.map((id) => ({ id }));
+  const { saved_objects: migrations } = await client.bulkGet(objects);
+  const error = migrations.find((migration) => migration.error)?.error;
+
+  if (error) {
+    throw new MigrationResponseError(error.message, error.statusCode);
+  }
+
+  return pipe(
+    migrations,
+    (ms) => validateEither(signalsMigrationSOs, ms),
+    fold(
+      (e) => Promise.reject(e),
+      (a) => Promise.resolve(a)
+    )
+  );
+};

x-pack/plugins/security_solution/server/lib/detection_engine/migrations/saved_objects_client.mock.ts

@@ -9,6 +9,7 @@ import { SignalsMigrationSOClient } from './saved_objects_client';
 
 const create = () =>
   ({
+    bulkGet: jest.fn(),
     create: jest.fn(),
     delete: jest.fn(),
     find: jest.fn(),

x-pack/plugins/security_solution/server/lib/detection_engine/migrations/saved_objects_client.ts

@@ -11,11 +11,18 @@ import {
   SavedObjectsUpdateResponse,
   SavedObjectsFindOptions,
   SavedObjectsFindResponse,
+  SavedObjectsBulkGetObject,
+  SavedObjectsBulkResponse,
+  SavedObjectsBaseOptions,
 } from 'src/core/server';
 import { signalsMigrationType } from './saved_objects';
 import { SignalsMigrationSOAttributes } from './saved_objects_schema';
 
 export interface SignalsMigrationSOClient {
+  bulkGet: (
+    objects: Array<Omit<SavedObjectsBulkGetObject, 'type'>>,
+    options?: SavedObjectsBaseOptions
+  ) => Promise<SavedObjectsBulkResponse<SignalsMigrationSOAttributes>>;
   find: (
     options?: Omit<SavedObjectsFindOptions, 'type'>
   ) => Promise<SavedObjectsFindResponse<SignalsMigrationSOAttributes>>;
@@ -32,6 +39,11 @@ export interface SignalsMigrationSOClient {
 export const signalsMigrationSOClient = (
   savedObjectsClient: SavedObjectsClientContract
 ): SignalsMigrationSOClient => ({
+  bulkGet: (objects, options) =>
+    savedObjectsClient.bulkGet(
+      objects.map((o) => ({ ...o, type: signalsMigrationType })),
+      options
+    ),
   find: (options) =>
     savedObjectsClient.find<SignalsMigrationSOAttributes>({
       ...options,

x-pack/plugins/security_solution/server/lib/detection_engine/migrations/saved_objects_schema.mock.ts

@@ -27,3 +27,16 @@ export const getSignalsMigrationSavedObjectMock = (
     ...overrides,
   },
 });
+
+export const getSignalsMigrationSavedObjectErrorMock = (
+  overrides: Partial<SignalsMigrationSO['error']> = {}
+): SignalsMigrationSO =>
+  ({
+    id: 'dne-migration',
+    error: {
+      statusCode: 404,
+      error: 'Not Found',
+      message: 'Saved object [security-solution-signals-migration/dne-migration] not found',
+      ...overrides,
+    },
+  } as SignalsMigrationSO);

x-pack/plugins/security_solution/server/lib/detection_engine/migrations/saved_objects_schema.ts

@@ -28,6 +28,12 @@ const signalsMigrationSOGeneratedAttributes = {
   updatedBy: t.string,
 };
 
+const signalsMigrationSOError = {
+  statusCode: t.number,
+  error: t.string,
+  message: t.string,
+};
+
 /**
  The attributes necessary to create a Signals Migration Saved Object
  */
@@ -59,11 +65,14 @@ export const signalsMigrationSOAttributes = t.exact(
 );
 export type SignalsMigrationSOAttributes = t.TypeOf<typeof signalsMigrationSOAttributes>;
 
-export const signalsMigrationSO = t.type({
-  id: t.string,
-  attributes: signalsMigrationSOAttributes,
-  type: t.string,
-});
+export const signalsMigrationSO = t.intersection([
+  t.type({
+    id: t.string,
+    attributes: signalsMigrationSOAttributes,
+    type: t.string,
+  }),
+  t.partial({ error: t.type(signalsMigrationSOError) }),
+]);
 export type SignalsMigrationSO = t.TypeOf<typeof signalsMigrationSO>;
 
 export const signalsMigrationSOs = t.array(signalsMigrationSO);

x-pack/test/detection_engine_api_integration/security_and_spaces/tests/delete_signals_migrations.ts

@@ -102,6 +102,19 @@ export default ({ getService }: FtrProviderContext): void => {
       );
     });
 
+    it('returns a 404 trying to delete a migration that does not exist', async () => {
+      const { body } = await supertest
+        .delete(DETECTION_ENGINE_SIGNALS_MIGRATION_URL)
+        .set('kbn-xsrf', 'true')
+        .send({ migration_ids: ['dne-migration'] })
+        .expect(404);
+
+      expect(body).to.eql({
+        message: 'Saved object [security-solution-signals-migration/dne-migration] not found',
+        status_code: 404,
+      });
+    });
+
     it('rejects the request if the user does not have sufficient privileges', async () => {
       await createUserAndRole(getService, ROLES.t1_analyst);
 

x-pack/test/detection_engine_api_integration/security_and_spaces/tests/finalize_signals_migrations.ts

@@ -221,14 +221,17 @@ export default ({ getService }: FtrProviderContext): void => {
       expect(indicesAfter).not.to.contain(createdMigration.index);
     });
 
-    it('returns an empty array indicating a no-op for DNE migrations', async () => {
+    it('returns a 404 for DNE migrations', async () => {
       const { body } = await supertest
         .post(DETECTION_ENGINE_SIGNALS_FINALIZE_MIGRATION_URL)
         .set('kbn-xsrf', 'true')
         .send({ migration_ids: ['dne-migration'] })
-        .expect(200);
+        .expect(404);
 
-      expect(body).to.eql({ migrations: [] });
+      expect(body).to.eql({
+        message: 'Saved object [security-solution-signals-migration/dne-migration] not found',
+        status_code: 404,
+      });
     });
 
     it('rejects the request if the user does not have sufficient privileges', async () => {