Skip to content

Commit

Permalink
Fix sub-feature privilege "minimumLicense" bug (#106008)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jportner
jportner authored Jul 19, 2021
1 parent cc1dd52 commit ea06239
Show file tree
Hide file tree
Showing 13 changed files with 178 additions and 297 deletions.
260 changes: 106 additions & 154 deletions x-pack/plugins/features/server/feature_privilege_iterator/feature_privilege_iterator.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,24 @@
* 2.0.
*/

import type { LicenseType } from '../../../licensing/common/types';
import { LICENSE_TYPE } from '../../../licensing/server';
import { KibanaFeature } from '../';
import { SubFeaturePrivilegeConfig } from '../../common';
import type { FeaturePrivilegeIteratorOptions } from './feature_privilege_iterator';
import { featurePrivilegeIterator } from './feature_privilege_iterator';

function getFeaturePrivilegeIterator(
feature: KibanaFeature,
options: Omit<FeaturePrivilegeIteratorOptions, 'licenseHasAtLeast'> & { licenseType: LicenseType }
) {
const { licenseType, ...otherOptions } = options;
const licenseHasAtLeast = (licenseTypeToCheck: LicenseType) => {
return LICENSE_TYPE[licenseTypeToCheck] <= LICENSE_TYPE[options.licenseType];
};
return featurePrivilegeIterator(feature, { licenseHasAtLeast, ...otherOptions });
}

describe('featurePrivilegeIterator', () => {
it('handles features with no privileges', () => {
const feature = new KibanaFeature({
Expand All @@ -19,7 +34,7 @@ describe('featurePrivilegeIterator', () => {
});

const actualPrivileges = Array.from(
featurePrivilegeIterator(feature, {
getFeaturePrivilegeIterator(feature, {
augmentWithSubFeaturePrivileges: true,
licenseType: 'basic',
})
Expand Down Expand Up @@ -90,7 +105,7 @@ describe('featurePrivilegeIterator', () => {
});

const actualPrivileges = Array.from(
featurePrivilegeIterator(feature, {
getFeaturePrivilegeIterator(feature, {
augmentWithSubFeaturePrivileges: true,
licenseType: 'basic',
})
Expand Down Expand Up @@ -219,7 +234,7 @@ describe('featurePrivilegeIterator', () => {
});

const actualPrivileges = Array.from(
featurePrivilegeIterator(feature, {
getFeaturePrivilegeIterator(feature, {
augmentWithSubFeaturePrivileges: true,
licenseType: 'basic',
predicate: (privilegeId) => privilegeId === 'all',
Expand Down Expand Up @@ -357,7 +372,7 @@ describe('featurePrivilegeIterator', () => {
});

const actualPrivileges = Array.from(
featurePrivilegeIterator(feature, {
getFeaturePrivilegeIterator(feature, {
augmentWithSubFeaturePrivileges: false,
licenseType: 'basic',
})
Expand Down Expand Up @@ -519,7 +534,7 @@ describe('featurePrivilegeIterator', () => {
});

const actualPrivileges = Array.from(
featurePrivilegeIterator(feature, {
getFeaturePrivilegeIterator(feature, {
augmentWithSubFeaturePrivileges: true,
licenseType: 'basic',
})
Expand Down Expand Up @@ -682,7 +697,7 @@ describe('featurePrivilegeIterator', () => {
});

const actualPrivileges = Array.from(
featurePrivilegeIterator(feature, {
getFeaturePrivilegeIterator(feature, {
augmentWithSubFeaturePrivileges: true,
licenseType: 'basic',
})
Expand Down Expand Up @@ -852,7 +867,7 @@ describe('featurePrivilegeIterator', () => {
});

const actualPrivileges = Array.from(
featurePrivilegeIterator(feature, {
getFeaturePrivilegeIterator(feature, {
augmentWithSubFeaturePrivileges: true,
licenseType: 'basic',
})
Expand Down Expand Up @@ -1020,7 +1035,7 @@ describe('featurePrivilegeIterator', () => {
});

const actualPrivileges = Array.from(
featurePrivilegeIterator(feature, {
getFeaturePrivilegeIterator(feature, {
augmentWithSubFeaturePrivileges: true,
licenseType: 'basic',
})
Expand Down Expand Up @@ -1088,168 +1103,105 @@ describe('featurePrivilegeIterator', () => {
]);
});

it('excludes sub feature privileges when the minimum license is not met', () => {
describe('excludes sub-feature privileges when the minimum license is not met', () => {
function createSubFeaturePrivilegeConfig(licenseType: LicenseType): SubFeaturePrivilegeConfig {
return {
// This is not a realistic sub-feature privilege config, but we only need the "api" string for our test cases
id: `${licenseType}-sub-feature`,
name: '',
includeIn: 'all',
minimumLicense: licenseType,
api: [`${licenseType}-api`],
savedObject: { all: [], read: [] },
ui: [],
};
}

const feature = new KibanaFeature({
id: 'foo',
name: 'foo',
id: 'feature',
name: 'feature-name',
app: [],
category: { id: 'foo', label: 'foo' },
category: { id: 'category-id', label: 'category-label' },
privileges: {
all: {
api: ['all-api', 'read-api'],
app: ['foo'],
catalogue: ['foo-catalogue'],
management: {
section: ['foo-management'],
},
savedObject: {
all: ['all-type'],
read: ['read-type'],
},
alerting: {
rule: {
all: ['alerting-all-type'],
},
alert: {
read: ['alerting-another-read-type'],
},
},
cases: {
all: ['cases-all-type'],
read: ['cases-read-type'],
},
ui: ['ui-action'],
},
read: {
api: ['read-api'],
app: ['foo'],
catalogue: ['foo-catalogue'],
management: {
section: ['foo-management'],
},
savedObject: {
all: [],
read: ['read-type'],
},
alerting: {
rule: {
read: ['alerting-read-type'],
},
alert: {
read: ['alerting-read-type'],
},
},
cases: {
read: ['cases-read-type'],
},
ui: ['ui-action'],
},
all: { savedObject: { all: ['obj-type'], read: [] }, ui: [] },
read: { savedObject: { all: [], read: ['obj-type'] }, ui: [] },
},
subFeatures: [
{
name: 'sub feature 1',
name: `sub-feature-name`,
privilegeGroups: [
{
groupType: 'independent',
privileges: [
{
id: 'sub-feature-priv-1',
name: 'first sub feature privilege',
includeIn: 'all',
minimumLicense: 'gold',
api: ['sub-feature-api'],
app: ['sub-app'],
catalogue: ['sub-catalogue'],
management: {
section: ['other-sub-management'],
kibana: ['sub-management'],
},
savedObject: {
all: ['all-sub-type'],
read: ['read-sub-type'],
},
alerting: {
alert: {
all: ['alerting-all-sub-type'],
},
},
cases: {
all: ['cases-all-sub-type'],
read: ['cases-read-sub-type'],
},
ui: ['ui-sub-type'],
},
createSubFeaturePrivilegeConfig('gold'),
createSubFeaturePrivilegeConfig('platinum'),
createSubFeaturePrivilegeConfig('enterprise'),
// Note: we intentionally do not include a sub-feature privilege config for the "trial" license because that should never be used
],
},
],
},
],
});

const actualPrivileges = Array.from(
featurePrivilegeIterator(feature, {
augmentWithSubFeaturePrivileges: true,
licenseType: 'basic',
})
);
// Each of the test cases below is a minimal check to make sure the correct sub-feature privileges are applied -- nothing more, nothing less
// Note: we do not include a test case for the "basic" license, because sub-feature privileges are not enabled at that license level

expect(actualPrivileges).toEqual([
{
privilegeId: 'all',
privilege: {
api: ['all-api', 'read-api'],
app: ['foo'],
catalogue: ['foo-catalogue'],
management: {
section: ['foo-management'],
},
savedObject: {
all: ['all-type'],
read: ['read-type'],
},
alerting: {
rule: {
all: ['alerting-all-type'],
},
alert: {
read: ['alerting-another-read-type'],
},
},
cases: {
all: ['cases-all-type'],
read: ['cases-read-type'],
},
ui: ['ui-action'],
},
},
{
privilegeId: 'read',
privilege: {
api: ['read-api'],
app: ['foo'],
catalogue: ['foo-catalogue'],
management: {
section: ['foo-management'],
},
savedObject: {
all: [],
read: ['read-type'],
},
alerting: {
rule: {
read: ['alerting-read-type'],
},
alert: {
read: ['alerting-read-type'],
},
},
cases: {
read: ['cases-read-type'],
},
ui: ['ui-action'],
},
},
]);
it('with a gold license', () => {
const actualPrivileges = Array.from(
getFeaturePrivilegeIterator(feature, {
augmentWithSubFeaturePrivileges: true,
licenseType: 'gold',
})
);
const expectedPrivilege = expect.objectContaining({ api: ['gold-api'] });
expect(actualPrivileges).toEqual(
expect.arrayContaining([{ privilegeId: 'all', privilege: expectedPrivilege }])
);
});

it('with a platinum license', () => {
const actualPrivileges = Array.from(
getFeaturePrivilegeIterator(feature, {
augmentWithSubFeaturePrivileges: true,
licenseType: 'platinum',
})
);
const expectedPrivilege = expect.objectContaining({ api: ['gold-api', 'platinum-api'] });
expect(actualPrivileges).toEqual(
expect.arrayContaining([{ privilegeId: 'all', privilege: expectedPrivilege }])
);
});

it('with an enterprise license', () => {
const actualPrivileges = Array.from(
getFeaturePrivilegeIterator(feature, {
augmentWithSubFeaturePrivileges: true,
licenseType: 'enterprise',
})
);
const expectedPrivilege = expect.objectContaining({
api: ['gold-api', 'platinum-api', 'enterprise-api'],
});
expect(actualPrivileges).toEqual(
expect.arrayContaining([{ privilegeId: 'all', privilege: expectedPrivilege }])
);
});

it('with a trial license', () => {
const actualPrivileges = Array.from(
getFeaturePrivilegeIterator(feature, {
augmentWithSubFeaturePrivileges: true,
licenseType: 'trial',
})
);
const expectedPrivilege = expect.objectContaining({
api: ['gold-api', 'platinum-api', 'enterprise-api'],
});
expect(actualPrivileges).toEqual(
expect.arrayContaining([{ privilegeId: 'all', privilege: expectedPrivilege }])
);
});
});

it(`can augment primary feature privileges even if they don't specify their own`, () => {
Expand Down Expand Up @@ -1316,7 +1268,7 @@ describe('featurePrivilegeIterator', () => {
});

const actualPrivileges = Array.from(
featurePrivilegeIterator(feature, {
getFeaturePrivilegeIterator(feature, {
augmentWithSubFeaturePrivileges: true,
licenseType: 'basic',
})
Expand Down Expand Up @@ -1470,7 +1422,7 @@ describe('featurePrivilegeIterator', () => {
});

const actualPrivileges = Array.from(
featurePrivilegeIterator(feature, {
getFeaturePrivilegeIterator(feature, {
augmentWithSubFeaturePrivileges: true,
licenseType: 'basic',
})
Expand Down
11 changes: 6 additions & 5 deletions x-pack/plugins/features/server/feature_privilege_iterator/feature_privilege_iterator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@ export interface FeaturePrivilegeIteratorOptions {
augmentWithSubFeaturePrivileges: boolean;

/**
* The current license type. Controls which sub-features are returned, as they may have different license terms than the overall feature.
* Function that returns whether the current license is equal to or greater than the given license type.
* Controls which sub-features are returned, as they may have different license terms than the overall feature.
*/
licenseType: LicenseType;
licenseHasAtLeast: (licenseType: LicenseType) => boolean | undefined;

/**
* Optional predicate to filter the returned set of privileges.
Expand Down Expand Up @@ -59,7 +60,7 @@ const featurePrivilegeIterator: FeaturePrivilegeIterator = function* featurePriv
if (options.augmentWithSubFeaturePrivileges) {
yield {
privilegeId,
privilege: mergeWithSubFeatures(privilegeId, privilege, feature, options.licenseType),
privilege: mergeWithSubFeatures(privilegeId, privilege, feature, options.licenseHasAtLeast),
};
} else {
yield { privilegeId, privilege };
Expand All @@ -71,10 +72,10 @@ function mergeWithSubFeatures(
privilegeId: string,
privilege: FeatureKibanaPrivileges,
feature: KibanaFeature,
licenseType: LicenseType
licenseHasAtLeast: FeaturePrivilegeIteratorOptions['licenseHasAtLeast']
) {
const mergedConfig = _.cloneDeep(privilege);
for (const subFeaturePrivilege of subFeaturePrivilegeIterator(feature, licenseType)) {
for (const subFeaturePrivilege of subFeaturePrivilegeIterator(feature, licenseHasAtLeast)) {
if (subFeaturePrivilege.includeIn !== 'read' && subFeaturePrivilege.includeIn !== privilegeId) {
continue;
}
Expand Down
Loading

0 comments on commit ea06239

Please sign in to comment.