diff --git a/x-pack/plugins/features/server/mocks.ts b/x-pack/plugins/features/server/mocks.ts index bb2292a45377f..15339b068e7e8 100644 --- a/x-pack/plugins/features/server/mocks.ts +++ b/x-pack/plugins/features/server/mocks.ts @@ -25,8 +25,8 @@ const createSetup = (): jest.Mocked<FeaturesPluginSetup> => { const createStart = (): jest.Mocked<FeaturesPluginStart> => { return { - getKibanaFeatures: jest.fn(), - getElasticsearchFeatures: jest.fn(), + getKibanaFeatures: jest.fn().mockReturnValue([]), + getElasticsearchFeatures: jest.fn().mockReturnValue([]), }; }; diff --git a/x-pack/plugins/features/server/plugin.ts b/x-pack/plugins/features/server/plugin.ts index 15888358bb773..9f6cae36f6aee 100644 --- a/x-pack/plugins/features/server/plugin.ts +++ b/x-pack/plugins/features/server/plugin.ts @@ -138,7 +138,8 @@ export class FeaturesPlugin this.featureRegistry.validateFeatures(); this.capabilities = uiCapabilitiesForFeatures( - this.featureRegistry.getAllKibanaFeatures(), + // Don't expose capabilities of the deprecated features. + this.featureRegistry.getAllKibanaFeatures({ omitDeprecated: true }), this.featureRegistry.getAllElasticsearchFeatures() ); diff --git a/x-pack/plugins/security/server/authorization/authorization_service.test.ts b/x-pack/plugins/security/server/authorization/authorization_service.test.ts index 275a6d2643f24..de3646166d8f9 100644 --- a/x-pack/plugins/security/server/authorization/authorization_service.test.ts +++ b/x-pack/plugins/security/server/authorization/authorization_service.test.ts @@ -145,12 +145,9 @@ describe('#start', () => { customBranding: mockCoreSetup.customBranding, }); - const featuresStart = featuresPluginMock.createStart(); - featuresStart.getKibanaFeatures.mockReturnValue([]); - authorizationService.start({ clusterClient: mockClusterClient, - features: featuresStart, + features: featuresPluginMock.createStart(), online$: statusSubject.asObservable(), }); @@ -217,12 +214,9 @@ it('#stop unsubscribes from license and ES updates.', async () => { customBranding: mockCoreSetup.customBranding, }); - const featuresStart = featuresPluginMock.createStart(); - featuresStart.getKibanaFeatures.mockReturnValue([]); - authorizationService.start({ clusterClient: mockClusterClient, - features: featuresStart, + features: featuresPluginMock.createStart(), online$: statusSubject.asObservable(), }); diff --git a/x-pack/plugins/security/server/plugin.test.ts b/x-pack/plugins/security/server/plugin.test.ts index 37c6e22a07fab..4b9479f51a0f3 100644 --- a/x-pack/plugins/security/server/plugin.test.ts +++ b/x-pack/plugins/security/server/plugin.test.ts @@ -64,10 +64,8 @@ describe('Security Plugin', () => { mockCoreStart = coreMock.createStart(); - const mockFeaturesStart = featuresPluginMock.createStart(); - mockFeaturesStart.getKibanaFeatures.mockReturnValue([]); mockStartDependencies = { - features: mockFeaturesStart, + features: featuresPluginMock.createStart(), licensing: licensingMock.createStart(), taskManager: taskManagerMock.createStart(), }; diff --git a/x-pack/plugins/spaces/server/capabilities/capabilities_switcher.test.ts b/x-pack/plugins/spaces/server/capabilities/capabilities_switcher.test.ts index d48095638babf..688f8297271a3 100644 --- a/x-pack/plugins/spaces/server/capabilities/capabilities_switcher.test.ts +++ b/x-pack/plugins/spaces/server/capabilities/capabilities_switcher.test.ts @@ -66,7 +66,7 @@ const features = [ category: { id: 'securitySolution' }, }, { - // feature 4 intentionally delcares the same items as feature 3 + // feature 4 intentionally declares the same items as feature 3 id: 'feature_4', name: 'Feature 4', app: ['feature3', 'feature3_app'], @@ -87,6 +87,32 @@ const features = [ }, category: { id: 'observability' }, }, + { + deprecated: { notice: 'It was a mistake.' }, + id: 'deprecated_feature', + name: 'Deprecated Feature', + // Expose the same `app` and `catalogue` entries as `feature_2` to make sure they are disabled + // when `feature_2` is disabled even if the deprecated feature isn't explicitly disabled. + app: ['feature2'], + catalogue: ['feature2Entry'], + category: { id: 'deprecated', label: 'deprecated' }, + privileges: { + all: { + savedObject: { all: [], read: [] }, + ui: ['ui_deprecated_all'], + app: ['feature2'], + catalogue: ['feature2Entry'], + replacedBy: [{ feature: 'feature_2', privileges: ['all'] }], + }, + read: { + savedObject: { all: [], read: [] }, + ui: ['ui_deprecated_read'], + app: ['feature2'], + catalogue: ['feature2Entry'], + replacedBy: [{ feature: 'feature_2', privileges: ['all'] }], + }, + }, + }, ] as unknown as KibanaFeature[]; const buildCapabilities = () => diff --git a/x-pack/plugins/spaces/server/capabilities/capabilities_switcher.ts b/x-pack/plugins/spaces/server/capabilities/capabilities_switcher.ts index 90ee85fece486..41d5dcdf2cb14 100644 --- a/x-pack/plugins/spaces/server/capabilities/capabilities_switcher.ts +++ b/x-pack/plugins/spaces/server/capabilities/capabilities_switcher.ts @@ -72,7 +72,7 @@ function toggleDisabledFeatures( (acc, feature) => { if (disabledFeatureKeys.includes(feature.id)) { acc.disabledFeatures.push(feature); - } else { + } else if (!feature.deprecated) { acc.enabledFeatures.push(feature); } return acc; diff --git a/x-pack/plugins/spaces/server/lib/utils/space_solution_disabled_features.ts b/x-pack/plugins/spaces/server/lib/utils/space_solution_disabled_features.ts index 066166e7e87dd..6d30645325535 100644 --- a/x-pack/plugins/spaces/server/lib/utils/space_solution_disabled_features.ts +++ b/x-pack/plugins/spaces/server/lib/utils/space_solution_disabled_features.ts @@ -39,6 +39,7 @@ const enabledFeaturesPerSolution: Record<SolutionId, string[]> = { * This function takes the current space's disabled features and the space solution and returns * the updated array of disabled features. * + * @param features The list of all Kibana registered features. * @param spaceDisabledFeatures The current space's disabled features * @param spaceSolution The current space's solution (es, oblt, security or classic) * @returns The updated array of disabled features diff --git a/x-pack/plugins/spaces/server/routes/api/external/post.test.ts b/x-pack/plugins/spaces/server/routes/api/external/post.test.ts index 984d684762159..88c846b77eb53 100644 --- a/x-pack/plugins/spaces/server/routes/api/external/post.test.ts +++ b/x-pack/plugins/spaces/server/routes/api/external/post.test.ts @@ -56,13 +56,9 @@ describe('Spaces Public API', () => { basePath: httpService.basePath, }); - const featuresPluginMockStart = featuresPluginMock.createStart(); - - featuresPluginMockStart.getKibanaFeatures.mockReturnValue([]); - const usageStatsServicePromise = Promise.resolve(usageStatsServiceMock.createSetupContract()); - const clientServiceStart = clientService.start(coreStart, featuresPluginMockStart); + const clientServiceStart = clientService.start(coreStart, featuresPluginMock.createStart()); const spacesServiceStart = service.start({ basePath: coreStart.http.basePath, diff --git a/x-pack/plugins/spaces/server/routes/api/external/put.test.ts b/x-pack/plugins/spaces/server/routes/api/external/put.test.ts index 8aa71d30fc4bb..cf2e9981fd024 100644 --- a/x-pack/plugins/spaces/server/routes/api/external/put.test.ts +++ b/x-pack/plugins/spaces/server/routes/api/external/put.test.ts @@ -56,13 +56,9 @@ describe('PUT /api/spaces/space', () => { basePath: httpService.basePath, }); - const featuresPluginMockStart = featuresPluginMock.createStart(); - - featuresPluginMockStart.getKibanaFeatures.mockReturnValue([]); - const usageStatsServicePromise = Promise.resolve(usageStatsServiceMock.createSetupContract()); - const clientServiceStart = clientService.start(coreStart, featuresPluginMockStart); + const clientServiceStart = clientService.start(coreStart, featuresPluginMock.createStart()); const spacesServiceStart = service.start({ basePath: coreStart.http.basePath, diff --git a/x-pack/plugins/spaces/server/spaces_client/spaces_client.test.ts b/x-pack/plugins/spaces/server/spaces_client/spaces_client.test.ts index 364afdcaba66a..4b7c1de0b3fcb 100644 --- a/x-pack/plugins/spaces/server/spaces_client/spaces_client.test.ts +++ b/x-pack/plugins/spaces/server/spaces_client/spaces_client.test.ts @@ -55,6 +55,37 @@ const features = [ catalogue: ['feature3Entry'], category: { id: 'securitySolution' }, }, + { + deprecated: { notice: 'It was a mistake.' }, + id: 'feature_4_deprecated', + name: 'Deprecated Feature', + app: ['feature2', 'feature3'], + catalogue: ['feature2Entry', 'feature3Entry'], + category: { id: 'deprecated', label: 'deprecated' }, + scope: ['spaces', 'security'], + privileges: { + all: { + savedObject: { all: [], read: [] }, + ui: [], + app: ['feature2', 'feature3'], + catalogue: ['feature2Entry', 'feature3Entry'], + replacedBy: [ + { feature: 'feature_2', privileges: ['all'] }, + { feature: 'feature_3', privileges: ['all'] }, + ], + }, + read: { + savedObject: { all: [], read: [] }, + ui: [], + app: ['feature2', 'feature3'], + catalogue: ['feature2Entry', 'feature3Entry'], + replacedBy: [ + { feature: 'feature_2', privileges: ['read'] }, + { feature: 'feature_3', privileges: ['read'] }, + ], + }, + }, + }, ] as unknown as KibanaFeature[]; const featuresStart = featuresPluginMock.createStart(); @@ -103,6 +134,17 @@ describe('#getAll', () => { bar: 'baz-bar', // an extra attribute that will be ignored during conversion }, }, + { + // alpha only has deprecated disabled features + id: 'alpha', + type: 'space', + references: [], + attributes: { + name: 'alpha-name', + description: 'alpha-description', + disabledFeatures: ['feature_1', 'feature_4_deprecated'], + }, + }, ]; const expectedSpaces: Space[] = [ @@ -130,6 +172,12 @@ describe('#getAll', () => { description: 'baz-description', disabledFeatures: [], }, + { + id: 'alpha', + name: 'alpha-name', + description: 'alpha-description', + disabledFeatures: ['feature_1', 'feature_2', 'feature_3'], + }, ]; test(`finds spaces using callWithRequestRepository`, async () => { diff --git a/x-pack/plugins/spaces/server/spaces_client/spaces_client.ts b/x-pack/plugins/spaces/server/spaces_client/spaces_client.ts index 5d7ae1159f5ea..66728636f9752 100644 --- a/x-pack/plugins/spaces/server/spaces_client/spaces_client.ts +++ b/x-pack/plugins/spaces/server/spaces_client/spaces_client.ts @@ -14,6 +14,7 @@ import type { SavedObject, } from '@kbn/core/server'; import type { LegacyUrlAliasTarget } from '@kbn/core-saved-objects-common'; +import type { KibanaFeature } from '@kbn/features-plugin/common'; import { KibanaFeatureScope } from '@kbn/features-plugin/common'; import type { FeaturesPluginStart } from '@kbn/features-plugin/server'; @@ -84,7 +85,13 @@ export interface ISpacesClient { * Client for interacting with spaces. */ export class SpacesClient implements ISpacesClient { - private isServerless = false; + private readonly isServerless: boolean; + + /** + * A map of deprecated feature IDs to the feature IDs that replace them used to transform the disabled features + * of a space to make sure they only reference non-deprecated features. + */ + private readonly deprecatedFeaturesReferences: Map<string, Set<string>>; constructor( private readonly debugLogger: (message: string) => void, @@ -95,6 +102,9 @@ export class SpacesClient implements ISpacesClient { private readonly features: FeaturesPluginStart ) { this.isServerless = this.buildFlavour === 'serverless'; + this.deprecatedFeaturesReferences = this.collectDeprecatedFeaturesReferences( + features.getKibanaFeatures() + ); } public async getAll(options: v1.GetAllSpacesOptions = {}): Promise<v1.GetSpaceResult[]> { @@ -247,6 +257,8 @@ export class SpacesClient implements ISpacesClient { }; private transformSavedObjectToSpace = (savedObject: SavedObject<any>): v1.Space => { + // Solution isn't supported in the serverless offering. + const solution = !this.isServerless ? savedObject.attributes.solution : undefined; return { id: savedObject.id, name: savedObject.attributes.name ?? '', @@ -256,11 +268,13 @@ export class SpacesClient implements ISpacesClient { imageUrl: savedObject.attributes.imageUrl, disabledFeatures: withSpaceSolutionDisabledFeatures( this.features.getKibanaFeatures(), - savedObject.attributes.disabledFeatures ?? [], - !this.isServerless ? savedObject.attributes.solution : undefined + savedObject.attributes.disabledFeatures?.flatMap((featureId: string) => + Array.from(this.deprecatedFeaturesReferences.get(featureId) ?? [featureId]) + ) ?? [], + solution ), _reserved: savedObject.attributes._reserved, - ...(!this.isServerless ? { solution: savedObject.attributes.solution } : {}), + ...(solution ? { solution } : {}), } as v1.Space; }; @@ -275,4 +289,41 @@ export class SpacesClient implements ISpacesClient { ...(!this.isServerless && space.solution ? { solution: space.solution } : {}), }; }; + + /** + * Collects a map of all deprecated feature IDs and the feature IDs that replace them. + * @param features A list of all available Kibana features including deprecated ones. + */ + private collectDeprecatedFeaturesReferences(features: KibanaFeature[]) { + const deprecatedFeatureReferences = new Map(); + for (const feature of features) { + if (!feature.deprecated || !feature.scope?.includes(KibanaFeatureScope.Spaces)) { + continue; + } + + // Collect all feature privileges including the ones provided by sub-features, if any. + const allPrivileges = Object.values(feature.privileges ?? {}).concat( + feature.subFeatures?.flatMap((subFeature) => + subFeature.privilegeGroups.flatMap(({ privileges }) => privileges) + ) ?? [] + ); + + // Collect all features IDs that are referenced by the deprecated feature privileges. + const referencedFeaturesIds = new Set(); + for (const privilege of allPrivileges) { + const replacedBy = privilege.replacedBy + ? 'default' in privilege.replacedBy + ? privilege.replacedBy.default.concat(privilege.replacedBy.minimal) + : privilege.replacedBy + : []; + for (const privilegeReference of replacedBy) { + referencedFeaturesIds.add(privilegeReference.feature); + } + } + + deprecatedFeatureReferences.set(feature.id, referencedFeaturesIds); + } + + return deprecatedFeatureReferences; + } } diff --git a/x-pack/test/security_api_integration/plugins/features_provider/server/index.ts b/x-pack/test/security_api_integration/plugins/features_provider/server/index.ts index 646fe327a0015..61100babefea7 100644 --- a/x-pack/test/security_api_integration/plugins/features_provider/server/index.ts +++ b/x-pack/test/security_api_integration/plugins/features_provider/server/index.ts @@ -9,8 +9,11 @@ import type { PluginSetupContract as AlertingPluginsSetup } from '@kbn/alerting- import { schema } from '@kbn/config-schema'; import type { CoreSetup, Plugin, PluginInitializer } from '@kbn/core/server'; import { DEFAULT_APP_CATEGORIES } from '@kbn/core/server'; +import { KibanaFeatureScope } from '@kbn/features-plugin/common'; import type { FeaturesPluginSetup, FeaturesPluginStart } from '@kbn/features-plugin/server'; +import { initRoutes } from './init_routes'; + export interface PluginSetupDependencies { features: FeaturesPluginSetup; alerting: AlertingPluginsSetup; @@ -23,7 +26,7 @@ export interface PluginStartDependencies { export const plugin: PluginInitializer<void, void> = async (): Promise< Plugin<void, void, PluginSetupDependencies, PluginStartDependencies> > => ({ - setup: (_: CoreSetup<PluginStartDependencies>, deps: PluginSetupDependencies) => { + setup: (core: CoreSetup<PluginStartDependencies>, deps: PluginSetupDependencies) => { // Case #1: feature A needs to be renamed to feature B. It's unfortunate, but the existing feature A // should be deprecated and re-created as a new feature with the same privileges. case1FeatureRename(deps); @@ -46,6 +49,8 @@ export const plugin: PluginInitializer<void, void> = async (): Promise< // * `case_4_feature_b_v2` (new, decoupled from `ab` SO, partially replaces `case_4_feature_b`) // * `case_4_feature_c` (new, only for `ab` SO access) case4FeatureExtract(deps); + + initRoutes(core); }, start: () => {}, stop: () => {}, @@ -61,6 +66,7 @@ function case1FeatureRename(deps: PluginSetupDependencies) { all: { savedObject: { all: ['one'], read: [] }, ui: ['ui_all'] }, read: { savedObject: { all: [], read: ['one'] }, ui: ['ui_read'] }, }, + scope: [KibanaFeatureScope.Security, KibanaFeatureScope.Spaces], }; // Step 2: mark feature A as deprecated and provide proper replacements for all feature and @@ -96,6 +102,8 @@ function case2FeatureSplit(deps: PluginSetupDependencies) { deps.features.registerKibanaFeature({ deprecated: { notice: 'Case #2 is deprecated.' }, + scope: [KibanaFeatureScope.Security, KibanaFeatureScope.Spaces], + app: ['app_one', 'app_two'], catalogue: ['cat_one', 'cat_two'], management: { kibana: ['management_one', 'management_two'] }, @@ -139,6 +147,8 @@ function case2FeatureSplit(deps: PluginSetupDependencies) { read: { savedObject: { all: [], read: ['one', 'two'] }, ui: ['ui_read_one', 'ui_read_two'], + catalogue: ['cat_one', 'cat_two'], + app: ['app_one', 'app_two'], replacedBy: [ { feature: 'case_2_feature_b', privileges: ['read'] }, { feature: 'case_2_feature_c', privileges: ['read'] }, @@ -149,6 +159,8 @@ function case2FeatureSplit(deps: PluginSetupDependencies) { // Step 2: define new features deps.features.registerKibanaFeature({ + scope: [KibanaFeatureScope.Security, KibanaFeatureScope.Spaces], + category: DEFAULT_APP_CATEGORIES.kibana, id: 'case_2_feature_b', name: 'Case #2 feature B', @@ -182,10 +194,14 @@ function case2FeatureSplit(deps: PluginSetupDependencies) { read: { savedObject: { all: [], read: ['one'] }, ui: ['ui_read_one'], + catalogue: ['cat_one'], + app: ['app_one'], }, }, }); deps.features.registerKibanaFeature({ + scope: [KibanaFeatureScope.Security, KibanaFeatureScope.Spaces], + category: DEFAULT_APP_CATEGORIES.kibana, id: 'case_2_feature_c', name: 'Case #2 feature C', @@ -219,6 +235,8 @@ function case2FeatureSplit(deps: PluginSetupDependencies) { read: { savedObject: { all: [], read: ['two'] }, ui: ['ui_read_two'], + app: ['app_two'], + catalogue: ['cat_two'], }, }, }); @@ -249,6 +267,8 @@ function case3FeatureSplitSubFeature(deps: PluginSetupDependencies) { deps.features.registerKibanaFeature({ deprecated: { notice: 'Case #3 is deprecated.' }, + scope: [KibanaFeatureScope.Security, KibanaFeatureScope.Spaces], + category: DEFAULT_APP_CATEGORIES.kibana, id: 'case_3_feature_a', name: 'Case #3 feature A (DEPRECATED)', @@ -275,6 +295,8 @@ function case3FeatureSplitSubFeature(deps: PluginSetupDependencies) { // Step 2: Create a new feature with the desired privileges structure. deps.features.registerKibanaFeature({ + scope: [KibanaFeatureScope.Security, KibanaFeatureScope.Spaces], + category: DEFAULT_APP_CATEGORIES.kibana, id: 'case_3_feature_a_v2', name: 'Case #3 feature A', @@ -324,6 +346,8 @@ function case4FeatureExtract(deps: PluginSetupDependencies) { deps.features.registerKibanaFeature({ deprecated: { notice: 'Case #4 is deprecated.' }, + scope: [KibanaFeatureScope.Security, KibanaFeatureScope.Spaces], + category: DEFAULT_APP_CATEGORIES.kibana, id: `case_4_feature_${suffix.toLowerCase()}`, name: `Case #4 feature ${suffix} (DEPRECATED)`, @@ -350,6 +374,8 @@ function case4FeatureExtract(deps: PluginSetupDependencies) { // Step 2: introduce new features (v2) with privileges that don't grant access to `ab`. deps.features.registerKibanaFeature({ + scope: [KibanaFeatureScope.Security, KibanaFeatureScope.Spaces], + category: DEFAULT_APP_CATEGORIES.kibana, id: `case_4_feature_${suffix.toLowerCase()}_v2`, name: `Case #4 feature ${suffix}`, @@ -363,6 +389,8 @@ function case4FeatureExtract(deps: PluginSetupDependencies) { // Step 3: introduce new feature C that only grants access to `ab`. deps.features.registerKibanaFeature({ + scope: [KibanaFeatureScope.Security, KibanaFeatureScope.Spaces], + category: DEFAULT_APP_CATEGORIES.kibana, id: 'case_4_feature_c', name: 'Case #4 feature C', diff --git a/x-pack/test/security_api_integration/plugins/features_provider/server/init_routes.ts b/x-pack/test/security_api_integration/plugins/features_provider/server/init_routes.ts new file mode 100644 index 0000000000000..d58f2f3078a3a --- /dev/null +++ b/x-pack/test/security_api_integration/plugins/features_provider/server/init_routes.ts @@ -0,0 +1,25 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import type { CoreSetup } from '@kbn/core/server'; + +import type { PluginStartDependencies } from '.'; + +export function initRoutes(core: CoreSetup<PluginStartDependencies>) { + const router = core.http.createRouter(); + + // This route mirrors existing `GET /api/features` route except that it also returns all deprecated features. + router.get( + { path: '/internal/features_provider/features', validate: false }, + async (context, request, response) => { + const [, pluginDeps] = await core.getStartServices(); + return response.ok({ + body: pluginDeps.features.getKibanaFeatures().map((feature) => feature.toRaw()), + }); + } + ); +} diff --git a/x-pack/test/security_api_integration/tests/features/deprecated_features.ts b/x-pack/test/security_api_integration/tests/features/deprecated_features.ts index 030f5ac704d8b..6e868fc5946ec 100644 --- a/x-pack/test/security_api_integration/tests/features/deprecated_features.ts +++ b/x-pack/test/security_api_integration/tests/features/deprecated_features.ts @@ -14,6 +14,7 @@ import type { FeatureKibanaPrivilegesReference, KibanaFeatureConfig, } from '@kbn/features-plugin/common'; +import { KibanaFeatureScope } from '@kbn/features-plugin/common'; import type { Role } from '@kbn/security-plugin-types-common'; import type { FtrProviderContext } from '../../ftr_provider_context'; @@ -163,11 +164,97 @@ export default function ({ getService }: FtrProviderContext) { } }); + it('all deprecated features are known', async () => { + const { body: features } = await supertest + .get('/internal/features_provider/features') + .expect(200); + + // **NOTE**: This test is to ensure the AppEx Security team has a chance to review all features marked as + // deprecated. If you’re adding a new deprecated feature, make sure to add it to the list below manually or by + // running the API integration test locally with the --updateSnapshot flag. + expectSnapshot( + (features as KibanaFeatureConfig[]).flatMap((f) => (f.deprecated ? [f.id] : [])).sort() + ).toMatchInline(` + Array [ + "case_1_feature_a", + "case_2_feature_a", + "case_3_feature_a", + "case_4_feature_a", + "case_4_feature_b", + ] + `); + }); + + it('all deprecated features are replaced by a single feature only', async () => { + const featuresResponse = await supertest + .get('/internal/features_provider/features') + .expect(200); + const features = featuresResponse.body as KibanaFeatureConfig[]; + + // **NOTE**: This test ensures that deprecated features displayed in the Space’s feature visibility toggles screen + // are only replaced by a single feature. This way, if a feature is toggled off for a particular Space, there + // won’t be any ambiguity about which replacement feature should also be toggled off. Currently, we don’t + // anticipate having a deprecated feature replaced by more than one feature, so this test is intended to catch + // such scenarios early. If there’s a need for a deprecated feature to be replaced by multiple features, please + // reach out to the AppEx Security team to discuss how this should affect Space’s feature visibility toggles. + const featureIdsThatSupportMultipleReplacements = new Set([ + 'case_2_feature_a', + 'case_4_feature_a', + 'case_4_feature_b', + ]); + for (const feature of features) { + if ( + !feature.deprecated || + !feature.scope?.includes(KibanaFeatureScope.Spaces) || + featureIdsThatSupportMultipleReplacements.has(feature.id) + ) { + continue; + } + + // Collect all feature privileges including the ones provided by sub-features, if any. + const allPrivileges = Object.values(feature.privileges ?? {}).concat( + feature.subFeatures?.flatMap((subFeature) => + subFeature.privilegeGroups.flatMap(({ privileges }) => privileges) + ) ?? [] + ); + + // Collect all features IDs that are referenced by the deprecated feature privileges. + const referencedFeaturesIds = new Set(); + for (const privilege of allPrivileges) { + const replacedBy = privilege.replacedBy + ? 'default' in privilege.replacedBy + ? privilege.replacedBy.default.concat(privilege.replacedBy.minimal) + : privilege.replacedBy + : []; + for (const privilegeReference of replacedBy) { + referencedFeaturesIds.add(privilegeReference.feature); + } + } + + if (referencedFeaturesIds.size > 1) { + throw new Error( + `Feature "${feature.id}" is deprecated and replaced by more than one feature: ${ + referencedFeaturesIds.size + } features: ${Array.from(referencedFeaturesIds).join( + ', ' + )}. If it's intentional, please contact the AppEx Security team.` + ); + } + } + }); + it('all privileges of the deprecated features should have a proper replacement', async () => { // Fetch all features first. - const featuresResponse = await supertest.get('/api/features').expect(200); + const featuresResponse = await supertest + .get('/internal/features_provider/features') + .expect(200); const features = featuresResponse.body as KibanaFeatureConfig[]; + // Check if the action provided by the deprecated feature is directly replaceable by other + // features. The `ui:`-prefixed actions are special since they are prefixed with a feature ID, + // and do not need to be replaced like any other privilege actions. + const isReplaceableAction = (action: string) => !action.startsWith('ui:'); + // Collect all deprecated features. const deprecatedFeatures = features.filter((f) => f.deprecated); log.info(`Found ${deprecatedFeatures.length} deprecated features.`); @@ -207,7 +294,10 @@ export default function ({ getService }: FtrProviderContext) { ); for (const deprecatedAction of deprecatedActions) { - if (!replacementActions.has(deprecatedAction)) { + if ( + isReplaceableAction(deprecatedAction) && + !replacementActions.has(deprecatedAction) + ) { throw new Error( `Action "${deprecatedAction}" granted by the privilege "${privilegeId}" of the deprecated feature "${feature.id}" is not properly replaced.` ); @@ -225,22 +315,23 @@ export default function ({ getService }: FtrProviderContext) { .send({ applications: [] }) .expect(200); - // Both deprecated and new UI capabilities should be toggled. + // Only new UI capabilities should be toggled, deprecated ones should not be present. expect(capabilities).toEqual( expect.objectContaining({ - // UI flags from the deprecated feature privilege. - case_2_feature_a: { - ui_all_one: true, - ui_all_two: true, - ui_read_one: false, - ui_read_two: false, - }, - // UI flags from the feature privileges that replace deprecated one. case_2_feature_b: { ui_all_one: true, ui_read_one: false }, case_2_feature_c: { ui_all_two: true, ui_read_two: false }, }) ); + for (const deprecatedFeatureId of [ + 'case_1_feature_a', + 'case_2_feature_a', + 'case_3_feature_a', + 'case_4_feature_a', + 'case_4_feature_b', + ]) { + expect(capabilities).not.toHaveProperty(deprecatedFeatureId); + } }); it('Cases privileges are properly handled for deprecated privileges', async () => { diff --git a/x-pack/test/ui_capabilities/common/config.ts b/x-pack/test/ui_capabilities/common/config.ts index ba40e613c0d69..18a96b8e26274 100644 --- a/x-pack/test/ui_capabilities/common/config.ts +++ b/x-pack/test/ui_capabilities/common/config.ts @@ -46,6 +46,10 @@ export function createTestConfig(name: string, options: CreateTestConfigOptions) .filter((k) => k !== 'security') .map((key) => `--xpack.${key}.enabled=false`), `--plugin-path=${path.resolve(__dirname, 'plugins/foo_plugin')}`, + `--plugin-path=${path.resolve( + __dirname, + '../../security_api_integration/plugins/features_provider' + )}`, ], }, };