Skip to content

Commit

Permalink
incluyde sub features in privilege check
Browse files Browse the repository at this point in the history
  • Loading branch information
gmmorris committed Jun 25, 2020
1 parent 8b2a423 commit 44a0c4e
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand All @@ -91,7 +154,12 @@ beforeEach(() => {
async executor() {},
producer: 'myApp',
}));
features.getFeatures.mockReturnValue([myAppFeature, myOtherAppFeature, myFeatureWithoutAlerting]);
features.getFeatures.mockReturnValue([
myAppFeature,
myOtherAppFeature,
myAppWithSubFeature,
myFeatureWithoutAlerting,
]);
});

describe('ensureAuthorized', () => {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -639,6 +707,7 @@ describe('filterByAlertTypeAuthorization', () => {
"alerts",
"myApp",
"myOtherApp",
"myAppWithSubFeature",
],
"defaultActionGroupId": "default",
"id": "myAppAlertType",
Expand All @@ -652,6 +721,7 @@ describe('filterByAlertTypeAuthorization', () => {
"alerts",
"myApp",
"myOtherApp",
"myAppWithSubFeature",
],
"defaultActionGroupId": "default",
"id": "alertingAlertType",
Expand Down
24 changes: 19 additions & 5 deletions x-pack/plugins/alerts/server/authorization/alerts_authorization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
);
}

0 comments on commit 44a0c4e

Please sign in to comment.