[8.x] fix(security, features): do not expose UI capabilities of the d…

…eprecated features (elastic#198656) (elastic#199147) # Backport This will backport the following commits from `main` to `8.x`: - [fix(security, features): do not expose UI capabilities of the deprecated features (elastic#198656)](elastic#198656)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Aleh Zasypkin <[email protected]>
gsoldevila · Nov 6, 2024 · 5d674b8 · 5d674b8
1 parent bee5c9d
commit 5d674b8
Show file tree

Hide file tree

Showing 15 changed files with 301 additions and 42 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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;
+  }
 }