diff --git a/x-pack/plugins/alerting/server/alerting_authorization_client_factory.test.ts b/x-pack/plugins/alerting/server/alerting_authorization_client_factory.test.ts index 77a15eda79cef0..2ba3580745d594 100644 --- a/x-pack/plugins/alerting/server/alerting_authorization_client_factory.test.ts +++ b/x-pack/plugins/alerting/server/alerting_authorization_client_factory.test.ts @@ -75,35 +75,6 @@ test('creates an alerting authorization client with proper constructor arguments auditLogger: expect.any(AlertingAuthorizationAuditLogger), getSpace: expect.any(Function), getSpaceId: expect.any(Function), - exemptConsumerIds: [], - }); - - expect(AlertingAuthorizationAuditLogger).toHaveBeenCalled(); - expect(securityPluginSetup.audit.getLogger).toHaveBeenCalledWith(ALERTS_FEATURE_ID); -}); - -test('creates an alerting authorization client with proper constructor arguments when exemptConsumerIds are specified', async () => { - const factory = new AlertingAuthorizationClientFactory(); - factory.initialize({ - securityPluginSetup, - securityPluginStart, - ...alertingAuthorizationClientFactoryParams, - }); - const request = KibanaRequest.from(fakeRequest); - const { AlertingAuthorizationAuditLogger } = jest.requireMock('./authorization/audit_logger'); - - factory.create(request, ['exemptConsumerA', 'exemptConsumerB']); - - const { AlertingAuthorization } = jest.requireMock('./authorization/alerting_authorization'); - expect(AlertingAuthorization).toHaveBeenCalledWith({ - request, - authorization: securityPluginStart.authz, - ruleTypeRegistry: alertingAuthorizationClientFactoryParams.ruleTypeRegistry, - features: alertingAuthorizationClientFactoryParams.features, - auditLogger: expect.any(AlertingAuthorizationAuditLogger), - getSpace: expect.any(Function), - getSpaceId: expect.any(Function), - exemptConsumerIds: ['exemptConsumerA', 'exemptConsumerB'], }); expect(AlertingAuthorizationAuditLogger).toHaveBeenCalled(); @@ -126,7 +97,6 @@ test('creates an alerting authorization client with proper constructor arguments auditLogger: expect.any(AlertingAuthorizationAuditLogger), getSpace: expect.any(Function), getSpaceId: expect.any(Function), - exemptConsumerIds: [], }); expect(AlertingAuthorizationAuditLogger).toHaveBeenCalled(); diff --git a/x-pack/plugins/alerting/server/alerting_authorization_client_factory.ts b/x-pack/plugins/alerting/server/alerting_authorization_client_factory.ts index 1df67ed8d4b792..27b2d92eba2561 100644 --- a/x-pack/plugins/alerting/server/alerting_authorization_client_factory.ts +++ b/x-pack/plugins/alerting/server/alerting_authorization_client_factory.ts @@ -45,7 +45,7 @@ export class AlertingAuthorizationClientFactory { this.getSpaceId = options.getSpaceId; } - public create(request: KibanaRequest, exemptConsumerIds: string[] = []): AlertingAuthorization { + public create(request: KibanaRequest): AlertingAuthorization { const { securityPluginSetup, securityPluginStart, features } = this; return new AlertingAuthorization({ authorization: securityPluginStart?.authz, @@ -57,7 +57,6 @@ export class AlertingAuthorizationClientFactory { auditLogger: new AlertingAuthorizationAuditLogger( securityPluginSetup?.audit.getLogger(ALERTS_FEATURE_ID) ), - exemptConsumerIds, }); } } 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 6314488af88d75..7f5b06031c18cd 100644 --- a/x-pack/plugins/alerting/server/authorization/alerting_authorization.test.ts +++ b/x-pack/plugins/alerting/server/authorization/alerting_authorization.test.ts @@ -37,8 +37,6 @@ const realAuditLogger = new AlertingAuthorizationAuditLogger(); const getSpace = jest.fn(); const getSpaceId = () => 'space1'; -const exemptConsumerIds: string[] = []; - const mockAuthorizationAction = ( type: string, app: string, @@ -235,7 +233,6 @@ describe('AlertingAuthorization', () => { auditLogger, getSpace, getSpaceId, - exemptConsumerIds, }); expect(getSpace).toHaveBeenCalledWith(request); @@ -251,7 +248,6 @@ describe('AlertingAuthorization', () => { auditLogger, getSpace, getSpaceId, - exemptConsumerIds, }); await alertAuthorization.ensureAuthorized({ @@ -275,7 +271,6 @@ describe('AlertingAuthorization', () => { auditLogger, getSpace, getSpaceId, - exemptConsumerIds, }); await alertAuthorization.ensureAuthorized({ @@ -302,7 +297,6 @@ describe('AlertingAuthorization', () => { auditLogger, getSpace, getSpaceId, - exemptConsumerIds, }); checkPrivileges.mockResolvedValueOnce({ @@ -359,7 +353,6 @@ describe('AlertingAuthorization', () => { auditLogger, getSpace, getSpaceId, - exemptConsumerIds, }); checkPrivileges.mockResolvedValueOnce({ @@ -402,7 +395,7 @@ describe('AlertingAuthorization', () => { `); }); - test('ensures the user has privileges to execute rules for the specified rule type and operation without consumer when consumer is exempt', async () => { + test('ensures the user has privileges to execute rules for the specified rule type and operation without consumer when consumer is alerts', async () => { const { authorization } = mockSecurity(); const checkPrivileges: jest.MockedFunction< ReturnType @@ -416,7 +409,6 @@ describe('AlertingAuthorization', () => { auditLogger, getSpace, getSpaceId, - exemptConsumerIds: ['exemptConsumer'], }); checkPrivileges.mockResolvedValueOnce({ @@ -427,7 +419,7 @@ describe('AlertingAuthorization', () => { await alertAuthorization.ensureAuthorized({ ruleTypeId: 'myType', - consumer: 'exemptConsumer', + consumer: 'alerts', operation: WriteOperations.Create, entity: AlertingAuthorizationEntity.Rule, }); @@ -437,7 +429,7 @@ describe('AlertingAuthorization', () => { expect(authorization.actions.alerting.get).toHaveBeenCalledTimes(2); expect(authorization.actions.alerting.get).toHaveBeenCalledWith( 'myType', - 'exemptConsumer', + 'alerts', 'rule', 'create' ); @@ -458,14 +450,14 @@ describe('AlertingAuthorization', () => { "some-user", "myType", 0, - "exemptConsumer", + "alerts", "create", "rule", ] `); }); - test('ensures the user has privileges to execute alerts for the specified rule type and operation without consumer when consumer is exempt', async () => { + test('ensures the user has privileges to execute alerts for the specified rule type and operation without consumer when consumer is alerts', async () => { const { authorization } = mockSecurity(); const checkPrivileges: jest.MockedFunction< ReturnType @@ -479,7 +471,6 @@ describe('AlertingAuthorization', () => { auditLogger, getSpace, getSpaceId, - exemptConsumerIds: ['exemptConsumer'], }); checkPrivileges.mockResolvedValueOnce({ @@ -490,7 +481,7 @@ describe('AlertingAuthorization', () => { await alertAuthorization.ensureAuthorized({ ruleTypeId: 'myType', - consumer: 'exemptConsumer', + consumer: 'alerts', operation: WriteOperations.Update, entity: AlertingAuthorizationEntity.Alert, }); @@ -500,7 +491,7 @@ describe('AlertingAuthorization', () => { expect(authorization.actions.alerting.get).toHaveBeenCalledTimes(2); expect(authorization.actions.alerting.get).toHaveBeenCalledWith( 'myType', - 'exemptConsumer', + 'alerts', 'alert', 'update' ); @@ -521,7 +512,7 @@ describe('AlertingAuthorization', () => { "some-user", "myType", 0, - "exemptConsumer", + "alerts", "update", "alert", ] @@ -548,7 +539,6 @@ describe('AlertingAuthorization', () => { auditLogger, getSpace, getSpaceId, - exemptConsumerIds, }); await alertAuthorization.ensureAuthorized({ @@ -614,7 +604,6 @@ describe('AlertingAuthorization', () => { auditLogger, getSpace, getSpaceId, - exemptConsumerIds, }); await alertAuthorization.ensureAuthorized({ @@ -674,7 +663,6 @@ describe('AlertingAuthorization', () => { auditLogger, getSpace, getSpaceId, - exemptConsumerIds, }); checkPrivileges.mockResolvedValueOnce({ @@ -733,7 +721,6 @@ describe('AlertingAuthorization', () => { auditLogger, getSpace, getSpaceId, - exemptConsumerIds, }); checkPrivileges.mockResolvedValueOnce({ @@ -796,7 +783,6 @@ describe('AlertingAuthorization', () => { auditLogger, getSpace, getSpaceId, - exemptConsumerIds, }); checkPrivileges.mockResolvedValueOnce({ @@ -855,7 +841,6 @@ describe('AlertingAuthorization', () => { auditLogger, getSpace, getSpaceId, - exemptConsumerIds, }); checkPrivileges.mockResolvedValueOnce({ @@ -947,7 +932,6 @@ describe('AlertingAuthorization', () => { auditLogger, getSpace, getSpaceId, - exemptConsumerIds, }); const { filter, @@ -970,7 +954,6 @@ describe('AlertingAuthorization', () => { auditLogger, getSpace, getSpaceId, - exemptConsumerIds, }); const { ensureRuleTypeIsAuthorized } = await alertAuthorization.getFindAuthorizationFilter( AlertingAuthorizationEntity.Rule, @@ -1005,7 +988,6 @@ describe('AlertingAuthorization', () => { auditLogger, getSpace, getSpaceId, - exemptConsumerIds, }); ruleTypeRegistry.list.mockReturnValue(setOfAlertTypes); expect( @@ -1020,11 +1002,54 @@ describe('AlertingAuthorization', () => { ).filter ).toEqual( esKuery.fromKueryExpression( - `((path.to.rule_type_id:myAppAlertType and consumer-field:(myApp or myOtherApp or myAppWithSubFeature)) or (path.to.rule_type_id:myOtherAppAlertType and consumer-field:(myApp or myOtherApp or myAppWithSubFeature)) or (path.to.rule_type_id:mySecondAppAlertType and consumer-field:(myApp or myOtherApp or myAppWithSubFeature)))` + `((path.to.rule_type_id:myAppAlertType and consumer-field:(alerts or myApp or myOtherApp or myAppWithSubFeature)) or (path.to.rule_type_id:myOtherAppAlertType and consumer-field:(alerts or myApp or myOtherApp or myAppWithSubFeature)) or (path.to.rule_type_id:mySecondAppAlertType and consumer-field:(alerts or myApp or myOtherApp or myAppWithSubFeature)))` ) ); expect(auditLogger.logAuthorizationSuccess).not.toHaveBeenCalled(); }); + test('throws if user has no privileges to any rule type', async () => { + const { authorization } = mockSecurity(); + const checkPrivileges: jest.MockedFunction< + ReturnType + > = jest.fn(); + authorization.checkPrivilegesDynamicallyWithRequest.mockReturnValue(checkPrivileges); + checkPrivileges.mockResolvedValueOnce({ + username: 'some-user', + hasAllRequested: false, + privileges: { + kibana: [ + { + privilege: mockAuthorizationAction('myType', 'myOtherApp', 'rule', 'create'), + authorized: false, + }, + { + privilege: mockAuthorizationAction('myType', 'myApp', 'rule', 'create'), + authorized: false, + }, + ], + }, + }); + const alertAuthorization = new AlertingAuthorization({ + request, + authorization, + ruleTypeRegistry, + features, + auditLogger, + getSpace, + getSpaceId, + }); + ruleTypeRegistry.list.mockReturnValue(setOfAlertTypes); + await expect( + alertAuthorization.getFindAuthorizationFilter(AlertingAuthorizationEntity.Rule, { + type: AlertingAuthorizationFilterType.KQL, + fieldNames: { + ruleTypeId: 'path.to.rule_type_id', + consumer: 'consumer-field', + }, + }) + ).rejects.toThrowErrorMatchingInlineSnapshot(`"Unauthorized some-user/find"`); + expect(auditLogger.logAuthorizationSuccess).not.toHaveBeenCalled(); + }); test('creates an `ensureRuleTypeIsAuthorized` function which throws if type is unauthorized', async () => { const { authorization } = mockSecurity(); const checkPrivileges: jest.MockedFunction< @@ -1068,7 +1093,6 @@ describe('AlertingAuthorization', () => { auditLogger, getSpace, getSpaceId, - exemptConsumerIds, }); ruleTypeRegistry.list.mockReturnValue(setOfAlertTypes); const { ensureRuleTypeIsAuthorized } = await alertAuthorization.getFindAuthorizationFilter( @@ -1142,7 +1166,6 @@ describe('AlertingAuthorization', () => { auditLogger, getSpace, getSpaceId, - exemptConsumerIds, }); ruleTypeRegistry.list.mockReturnValue(setOfAlertTypes); const { ensureRuleTypeIsAuthorized } = await alertAuthorization.getFindAuthorizationFilter( @@ -1217,7 +1240,6 @@ describe('AlertingAuthorization', () => { auditLogger, getSpace, getSpaceId, - exemptConsumerIds, }); ruleTypeRegistry.list.mockReturnValue(setOfAlertTypes); const { @@ -1270,7 +1292,6 @@ describe('AlertingAuthorization', () => { auditLogger, getSpace, getSpaceId, - exemptConsumerIds, }); const { filter } = await alertAuthorization.getFindAuthorizationFilter( AlertingAuthorizationEntity.Alert, @@ -1325,89 +1346,6 @@ describe('AlertingAuthorization', () => { auditLogger, getSpace, getSpaceId, - exemptConsumerIds, - }); - ruleTypeRegistry.list.mockReturnValue(setOfAlertTypes); - - await expect( - alertAuthorization.filterByRuleTypeAuthorization( - new Set([myAppAlertType, myOtherAppAlertType]), - [WriteOperations.Create], - AlertingAuthorizationEntity.Rule - ) - ).resolves.toMatchInlineSnapshot(` - Set { - Object { - "actionGroups": Array [], - "actionVariables": undefined, - "authorizedConsumers": Object { - "myApp": Object { - "all": true, - "read": true, - }, - "myAppWithSubFeature": Object { - "all": true, - "read": true, - }, - "myOtherApp": Object { - "all": true, - "read": true, - }, - }, - "defaultActionGroupId": "default", - "enabledInLicense": true, - "id": "myAppAlertType", - "isExportable": true, - "minimumLicenseRequired": "basic", - "name": "myAppAlertType", - "producer": "myApp", - "recoveryActionGroup": Object { - "id": "recovered", - "name": "Recovered", - }, - }, - Object { - "actionGroups": Array [], - "actionVariables": undefined, - "authorizedConsumers": Object { - "myApp": Object { - "all": true, - "read": true, - }, - "myAppWithSubFeature": Object { - "all": true, - "read": true, - }, - "myOtherApp": Object { - "all": true, - "read": true, - }, - }, - "defaultActionGroupId": "default", - "enabledInLicense": true, - "id": "myOtherAppAlertType", - "isExportable": true, - "minimumLicenseRequired": "basic", - "name": "myOtherAppAlertType", - "producer": "myOtherApp", - "recoveryActionGroup": Object { - "id": "recovered", - "name": "Recovered", - }, - }, - } - `); - }); - - test('augments a list of types with all features and exempt consumer ids when there is no authorization api', async () => { - const alertAuthorization = new AlertingAuthorization({ - request, - ruleTypeRegistry, - features, - auditLogger, - getSpace, - getSpaceId, - exemptConsumerIds: ['exemptConsumerA', 'exemptConsumerB'], }); ruleTypeRegistry.list.mockReturnValue(setOfAlertTypes); @@ -1423,11 +1361,7 @@ describe('AlertingAuthorization', () => { "actionGroups": Array [], "actionVariables": undefined, "authorizedConsumers": Object { - "exemptConsumerA": Object { - "all": true, - "read": true, - }, - "exemptConsumerB": Object { + "alerts": Object { "all": true, "read": true, }, @@ -1460,11 +1394,7 @@ describe('AlertingAuthorization', () => { "actionGroups": Array [], "actionVariables": undefined, "authorizedConsumers": Object { - "exemptConsumerA": Object { - "all": true, - "read": true, - }, - "exemptConsumerB": Object { + "alerts": Object { "all": true, "read": true, }, @@ -1541,7 +1471,6 @@ describe('AlertingAuthorization', () => { auditLogger, getSpace, getSpaceId, - exemptConsumerIds, }); ruleTypeRegistry.list.mockReturnValue(setOfAlertTypes); @@ -1578,113 +1507,7 @@ describe('AlertingAuthorization', () => { "actionGroups": Array [], "actionVariables": undefined, "authorizedConsumers": Object { - "myApp": Object { - "all": true, - "read": true, - }, - "myOtherApp": Object { - "all": true, - "read": true, - }, - }, - "defaultActionGroupId": "default", - "enabledInLicense": true, - "id": "myAppAlertType", - "isExportable": true, - "minimumLicenseRequired": "basic", - "name": "myAppAlertType", - "producer": "myApp", - "recoveryActionGroup": Object { - "id": "recovered", - "name": "Recovered", - }, - }, - } - `); - }); - - test('augments a list of types with consumers and exempt consumer ids under which the operation is authorized', async () => { - const { authorization } = mockSecurity(); - const checkPrivileges: jest.MockedFunction< - ReturnType - > = jest.fn(); - authorization.checkPrivilegesDynamicallyWithRequest.mockReturnValue(checkPrivileges); - checkPrivileges.mockResolvedValueOnce({ - username: 'some-user', - hasAllRequested: false, - privileges: { - kibana: [ - { - privilege: mockAuthorizationAction('myOtherAppAlertType', 'myApp', 'rule', 'create'), - authorized: true, - }, - { - privilege: mockAuthorizationAction( - 'myOtherAppAlertType', - 'myOtherApp', - 'rule', - 'create' - ), - authorized: false, - }, - { - privilege: mockAuthorizationAction('myAppAlertType', 'myApp', 'rule', 'create'), - authorized: true, - }, - { - privilege: mockAuthorizationAction('myAppAlertType', 'myOtherApp', 'rule', 'create'), - authorized: true, - }, - ], - }, - }); - - const alertAuthorization = new AlertingAuthorization({ - request, - authorization, - ruleTypeRegistry, - features, - auditLogger, - getSpace, - getSpaceId, - exemptConsumerIds: ['exemptConsumerA'], - }); - ruleTypeRegistry.list.mockReturnValue(setOfAlertTypes); - - await expect( - alertAuthorization.filterByRuleTypeAuthorization( - new Set([myAppAlertType, myOtherAppAlertType]), - [WriteOperations.Create], - AlertingAuthorizationEntity.Rule - ) - ).resolves.toMatchInlineSnapshot(` - Set { - Object { - "actionGroups": Array [], - "actionVariables": undefined, - "authorizedConsumers": Object { - "myApp": Object { - "all": true, - "read": true, - }, - }, - "defaultActionGroupId": "default", - "enabledInLicense": true, - "id": "myOtherAppAlertType", - "isExportable": true, - "minimumLicenseRequired": "basic", - "name": "myOtherAppAlertType", - "producer": "myOtherApp", - "recoveryActionGroup": Object { - "id": "recovered", - "name": "Recovered", - }, - }, - Object { - "actionGroups": Array [], - "actionVariables": undefined, - "authorizedConsumers": Object { - "exemptConsumerA": Object { + "alerts": Object { "all": true, "read": true, }, @@ -1713,7 +1536,7 @@ describe('AlertingAuthorization', () => { `); }); - test('authorizes user under exempt consumers when they are authorized by the producer', async () => { + test('authorizes user under the `alerts` consumer when they are authorized by the producer', async () => { const { authorization } = mockSecurity(); const checkPrivileges: jest.MockedFunction< ReturnType @@ -1744,7 +1567,6 @@ describe('AlertingAuthorization', () => { auditLogger, getSpace, getSpaceId, - exemptConsumerIds: ['exemptConsumerA'], }); ruleTypeRegistry.list.mockReturnValue(setOfAlertTypes); @@ -1760,7 +1582,7 @@ describe('AlertingAuthorization', () => { "actionGroups": Array [], "actionVariables": undefined, "authorizedConsumers": Object { - "exemptConsumerA": Object { + "alerts": Object { "all": true, "read": true, }, @@ -1850,7 +1672,6 @@ describe('AlertingAuthorization', () => { auditLogger, getSpace, getSpaceId, - exemptConsumerIds, }); ruleTypeRegistry.list.mockReturnValue(setOfAlertTypes); @@ -1866,6 +1687,10 @@ describe('AlertingAuthorization', () => { "actionGroups": Array [], "actionVariables": undefined, "authorizedConsumers": Object { + "alerts": Object { + "all": false, + "read": true, + }, "myApp": Object { "all": true, "read": true, @@ -1891,6 +1716,10 @@ describe('AlertingAuthorization', () => { "actionGroups": Array [], "actionVariables": undefined, "authorizedConsumers": Object { + "alerts": Object { + "all": false, + "read": true, + }, "myApp": Object { "all": false, "read": true, @@ -1960,7 +1789,6 @@ describe('AlertingAuthorization', () => { auditLogger, getSpace, getSpaceId, - exemptConsumerIds, }); ruleTypeRegistry.list.mockReturnValue(setOfAlertTypes); @@ -1976,6 +1804,10 @@ describe('AlertingAuthorization', () => { "actionGroups": Array [], "actionVariables": undefined, "authorizedConsumers": Object { + "alerts": Object { + "all": true, + "read": true, + }, "myApp": Object { "all": true, "read": true, @@ -2067,7 +1899,6 @@ describe('AlertingAuthorization', () => { auditLogger, getSpace, getSpaceId, - exemptConsumerIds, }); ruleTypeRegistry.list.mockReturnValue(setOfAlertTypes); @@ -2142,7 +1973,6 @@ describe('AlertingAuthorization', () => { auditLogger, getSpace, getSpaceId, - exemptConsumerIds, }); ruleTypeRegistry.list.mockReturnValue(setOfAlertTypes); diff --git a/x-pack/plugins/alerting/server/authorization/alerting_authorization.ts b/x-pack/plugins/alerting/server/authorization/alerting_authorization.ts index ed14c2dd7f0ae3..101ab675bb5530 100644 --- a/x-pack/plugins/alerting/server/authorization/alerting_authorization.ts +++ b/x-pack/plugins/alerting/server/authorization/alerting_authorization.ts @@ -9,7 +9,7 @@ import Boom from '@hapi/boom'; import { map, mapValues, fromPairs, has } from 'lodash'; import { KibanaRequest } from 'src/core/server'; import { JsonObject } from '@kbn/utility-types'; -import { RuleTypeRegistry } from '../types'; +import { ALERTS_FEATURE_ID, RuleTypeRegistry } from '../types'; import { SecurityPluginSetup } from '../../../security/server'; import { RegistryRuleType } from '../rule_type_registry'; import { PluginStartContract as FeaturesPluginStart } from '../../../features/server'; @@ -71,7 +71,6 @@ export interface ConstructorOptions { getSpace: (request: KibanaRequest) => Promise; getSpaceId: (request: KibanaRequest) => string | undefined; auditLogger: AlertingAuthorizationAuditLogger; - exemptConsumerIds: string[]; authorization?: SecurityPluginSetup['authz']; } @@ -82,7 +81,6 @@ export class AlertingAuthorization { private readonly auditLogger: AlertingAuthorizationAuditLogger; private readonly featuresIds: Promise>; private readonly allPossibleConsumers: Promise; - private readonly exemptConsumerIds: string[]; private readonly spaceId: string | undefined; constructor({ @@ -93,18 +91,12 @@ export class AlertingAuthorization { auditLogger, getSpace, getSpaceId, - exemptConsumerIds, }: ConstructorOptions) { this.request = request; this.authorization = authorization; this.ruleTypeRegistry = ruleTypeRegistry; this.auditLogger = auditLogger; - // List of consumer ids that are exempt from privilege check. This should be used sparingly. - // An example of this is the Rules Management `consumer` as we don't want to have to - // manually authorize each rule type in the management UI. - this.exemptConsumerIds = exemptConsumerIds; - this.spaceId = getSpaceId(request); this.featuresIds = getSpace(request) @@ -132,7 +124,7 @@ export class AlertingAuthorization { this.allPossibleConsumers = this.featuresIds.then((featuresIds) => { return featuresIds.size - ? asAuthorizedConsumers([...this.exemptConsumerIds, ...featuresIds], { + ? asAuthorizedConsumers([ALERTS_FEATURE_ID, ...featuresIds], { read: true, all: true, }) @@ -185,8 +177,10 @@ export class AlertingAuthorization { ), }; - // Skip authorizing consumer if it is in the list of exempt consumer ids - const shouldAuthorizeConsumer = !this.exemptConsumerIds.includes(consumer); + // Skip authorizing consumer if consumer is the Rules Management consumer (`alerts`) + // This means that rules and their derivative alerts created in the Rules Management UI + // will only be subject to checking if user has access to the rule producer. + const shouldAuthorizeConsumer = consumer !== ALERTS_FEATURE_ID; const checkPrivileges = authorization.checkPrivilegesDynamicallyWithRequest(this.request); const { hasAllRequested, username, privileges } = await checkPrivileges({ @@ -199,8 +193,8 @@ export class AlertingAuthorization { requiredPrivilegesByScope.producer, ] : [ - // skip consumer privilege checks for exempt consumer ids as all rule types can - // be created for exempt consumers if user has producer level privileges + // skip consumer privilege checks under `alerts` as all rule types can + // be created under `alerts` if you have producer level privileges requiredPrivilegesByScope.producer, ], }); @@ -448,14 +442,12 @@ export class AlertingAuthorization { ruleType.authorizedConsumers[feature] ); - if (isAuthorizedAtProducerLevel && this.exemptConsumerIds.length > 0) { - // granting privileges under the producer automatically authorized exempt consumer IDs as well - this.exemptConsumerIds.forEach((exemptId: string) => { - ruleType.authorizedConsumers[exemptId] = mergeHasPrivileges( - hasPrivileges, - ruleType.authorizedConsumers[exemptId] - ); - }); + if (isAuthorizedAtProducerLevel) { + // granting privileges under the producer automatically authorized the Rules Management UI as well + ruleType.authorizedConsumers[ALERTS_FEATURE_ID] = mergeHasPrivileges( + hasPrivileges, + ruleType.authorizedConsumers[ALERTS_FEATURE_ID] + ); } authorizedRuleTypes.add(ruleType); } diff --git a/x-pack/plugins/alerting/server/rules_client_factory.test.ts b/x-pack/plugins/alerting/server/rules_client_factory.test.ts index df59205cf10b3e..188ec652f40ce3 100644 --- a/x-pack/plugins/alerting/server/rules_client_factory.test.ts +++ b/x-pack/plugins/alerting/server/rules_client_factory.test.ts @@ -21,7 +21,6 @@ import { securityMock } from '../../security/server/mocks'; import { PluginStartContract as ActionsStartContract } from '../../actions/server'; import { actionsMock, actionsAuthorizationMock } from '../../actions/server/mocks'; import { LegacyAuditLogger } from '../../security/server'; -import { ALERTS_FEATURE_ID } from '../common'; import { eventLogMock } from '../../event_log/server/mocks'; import { alertingAuthorizationMock } from './authorization/alerting_authorization.mock'; import { alertingAuthorizationClientFactoryMock } from './alerting_authorization_client_factory.mock'; @@ -105,9 +104,7 @@ test('creates an alerts client with proper constructor arguments when security i includedHiddenTypes: ['alert', 'api_key_pending_invalidation'], }); - expect(alertingAuthorizationClientFactory.create).toHaveBeenCalledWith(request, [ - ALERTS_FEATURE_ID, - ]); + expect(alertingAuthorizationClientFactory.create).toHaveBeenCalledWith(request); expect(rulesClientFactoryParams.actions.getActionsAuthorizationWithRequest).toHaveBeenCalledWith( request @@ -148,9 +145,7 @@ test('creates an alerts client with proper constructor arguments', async () => { includedHiddenTypes: ['alert', 'api_key_pending_invalidation'], }); - expect(alertingAuthorizationClientFactory.create).toHaveBeenCalledWith(request, [ - ALERTS_FEATURE_ID, - ]); + expect(alertingAuthorizationClientFactory.create).toHaveBeenCalledWith(request); expect(jest.requireMock('./rules_client').RulesClient).toHaveBeenCalledWith({ unsecuredSavedObjectsClient: savedObjectsClient, diff --git a/x-pack/plugins/alerting/server/rules_client_factory.ts b/x-pack/plugins/alerting/server/rules_client_factory.ts index 336c8e6de20e6f..7961d3761d3efe 100644 --- a/x-pack/plugins/alerting/server/rules_client_factory.ts +++ b/x-pack/plugins/alerting/server/rules_client_factory.ts @@ -19,7 +19,6 @@ import { EncryptedSavedObjectsClient } from '../../encrypted_saved_objects/serve import { TaskManagerStartContract } from '../../task_manager/server'; import { IEventLogClientService } from '../../../plugins/event_log/server'; import { AlertingAuthorizationClientFactory } from './alerting_authorization_client_factory'; -import { ALERTS_FEATURE_ID } from '../common'; export interface RulesClientFactoryOpts { logger: Logger; taskManager: TaskManagerStartContract; @@ -87,7 +86,7 @@ export class RulesClientFactory { excludedWrappers: ['security'], includedHiddenTypes: ['alert', 'api_key_pending_invalidation'], }), - authorization: this.authorization.create(request, [ALERTS_FEATURE_ID]), + authorization: this.authorization.create(request), actionsAuthorization: actions.getActionsAuthorizationWithRequest(request), namespace: this.spaceIdToNamespace(spaceId), encryptedSavedObjectsClient: this.encryptedSavedObjectsClient,