update meta field only when api key is updated

elastic · Sep 9, 2020 · 2a9694f · 2a9694f
1 parent 1ef7d8b
commit 2a9694f
Show file tree

Hide file tree

Showing 11 changed files with 47 additions and 55 deletions.
diff --git a/x-pack/plugins/actions/README.md b/x-pack/plugins/actions/README.md
@@ -280,7 +280,7 @@ The following table describes the properties of the `options` object.
 | params   | The `params` value to give the action type executor.                                                   | object |
 | spaceId  | The space id the action is within.                                                                     | string |
 | apiKey   | The Elasticsearch API key to use for context. (Note: only required and used when security is enabled). | string |
-| source   | The source of the execution, either a reponse to an HTTP request or a references to a Saved Object. | object, optional |
+| source   | The source of the execution, either an HTTP request or a reference to a Saved Object.                  | object, optional |
 
 ## Example
 
@@ -308,11 +308,11 @@ This api runs the action and asynchronously returns the result of running the ac
 
 The following table describes the properties of the `options` object.
 
-| Property | Description                                          | Type   |
-| -------- | ---------------------------------------------------- | ------ |
-| id       | The id of the action you want to execute.            | string |
-| params   | The `params` value to give the action type executor. | object |
-| source   | The source of the execution, either a reponse to an HTTP request or a references to a Saved Object. | object, optional |
+| Property | Description                                                                          | Type   |
+| -------- | ------------------------------------------------------------------------------------ | ------ |
+| id       | The id of the action you want to execute.                                            | string |
+| params   | The `params` value to give the action type executor.                                 | object |
+| source   | The source of the execution, either an HTTP request or a reference to a Saved Object.| object, optional |
 
 ## Example
 

