Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove circular dependency between features and security #100206

Merged
merged 10 commits into from
May 19, 2021
60 changes: 48 additions & 12 deletions api_docs/features.json
Original file line number Diff line number Diff line change
Expand Up @@ -2101,7 +2101,7 @@
],
"source": {
"path": "x-pack/plugins/features/server/plugin.ts",
"lineNumber": 33
"lineNumber": 41
},
"deprecated": false,
"children": [
Expand All @@ -2125,7 +2125,7 @@
],
"source": {
"path": "x-pack/plugins/features/server/plugin.ts",
"lineNumber": 34
"lineNumber": 42
},
"deprecated": false,
"children": [
Expand All @@ -2147,7 +2147,7 @@
],
"source": {
"path": "x-pack/plugins/features/server/plugin.ts",
"lineNumber": 34
"lineNumber": 42
},
"deprecated": false,
"isRequired": true
Expand Down Expand Up @@ -2175,7 +2175,7 @@
],
"source": {
"path": "x-pack/plugins/features/server/plugin.ts",
"lineNumber": 35
"lineNumber": 43
},
"deprecated": false,
"children": [
Expand All @@ -2197,7 +2197,7 @@
],
"source": {
"path": "x-pack/plugins/features/server/plugin.ts",
"lineNumber": 35
"lineNumber": 43
},
"deprecated": false,
"isRequired": true
Expand Down Expand Up @@ -2225,7 +2225,7 @@
],
"source": {
"path": "x-pack/plugins/features/server/plugin.ts",
"lineNumber": 41
"lineNumber": 49
},
"deprecated": false,
"children": [],
Expand All @@ -2251,7 +2251,7 @@
],
"source": {
"path": "x-pack/plugins/features/server/plugin.ts",
"lineNumber": 47
"lineNumber": 55
},
"deprecated": false,
"children": [],
Expand All @@ -2270,7 +2270,7 @@
],
"source": {
"path": "x-pack/plugins/features/server/plugin.ts",
"lineNumber": 48
"lineNumber": 56
},
"deprecated": false,
"children": [],
Expand All @@ -2288,11 +2288,47 @@
],
"source": {
"path": "x-pack/plugins/features/server/plugin.ts",
"lineNumber": 56
"lineNumber": 64
},
"deprecated": false,
"children": [],
"returnComment": []
},
{
"parentPluginId": "features",
"id": "def-server.PluginSetupContract.featurePrivilegeIterator",
"type": "Function",
"tags": [],
"label": "featurePrivilegeIterator",
"description": [
"\nUtility for iterating through all privileges belonging to a specific feature.\n{@see FeaturePrivilegeIterator }"
],
"signature": [
"FeaturePrivilegeIterator"
],
"source": {
"path": "x-pack/plugins/features/server/plugin.ts",
"lineNumber": 70
},
"deprecated": false
},
{
"parentPluginId": "features",
"id": "def-server.PluginSetupContract.subFeaturePrivilegeIterator",
"type": "Function",
"tags": [],
"label": "subFeaturePrivilegeIterator",
"description": [
"\nUtility for iterating through all sub-feature privileges belonging to a specific feature.\n{@see SubFeaturePrivilegeIterator }"
],
"signature": [
"SubFeaturePrivilegeIterator"
],
"source": {
"path": "x-pack/plugins/features/server/plugin.ts",
"lineNumber": 76
},
"deprecated": false
}
],
"initialIsOpen": false
Expand All @@ -2306,7 +2342,7 @@
"description": [],
"source": {
"path": "x-pack/plugins/features/server/plugin.ts",
"lineNumber": 59
"lineNumber": 79
},
"deprecated": false,
"children": [
Expand All @@ -2330,7 +2366,7 @@
],
"source": {
"path": "x-pack/plugins/features/server/plugin.ts",
"lineNumber": 60
"lineNumber": 80
},
"deprecated": false,
"children": [],
Expand All @@ -2356,7 +2392,7 @@
],
"source": {
"path": "x-pack/plugins/features/server/plugin.ts",
"lineNumber": 61
"lineNumber": 81
},
"deprecated": false,
"children": [],
Expand Down
11 changes: 0 additions & 11 deletions x-pack/plugins/features/server/feature_privilege_iterator.js

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { KibanaFeature } from '../../../../../features/server';
import { KibanaFeature } from '../';
import { featurePrivilegeIterator } from './feature_privilege_iterator';

