Skip to content

Commit

Permalink
fix(security, features): do not expose UI capabilities of the depreca…
Browse files Browse the repository at this point in the history
…ted features
  • Loading branch information
azasypkin committed Nov 1, 2024
1 parent bd21496 commit 6558b95
Show file tree
Hide file tree
Showing 15 changed files with 301 additions and 42 deletions.
4 changes: 2 additions & 2 deletions x-pack/plugins/features/server/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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([]),
};
};

Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/features/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
});

Expand Down Expand Up @@ -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(),
});

Expand Down
4 changes: 1 addition & 3 deletions x-pack/plugins/security/server/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand All @@ -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 = () =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const getFeatureIdsForCategories = (
* 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 1 addition & 5 deletions x-pack/plugins/spaces/server/routes/api/external/put.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
48 changes: 48 additions & 0 deletions x-pack/plugins/spaces/server/spaces_client/spaces_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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[] = [
Expand Down Expand Up @@ -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 () => {
Expand Down
59 changes: 55 additions & 4 deletions x-pack/plugins/spaces/server/spaces_client/spaces_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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,
Expand All @@ -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[]> {
Expand Down Expand Up @@ -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 ?? '',
Expand All @@ -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;
};

Expand All @@ -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;
}
}
Loading

0 comments on commit 6558b95

Please sign in to comment.