From 6114d3a0848addea9207c6a7176a20d669b5031f Mon Sep 17 00:00:00 2001 From: Joe Portner <5295965+jportner@users.noreply.github.com> Date: Mon, 19 Jul 2021 10:47:41 -0400 Subject: [PATCH] Fix sub-feature privilege "minimumLicense" bug (#106008) --- .../feature_privilege_iterator.test.ts | 260 +++++++----------- .../feature_privilege_iterator.ts | 11 +- .../sub_feature_privilege_iterator.ts | 6 +- .../features/server/oss_features.test.ts | 5 +- .../security/common/licensing/index.mock.ts | 18 +- .../common/licensing/license_service.ts | 4 +- .../roles/__fixtures__/kibana_privileges.ts | 3 + .../elasticsearch_privileges.test.tsx.snap | 2 +- .../index_privileges.test.tsx.snap | 2 +- .../plugins/security/public/plugin.test.tsx | 4 +- .../privileges/privileges.test.ts | 146 ++-------- .../authorization/privileges/privileges.ts | 12 +- x-pack/plugins/security/server/plugin.test.ts | 2 +- 13 files changed, 178 insertions(+), 297 deletions(-) diff --git a/x-pack/plugins/features/server/feature_privilege_iterator/feature_privilege_iterator.test.ts b/x-pack/plugins/features/server/feature_privilege_iterator/feature_privilege_iterator.test.ts index 6275d6aa9dce9..d7ba5f05e691b 100644 --- a/x-pack/plugins/features/server/feature_privilege_iterator/feature_privilege_iterator.test.ts +++ b/x-pack/plugins/features/server/feature_privilege_iterator/feature_privilege_iterator.test.ts @@ -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 & { 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({ @@ -19,7 +34,7 @@ describe('featurePrivilegeIterator', () => { }); const actualPrivileges = Array.from( - featurePrivilegeIterator(feature, { + getFeaturePrivilegeIterator(feature, { augmentWithSubFeaturePrivileges: true, licenseType: 'basic', }) @@ -90,7 +105,7 @@ describe('featurePrivilegeIterator', () => { }); const actualPrivileges = Array.from( - featurePrivilegeIterator(feature, { + getFeaturePrivilegeIterator(feature, { augmentWithSubFeaturePrivileges: true, licenseType: 'basic', }) @@ -219,7 +234,7 @@ describe('featurePrivilegeIterator', () => { }); const actualPrivileges = Array.from( - featurePrivilegeIterator(feature, { + getFeaturePrivilegeIterator(feature, { augmentWithSubFeaturePrivileges: true, licenseType: 'basic', predicate: (privilegeId) => privilegeId === 'all', @@ -357,7 +372,7 @@ describe('featurePrivilegeIterator', () => { }); const actualPrivileges = Array.from( - featurePrivilegeIterator(feature, { + getFeaturePrivilegeIterator(feature, { augmentWithSubFeaturePrivileges: false, licenseType: 'basic', }) @@ -519,7 +534,7 @@ describe('featurePrivilegeIterator', () => { }); const actualPrivileges = Array.from( - featurePrivilegeIterator(feature, { + getFeaturePrivilegeIterator(feature, { augmentWithSubFeaturePrivileges: true, licenseType: 'basic', }) @@ -682,7 +697,7 @@ describe('featurePrivilegeIterator', () => { }); const actualPrivileges = Array.from( - featurePrivilegeIterator(feature, { + getFeaturePrivilegeIterator(feature, { augmentWithSubFeaturePrivileges: true, licenseType: 'basic', }) @@ -852,7 +867,7 @@ describe('featurePrivilegeIterator', () => { }); const actualPrivileges = Array.from( - featurePrivilegeIterator(feature, { + getFeaturePrivilegeIterator(feature, { augmentWithSubFeaturePrivileges: true, licenseType: 'basic', }) @@ -1020,7 +1035,7 @@ describe('featurePrivilegeIterator', () => { }); const actualPrivileges = Array.from( - featurePrivilegeIterator(feature, { + getFeaturePrivilegeIterator(feature, { augmentWithSubFeaturePrivileges: true, licenseType: 'basic', }) @@ -1088,97 +1103,40 @@ 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 ], }, ], @@ -1186,70 +1144,64 @@ describe('featurePrivilegeIterator', () => { ], }); - 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`, () => { @@ -1316,7 +1268,7 @@ describe('featurePrivilegeIterator', () => { }); const actualPrivileges = Array.from( - featurePrivilegeIterator(feature, { + getFeaturePrivilegeIterator(feature, { augmentWithSubFeaturePrivileges: true, licenseType: 'basic', }) @@ -1470,7 +1422,7 @@ describe('featurePrivilegeIterator', () => { }); const actualPrivileges = Array.from( - featurePrivilegeIterator(feature, { + getFeaturePrivilegeIterator(feature, { augmentWithSubFeaturePrivileges: true, licenseType: 'basic', }) diff --git a/x-pack/plugins/features/server/feature_privilege_iterator/feature_privilege_iterator.ts b/x-pack/plugins/features/server/feature_privilege_iterator/feature_privilege_iterator.ts index 8843423ed3c43..86a232ca68d2f 100644 --- a/x-pack/plugins/features/server/feature_privilege_iterator/feature_privilege_iterator.ts +++ b/x-pack/plugins/features/server/feature_privilege_iterator/feature_privilege_iterator.ts @@ -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. @@ -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 }; @@ -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; } diff --git a/x-pack/plugins/features/server/feature_privilege_iterator/sub_feature_privilege_iterator.ts b/x-pack/plugins/features/server/feature_privilege_iterator/sub_feature_privilege_iterator.ts index e4cc52bca81bc..4e43643338830 100644 --- a/x-pack/plugins/features/server/feature_privilege_iterator/sub_feature_privilege_iterator.ts +++ b/x-pack/plugins/features/server/feature_privilege_iterator/sub_feature_privilege_iterator.ts @@ -16,17 +16,17 @@ import type { LicenseType } from '../../../licensing/server'; */ export type SubFeaturePrivilegeIterator = ( feature: KibanaFeature, - licenseType: LicenseType + licenseHasAtLeast: (licenseType: LicenseType) => boolean | undefined ) => IterableIterator; const subFeaturePrivilegeIterator: SubFeaturePrivilegeIterator = function* subFeaturePrivilegeIterator( feature: KibanaFeature, - licenseType: LicenseType + licenseHasAtLeast: (licenseType: LicenseType) => boolean | undefined ): IterableIterator { for (const subFeature of feature.subFeatures) { for (const group of subFeature.privilegeGroups) { yield* group.privileges.filter( - (privilege) => !privilege.minimumLicense || privilege.minimumLicense <= licenseType + (privilege) => !privilege.minimumLicense || licenseHasAtLeast(privilege.minimumLicense) ); } } diff --git a/x-pack/plugins/features/server/oss_features.test.ts b/x-pack/plugins/features/server/oss_features.test.ts index 207abaeee9472..39bb12d90ea1c 100644 --- a/x-pack/plugins/features/server/oss_features.test.ts +++ b/x-pack/plugins/features/server/oss_features.test.ts @@ -8,7 +8,7 @@ import { buildOSSFeatures } from './oss_features'; import { featurePrivilegeIterator } from './feature_privilege_iterator'; import { KibanaFeature } from '.'; -import { LicenseType } from '../../licensing/server'; +import { LicenseType, LICENSE_TYPE } from '../../licensing/server'; describe('buildOSSFeatures', () => { it('returns features including timelion', () => { @@ -86,7 +86,8 @@ Array [ new KibanaFeature(featureConfig), { augmentWithSubFeaturePrivileges: true, - licenseType, + licenseHasAtLeast: (licenseTypeToCheck: LicenseType) => + LICENSE_TYPE[licenseTypeToCheck] <= LICENSE_TYPE[licenseType], } )) { privileges.push(featurePrivilege); diff --git a/x-pack/plugins/security/common/licensing/index.mock.ts b/x-pack/plugins/security/common/licensing/index.mock.ts index 319faa1a5b344..bd85df3dea638 100644 --- a/x-pack/plugins/security/common/licensing/index.mock.ts +++ b/x-pack/plugins/security/common/licensing/index.mock.ts @@ -7,15 +7,27 @@ import { of } from 'rxjs'; +// eslint-disable-next-line @kbn/eslint/no-restricted-paths +import { LICENSE_TYPE } from '../../../licensing/server'; +// eslint-disable-next-line @kbn/eslint/no-restricted-paths +import type { LicenseType } from '../../../licensing/server'; import type { SecurityLicenseFeatures } from './license_features'; import type { SecurityLicense } from './license_service'; export const licenseMock = { - create: (features?: Partial): jest.Mocked => ({ + create: ( + features: Partial = {}, + licenseType: LicenseType = 'basic' // default to basic if this is not specified + ): jest.Mocked => ({ isLicenseAvailable: jest.fn().mockReturnValue(true), isEnabled: jest.fn().mockReturnValue(true), - getType: jest.fn().mockReturnValue('basic'), - getFeatures: jest.fn(), + getFeatures: jest.fn().mockReturnValue(features), + hasAtLeast: jest + .fn() + .mockImplementation( + (licenseTypeToCheck: LicenseType) => + LICENSE_TYPE[licenseTypeToCheck] <= LICENSE_TYPE[licenseType] + ), features$: features ? of(features as SecurityLicenseFeatures) : of(), }), }; diff --git a/x-pack/plugins/security/common/licensing/license_service.ts b/x-pack/plugins/security/common/licensing/license_service.ts index 7bebf65b48205..51093428e84a0 100644 --- a/x-pack/plugins/security/common/licensing/license_service.ts +++ b/x-pack/plugins/security/common/licensing/license_service.ts @@ -14,8 +14,8 @@ import type { SecurityLicenseFeatures } from './license_features'; export interface SecurityLicense { isLicenseAvailable(): boolean; isEnabled(): boolean; - getType(): LicenseType | undefined; getFeatures(): SecurityLicenseFeatures; + hasAtLeast(licenseType: LicenseType): boolean | undefined; features$: Observable; } @@ -39,7 +39,7 @@ export class SecurityLicenseService { isEnabled: () => this.isSecurityEnabledFromRawLicense(rawLicense), - getType: () => rawLicense?.type, + hasAtLeast: (licenseType: LicenseType) => rawLicense?.hasAtLeast(licenseType), getFeatures: () => this.calculateFeaturesFromRawLicense(rawLicense), diff --git a/x-pack/plugins/security/public/management/roles/__fixtures__/kibana_privileges.ts b/x-pack/plugins/security/public/management/roles/__fixtures__/kibana_privileges.ts index de829ea003b00..cd72085c9a16d 100644 --- a/x-pack/plugins/security/public/management/roles/__fixtures__/kibana_privileges.ts +++ b/x-pack/plugins/security/public/management/roles/__fixtures__/kibana_privileges.ts @@ -8,6 +8,8 @@ import type { KibanaFeature } from '../../../../../features/public'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { featuresPluginMock } from '../../../../../features/server/mocks'; +// eslint-disable-next-line @kbn/eslint/no-restricted-paths +import type { LicenseType } from '../../../../../licensing/server'; import type { SecurityLicenseFeatures } from '../../../../common/licensing'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { Actions } from '../../../../server/authorization'; @@ -25,6 +27,7 @@ export const createRawKibanaPrivileges = ( const licensingService = { getFeatures: () => ({ allowSubFeaturePrivileges } as SecurityLicenseFeatures), getType: () => 'basic' as const, + hasAtLeast: (licenseType: LicenseType) => licenseType === 'basic', }; return privilegesFactory( diff --git a/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/__snapshots__/elasticsearch_privileges.test.tsx.snap b/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/__snapshots__/elasticsearch_privileges.test.tsx.snap index 5413bad801872..9db96be27faaa 100644 --- a/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/__snapshots__/elasticsearch_privileges.test.tsx.snap +++ b/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/__snapshots__/elasticsearch_privileges.test.tsx.snap @@ -183,7 +183,7 @@ exports[`it renders without crashing 1`] = ` "_subscribe": [Function], }, "getFeatures": [MockFunction], - "getType": [MockFunction], + "hasAtLeast": [MockFunction], "isEnabled": [MockFunction], "isLicenseAvailable": [MockFunction], } diff --git a/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/__snapshots__/index_privileges.test.tsx.snap b/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/__snapshots__/index_privileges.test.tsx.snap index 6dc3165cbea17..12824eb9193d1 100644 --- a/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/__snapshots__/index_privileges.test.tsx.snap +++ b/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/__snapshots__/index_privileges.test.tsx.snap @@ -24,7 +24,7 @@ exports[`it renders without crashing 1`] = ` "_subscribe": [Function], }, "getFeatures": [MockFunction], - "getType": [MockFunction], + "hasAtLeast": [MockFunction], "isEnabled": [MockFunction], "isLicenseAvailable": [MockFunction], } diff --git a/x-pack/plugins/security/public/plugin.test.tsx b/x-pack/plugins/security/public/plugin.test.tsx index c4c551e4bb5b5..bcc69bbfbd853 100644 --- a/x-pack/plugins/security/public/plugin.test.tsx +++ b/x-pack/plugins/security/public/plugin.test.tsx @@ -46,8 +46,8 @@ describe('Security Plugin', () => { license: { isLicenseAvailable: expect.any(Function), isEnabled: expect.any(Function), - getType: expect.any(Function), getFeatures: expect.any(Function), + hasAtLeast: expect.any(Function), features$: expect.any(Observable), }, }); @@ -74,8 +74,8 @@ describe('Security Plugin', () => { license: { isLicenseAvailable: expect.any(Function), isEnabled: expect.any(Function), - getType: expect.any(Function), getFeatures: expect.any(Function), + hasAtLeast: expect.any(Function), features$: expect.any(Observable), }, management: managementSetupMock, diff --git a/x-pack/plugins/security/server/authorization/privileges/privileges.test.ts b/x-pack/plugins/security/server/authorization/privileges/privileges.test.ts index dfe6bef1e00e0..5264e74861be1 100644 --- a/x-pack/plugins/security/server/authorization/privileges/privileges.test.ts +++ b/x-pack/plugins/security/server/authorization/privileges/privileges.test.ts @@ -7,11 +7,19 @@ import { KibanaFeature } from '../../../../features/server'; import { featuresPluginMock } from '../../../../features/server/mocks'; +import { licenseMock } from '../../../common/licensing/index.mock'; import { Actions } from '../actions'; import { privilegesFactory } from './privileges'; const actions = new Actions('1.0.0-zeta1'); +const mockLicenseServiceBasic = licenseMock.create({ allowSubFeaturePrivileges: false }, 'basic'); +const mockLicenseServiceGold = licenseMock.create({ allowSubFeaturePrivileges: true }, 'gold'); +const mockLicenseServicePlatinum = licenseMock.create( + { allowSubFeaturePrivileges: true }, + 'platinum' +); + describe('features', () => { test('actions defined at the feature do not cascade to the privileges', () => { const features: KibanaFeature[] = [ @@ -43,14 +51,10 @@ describe('features', () => { }), ]; - const mockFeaturesService = featuresPluginMock.createSetup(); - mockFeaturesService.getKibanaFeatures.mockReturnValue(features); + const mockFeaturesPlugin = featuresPluginMock.createSetup(); + mockFeaturesPlugin.getKibanaFeatures.mockReturnValue(features); - const mockLicenseService = { - getFeatures: jest.fn().mockReturnValue({ allowSubFeaturePrivileges: true }), - getType: jest.fn().mockReturnValue('basic'), - }; - const privileges = privilegesFactory(actions, mockFeaturesService, mockLicenseService); + const privileges = privilegesFactory(actions, mockFeaturesPlugin, mockLicenseServiceBasic); const actual = privileges.get(); expect(actual).toHaveProperty('features.foo-feature', { @@ -87,11 +91,7 @@ describe('features', () => { const mockFeaturesPlugin = featuresPluginMock.createSetup(); mockFeaturesPlugin.getKibanaFeatures.mockReturnValue(features); - const mockLicenseService = { - getFeatures: jest.fn().mockReturnValue({ allowSubFeaturePrivileges: true }), - getType: jest.fn().mockReturnValue('basic'), - }; - const privileges = privilegesFactory(actions, mockFeaturesPlugin as any, mockLicenseService); + const privileges = privilegesFactory(actions, mockFeaturesPlugin, mockLicenseServiceBasic); const expectedAllPrivileges = [ actions.login, @@ -191,11 +191,7 @@ describe('features', () => { const mockFeaturesPlugin = featuresPluginMock.createSetup(); mockFeaturesPlugin.getKibanaFeatures.mockReturnValue(features); - const mockLicenseService = { - getFeatures: jest.fn().mockReturnValue({ allowSubFeaturePrivileges: true }), - getType: jest.fn().mockReturnValue('basic'), - }; - const privileges = privilegesFactory(actions, mockFeaturesPlugin as any, mockLicenseService); + const privileges = privilegesFactory(actions, mockFeaturesPlugin, mockLicenseServiceBasic); const actual = privileges.get(); expect(actual).not.toHaveProperty('features.foo'); @@ -268,15 +264,7 @@ describe('features', () => { const mockFeaturesPlugin = featuresPluginMock.createSetup(); mockFeaturesPlugin.getKibanaFeatures.mockReturnValue(features); - const mockLicenseService = { - getFeatures: jest.fn().mockReturnValue({ allowSubFeaturePrivileges: true }), - getType: jest.fn().mockReturnValue('basic'), - }; - const privileges = privilegesFactory( - actions, - mockFeaturesPlugin as any, - mockLicenseService - ); + const privileges = privilegesFactory(actions, mockFeaturesPlugin, mockLicenseServiceBasic); const actual = privileges.get(); expect(actual).toHaveProperty(`${group}.all`, [ @@ -412,15 +400,7 @@ describe('features', () => { const mockFeaturesPlugin = featuresPluginMock.createSetup(); mockFeaturesPlugin.getKibanaFeatures.mockReturnValue(features); - const mockLicenseService = { - getFeatures: jest.fn().mockReturnValue({ allowSubFeaturePrivileges: true }), - getType: jest.fn().mockReturnValue('basic'), - }; - const privileges = privilegesFactory( - actions, - mockFeaturesPlugin as any, - mockLicenseService - ); + const privileges = privilegesFactory(actions, mockFeaturesPlugin, mockLicenseServiceBasic); const actual = privileges.get(); expect(actual).toHaveProperty(`${group}.read`, [ @@ -500,15 +480,7 @@ describe('features', () => { const mockFeaturesPlugin = featuresPluginMock.createSetup(); mockFeaturesPlugin.getKibanaFeatures.mockReturnValue(features); - const mockLicenseService = { - getFeatures: jest.fn().mockReturnValue({ allowSubFeaturePrivileges: true }), - getType: jest.fn().mockReturnValue('basic'), - }; - const privileges = privilegesFactory( - actions, - mockFeaturesPlugin as any, - mockLicenseService - ); + const privileges = privilegesFactory(actions, mockFeaturesPlugin, mockLicenseServiceBasic); const actual = privileges.get(); expect(actual).toHaveProperty(`${group}.all`, [ @@ -574,15 +546,7 @@ describe('features', () => { const mockFeaturesPlugin = featuresPluginMock.createSetup(); mockFeaturesPlugin.getKibanaFeatures.mockReturnValue(features); - const mockLicenseService = { - getFeatures: jest.fn().mockReturnValue({ allowSubFeaturePrivileges: true }), - getType: jest.fn().mockReturnValue('basic'), - }; - const privileges = privilegesFactory( - actions, - mockFeaturesPlugin as any, - mockLicenseService - ); + const privileges = privilegesFactory(actions, mockFeaturesPlugin, mockLicenseServiceBasic); const actual = privileges.get(); expect(actual).toHaveProperty(`${group}.all`, [ @@ -649,15 +613,7 @@ describe('features', () => { const mockFeaturesPlugin = featuresPluginMock.createSetup(); mockFeaturesPlugin.getKibanaFeatures.mockReturnValue(features); - const mockLicenseService = { - getFeatures: jest.fn().mockReturnValue({ allowSubFeaturePrivileges: true }), - getType: jest.fn().mockReturnValue('basic'), - }; - const privileges = privilegesFactory( - actions, - mockFeaturesPlugin as any, - mockLicenseService - ); + const privileges = privilegesFactory(actions, mockFeaturesPlugin, mockLicenseServiceBasic); const actual = privileges.get(); expect(actual).toHaveProperty(`${group}.all`, [ @@ -718,11 +674,7 @@ describe('reserved', () => { const mockFeaturesPlugin = featuresPluginMock.createSetup(); mockFeaturesPlugin.getKibanaFeatures.mockReturnValue(features); - const mockLicenseService = { - getFeatures: jest.fn().mockReturnValue({ allowSubFeaturePrivileges: true }), - getType: jest.fn().mockReturnValue('basic'), - }; - const privileges = privilegesFactory(actions, mockFeaturesPlugin as any, mockLicenseService); + const privileges = privilegesFactory(actions, mockFeaturesPlugin, mockLicenseServiceBasic); const actual = privileges.get(); expect(actual).toHaveProperty('reserved.foo', [actions.version]); @@ -756,11 +708,7 @@ describe('reserved', () => { const mockFeaturesPlugin = featuresPluginMock.createSetup(); mockFeaturesPlugin.getKibanaFeatures.mockReturnValue(features); - const mockLicenseService = { - getFeatures: jest.fn().mockReturnValue({ allowSubFeaturePrivileges: true }), - getType: jest.fn().mockReturnValue('basic'), - }; - const privileges = privilegesFactory(actions, mockFeaturesPlugin as any, mockLicenseService); + const privileges = privilegesFactory(actions, mockFeaturesPlugin, mockLicenseServiceBasic); const actual = privileges.get(); expect(actual).toHaveProperty('reserved.foo', [ @@ -830,11 +778,7 @@ describe('reserved', () => { const mockFeaturesPlugin = featuresPluginMock.createSetup(); mockFeaturesPlugin.getKibanaFeatures.mockReturnValue(features); - const mockLicenseService = { - getFeatures: jest.fn().mockReturnValue({ allowSubFeaturePrivileges: true }), - getType: jest.fn().mockReturnValue('basic'), - }; - const privileges = privilegesFactory(actions, mockFeaturesPlugin as any, mockLicenseService); + const privileges = privilegesFactory(actions, mockFeaturesPlugin, mockLicenseServiceBasic); const actual = privileges.get(); expect(actual).not.toHaveProperty('reserved.foo'); @@ -893,11 +837,7 @@ describe('subFeatures', () => { const mockFeaturesPlugin = featuresPluginMock.createSetup(); mockFeaturesPlugin.getKibanaFeatures.mockReturnValue(features); - const mockLicenseService = { - getFeatures: jest.fn().mockReturnValue({ allowSubFeaturePrivileges: true }), - getType: jest.fn().mockReturnValue('basic'), - }; - const privileges = privilegesFactory(actions, mockFeaturesPlugin as any, mockLicenseService); + const privileges = privilegesFactory(actions, mockFeaturesPlugin, mockLicenseServiceGold); const actual = privileges.get(); expect(actual.features).toHaveProperty(`foo.subFeaturePriv1`, [ @@ -1027,11 +967,7 @@ describe('subFeatures', () => { const mockFeaturesPlugin = featuresPluginMock.createSetup(); mockFeaturesPlugin.getKibanaFeatures.mockReturnValue(features); - const mockLicenseService = { - getFeatures: jest.fn().mockReturnValue({ allowSubFeaturePrivileges: true }), - getType: jest.fn().mockReturnValue('basic'), - }; - const privileges = privilegesFactory(actions, mockFeaturesPlugin as any, mockLicenseService); + const privileges = privilegesFactory(actions, mockFeaturesPlugin, mockLicenseServiceGold); const actual = privileges.get(); expect(actual.features).toHaveProperty(`foo.subFeaturePriv1`, [ @@ -1264,11 +1200,7 @@ describe('subFeatures', () => { const mockFeaturesPlugin = featuresPluginMock.createSetup(); mockFeaturesPlugin.getKibanaFeatures.mockReturnValue(features); - const mockLicenseService = { - getFeatures: jest.fn().mockReturnValue({ allowSubFeaturePrivileges: true }), - getType: jest.fn().mockReturnValue('basic'), - }; - const privileges = privilegesFactory(actions, mockFeaturesPlugin as any, mockLicenseService); + const privileges = privilegesFactory(actions, mockFeaturesPlugin, mockLicenseServiceGold); const actual = privileges.get(); expect(actual.features).toHaveProperty(`foo.subFeaturePriv1`, [ @@ -1424,11 +1356,7 @@ describe('subFeatures', () => { const mockFeaturesPlugin = featuresPluginMock.createSetup(); mockFeaturesPlugin.getKibanaFeatures.mockReturnValue(features); - const mockLicenseService = { - getFeatures: jest.fn().mockReturnValue({ allowSubFeaturePrivileges: true }), - getType: jest.fn().mockReturnValue('basic'), - }; - const privileges = privilegesFactory(actions, mockFeaturesPlugin as any, mockLicenseService); + const privileges = privilegesFactory(actions, mockFeaturesPlugin, mockLicenseServiceGold); const actual = privileges.get(); expect(actual.features).toHaveProperty(`foo.subFeaturePriv1`, [ @@ -1610,11 +1538,7 @@ describe('subFeatures', () => { const mockFeaturesPlugin = featuresPluginMock.createSetup(); mockFeaturesPlugin.getKibanaFeatures.mockReturnValue(features); - const mockLicenseService = { - getFeatures: jest.fn().mockReturnValue({ allowSubFeaturePrivileges: true }), - getType: jest.fn().mockReturnValue('basic'), - }; - const privileges = privilegesFactory(actions, mockFeaturesPlugin as any, mockLicenseService); + const privileges = privilegesFactory(actions, mockFeaturesPlugin, mockLicenseServiceGold); const actual = privileges.get(); expect(actual.features).toHaveProperty(`foo.subFeaturePriv1`, [ @@ -1753,11 +1677,7 @@ describe('subFeatures', () => { const mockFeaturesPlugin = featuresPluginMock.createSetup(); mockFeaturesPlugin.getKibanaFeatures.mockReturnValue(features); - const mockLicenseService = { - getFeatures: jest.fn().mockReturnValue({ allowSubFeaturePrivileges: false }), - getType: jest.fn().mockReturnValue('basic'), - }; - const privileges = privilegesFactory(actions, mockFeaturesPlugin as any, mockLicenseService); + const privileges = privilegesFactory(actions, mockFeaturesPlugin, mockLicenseServiceBasic); const actual = privileges.get(); expect(actual.features).not.toHaveProperty(`foo.subFeaturePriv1`); @@ -1979,11 +1899,7 @@ describe('subFeatures', () => { const mockFeaturesPlugin = featuresPluginMock.createSetup(); mockFeaturesPlugin.getKibanaFeatures.mockReturnValue(features); - const mockLicenseService = { - getFeatures: jest.fn().mockReturnValue({ allowSubFeaturePrivileges: true }), - getType: jest.fn().mockReturnValue('gold'), - }; - const privileges = privilegesFactory(actions, mockFeaturesPlugin as any, mockLicenseService); + const privileges = privilegesFactory(actions, mockFeaturesPlugin, mockLicenseServiceGold); const actual = privileges.get(); expect(actual.features).toHaveProperty(`foo.subFeaturePriv1`); @@ -2214,11 +2130,7 @@ describe('subFeatures', () => { const mockFeaturesPlugin = featuresPluginMock.createSetup(); mockFeaturesPlugin.getKibanaFeatures.mockReturnValue(features); - const mockLicenseService = { - getFeatures: jest.fn().mockReturnValue({ allowSubFeaturePrivileges: true }), - getType: jest.fn().mockReturnValue('platinum'), - }; - const privileges = privilegesFactory(actions, mockFeaturesPlugin as any, mockLicenseService); + const privileges = privilegesFactory(actions, mockFeaturesPlugin, mockLicenseServicePlatinum); const actual = privileges.get(); expect(actual.features).toHaveProperty(`foo.subFeaturePriv1`); diff --git a/x-pack/plugins/security/server/authorization/privileges/privileges.ts b/x-pack/plugins/security/server/authorization/privileges/privileges.ts index 9f50fd0fb1d53..c38a5c9a44f57 100644 --- a/x-pack/plugins/security/server/authorization/privileges/privileges.ts +++ b/x-pack/plugins/security/server/authorization/privileges/privileges.ts @@ -23,7 +23,7 @@ export interface PrivilegesService { export function privilegesFactory( actions: Actions, featuresService: FeaturesPluginSetup, - licenseService: Pick + licenseService: Pick ) { const featurePrivilegeBuilder = featurePrivilegeBuilderFactory(actions); @@ -31,7 +31,7 @@ export function privilegesFactory( get() { const features = featuresService.getKibanaFeatures(); const { allowSubFeaturePrivileges } = licenseService.getFeatures(); - const licenseType = licenseService.getType()!; + const { hasAtLeast: licenseHasAtLeast } = licenseService; const basePrivilegeFeatures = features.filter( (feature) => !feature.excludeFromBasePrivileges ); @@ -42,7 +42,7 @@ export function privilegesFactory( basePrivilegeFeatures.forEach((feature) => { for (const { privilegeId, privilege } of featuresService.featurePrivilegeIterator(feature, { augmentWithSubFeaturePrivileges: true, - licenseType, + licenseHasAtLeast, predicate: (pId, featurePrivilege) => !featurePrivilege.excludeFromBasePrivileges, })) { const privilegeActions = featurePrivilegeBuilder.getActions(privilege, feature); @@ -61,7 +61,7 @@ export function privilegesFactory( featurePrivileges[feature.id] = {}; for (const featurePrivilege of featuresService.featurePrivilegeIterator(feature, { augmentWithSubFeaturePrivileges: true, - licenseType, + licenseHasAtLeast, })) { featurePrivileges[feature.id][featurePrivilege.privilegeId] = [ actions.login, @@ -73,7 +73,7 @@ export function privilegesFactory( if (allowSubFeaturePrivileges && feature.subFeatures?.length > 0) { for (const featurePrivilege of featuresService.featurePrivilegeIterator(feature, { augmentWithSubFeaturePrivileges: false, - licenseType, + licenseHasAtLeast, })) { featurePrivileges[feature.id][`minimal_${featurePrivilege.privilegeId}`] = [ actions.login, @@ -84,7 +84,7 @@ export function privilegesFactory( for (const subFeaturePrivilege of featuresService.subFeaturePrivilegeIterator( feature, - licenseType + licenseHasAtLeast )) { featurePrivileges[feature.id][subFeaturePrivilege.id] = [ actions.login, diff --git a/x-pack/plugins/security/server/plugin.test.ts b/x-pack/plugins/security/server/plugin.test.ts index 2d17e75527c6f..7df717ddefef9 100644 --- a/x-pack/plugins/security/server/plugin.test.ts +++ b/x-pack/plugins/security/server/plugin.test.ts @@ -119,7 +119,7 @@ describe('Security Plugin', () => { }, }, "getFeatures": [Function], - "getType": [Function], + "hasAtLeast": [Function], "isEnabled": [Function], "isLicenseAvailable": [Function], },