describe('featurePrivilegeIterator', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,48 @@

import _ from 'lodash';

import type { FeatureKibanaPrivileges, KibanaFeature } from '../../../../../features/server';
import type { LicenseType } from '../../../../../licensing/server';
import type { FeatureKibanaPrivileges, KibanaFeature } from '../';
import type { LicenseType } from '../../../licensing/server';
import { subFeaturePrivilegeIterator } from './sub_feature_privilege_iterator';

interface IteratorOptions {
/**
* Options to control feature privilege iteration.
*/
export interface FeaturePrivilegeIteratorOptions {
/**
* Augment each privilege definition with its sub-feature privileges.
*/
augmentWithSubFeaturePrivileges: boolean;

/**
* The current license type. Controls which sub-features are returned, as they may have different license terms than the overall feature.
*/
licenseType: LicenseType;

/**
* Optional predicate to filter the returned set of privileges.
*/
predicate?: (privilegeId: string, privilege: FeatureKibanaPrivileges) => boolean;
}

export function* featurePrivilegeIterator(
/**
* Utility for iterating through all privileges belonging to a specific feature.
* Iteration can be customized in several ways:
* - Filter privileges with a given predicate.
* - Augment privileges with their respective sub-feature privileges.
*
* @param feature the feature whose privileges to iterate through.
* @param options options to control iteration.
*/
export type FeaturePrivilegeIterator = (
feature: KibanaFeature,
options: IteratorOptions
): IterableIterator<{ privilegeId: string; privilege: FeatureKibanaPrivileges }> {
options: FeaturePrivilegeIteratorOptions
) => IterableIterator<{ privilegeId: string; privilege: FeatureKibanaPrivileges }>;

const featurePrivilegeIterator: FeaturePrivilegeIterator = function* featurePrivilegeIterator(
feature: KibanaFeature,
options: FeaturePrivilegeIteratorOptions
) {
for (const entry of Object.entries(feature.privileges ?? {})) {
const [privilegeId, privilege] = entry;

Expand All @@ -37,7 +65,7 @@ export function* featurePrivilegeIterator(
yield { privilegeId, privilege };
}
}
}
};

function mergeWithSubFeatures(
privilegeId: string,
Expand Down Expand Up @@ -97,3 +125,5 @@ function mergeArrays(input1: readonly string[] | undefined, input2: readonly str
const second = input2 ?? [];
return Array.from(new Set([...first, ...second]));
}

export { featurePrivilegeIterator };
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,10 @@
* 2.0.
*/

export type {
FeaturePrivilegeIterator,
FeaturePrivilegeIteratorOptions,
} from './feature_privilege_iterator';
export type { SubFeaturePrivilegeIterator } from './sub_feature_privilege_iterator';
export { featurePrivilegeIterator } from './feature_privilege_iterator';
export { subFeaturePrivilegeIterator } from './sub_feature_privilege_iterator';
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,21 @@
* 2.0.
*/

import type { KibanaFeature, SubFeaturePrivilegeConfig } from '../../../../../features/common';
import type { LicenseType } from '../../../../../licensing/server';
import type { KibanaFeature, SubFeaturePrivilegeConfig } from '../../common';
import type { LicenseType } from '../../../licensing/server';

export function* subFeaturePrivilegeIterator(
/**
* Utility for iterating through all sub-feature privileges belonging to a specific feature.
*
* @param feature the feature whose sub-feature privileges to iterate through.
* @param licenseType the current license.
*/
export type SubFeaturePrivilegeIterator = (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: you added a TSDoc comment for FeaturePrivilegeIterator, but missed it for SubFeaturePrivilegeIterator here.

feature: KibanaFeature,
licenseType: LicenseType
) => IterableIterator<SubFeaturePrivilegeConfig>;

const subFeaturePrivilegeIterator: SubFeaturePrivilegeIterator = function* subFeaturePrivilegeIterator(
feature: KibanaFeature,
licenseType: LicenseType
): IterableIterator<SubFeaturePrivilegeConfig> {
Expand All @@ -19,4 +30,6 @@ export function* subFeaturePrivilegeIterator(
);
}
}
}
};

export { subFeaturePrivilegeIterator };
6 changes: 6 additions & 0 deletions x-pack/plugins/features/server/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
*/

import { PluginSetupContract, PluginStartContract } from './plugin';
import {
featurePrivilegeIterator,
subFeaturePrivilegeIterator,
} from './feature_privilege_iterator';

const createSetup = (): jest.Mocked<PluginSetupContract> => {
return {
Expand All @@ -15,6 +19,8 @@ const createSetup = (): jest.Mocked<PluginSetupContract> => {
registerKibanaFeature: jest.fn(),
registerElasticsearchFeature: jest.fn(),
enableReportingUiCapabilities: jest.fn(),
featurePrivilegeIterator: jest.fn().mockImplementation(featurePrivilegeIterator),
subFeaturePrivilegeIterator: jest.fn().mockImplementation(subFeaturePrivilegeIterator),
};
};

Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/features/server/oss_features.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
*/

import { buildOSSFeatures } from './oss_features';
// @ts-expect-error
import { featurePrivilegeIterator } from './feature_privilege_iterator';
import { KibanaFeature } from '.';
import { LicenseType } from '../../licensing/server';
Expand Down
22 changes: 22 additions & 0 deletions x-pack/plugins/features/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,14 @@ import {
KibanaFeature,
KibanaFeatureConfig,
} from '../common';
import type {
FeaturePrivilegeIterator,
SubFeaturePrivilegeIterator,
} from './feature_privilege_iterator';
import {
featurePrivilegeIterator,
subFeaturePrivilegeIterator,
} from './feature_privilege_iterator';

/**
* Describes public Features plugin contract returned at the `setup` stage.
Expand Down Expand Up @@ -54,6 +62,18 @@ export interface PluginSetupContract {
* `features` to include Reporting when registering OSS features.
*/
enableReportingUiCapabilities(): void;

/**
* Utility for iterating through all privileges belonging to a specific feature.
* {@see FeaturePrivilegeIterator }
*/
featurePrivilegeIterator: FeaturePrivilegeIterator;

/**
* Utility for iterating through all sub-feature privileges belonging to a specific feature.
* {@see SubFeaturePrivilegeIterator }
*/
subFeaturePrivilegeIterator: SubFeaturePrivilegeIterator;
}

export interface PluginStartContract {
Expand Down Expand Up @@ -110,6 +130,8 @@ export class FeaturesPlugin
),
getFeaturesUICapabilities,
enableReportingUiCapabilities: this.enableReportingUiCapabilities.bind(this),
featurePrivilegeIterator,
subFeaturePrivilegeIterator,
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ import { validateReservedPrivileges } from './validate_reserved_privileges';

export { Actions } from './actions';
export { CheckSavedObjectsPrivileges } from './check_saved_objects_privileges';
export { featurePrivilegeIterator } from './privileges';

interface AuthorizationServiceSetupParams {
packageVersion: string;
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/security/server/authorization/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,4 @@
export { Actions } from './actions';
export { AuthorizationService, AuthorizationServiceSetup } from './authorization_service';
export { CheckSavedObjectsPrivileges } from './check_saved_objects_privileges';
export { featurePrivilegeIterator } from './privileges';
export { CheckPrivilegesPayload } from './types';
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,3 @@
*/

export { privilegesFactory, PrivilegesService } from './privileges';
export { featurePrivilegeIterator } from './feature_privilege_iterator';
Loading