Add support for custom alert ids (#89814)

* Add support for custom alert ids * UUID v4 also supported * Change ESO custom id error message * Update api integration test * Use errors.createBadRequestError
2025-04-24 17:59:23 -04:00 · 2021-02-01 14:00:33 -05:00 · 2021-02-01 14:00:33 -05:00 · 51cfa90dc5
commit 51cfa90dc5
parent 3f96892430
7 changed files with 244 additions and 29 deletions
--- a/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts
+++ b/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts
@ -149,6 +149,7 @@ export interface CreateOptions<Params extends AlertTypeParams> {
    | 'executionStatus'
  > & { actions: NormalizedAlertAction[] };
  options?: {
+    id?: string;
    migrationVersion?: Record<string, string>;
  };
 }
@ -226,7 +227,7 @@ export class AlertsClient {
    data,
    options,
  }: CreateOptions<Params>): Promise<Alert<Params>> {
-    const id = SavedObjectsUtils.generateId();
+    const id = options?.id || SavedObjectsUtils.generateId();

    try {
      await this.authorization.ensureAuthorized(
--- a/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts
+++ b/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts
@ -462,6 +462,73 @@ describe('create()', () => {
    expect(actionsClient.isActionTypeEnabled).toHaveBeenCalledWith('test', { notifyUsage: true });
  });

+  test('creates an alert with a custom id', async () => {
+    const data = getMockData();
+    const createdAttributes = {
+      ...data,
+      alertTypeId: '123',
+      schedule: { interval: '10s' },
+      params: {
+        bar: true,
+      },
+      createdAt: '2019-02-12T21:01:22.479Z',
+      createdBy: 'elastic',
+      updatedBy: 'elastic',
+      updatedAt: '2019-02-12T21:01:22.479Z',
+      muteAll: false,
+      mutedInstanceIds: [],
+      actions: [
+        {
+          group: 'default',
+          actionRef: 'action_0',
+          actionTypeId: 'test',
+          params: {
+            foo: true,
+          },
+        },
+      ],
+    };
+    unsecuredSavedObjectsClient.create.mockResolvedValueOnce({
+      id: '123',
+      type: 'alert',
+      attributes: createdAttributes,
+      references: [
+        {
+          name: 'action_0',
+          type: 'action',
+          id: '1',
+        },
+      ],
+    });
+    taskManager.schedule.mockResolvedValueOnce({
+      id: 'task-123',
+      taskType: 'alerting:123',
+      scheduledAt: new Date(),
+      attempts: 1,
+      status: TaskStatus.Idle,
+      runAt: new Date(),
+      startedAt: null,
+      retryAt: null,
+      state: {},
+      params: {},
+      ownerId: null,
+    });
+    const result = await alertsClient.create({ data, options: { id: '123' } });
+    expect(result.id).toEqual('123');
+    expect(unsecuredSavedObjectsClient.create.mock.calls[0][2]).toMatchInlineSnapshot(`
+      Object {
+        "id": "123",
+        "references": Array [
+          Object {
+            "id": "1",
+            "name": "action_0",
+            "type": "action",
+          },
+        ],
+      }
+    `);
+  });
+
  test('creates an alert with multiple actions', async () => {
    const data = getMockData({
      actions: [
--- a/x-pack/plugins/alerts/server/routes/create.test.ts
+++ b/x-pack/plugins/alerts/server/routes/create.test.ts
@ -82,7 +82,7 @@ describe('createAlertRoute', () => {

    const [config, handler] = router.post.mock.calls[0];

-    expect(config.path).toMatchInlineSnapshot(`"/api/alerts/alert"`);
+    expect(config.path).toMatchInlineSnapshot(`"/api/alerts/alert/{id?}"`);

    alertsClient.create.mockResolvedValueOnce(createResult);

@ -125,6 +125,9 @@ describe('createAlertRoute', () => {
            ],
            "throttle": "30s",
          },
+          "options": Object {
+            "id": undefined,
+          },
        },
      ]
    `);
@ -134,6 +137,74 @@ describe('createAlertRoute', () => {
    });
  });

+  it('allows providing a custom id', async () => {
+    const expectedResult = {
+      ...createResult,
+      id: 'custom-id',
+    };
+    const licenseState = licenseStateMock.create();
+    const router = httpServiceMock.createRouter();
+
+    createAlertRoute(router, licenseState);
+
+    const [config, handler] = router.post.mock.calls[0];
+
+    expect(config.path).toMatchInlineSnapshot(`"/api/alerts/alert/{id?}"`);
+
+    alertsClient.create.mockResolvedValueOnce(expectedResult);
+
+    const [context, req, res] = mockHandlerArguments(
+      { alertsClient },
+      {
+        params: { id: 'custom-id' },
+        body: mockedAlert,
+      },
+      ['ok']
+    );
+
+    expect(await handler(context, req, res)).toEqual({ body: expectedResult });
+
+    expect(alertsClient.create).toHaveBeenCalledTimes(1);
+    expect(alertsClient.create.mock.calls[0]).toMatchInlineSnapshot(`
+      Array [
+        Object {
+          "data": Object {
+            "actions": Array [
+              Object {
+                "group": "default",
+                "id": "2",
+                "params": Object {
+                  "foo": true,
+                },
+              },
+            ],
+            "alertTypeId": "1",
+            "consumer": "bar",
+            "name": "abc",
+            "notifyWhen": "onActionGroupChange",
+            "params": Object {
+              "bar": true,
+            },
+            "schedule": Object {
+              "interval": "10s",
+            },
+            "tags": Array [
+              "foo",
+            ],
+            "throttle": "30s",
+          },
+          "options": Object {
+            "id": "custom-id",
+          },
+        },
+      ]
+    `);
+
+    expect(res.ok).toHaveBeenCalledWith({
+      body: expectedResult,
+    });
+  });
+
  it('ensures the license allows creating alerts', async () => {
    const licenseState = licenseStateMock.create();
    const router = httpServiceMock.createRouter();
--- a/x-pack/plugins/alerts/server/routes/create.ts
+++ b/x-pack/plugins/alerts/server/routes/create.ts
@ -45,8 +45,13 @@ export const bodySchema = schema.object({
 export const createAlertRoute = (router: AlertingRouter, licenseState: ILicenseState) => {
  router.post(
    {
-      path: `${BASE_ALERT_API_PATH}/alert`,
+      path: `${BASE_ALERT_API_PATH}/alert/{id?}`,
      validate: {
+        params: schema.maybe(
+          schema.object({
+            id: schema.maybe(schema.string()),
+          })
+        ),
        body: bodySchema,
      },
    },
@ -59,10 +64,12 @@ export const createAlertRoute = (router: AlertingRouter, licenseState: ILicenseS
        }
        const alertsClient = context.alerting.getAlertsClient();
        const alert = req.body;
+        const params = req.params;
        const notifyWhen = alert?.notifyWhen ? (alert.notifyWhen as AlertNotifyWhenType) : null;
        try {
          const alertRes: Alert<AlertTypeParams> = await alertsClient.create<AlertTypeParams>({
            data: { ...alert, notifyWhen },
+            options: { id: params?.id },
          });
          return res.ok({
            body: alertRes,
--- a/x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.test.ts
+++ b/x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.test.ts
@ -83,11 +83,11 @@ describe('#create', () => {
    expect(mockBaseClient.create).toHaveBeenCalledWith('unknown-type', attributes, options);
  });

-  it('fails if type is registered and ID is specified', async () => {
+  it('fails if type is registered and non-UUID ID is specified', async () => {
    const attributes = { attrOne: 'one', attrSecret: 'secret', attrThree: 'three' };

    await expect(wrapper.create('known-type', attributes, { id: 'some-id' })).rejects.toThrowError(
-      'Predefined IDs are not allowed for saved objects with encrypted attributes, unless the ID has been generated using `SavedObjectsUtils.generateId`.'
+      'Predefined IDs are not allowed for saved objects with encrypted attributes unless the ID is a UUID.'
    );

    expect(mockBaseClient.create).not.toHaveBeenCalled();
@ -310,7 +310,7 @@ describe('#bulkCreate', () => {
    );
  });

-  it('fails if ID is specified for registered type', async () => {
+  it('fails if non-UUID ID is specified for registered type', async () => {
    const attributes = { attrOne: 'one', attrSecret: 'secret', attrThree: 'three' };

    const bulkCreateParams = [
@ -319,7 +319,7 @@ describe('#bulkCreate', () => {
    ];

    await expect(wrapper.bulkCreate(bulkCreateParams)).rejects.toThrowError(
-      'Predefined IDs are not allowed for saved objects with encrypted attributes, unless the ID has been generated using `SavedObjectsUtils.generateId`.'
+      'Predefined IDs are not allowed for saved objects with encrypted attributes unless the ID is a UUID.'
    );

    expect(mockBaseClient.bulkCreate).not.toHaveBeenCalled();
--- a/x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.ts
+++ b/x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.ts
@ -59,7 +59,7 @@ export class EncryptedSavedObjectsClientWrapper implements SavedObjectsClientCon
      return await this.options.baseClient.create(type, attributes, options);
    }

-    const id = getValidId(options.id, options.version, options.overwrite);
+    const id = this.getValidId(options.id, options.version, options.overwrite);
    const namespace = getDescriptorNamespace(
      this.options.baseTypeRegistry,
      type,
@ -93,7 +93,7 @@ export class EncryptedSavedObjectsClientWrapper implements SavedObjectsClientCon
          return object;
        }

-        const id = getValidId(object.id, object.version, options?.overwrite);
+        const id = this.getValidId(object.id, object.version, options?.overwrite);
        const namespace = getDescriptorNamespace(
          this.options.baseTypeRegistry,
          object.type,
@ -307,27 +307,27 @@ export class EncryptedSavedObjectsClientWrapper implements SavedObjectsClientCon

    return response;
  }
-}

-// Saved objects with encrypted attributes should have IDs that are hard to guess especially
-// since IDs are part of the AAD used during encryption, that's why we control them within this
-// wrapper and don't allow consumers to specify their own IDs directly unless overwriting the original document.
-function getValidId(
-  id: string | undefined,
-  version: string | undefined,
-  overwrite: boolean | undefined
-) {
-  if (id) {
-    // only allow a specified ID if we're overwriting an existing ESO with a Version
-    // this helps us ensure that the document really was previously created using ESO
-    // and not being used to get around the specified ID limitation
-    const canSpecifyID = (overwrite && version) || SavedObjectsUtils.isRandomId(id);
-    if (!canSpecifyID) {
-      throw new Error(
-        'Predefined IDs are not allowed for saved objects with encrypted attributes, unless the ID has been generated using `SavedObjectsUtils.generateId`.'
-      );
+  // Saved objects with encrypted attributes should have IDs that are hard to guess especially
+  // since IDs are part of the AAD used during encryption, that's why we control them within this
+  // wrapper and don't allow consumers to specify their own IDs directly unless overwriting the original document.
+  private getValidId(
+    id: string | undefined,
+    version: string | undefined,
+    overwrite: boolean | undefined
+  ) {
+    if (id) {
+      // only allow a specified ID if we're overwriting an existing ESO with a Version
+      // this helps us ensure that the document really was previously created using ESO
+      // and not being used to get around the specified ID limitation
+      const canSpecifyID = (overwrite && version) || SavedObjectsUtils.isRandomId(id);
+      if (!canSpecifyID) {
+        throw this.errors.createBadRequestError(
+          'Predefined IDs are not allowed for saved objects with encrypted attributes unless the ID is a UUID.'
+        );
+      }
+      return id;
    }
-    return id;
+    return SavedObjectsUtils.generateId();
  }
-  return SavedObjectsUtils.generateId();
 }
--- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/create.ts
+++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/create.ts
@ -111,6 +111,75 @@ export default function createAlertTests({ getService }: FtrProviderContext) {
      });
    });

+    it('should allow providing custom saved object ids (uuid v1)', async () => {
+      const customId = '09570bb0-6299-11eb-8fde-9fe5ce6ea450';
+      const response = await supertest
+        .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerts/alert/${customId}`)
+        .set('kbn-xsrf', 'foo')
+        .send(getTestAlertData());
+
+      expect(response.status).to.eql(200);
+      objectRemover.add(Spaces.space1.id, response.body.id, 'alert', 'alerts');
+      expect(response.body.id).to.eql(customId);
+      // Ensure AAD isn't broken
+      await checkAAD({
+        supertest,
+        spaceId: Spaces.space1.id,
+        type: 'alert',
+        id: customId,
+      });
+    });
+
+    it('should allow providing custom saved object ids (uuid v4)', async () => {
+      const customId = 'b3bc6d83-3192-4ffd-9702-ad4fb88617ba';
+      const response = await supertest
+        .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerts/alert/${customId}`)
+        .set('kbn-xsrf', 'foo')
+        .send(getTestAlertData());
+
+      expect(response.status).to.eql(200);
+      objectRemover.add(Spaces.space1.id, response.body.id, 'alert', 'alerts');
+      expect(response.body.id).to.eql(customId);
+      // Ensure AAD isn't broken
+      await checkAAD({
+        supertest,
+        spaceId: Spaces.space1.id,
+        type: 'alert',
+        id: customId,
+      });
+    });
+
+    it('should not allow providing simple custom ids (non uuid)', async () => {
+      const customId = '1';
+      const response = await supertest
+        .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerts/alert/${customId}`)
+        .set('kbn-xsrf', 'foo')
+        .send(getTestAlertData());
+
+      expect(response.status).to.eql(400);
+      expect(response.body).to.eql({
+        statusCode: 400,
+        error: 'Bad Request',
+        message:
+          'Predefined IDs are not allowed for saved objects with encrypted attributes unless the ID is a UUID.: Bad Request',
+      });
+    });
+
+    it('should return 409 when document with id already exists', async () => {
+      const customId = '5031f8f0-629a-11eb-b500-d1931a8e5df7';
+      const createdAlertResponse = await supertest
+        .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerts/alert/${customId}`)
+        .set('kbn-xsrf', 'foo')
+        .send(getTestAlertData())
+        .expect(200);
+      objectRemover.add(Spaces.space1.id, createdAlertResponse.body.id, 'alert', 'alerts');
+      await supertest
+        .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerts/alert/${customId}`)
+        .set('kbn-xsrf', 'foo')
+        .send(getTestAlertData())
+        .expect(409);
+    });
+
    it('should handle create alert request appropriately when consumer is unknown', async () => {
      const response = await supertest
        .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerts/alert`)