From 715fd54e1ab2d4c79d35763f9b4426429c6d9e26 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mike=20C=C3=B4t=C3=A9?= <mikecote@users.noreply.github.com>
Date: Tue, 26 Jan 2021 11:38:02 -0500
Subject: [PATCH] Fix flaky test for legacy authorization (#87642) (#89307)

* Unskip test

* Increase attempts to 2 for retryIfConflicts

* Cleanup authorization for updateApiKey
---
 .../plugins/alerts/server/alerts_client/alerts_client.ts   | 5 +----
 .../server/authorization/alerts_authorization.mock.ts      | 1 -
 .../alerts/server/authorization/alerts_authorization.ts    | 7 +------
 x-pack/plugins/alerts/server/lib/retry_if_conflicts.ts     | 4 +---
 4 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts b/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts
index 095823952722be..5a63da4ab1574a 100644
--- a/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts
+++ b/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts
@@ -798,10 +798,7 @@ export class AlertsClient {
         attributes.consumer,
         WriteOperations.UpdateApiKey
       );
-      if (
-        attributes.actions.length &&
-        !this.authorization.shouldUseLegacyAuthorization(attributes)
-      ) {
+      if (attributes.actions.length) {
         await this.actionsAuthorization.ensureAuthorized('execute');
       }
     } catch (error) {
diff --git a/x-pack/plugins/alerts/server/authorization/alerts_authorization.mock.ts b/x-pack/plugins/alerts/server/authorization/alerts_authorization.mock.ts
index 171e3978d0d0d2..30de2c79732ced 100644
--- a/x-pack/plugins/alerts/server/authorization/alerts_authorization.mock.ts
+++ b/x-pack/plugins/alerts/server/authorization/alerts_authorization.mock.ts
@@ -14,7 +14,6 @@ const createAlertsAuthorizationMock = () => {
     ensureAuthorized: jest.fn(),
     filterByAlertTypeAuthorization: jest.fn(),
     getFindAuthorizationFilter: jest.fn(),
-    shouldUseLegacyAuthorization: jest.fn(),
   };
   return mocked;
 };
diff --git a/x-pack/plugins/alerts/server/authorization/alerts_authorization.ts b/x-pack/plugins/alerts/server/authorization/alerts_authorization.ts
index 6814e4ac1cc1bf..29f2078bc61e41 100644
--- a/x-pack/plugins/alerts/server/authorization/alerts_authorization.ts
+++ b/x-pack/plugins/alerts/server/authorization/alerts_authorization.ts
@@ -8,13 +8,12 @@ import Boom from '@hapi/boom';
 import { map, mapValues, fromPairs, has } from 'lodash';
 import { KibanaRequest } from 'src/core/server';
 import { ALERTS_FEATURE_ID } from '../../common';
-import { AlertTypeRegistry, RawAlert } from '../types';
+import { AlertTypeRegistry } from '../types';
 import { SecurityPluginSetup } from '../../../security/server';
 import { RegistryAlertType } from '../alert_type_registry';
 import { PluginStartContract as FeaturesPluginStart } from '../../../features/server';
 import { AlertsAuthorizationAuditLogger, ScopeType } from './audit_logger';
 import { Space } from '../../../spaces/server';
-import { LEGACY_LAST_MODIFIED_VERSION } from '../saved_objects/migrations';
 import { asFiltersByAlertTypeAndConsumer } from './alerts_authorization_kuery';
 import { KueryNode } from '../../../../../src/plugins/data/server';
 
@@ -112,10 +111,6 @@ export class AlertsAuthorization {
     );
   }
 
-  public shouldUseLegacyAuthorization(alert: RawAlert): boolean {
-    return alert.meta?.versionApiKeyLastmodified === LEGACY_LAST_MODIFIED_VERSION;
-  }
-
   private shouldCheckAuthorization(): boolean {
     return this.authorization?.mode?.useRbacForRequest(this.request) ?? false;
   }
diff --git a/x-pack/plugins/alerts/server/lib/retry_if_conflicts.ts b/x-pack/plugins/alerts/server/lib/retry_if_conflicts.ts
index 9cb1d7975855c2..59ecc59ab57f8c 100644
--- a/x-pack/plugins/alerts/server/lib/retry_if_conflicts.ts
+++ b/x-pack/plugins/alerts/server/lib/retry_if_conflicts.ts
@@ -15,9 +15,7 @@ import { Logger, SavedObjectsErrorHelpers } from '../../../../../src/core/server
 type RetryableForConflicts<T> = () => Promise<T>;
 
 // number of times to retry when conflicts occur
-// note: it seems unlikely that we'd need more than one retry, but leaving
-// this statically configurable in case we DO need > 1
-export const RetryForConflictsAttempts = 1;
+export const RetryForConflictsAttempts = 2;
 
 // milliseconds to wait before retrying when conflicts occur
 // note: we considered making this random, to help avoid a stampede, but