From e23db1b2ccd64eadf84848f6340f816de5119da7 Mon Sep 17 00:00:00 2001 From: Alexi Doak <109488926+doakalexi@users.noreply.github.com> Date: Tue, 22 Nov 2022 08:36:25 -0500 Subject: [PATCH] Revert "[ResponseOps][Alerting] Hiding all features in a space causes rules to stop running (#145372)" This reverts commit 6a3f8ad877ad65475986408a320bdc7d13d0a6b4. --- .../alerting_authorization.test.ts | 49 ------------------- .../authorization/alerting_authorization.ts | 10 ++-- .../group1/tests/alerting/create.ts | 7 +++ .../group1/tests/alerting/delete.ts | 10 +++- .../group1/tests/alerting/enable.ts | 7 +++ .../group1/tests/alerting/get.ts | 11 +++++ .../group2/tests/alerting/mute_all.ts | 11 +++++ .../group2/tests/alerting/mute_instance.ts | 11 +++++ .../group2/tests/alerting/snooze.ts | 11 +++++ .../group2/tests/alerting/unmute_all.ts | 11 +++++ .../group2/tests/alerting/unmute_instance.ts | 11 +++++ .../group2/tests/alerting/update.ts | 11 +++++ .../group2/tests/alerting/update_api_key.ts | 11 +++++ 13 files changed, 117 insertions(+), 54 deletions(-) diff --git a/x-pack/plugins/alerting/server/authorization/alerting_authorization.test.ts b/x-pack/plugins/alerting/server/authorization/alerting_authorization.test.ts index 6ad38fb7ca197..1b102fa1a8f39 100644 --- a/x-pack/plugins/alerting/server/authorization/alerting_authorization.test.ts +++ b/x-pack/plugins/alerting/server/authorization/alerting_authorization.test.ts @@ -550,55 +550,6 @@ describe('AlertingAuthorization', () => { }); }); - test('ensures the user has privileges to execute alerts when all features are disabled', async () => { - features.getKibanaFeatures.mockReturnValue([]); - const { authorization } = mockSecurity(); - const checkPrivileges: jest.MockedFunction< - ReturnType - > = jest.fn(); - authorization.checkPrivilegesDynamicallyWithRequest.mockReturnValue(checkPrivileges); - const alertAuthorization = new AlertingAuthorization({ - request, - authorization, - ruleTypeRegistry, - features, - getSpace, - getSpaceId, - }); - - checkPrivileges.mockResolvedValueOnce({ - username: 'some-user', - hasAllRequested: true, - privileges: { kibana: [] }, - }); - - await alertAuthorization.ensureAuthorized({ - ruleTypeId: 'myType', - consumer: 'alerts', - operation: WriteOperations.Update, - entity: AlertingAuthorizationEntity.Alert, - }); - - expect(ruleTypeRegistry.get).toHaveBeenCalledWith('myType'); - - expect(authorization.actions.alerting.get).toHaveBeenCalledTimes(2); - expect(authorization.actions.alerting.get).toHaveBeenCalledWith( - 'myType', - 'alerts', - 'alert', - 'update' - ); - expect(authorization.actions.alerting.get).toHaveBeenCalledWith( - 'myType', - 'myApp', - 'alert', - 'update' - ); - expect(checkPrivileges).toHaveBeenCalledWith({ - kibana: [mockAuthorizationAction('myType', 'myApp', 'alert', 'update')], - }); - }); - test('throws if user lacks the required rule privileges for the consumer', async () => { const { authorization } = mockSecurity(); const checkPrivileges: jest.MockedFunction< diff --git a/x-pack/plugins/alerting/server/authorization/alerting_authorization.ts b/x-pack/plugins/alerting/server/authorization/alerting_authorization.ts index c85824fd3d2de..403826dc4668d 100644 --- a/x-pack/plugins/alerting/server/authorization/alerting_authorization.ts +++ b/x-pack/plugins/alerting/server/authorization/alerting_authorization.ts @@ -128,10 +128,12 @@ export class AlertingAuthorization { }); this.allPossibleConsumers = this.featuresIds.then((featuresIds) => { - return asAuthorizedConsumers([ALERTS_FEATURE_ID, ...featuresIds], { - read: true, - all: true, - }); + return featuresIds.size + ? asAuthorizedConsumers([ALERTS_FEATURE_ID, ...featuresIds], { + read: true, + all: true, + }) + : {}; }); } diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/create.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/create.ts index 014d7720409c1..cebfe68e279b0 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/create.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/create.ts @@ -260,6 +260,13 @@ export default function createAlertTests({ getService }: FtrProviderContext) { switch (scenario.id) { case 'no_kibana_privileges at space1': case 'space_1_all at space2': + expect(response.statusCode).to.eql(403); + expect(response.body).to.eql({ + error: 'Forbidden', + message: getConsumerUnauthorizedErrorMessage('create', 'test.noop', 'alerts'), + statusCode: 403, + }); + break; case 'global_read at space1': expect(response.statusCode).to.eql(403); expect(response.body).to.eql({ diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/delete.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/delete.ts index 307cd7a943916..2b6086cf38d92 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/delete.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/delete.ts @@ -224,9 +224,17 @@ export default function createDeleteTests({ getService }: FtrProviderContext) { .auth(user.username, user.password); switch (scenario.id) { - case 'global_read at space1': case 'no_kibana_privileges at space1': case 'space_1_all at space2': + expect(response.statusCode).to.eql(403); + expect(response.body).to.eql({ + error: 'Forbidden', + message: getConsumerUnauthorizedErrorMessage('delete', 'test.noop', 'alerts'), + statusCode: 403, + }); + objectRemover.add(space.id, createdAlert.id, 'rule', 'alerting'); + break; + case 'global_read at space1': expect(response.statusCode).to.eql(403); expect(response.body).to.eql({ error: 'Forbidden', diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/enable.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/enable.ts index 2e7d99c4fab22..73842073a542b 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/enable.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/enable.ts @@ -265,6 +265,13 @@ export default function createEnableAlertTests({ getService }: FtrProviderContex switch (scenario.id) { case 'no_kibana_privileges at space1': case 'space_1_all at space2': + expect(response.statusCode).to.eql(403); + expect(response.body).to.eql({ + error: 'Forbidden', + message: getConsumerUnauthorizedErrorMessage('enable', 'test.noop', 'alerts'), + statusCode: 403, + }); + break; case 'global_read at space1': expect(response.statusCode).to.eql(403); expect(response.body).to.eql({ diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/get.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/get.ts index 8efcf9f155a05..b0900f74993cb 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/get.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/get.ts @@ -231,6 +231,17 @@ const getTestUtils = ( switch (scenario.id) { case 'no_kibana_privileges at space1': case 'space_1_all at space2': + expect(response.statusCode).to.eql(403); + expect(response.body).to.eql({ + error: 'Forbidden', + message: getConsumerUnauthorizedErrorMessage( + 'get', + 'test.restricted-noop', + 'alerts' + ), + statusCode: 403, + }); + break; case 'space_1_all at space1': case 'space_1_all_alerts_none_actions at space1': expect(response.statusCode).to.eql(403); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/mute_all.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/mute_all.ts index 65861aed1027c..ca3570a511d17 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/mute_all.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/mute_all.ts @@ -254,6 +254,17 @@ export default function createMuteAlertTests({ getService }: FtrProviderContext) switch (scenario.id) { case 'no_kibana_privileges at space1': case 'space_1_all at space2': + expect(response.statusCode).to.eql(403); + expect(response.body).to.eql({ + error: 'Forbidden', + message: getConsumerUnauthorizedErrorMessage( + 'muteAll', + 'test.restricted-noop', + 'alerts' + ), + statusCode: 403, + }); + break; case 'global_read at space1': case 'space_1_all at space1': case 'space_1_all_alerts_none_actions at space1': diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/mute_instance.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/mute_instance.ts index d062f11df8bb8..63a285e0f4cb8 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/mute_instance.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/mute_instance.ts @@ -254,6 +254,17 @@ export default function createMuteAlertInstanceTests({ getService }: FtrProvider switch (scenario.id) { case 'no_kibana_privileges at space1': case 'space_1_all at space2': + expect(response.statusCode).to.eql(403); + expect(response.body).to.eql({ + error: 'Forbidden', + message: getConsumerUnauthorizedErrorMessage( + 'muteAlert', + 'test.restricted-noop', + 'alerts' + ), + statusCode: 403, + }); + break; case 'global_read at space1': case 'space_1_all at space1': case 'space_1_all_alerts_none_actions at space1': diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/snooze.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/snooze.ts index 33883d7e955ed..2376e05635e9c 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/snooze.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/snooze.ts @@ -284,6 +284,17 @@ export default function createSnoozeRuleTests({ getService }: FtrProviderContext switch (scenario.id) { case 'no_kibana_privileges at space1': case 'space_1_all at space2': + expect(response.statusCode).to.eql(403); + expect(response.body).to.eql({ + error: 'Forbidden', + message: getConsumerUnauthorizedErrorMessage( + 'snooze', + 'test.restricted-noop', + 'alerts' + ), + statusCode: 403, + }); + break; case 'global_read at space1': case 'space_1_all at space1': case 'space_1_all_alerts_none_actions at space1': diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/unmute_all.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/unmute_all.ts index 507c34e48d77c..b4576650c58c8 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/unmute_all.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/unmute_all.ts @@ -274,6 +274,17 @@ export default function createUnmuteAlertTests({ getService }: FtrProviderContex switch (scenario.id) { case 'no_kibana_privileges at space1': case 'space_1_all at space2': + expect(response.statusCode).to.eql(403); + expect(response.body).to.eql({ + error: 'Forbidden', + message: getConsumerUnauthorizedErrorMessage( + 'unmuteAll', + 'test.restricted-noop', + 'alerts' + ), + statusCode: 403, + }); + break; case 'global_read at space1': case 'space_1_all at space1': case 'space_1_all_alerts_none_actions at space1': diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/unmute_instance.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/unmute_instance.ts index d05df28935309..1aa84f64a7e79 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/unmute_instance.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/unmute_instance.ts @@ -274,6 +274,17 @@ export default function createMuteAlertInstanceTests({ getService }: FtrProvider switch (scenario.id) { case 'no_kibana_privileges at space1': case 'space_1_all at space2': + expect(response.statusCode).to.eql(403); + expect(response.body).to.eql({ + error: 'Forbidden', + message: getConsumerUnauthorizedErrorMessage( + 'unmuteAlert', + 'test.restricted-noop', + 'alerts' + ), + statusCode: 403, + }); + break; case 'global_read at space1': case 'space_1_all at space1': case 'space_1_all_alerts_none_actions at space1': diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/update.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/update.ts index 4a27d4b814bad..430d69274041a 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/update.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/update.ts @@ -378,6 +378,17 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { switch (scenario.id) { case 'no_kibana_privileges at space1': case 'space_1_all at space2': + expect(response.statusCode).to.eql(403); + expect(response.body).to.eql({ + error: 'Forbidden', + message: getConsumerUnauthorizedErrorMessage( + 'update', + 'test.restricted-noop', + 'alerts' + ), + statusCode: 403, + }); + break; case 'global_read at space1': case 'space_1_all at space1': case 'space_1_all_alerts_none_actions at space1': diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/update_api_key.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/update_api_key.ts index 60741756c9bf6..a0d1eb4dd0756 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/update_api_key.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/update_api_key.ts @@ -247,6 +247,17 @@ export default function createUpdateApiKeyTests({ getService }: FtrProviderConte switch (scenario.id) { case 'no_kibana_privileges at space1': case 'space_1_all at space2': + expect(response.statusCode).to.eql(403); + expect(response.body).to.eql({ + error: 'Forbidden', + message: getConsumerUnauthorizedErrorMessage( + 'updateApiKey', + 'test.restricted-noop', + 'alerts' + ), + statusCode: 403, + }); + break; case 'global_read at space1': case 'space_1_all at space1': case 'space_1_all_alerts_none_actions at space1':