From 44a0c4ebebce2906a3529ea415fd94e59dcb32db Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Thu, 25 Jun 2020 15:03:11 +0100 Subject: [PATCH] incluyde sub features in privilege check --- .../alerts_authorization.test.ts | 74 ++++++++++++++++++- .../authorization/alerts_authorization.ts | 24 ++++-- 2 files changed, 91 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/alerts/server/authorization/alerts_authorization.test.ts b/x-pack/plugins/alerts/server/authorization/alerts_authorization.test.ts index 32e5e36f4db10..280798e002822 100644 --- a/x-pack/plugins/alerts/server/authorization/alerts_authorization.test.ts +++ b/x-pack/plugins/alerts/server/authorization/alerts_authorization.test.ts @@ -68,8 +68,71 @@ function mockFeature(appName: string, typeName?: string) { }); } +function mockFeatureWithSubFeature(appName: string, typeName: string) { + return new Feature({ + id: appName, + name: appName, + app: [], + privileges: { + all: { + savedObject: { + all: [], + read: [], + }, + ui: [], + }, + read: { + savedObject: { + all: [], + read: [], + }, + ui: [], + }, + }, + subFeatures: [ + { + name: appName, + privilegeGroups: [ + { + groupType: 'independent', + privileges: [ + { + id: 'doSomethingAlertRelated', + name: 'sub feature alert', + includeIn: 'all', + alerting: { + all: [typeName], + }, + savedObject: { + all: [], + read: [], + }, + ui: ['doSomethingAlertRelated'], + }, + { + id: 'doSomethingAlertRelated', + name: 'sub feature alert', + includeIn: 'read', + alerting: { + read: [typeName], + }, + savedObject: { + all: [], + read: [], + }, + ui: ['doSomethingAlertRelated'], + }, + ], + }, + ], + }, + ], + }); +} + const myAppFeature = mockFeature('myApp', 'myType'); const myOtherAppFeature = mockFeature('myOtherApp', 'myType'); +const myAppWithSubFeature = mockFeatureWithSubFeature('myAppWithSubFeature', 'myType'); const myFeatureWithoutAlerting = mockFeature('myOtherApp'); beforeEach(() => { @@ -91,7 +154,12 @@ beforeEach(() => { async executor() {}, producer: 'myApp', })); - features.getFeatures.mockReturnValue([myAppFeature, myOtherAppFeature, myFeatureWithoutAlerting]); + features.getFeatures.mockReturnValue([ + myAppFeature, + myOtherAppFeature, + myAppWithSubFeature, + myFeatureWithoutAlerting, + ]); }); describe('ensureAuthorized', () => { @@ -460,7 +528,7 @@ describe('getFindAuthorizationFilter', () => { alertTypeRegistry.list.mockReturnValue(setOfAlertTypes); expect((await alertAuthorization.getFindAuthorizationFilter()).filter).toMatchInlineSnapshot( - `"((alert.attributes.alertTypeId:myAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp)) or (alert.attributes.alertTypeId:alertingAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp)))"` + `"((alert.attributes.alertTypeId:myAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)) or (alert.attributes.alertTypeId:alertingAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)))"` ); expect(auditLogger.alertsAuthorizationSuccess).not.toHaveBeenCalled(); @@ -639,6 +707,7 @@ describe('filterByAlertTypeAuthorization', () => { "alerts", "myApp", "myOtherApp", + "myAppWithSubFeature", ], "defaultActionGroupId": "default", "id": "myAppAlertType", @@ -652,6 +721,7 @@ describe('filterByAlertTypeAuthorization', () => { "alerts", "myApp", "myOtherApp", + "myAppWithSubFeature", ], "defaultActionGroupId": "default", "id": "alertingAlertType", diff --git a/x-pack/plugins/alerts/server/authorization/alerts_authorization.ts b/x-pack/plugins/alerts/server/authorization/alerts_authorization.ts index 3b70905d24c5a..ddb7bbc404169 100644 --- a/x-pack/plugins/alerts/server/authorization/alerts_authorization.ts +++ b/x-pack/plugins/alerts/server/authorization/alerts_authorization.ts @@ -11,6 +11,7 @@ import { ALERTS_FEATURE_ID } from '../../common'; import { AlertTypeRegistry } from '../types'; import { SecurityPluginSetup } from '../../../security/server'; import { RegistryAlertType } from '../alert_type_registry'; +import { FeatureKibanaPrivileges, SubFeaturePrivilegeConfig } from '../../../features/common'; import { PluginStartContract as FeaturesPluginStart } from '../../../features/server'; import { AlertsAuthorizationAuditLogger, ScopeType } from './audit_logger'; @@ -189,12 +190,17 @@ export class AlertsAuthorization { const featuresIds = this.features .getFeatures() // ignore features which don't grant privileges to alerting - .filter(({ privileges }) => { + .filter(({ privileges, subFeatures }) => { return ( - (privileges?.all.alerting?.all?.length ?? 0 > 0) || - (privileges?.all.alerting?.read?.length ?? 0 > 0) || - (privileges?.read.alerting?.all?.length ?? 0 > 0) || - (privileges?.read.alerting?.read?.length ?? 0 > 0) + hasAnyAlertingPrivileges(privileges?.all) || + hasAnyAlertingPrivileges(privileges?.read) || + subFeatures.some((subFeature) => + subFeature.privilegeGroups.some((privilegeGroup) => + privilegeGroup.privileges.some((subPrivileges) => + hasAnyAlertingPrivileges(subPrivileges) + ) + ) + ) ); }) .map((feature) => feature.id); @@ -294,3 +300,11 @@ export function ensureFieldIsSafeForQuery(field: string, value: string): boolean } return true; } + +function hasAnyAlertingPrivileges( + privileges?: FeatureKibanaPrivileges | SubFeaturePrivilegeConfig +): boolean { + return ( + ((privileges?.alerting?.all?.length ?? 0) || (privileges?.alerting?.read?.length ?? 0)) > 0 + ); +}