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

[8.x] fix(security, features): do not expose UI capabilities of the deprecated features (#198656) #199147

Merged
merged 1 commit into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -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
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
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
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