diff --git a/x-pack/plugins/actions/server/authorization/should_legacy_rbac_apply_by_source.test.ts b/x-pack/plugins/actions/server/authorization/should_legacy_rbac_apply_by_source.test.ts
@@ -44,7 +44,7 @@ describe(`#shouldLegacyRbacApplyBySource`, () => {
   test('should return true if source alert is marked as legacy', async () => {
     const id = uuid.v4();
     unsecuredSavedObjectsClient.get.mockResolvedValue(
-      mockAlert({ id, attributes: { meta: { versionLastmodified: 'pre-7.10.0' } } })
+      mockAlert({ id, attributes: { meta: { versionApiKeyLastmodified: 'pre-7.10.0' } } })
     );
     expect(
       await shouldLegacyRbacApplyBySource(
@@ -60,7 +60,7 @@ describe(`#shouldLegacyRbacApplyBySource`, () => {
   test('should return false if source alert is marked as modern', async () => {
     const id = uuid.v4();
     unsecuredSavedObjectsClient.get.mockResolvedValue(
-      mockAlert({ id, attributes: { meta: { versionLastmodified: '7.10.0' } } })
+      mockAlert({ id, attributes: { meta: { versionApiKeyLastmodified: '7.10.0' } } })
     );
     expect(
       await shouldLegacyRbacApplyBySource(

diff --git a/x-pack/plugins/actions/server/authorization/should_legacy_rbac_apply_by_source.ts b/x-pack/plugins/actions/server/authorization/should_legacy_rbac_apply_by_source.ts
@@ -19,9 +19,9 @@ export async function shouldLegacyRbacApplyBySource(
     ? (
         await unsecuredSavedObjectsClient.get<{
           meta?: {
-            versionLastmodified?: string;
+            versionApiKeyLastmodified?: string;
           };
         }>(ALERT_SAVED_OBJECT_TYPE, executionSource.source.id)
-      ).attributes.meta?.versionLastmodified === LEGACY_VERSION
+      ).attributes.meta?.versionApiKeyLastmodified === LEGACY_VERSION
     : false;
 }
diff --git a/x-pack/plugins/alerts/server/alerts_client.test.ts b/x-pack/plugins/alerts/server/alerts_client.test.ts
@@ -375,7 +375,7 @@ describe('create()', () => {
         "createdBy": "elastic",
         "enabled": true,
         "meta": Object {
-          "versionLastmodified": "v7.10.0",
+          "versionApiKeyLastmodified": "v7.10.0",
         },
         "muteAll": false,
         "mutedInstanceIds": Array [],
@@ -1003,7 +1003,7 @@ describe('create()', () => {
         updatedBy: 'elastic',
         enabled: true,
         meta: {
-          versionLastmodified: 'v7.10.0',
+          versionApiKeyLastmodified: 'v7.10.0',
         },
         schedule: { interval: '10s' },
         throttle: null,
@@ -1119,7 +1119,7 @@ describe('create()', () => {
         updatedBy: 'elastic',
         enabled: false,
         meta: {
-          versionLastmodified: 'v7.10.0',
+          versionApiKeyLastmodified: 'v7.10.0',
         },
         schedule: { interval: '10s' },
         throttle: null,
@@ -1247,7 +1247,7 @@ describe('enable()', () => {
         consumer: 'myApp',
         enabled: true,
         meta: {
-          versionLastmodified: kibanaVersion,
+          versionApiKeyLastmodified: kibanaVersion,
         },
         updatedBy: 'elastic',
         apiKey: null,
@@ -1335,7 +1335,7 @@ describe('enable()', () => {
         consumer: 'myApp',
         enabled: true,
         meta: {
-          versionLastmodified: kibanaVersion,
+          versionApiKeyLastmodified: kibanaVersion,
         },
         apiKey: Buffer.from('123:abc').toString('base64'),
         apiKeyOwner: 'elastic',
@@ -1503,7 +1503,7 @@ describe('disable()', () => {
         apiKeyOwner: null,
         enabled: false,
         meta: {
-          versionLastmodified: kibanaVersion,
+          versionApiKeyLastmodified: kibanaVersion,
         },
         scheduledTaskId: null,
         updatedBy: 'elastic',
@@ -1546,7 +1546,7 @@ describe('disable()', () => {
         apiKeyOwner: null,
         enabled: false,
         meta: {
-          versionLastmodified: kibanaVersion,
+          versionApiKeyLastmodified: kibanaVersion,
         },
         scheduledTaskId: null,
         updatedBy: 'elastic',
@@ -1656,9 +1656,6 @@ describe('muteAll()', () => {
 
     await alertsClient.muteAll({ id: '1' });
     expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledWith('alert', '1', {
-      meta: {
-        versionLastmodified: kibanaVersion,
-      },
       muteAll: true,
       mutedInstanceIds: [],
       updatedBy: 'elastic',
@@ -1744,9 +1741,6 @@ describe('unmuteAll()', () => {
 
     await alertsClient.unmuteAll({ id: '1' });
     expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledWith('alert', '1', {
-      meta: {
-        versionLastmodified: kibanaVersion,
-      },
       muteAll: false,
       mutedInstanceIds: [],
       updatedBy: 'elastic',
@@ -1830,9 +1824,6 @@ describe('muteInstance()', () => {
       'alert',
       '1',
       {
-        meta: {
-          versionLastmodified: kibanaVersion,
-        },
         mutedInstanceIds: ['2'],
         updatedBy: 'elastic',
       },
@@ -1966,9 +1957,6 @@ describe('unmuteInstance()', () => {
       'alert',
       '1',
       {
-        meta: {
-          versionLastmodified: kibanaVersion,
-        },
         mutedInstanceIds: [],
         updatedBy: 'elastic',
       },
@@ -3235,7 +3223,7 @@ describe('update()', () => {
         "consumer": "myApp",
         "enabled": true,
         "meta": Object {
-          "versionLastmodified": "v7.10.0",
+          "versionApiKeyLastmodified": "v7.10.0",
         },
         "name": "abc",
         "params": Object {
@@ -3395,7 +3383,7 @@ describe('update()', () => {
         "consumer": "myApp",
         "enabled": true,
         "meta": Object {
-          "versionLastmodified": "v7.10.0",
+          "versionApiKeyLastmodified": "v7.10.0",
         },
         "name": "abc",
         "params": Object {
@@ -3549,7 +3537,7 @@ describe('update()', () => {
         "consumer": "myApp",
         "enabled": false,
         "meta": Object {
-          "versionLastmodified": "v7.10.0",
+          "versionApiKeyLastmodified": "v7.10.0",
         },
         "name": "abc",
         "params": Object {
@@ -4237,7 +4225,7 @@ describe('updateApiKey()', () => {
           },
         ],
         meta: {
-          versionLastmodified: kibanaVersion,
+          versionApiKeyLastmodified: kibanaVersion,
         },
       },
       { version: '123' }
@@ -4276,7 +4264,7 @@ describe('updateApiKey()', () => {
           },
         ],
         meta: {
-          versionLastmodified: kibanaVersion,
+          versionApiKeyLastmodified: kibanaVersion,
         },
       },
       { version: '123' }

diff --git a/x-pack/plugins/alerts/server/alerts_client.ts b/x-pack/plugins/alerts/server/alerts_client.ts
@@ -964,8 +964,10 @@ export class AlertsClient {
   }
 
   private updateMeta<T extends Partial<RawAlert>>(alertAttributes: T): T {
-    alertAttributes.meta = alertAttributes.meta ?? {};
-    alertAttributes.meta.versionLastmodified = this.kibanaVersion;
+    if (alertAttributes.hasOwnProperty('apiKey') || alertAttributes.hasOwnProperty('apiKeyOwner')) {
+      alertAttributes.meta = alertAttributes.meta ?? {};
+      alertAttributes.meta.versionApiKeyLastmodified = this.kibanaVersion;
+    }
     return alertAttributes;
   }
 }

diff --git a/x-pack/plugins/alerts/server/authorization/alerts_authorization.ts b/x-pack/plugins/alerts/server/authorization/alerts_authorization.ts
@@ -111,7 +111,7 @@ export class AlertsAuthorization {
   }
 
   public shouldUseLegacyAuthorization(alert: RawAlert): boolean {
-    return alert.meta?.versionLastmodified === LEGACY_LAST_MODIFIED_VERSION;
+    return alert.meta?.versionApiKeyLastmodified === LEGACY_LAST_MODIFIED_VERSION;
   }
 
   private shouldCheckAuthorization(): boolean {

diff --git a/x-pack/plugins/alerts/server/saved_objects/mappings.json b/x-pack/plugins/alerts/server/saved_objects/mappings.json
@@ -79,7 +79,7 @@
       },
       "meta": {
         "properties": {
-          "versionLastmodified": {
+          "versionApiKeyLastmodified": {
             "type": "keyword"
           }
         }

diff --git a/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts b/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts
@@ -29,7 +29,7 @@ describe('7.10.0', () => {
       attributes: {
         ...alert.attributes,
         meta: {
-          versionLastmodified: 'pre-7.10.0',
+          versionApiKeyLastmodified: 'pre-7.10.0',
         },
       },
     });
@@ -46,7 +46,7 @@ describe('7.10.0', () => {
         ...alert.attributes,
         consumer: 'infrastructure',
         meta: {
-          versionLastmodified: 'pre-7.10.0',
+          versionApiKeyLastmodified: 'pre-7.10.0',
         },
       },
     });
@@ -63,7 +63,7 @@ describe('7.10.0', () => {
         ...alert.attributes,
         consumer: 'alerts',
         meta: {
-          versionLastmodified: 'pre-7.10.0',
+          versionApiKeyLastmodified: 'pre-7.10.0',
         },
       },
     });
@@ -91,7 +91,7 @@ describe('7.10.0 migrates with failure', () => {
       },
     });
     expect(log.error).toHaveBeenCalledWith(
-      `encryptedSavedObject migration failed for alert ${alert.id} with error: Can't migrate!`,
+      `encryptedSavedObject 7.10.0 migration failed for alert ${alert.id} with error: Can't migrate!`,
       {
         alertDocument: {
           ...alert,

diff --git a/x-pack/plugins/alerts/server/saved_objects/migrations.ts b/x-pack/plugins/alerts/server/saved_objects/migrations.ts
@@ -20,36 +20,36 @@ export function getMigrations(
   const migrationWhenRBACWasIntroduced = markAsLegacyAndChangeConsumer(encryptedSavedObjects);
 
   return {
-    '7.10.0': executeMigrationWithErrorHandling(migrationWhenRBACWasIntroduced),
+    '7.10.0': executeMigrationWithErrorHandling(migrationWhenRBACWasIntroduced, '7.10.0'),
   };
 }
 
 function executeMigrationWithErrorHandling(
-  migrationFunc: SavedObjectMigrationFn<RawAlert, RawAlert>
+  migrationFunc: SavedObjectMigrationFn<RawAlert, RawAlert>,
+  version: string
 ) {
   return (doc: SavedObjectUnsanitizedDoc<RawAlert>, context: SavedObjectMigrationContext) => {
     try {
       return migrationFunc(doc, context);
     } catch (ex) {
       context.log.error(
-        `encryptedSavedObject migration failed for alert ${doc.id} with error: ${ex.message}`,
+        `encryptedSavedObject ${version} migration failed for alert ${doc.id} with error: ${ex.message}`,
         { alertDocument: doc }
       );
     }
     return doc;
   };
 }
 
+const consumersToChange: Map<string, string> = new Map(
+  Object.entries({
+    alerting: 'alerts',
+    metrics: 'infrastructure',
+  })
+);
 function markAsLegacyAndChangeConsumer(
   encryptedSavedObjects: EncryptedSavedObjectsPluginSetup
 ): SavedObjectMigrationFn<RawAlert, RawAlert> {
-  const consumersToChange: Map<string, string> = new Map(
-    Object.entries({
-      alerting: 'alerts',
-      metrics: 'infrastructure',
-    })
-  );
-
   return encryptedSavedObjects.createMigration<RawAlert, RawAlert>(
     function shouldBeMigrated(doc): doc is SavedObjectUnsanitizedDoc<RawAlert> {
       // migrate all documents in 7.10 in order to add the "meta" RBAC field
@@ -66,7 +66,7 @@ function markAsLegacyAndChangeConsumer(
           consumer: consumersToChange.get(consumer) ?? consumer,
           // mark any alert predating 7.10 as a legacy alert
           meta: {
-            versionLastmodified: LEGACY_LAST_MODIFIED_VERSION,
+            versionApiKeyLastmodified: LEGACY_LAST_MODIFIED_VERSION,
           },
         },
       };

diff --git a/x-pack/plugins/alerts/server/types.ts b/x-pack/plugins/alerts/server/types.ts
@@ -112,7 +112,7 @@ export interface RawAlertAction extends SavedObjectAttributes {
 }
 
 export interface AlertMeta extends SavedObjectAttributes {
-  versionLastmodified?: string;
+  versionApiKeyLastmodified?: string;
 }
 
 export type PartialAlert = Pick<Alert, 'id'> & Partial<Omit<Alert, 'id'>>;

diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/rbac_legacy.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/rbac_legacy.ts
@@ -156,7 +156,9 @@ export default function alertTests({ getService }: FtrProviderContext) {
             // this is important as proper update *should* update the legacy status of the alert
             // and we want to ensure we don't accidentally introduce a change that might break our support of legacy alerts
             expect(swapResponse.body.id).to.eql(alertId);
-            expect(swapResponse.body.attributes.meta.versionLastmodified).to.eql('pre-7.10.0');
+            expect(swapResponse.body.attributes.meta.versionApiKeyLastmodified).to.eql(
+              'pre-7.10.0'
+            );
 
             // loading the archive likely caused the task to fail so ensure it's rescheduled to run in 2 seconds,
             // otherwise this test will stall for 5 minutes