Skip to content

Commit

Permalink
WIP: feat(rbac): pattern on rbac policy objects
Browse files Browse the repository at this point in the history
Motivation:
There is no way to capture multiple objects in a single policy today and
match against it. The outcome is that policies for basic permission,
where the objects are not catalog entities, are not fine grained. Either
you have permission on an action on all the category, or not at all.
For example the workflows that the orchestrator plugin exposes are not
a catalog entity. They only have a workflow ID the plugin exposes and
there is no way today to deny or allow some of the workflows usage.
It has today this permission:

    orchestrator.workflow.read, read, allow

Modification:
The only modification for the rbac plugin is to use a regexp match:

    [matchers]
    m = g(r.sub, p.sub) && regexMatch(r.obj, p.obj) && r.act == p.act

Result:
With this change we have the permission be matched by a regexp:

    orchestrator.workflow ,       read, allow  // catches all workflow
    orchestrator.workflow.banned ,read, deny   // catches banned workflow

Resolves: #2025

Signed-off-by: Roy Golan <[email protected]>
  • Loading branch information
rgolangh committed Sep 13, 2024
1 parent 5291d32 commit 5289c2a
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 11 deletions.
28 changes: 22 additions & 6 deletions plugins/orchestrator-backend/src/service/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { DiscoveryApi } from '@backstage/core-plugin-api';
import {
AuthorizeResult,
BasicPermission,
createPermission,
} from '@backstage/plugin-permission-common';
import { createPermissionIntegrationRouter } from '@backstage/plugin-permission-node';
import { JsonObject, JsonValue } from '@backstage/types';
Expand Down Expand Up @@ -352,7 +353,10 @@ function setupInternalRoutes(

const decision = await authorize(
_req,
orchestratorWorkflowReadPermission,
createPermission({
name: 'orchestrator.workflow.' + workflowId,
attributes: { action: 'read' },
}),
permissions,
httpAuth,
);
Expand Down Expand Up @@ -393,7 +397,10 @@ function setupInternalRoutes(

const decision = await authorize(
req,
orchestratorWorkflowExecutePermission,
createPermission({
name: 'orchestrator.workflow.' + workflowId,
attributes: {},
}),
permissions,
httpAuth,
);
Expand Down Expand Up @@ -439,7 +446,10 @@ function setupInternalRoutes(

const decision = await authorize(
_req,
orchestratorWorkflowReadPermission,
createPermission({
name: 'orchestrator.workflow.' + workflowId,
attributes: { action: 'read' },
}),
permissions,
httpAuth,
);
Expand Down Expand Up @@ -473,7 +483,10 @@ function setupInternalRoutes(

const decision = await authorize(
req,
orchestratorWorkflowReadPermission,
createPermission({
name: 'orchestrator.workflow.' + workflowId,
attributes: { action: 'read' },
}),
permissions,
httpAuth,
);
Expand Down Expand Up @@ -676,7 +689,10 @@ function setupInternalRoutes(
});
const decision = await authorize(
req,
orchestratorWorkflowInstanceReadPermission,
createPermission({
name: 'orchestrator.workflow.' + workflowId,
attributes: { action: 'read' },
}),
permissions,
httpAuth,
);
Expand Down Expand Up @@ -929,7 +945,7 @@ function setupInternalRoutes(

const decision = await authorize(
req,
orchestratorWorkflowExecutePermission,
createPermission({ name: 'orchestrator.workflow', attributes: {} }),
permissions,
httpAuth,
);
Expand Down
10 changes: 7 additions & 3 deletions plugins/orchestrator/docs/rbac-policy.csv
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
p, role:default/workflowViewer, orchestrator.workflowInstances.read, read, allow
p, role:default/workflowViewer, orchestrator.workflowInstance.read, read, allow

p, role:default/workflowAdmin, orchestrator.workflow.read, read, allow
p, role:default/workflowAdmin, orchestrator.workflow.execute, use, allow
p, role:default/workflowAdmin, orchestrator.workflow, read, allow
p, role:default/workflowAdmin, orchestrator.workflow.hello_world, read, deny

p, role:default/workflowAdmin, orchestrator.workflow, use, allow
p, role:default/workflowAdmin, orchestrator.workflow.ansible, use, deny

p, role:default/workflowAdmin, orchestrator.workflowInstance.abort, use, allow
p, role:default/workflowAdmin, orchestrator.workflowInstances.read, read, allow
p, role:default/workflowAdmin, orchestrator.workflowInstance.read, read, allow

g, user:default/guest, role:default/workflowViewer
g, user:default/rgolangh, role:default/workflowAdmin
g, user:development/guest, role:default/workflowAdmin
4 changes: 3 additions & 1 deletion plugins/rbac-backend/src/service/enforcer-delegate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,9 @@ export class EnforcerDelegate implements RoleEventEmitter<RoleEvents> {

await tempEnforcer.loadFilteredPolicy(filter);

return await tempEnforcer.enforce(entityRef, resourceType, action);
// this is a workaround because the tempEnforcer doesn't work with the regexpMatch probably because the policy filtering.
//return await tempEnforcer.enforce(entityRef, resourceType, action);
return await this.enforcer.enforce(entityRef, resourceType, action);
}

async getImplicitPermissionsForUser(user: string): Promise<string[][]> {
Expand Down
2 changes: 1 addition & 1 deletion plugins/rbac-backend/src/service/permission-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,5 @@ e = some(where (p.eft == allow)) && !some(where (p.eft == deny))
g = _, _
[matchers]
m = g(r.sub, p.sub) && r.obj == p.obj && r.act == p.act
m = g(r.sub, p.sub) && regexMatch(r.obj, p.obj) && r.act == p.act
`;

0 comments on commit 5289c2a

Please sign in to comment.