From 5811f51be9c431f3d9e71a6e85f1d0da2a2d3f5c Mon Sep 17 00:00:00 2001 From: Mike Cote Date: Mon, 4 Jan 2021 10:40:29 -0500 Subject: [PATCH 1/3] Unskip test --- .../security_and_spaces/tests/alerting/rbac_legacy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 992d9210b9761b..2b25c82cc92e5b 100644 --- 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 @@ -62,7 +62,7 @@ export default function alertTests({ getService }: FtrProviderContext) { }); }); - it.skip('should schedule actions on legacy alerts', async () => { + it('should schedule actions on legacy alerts', async () => { const reference = `alert:migrated-to-7.10:${user.username}`; const migratedAlertId = MIGRATED_ALERT_ID[user.username]; From 4b3c3edf8767751d3e7cd4c81e51b799b76f2009 Mon Sep 17 00:00:00 2001 From: Mike Cote Date: Thu, 7 Jan 2021 09:13:06 -0500 Subject: [PATCH 2/3] Increase attempts to 2 for retryIfConflicts --- x-pack/plugins/alerts/server/lib/retry_if_conflicts.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) 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 = () => Promise; // 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 From aedd62081d7ed01b51140cb9ed363b72abf1cdb9 Mon Sep 17 00:00:00 2001 From: Mike Cote Date: Thu, 7 Jan 2021 09:44:36 -0500 Subject: [PATCH 3/3] 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 +------ 3 files changed, 2 insertions(+), 11 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 e21fee4ce3d61f..25bd69d4378e0b 100644 --- a/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts +++ b/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts @@ -818,10 +818,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; }