fix(security): topologically sort composable feature privileges before composing actions (#211035)

## Summary

This PR changes the privilege's actions merging logic for the composable
and deprecated Kibana features. The change makes it possible to have any
number of composable and deprecated features chained by `replaceBy`,
`composedOf`, or a combination of the two.

Under the hood, the privileges factory sorts all deprecated and
composable privileges using Kahn's algorithm for topological sorting,
similar to what is used to sort Kibana plugin dependencies. This allows
us to not only detect cyclical dependencies but also sort privileges in
the proper order depending on their dependency chain.

The use cases addressed by this change are best illustrated by the
`actions should respect composedOf when specified with replaceBy at the
privilege` test in `privileges.test.ts`.
This commit is contained in:
Aleh Zasypkin 2025-03-24 14:09:36 +01:00 committed by GitHub
parent 2654b8c702
commit 6bbc4b67a5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 1016 additions and 1444 deletions

View file

@ -26,6 +26,16 @@ export interface PrivilegesService {
get(respectLicenseLevel?: boolean): RawKibanaPrivileges;
}
interface ComposablePrivilege {
featureId: string;
privilegeId: string;
excludeFromBasePrivileges?: boolean;
referenceGroups: Array<{
references: readonly FeatureKibanaPrivilegesReference[];
actionsFilter?: (action: string) => boolean;
}>;
}
export function privilegesFactory(
actions: Actions,
featuresService: FeaturesPluginSetup,
@ -53,7 +63,7 @@ export function privilegesFactory(
for (const { privilegeId, privilege } of featuresService.featurePrivilegeIterator(feature, {
augmentWithSubFeaturePrivileges: true,
licenseHasAtLeast,
predicate: (pId, featurePrivilege) => !featurePrivilege.excludeFromBasePrivileges,
predicate: (_, featurePrivilege) => !featurePrivilege.excludeFromBasePrivileges,
})) {
const privilegeActions = featurePrivilegeBuilder.getActions(privilege, feature);
privilegeActions.forEach((action) => {
@ -67,13 +77,7 @@ export function privilegesFactory(
// Remember privilege as composable to update it later, once actions for all referenced privileges are also
// calculated and registered.
const composablePrivileges: Array<{
featureId: string;
privilegeId: string;
references: readonly FeatureKibanaPrivilegesReference[];
excludeFromBasePrivileges?: boolean;
actionsFilter?: (action: string) => boolean;
}> = [];
const composablePrivileges: Map<string, ComposablePrivilege> = new Map();
const tryStoreComposablePrivilege = (
feature: KibanaFeature,
privilegeId: string,
@ -81,14 +85,9 @@ export function privilegesFactory(
) => {
// If privilege is configured with `composedOf` it should be complemented with **all**
// actions from referenced privileges.
const referenceGroups: ComposablePrivilege['referenceGroups'] = [];
if (privilege.composedOf) {
composablePrivileges.push({
featureId: feature.id,
privilegeId,
references: privilege.composedOf,
excludeFromBasePrivileges:
feature.excludeFromBasePrivileges || privilege.excludeFromBasePrivileges,
});
referenceGroups.push({ references: privilege.composedOf });
}
// If a privilege is configured with `replacedBy`, it's part of the deprecated feature and
@ -98,13 +97,21 @@ export function privilegesFactory(
// use only non-deprecated UI capabilities.
const replacedBy = getReplacedByForPrivilege(privilegeId, privilege);
if (replacedBy) {
composablePrivileges.push({
featureId: feature.id,
privilegeId,
referenceGroups.push({
references: replacedBy,
actionsFilter: (action) => actions.ui.isValid(action),
});
}
if (referenceGroups.length > 0) {
composablePrivileges.set(getPrivilegeGlobalId(feature.id, privilegeId), {
featureId: feature.id,
privilegeId,
excludeFromBasePrivileges:
feature.excludeFromBasePrivileges || privilege.excludeFromBasePrivileges,
referenceGroups,
});
}
};
const hiddenFeatures = new Set<string>();
@ -165,24 +172,28 @@ export function privilegesFactory(
// another feature. This could potentially enable functionality in a license lower than originally intended. It
// might or might not be desired, but we're accepting this for now, as every attempt to compose a feature
// undergoes a stringent review process.
for (const composableFeature of composablePrivileges) {
const composedActions = composableFeature.references.flatMap((privilegeReference) =>
privilegeReference.privileges.flatMap((privilege) => {
const privilegeActions = featurePrivileges[privilegeReference.feature][privilege] ?? [];
return composableFeature.actionsFilter
? privilegeActions.filter(composableFeature.actionsFilter)
: privilegeActions;
})
for (const composablePrivilegeId of getSortedComposablePrivileges(composablePrivileges)) {
const composablePrivilege = composablePrivileges.get(composablePrivilegeId)!;
const composedActions = composablePrivilege.referenceGroups.flatMap((group) =>
group.references.flatMap((privilegeReference) =>
privilegeReference.privileges.flatMap((privilege) => {
const privilegeActions =
featurePrivileges[privilegeReference.feature][privilege] ?? [];
return group.actionsFilter
? privilegeActions.filter(group.actionsFilter)
: privilegeActions;
})
)
);
featurePrivileges[composableFeature.featureId][composableFeature.privilegeId] = [
featurePrivileges[composablePrivilege.featureId][composablePrivilege.privilegeId] = [
...new Set(
featurePrivileges[composableFeature.featureId][composableFeature.privilegeId].concat(
composedActions
)
featurePrivileges[composablePrivilege.featureId][
composablePrivilege.privilegeId
].concat(composedActions)
),
];
if (!composableFeature.excludeFromBasePrivileges) {
if (!composablePrivilege.excludeFromBasePrivileges) {
for (const action of composedActions) {
// Login action is special since it's added explicitly for feature and base privileges.
if (action === actions.login) {
@ -190,7 +201,7 @@ export function privilegesFactory(
}
allActionsSet.add(action);
if (composableFeature.privilegeId === 'read') {
if (composablePrivilege.privilegeId === 'read') {
readActionsSet.add(action);
}
}
@ -271,3 +282,70 @@ export function getReplacedByForPrivilege(
: replacedBy.default
: replacedBy;
}
const getPrivilegeGlobalId = (featureId: string, privilegeId: string) =>
`${featureId}.${privilegeId}`;
// Gets topologically sorted composable privileges. Ordering is possible if and only if the
// privileges graph has no directed cycles (among composable privileges), meaning it is a directed
// acyclic graph (DAG). If the privileges cannot be ordered, an error is thrown. Uses Kahn's
// Algorithm to sort the graph.
function getSortedComposablePrivileges(privileges: Map<string, ComposablePrivilege>) {
// We clone map so we can remove handled nodes while we perform the topological ordering.
// If the cloned graph is _not_ empty at the end, we know we were not able to topologically
// order the graph. The keys of the graph are the global privilege IDs, and the values are
// the global privilege IDs of the privileges that the target privilege depends on.
const privilegesGraph = new Map(
[...privileges.entries()].map(([privilegeGlobalId, privilege]) => {
// Collect IDs of all referenced privileges that are part of the composable set.
const referencedPrivilegeIds = new Set();
privilege.referenceGroups.forEach((refGroup) =>
refGroup.references.forEach((ref) =>
ref.privileges.forEach((privilegeId) => {
// Include only privileges that are part of the composable set.
const globalPrivilegeId = getPrivilegeGlobalId(ref.feature, privilegeId);
if (privileges.has(globalPrivilegeId)) {
referencedPrivilegeIds.add(globalPrivilegeId);
}
})
)
);
return [privilegeGlobalId, referencedPrivilegeIds] as [string, Set<string>];
})
);
// First, find a list of privileges ("start nodes") which have no dependencies ("outgoing nodes")
// that require sorting. At least one such node must exist in a non-empty acyclic graph.
const privilegesWithAllDependenciesSorted = [...privilegesGraph.keys()].filter(
(globalPrivilegeId) => privilegesGraph.get(globalPrivilegeId)?.size === 0
);
const sortedPrivilegeGlobalIds = new Set<string>();
while (privilegesWithAllDependenciesSorted.length > 0) {
const sortedPrivilegeGlobalId = privilegesWithAllDependenciesSorted.pop()!;
// We know this privilege has all its dependencies sorted, so we can remove it and include
// into the final result.
privilegesGraph.delete(sortedPrivilegeGlobalId);
sortedPrivilegeGlobalIds.add(sortedPrivilegeGlobalId);
// Go through the rest of the privileges and remove `sortedPrivilegeGlobalId` from their
// unsorted dependencies.
for (const [privilegeGlobalId, dependencies] of privilegesGraph) {
// If we managed to delete sortedPrivilegeGlobalId from dependencies, let's check whether it
// was the last one and if we can mark the privilege as sorted.
if (dependencies.delete(sortedPrivilegeGlobalId) && dependencies.size === 0) {
privilegesWithAllDependenciesSorted.push(privilegeGlobalId);
}
}
}
if (privilegesGraph.size > 0) {
const edgesLeft = JSON.stringify([...privilegesGraph.keys()]);
throw new Error(
`Topological ordering of privileges did not complete, these feature privileges have cyclic dependencies: ${edgesLeft}`
);
}
return sortedPrivilegeGlobalIds;
}

View file

@ -892,6 +892,18 @@ export default function ({ getService }: FtrProviderContext) {
"ui:dashboard_v2/storeSearchSession",
"ui:dashboard_v2/generateScreenshot",
"ui:dashboard_v2/downloadCsv",
"app:maps",
"ui:catalogue/maps",
"ui:navLinks/maps",
"saved_object:map/create",
"saved_object:map/bulk_create",
"saved_object:map/update",
"saved_object:map/bulk_update",
"saved_object:map/delete",
"saved_object:map/bulk_delete",
"saved_object:map/share_to_space",
"ui:maps_v2/save",
"ui:maps_v2/show",
"app:visualize",
"app:lens",
"ui:catalogue/visualize",
@ -916,18 +928,6 @@ export default function ({ getService }: FtrProviderContext) {
"ui:visualize_v2/save",
"ui:visualize_v2/createShortUrl",
"ui:visualize_v2/generateScreenshot",
"app:maps",
"ui:catalogue/maps",
"ui:navLinks/maps",
"saved_object:map/create",
"saved_object:map/bulk_create",
"saved_object:map/update",
"saved_object:map/bulk_update",
"saved_object:map/delete",
"saved_object:map/bulk_delete",
"saved_object:map/share_to_space",
"ui:maps_v2/save",
"ui:maps_v2/show",
],
"blocklist_all": Array [
"login:",
@ -1733,6 +1733,18 @@ export default function ({ getService }: FtrProviderContext) {
"ui:dashboard_v2/storeSearchSession",
"ui:dashboard_v2/generateScreenshot",
"ui:dashboard_v2/downloadCsv",
"app:maps",
"ui:catalogue/maps",
"ui:navLinks/maps",
"saved_object:map/create",
"saved_object:map/bulk_create",
"saved_object:map/update",
"saved_object:map/bulk_update",
"saved_object:map/delete",
"saved_object:map/bulk_delete",
"saved_object:map/share_to_space",
"ui:maps_v2/save",
"ui:maps_v2/show",
"app:visualize",
"app:lens",
"ui:catalogue/visualize",
@ -1757,18 +1769,6 @@ export default function ({ getService }: FtrProviderContext) {
"ui:visualize_v2/save",
"ui:visualize_v2/createShortUrl",
"ui:visualize_v2/generateScreenshot",
"app:maps",
"ui:catalogue/maps",
"ui:navLinks/maps",
"saved_object:map/create",
"saved_object:map/bulk_create",
"saved_object:map/update",
"saved_object:map/bulk_update",
"saved_object:map/delete",
"saved_object:map/bulk_delete",
"saved_object:map/share_to_space",
"ui:maps_v2/save",
"ui:maps_v2/show",
],
"minimal_read": Array [
"login:",
@ -2091,6 +2091,10 @@ export default function ({ getService }: FtrProviderContext) {
"saved_object:dashboard/close_point_in_time",
"ui:dashboard_v2/show",
"ui:dashboard_v2/createShortUrl",
"app:maps",
"ui:catalogue/maps",
"ui:navLinks/maps",
"ui:maps_v2/show",
"app:visualize",
"app:lens",
"ui:catalogue/visualize",
@ -2098,10 +2102,6 @@ export default function ({ getService }: FtrProviderContext) {
"ui:navLinks/lens",
"ui:visualize_v2/show",
"ui:visualize_v2/createShortUrl",
"app:maps",
"ui:catalogue/maps",
"ui:navLinks/maps",
"ui:maps_v2/show",
],
"policy_management_all": Array [
"login:",
@ -2460,6 +2460,10 @@ export default function ({ getService }: FtrProviderContext) {
"saved_object:dashboard/close_point_in_time",
"ui:dashboard_v2/show",
"ui:dashboard_v2/createShortUrl",
"app:maps",
"ui:catalogue/maps",
"ui:navLinks/maps",
"ui:maps_v2/show",
"app:visualize",
"app:lens",
"ui:catalogue/visualize",
@ -2467,10 +2471,6 @@ export default function ({ getService }: FtrProviderContext) {
"ui:navLinks/lens",
"ui:visualize_v2/show",
"ui:visualize_v2/createShortUrl",
"app:maps",
"ui:catalogue/maps",
"ui:navLinks/maps",
"ui:maps_v2/show",
],
"scan_operations_all": Array [
"login:",