diff --git a/plugins/rbac-backend/package.json b/plugins/rbac-backend/package.json index ffdf9334f0..229f4f27e5 100644 --- a/plugins/rbac-backend/package.json +++ b/plugins/rbac-backend/package.json @@ -24,6 +24,7 @@ }, "dependencies": { "@backstage/backend-common": "^0.19.1", + "@backstage/catalog-client": "^1.4.3", "@backstage/catalog-model": "^1.4.1", "@backstage/config": "^1.0.8", "@backstage/core-plugin-api": "^1.5.3", @@ -32,6 +33,7 @@ "@backstage/plugin-permission-backend": "^0.5.20", "@backstage/plugin-permission-common": "^0.7.7", "@backstage/plugin-permission-node": "^0.7.10", + "@dagrejs/graphlib": "^2.1.13", "@janus-idp/backstage-plugin-rbac-common": "1.0.0", "casbin": "5.27.0", "express": "^4.17.1", diff --git a/plugins/rbac-backend/src/service/permission-model.ts b/plugins/rbac-backend/src/service/permission-model.ts index c1fbb41009..4cf469e812 100644 --- a/plugins/rbac-backend/src/service/permission-model.ts +++ b/plugins/rbac-backend/src/service/permission-model.ts @@ -12,5 +12,5 @@ e = some(where (p.eft == allow)) && !some(where (p.eft == deny)) g = _, _ [matchers] -m = r.sub == p.sub && r.obj == p.obj && r.act == p.act +m = g(r.sub, p.sub) && r.obj == p.obj && r.act == p.act `; diff --git a/plugins/rbac-backend/src/service/permission-policy.test.ts b/plugins/rbac-backend/src/service/permission-policy.test.ts index e332e333da..8216cdaf98 100644 --- a/plugins/rbac-backend/src/service/permission-policy.test.ts +++ b/plugins/rbac-backend/src/service/permission-policy.test.ts @@ -1,4 +1,5 @@ import { getVoidLogger } from '@backstage/backend-common'; +import { Entity } from '@backstage/catalog-model'; import { ConfigReader } from '@backstage/config'; import { BackstageIdentityResponse } from '@backstage/plugin-auth-node'; import { @@ -7,26 +8,65 @@ import { } from '@backstage/plugin-permission-common'; import { PolicyQuery } from '@backstage/plugin-permission-node'; -import { newEnforcer, newModelFromString, StringAdapter } from 'casbin'; +import { + Adapter, + Enforcer, + Model, + newEnforcer, + newModelFromString, + StringAdapter, +} from 'casbin'; +import { Logger } from 'winston'; import { resolve } from 'path'; import { MODEL } from './permission-model'; import { RBACPermissionPolicy } from './permission-policy'; +import { BackstageRoleManager } from './role-manager'; + +const catalogApi = { + getEntityAncestors: jest.fn().mockImplementation(), + getLocationById: jest.fn().mockImplementation(), + getEntities: jest.fn().mockImplementation(), + getEntitiesByRefs: jest.fn().mockImplementation(), + queryEntities: jest.fn().mockImplementation(), + getEntityByRef: jest.fn().mockImplementation(), + refreshEntity: jest.fn().mockImplementation(), + getEntityFacets: jest.fn().mockImplementation(), + addLocation: jest.fn().mockImplementation(), + getLocationByRef: jest.fn().mockImplementation(), + removeLocationById: jest.fn().mockImplementation(), + removeEntityByUid: jest.fn().mockImplementation(), + validateEntity: jest.fn().mockImplementation(), +}; + +async function createEnforcer( + theModel: Model, + adapter: Adapter, + logger: Logger, +): Promise { + const enf = await newEnforcer(theModel, adapter); + + const rm = new BackstageRoleManager(catalogApi, logger); + enf.setRoleManager(rm); + enf.enableAutoBuildRoleLinks(false); + await enf.buildRoleLinks(); + + return enf; +} describe('RBACPermissionPolicy Tests', () => { it('should build', async () => { const adapter = new StringAdapter( - `p, known_user, test-resource, update, allow `, + `p, user:default/known_user, test-resource, update, allow `, ); const config = newConfigReader(); const theModel = newModelFromString(MODEL); - const enf = await newEnforcer(theModel, adapter); - const policy = await RBACPermissionPolicy.build( - getVoidLogger(), - config, - enf, - ); + const logger = getVoidLogger(); + const enf = await createEnforcer(theModel, adapter, logger); + + const policy = await RBACPermissionPolicy.build(logger, config, enf); + expect(policy).not.toBeNull(); }); @@ -36,7 +76,7 @@ describe('RBACPermissionPolicy Tests', () => { beforeEach(async () => { const adapter = new StringAdapter( ` - p, known_user, test.resource.deny, use, allow + p, user:default/known_user, test.resource.deny, use, allow `, ); const csvPermFile = resolve(__dirname, './test/data/rbac-policy.csv'); @@ -48,8 +88,12 @@ describe('RBACPermissionPolicy Tests', () => { }, }); const theModel = newModelFromString(MODEL); - const enf = await newEnforcer(theModel, adapter); - policy = await RBACPermissionPolicy.build(getVoidLogger(), config, enf); + const logger = getVoidLogger(); + const enf = await createEnforcer(theModel, adapter, logger); + + policy = await RBACPermissionPolicy.build(logger, config, enf); + + catalogApi.getEntities.mockReturnValue({ items: [] }); }); // case1 @@ -75,52 +119,58 @@ describe('RBACPermissionPolicy Tests', () => { }); // case3 - it('should allow deny access to resource permission for known_user', async () => { + it('should allow deny access to resource permission for user:default/known_user', async () => { const decision = await policy.handle( newPolicyQueryWithBasicPermission('test.resource.deny'), - newIdentityResponse('known_user'), + newIdentityResponse('user:default/known_user'), ); expect(decision.result).toBe(AuthorizeResult.ALLOW); }); }); - describe('Policy checks', () => { + describe('Policy checks for users', () => { let policy: RBACPermissionPolicy; beforeEach(async () => { const adapter = new StringAdapter( ` - p, known_user, test.resource.deny, use, deny - p, duplicated, test.resource, use, allow - p, duplicated, test.resource, use, deny - p, known_user, test.resource, use, allow - - p, known_user, test-resource-deny, update, deny - p, duplicated, test-resource, update, allow - p, duplicated, test-resource, update, deny - p, known_user, test-resource, update, allow + # basic type permission policies + p, user:default/known_user, test.resource.deny, use, deny + p, user:default/duplicated, test.resource, use, allow + p, user:default/duplicated, test.resource, use, deny + p, user:default/known_user, test.resource, use, allow - p, group, test.resource.deny, use, deny + # resource type permission policies + p, user:default/known_user, test-resource-deny, update, deny + p, user:default/duplicated, test-resource, update, allow + p, user:default/duplicated, test-resource, update, deny + p, user:default/known_user, test-resource, update, allow `, ); const config = newConfigReader(); const theModel = newModelFromString(MODEL); - const enf = await newEnforcer(theModel, adapter); - policy = await RBACPermissionPolicy.build(getVoidLogger(), config, enf); + const logger = getVoidLogger(); + const enf = await createEnforcer(theModel, adapter, logger); + + policy = await RBACPermissionPolicy.build(logger, config, enf); + + catalogApi.getEntities.mockReturnValue({ items: [] }); }); // +-------+------+------------------------+ // | allow | deny | result | // +-------+------+------------------------+ // | N | Y | deny | 1 // | N | N | deny (user not listed) | 2 - // | Y | Y | deny (duplicated) | 3 + // | Y | Y | deny (user:default/duplicated) | 3 // | Y | N | allow | 4 + // Tests for Resource basic type permission + // case1 it('should deny access to basic permission for listed user with deny action', async () => { const decision = await policy.handle( newPolicyQueryWithBasicPermission('test.resource.deny'), - newIdentityResponse('known_user'), + newIdentityResponse('user:default/known_user'), ); expect(decision.result).toBe(AuthorizeResult.DENY); }); @@ -128,7 +178,7 @@ describe('RBACPermissionPolicy Tests', () => { it('should deny access to basic permission for unlisted user', async () => { const decision = await policy.handle( newPolicyQueryWithBasicPermission('test.resource'), - newIdentityResponse('unknown_user'), + newIdentityResponse('unuser:default/known_user'), ); expect(decision.result).toBe(AuthorizeResult.DENY); }); @@ -136,7 +186,7 @@ describe('RBACPermissionPolicy Tests', () => { it('should deny access to basic permission for listed user deny and allow', async () => { const decision = await policy.handle( newPolicyQueryWithBasicPermission('test.resource'), - newIdentityResponse('duplicated'), + newIdentityResponse('user:default/duplicated'), ); expect(decision.result).toBe(AuthorizeResult.DENY); }); @@ -144,7 +194,7 @@ describe('RBACPermissionPolicy Tests', () => { it('should allow access to basic permission for user listed on policy', async () => { const decision = await policy.handle( newPolicyQueryWithBasicPermission('test.resource'), - newIdentityResponse('known_user'), + newIdentityResponse('user:default/known_user'), ); expect(decision.result).toBe(AuthorizeResult.ALLOW); }); @@ -159,7 +209,7 @@ describe('RBACPermissionPolicy Tests', () => { 'test-resource-deny', 'update', ), - newIdentityResponse('known_user'), + newIdentityResponse('user:default/known_user'), ); expect(decision.result).toBe(AuthorizeResult.DENY); }); @@ -171,7 +221,7 @@ describe('RBACPermissionPolicy Tests', () => { 'test-resource', 'update', ), - newIdentityResponse('unknown_user'), + newIdentityResponse('unuser:default/known_user'), ); expect(decision.result).toBe(AuthorizeResult.DENY); }); @@ -183,7 +233,7 @@ describe('RBACPermissionPolicy Tests', () => { 'test-resource', 'update', ), - newIdentityResponse('duplicated'), + newIdentityResponse('user:default/duplicated'), ); expect(decision.result).toBe(AuthorizeResult.DENY); }); @@ -195,7 +245,7 @@ describe('RBACPermissionPolicy Tests', () => { 'test-resource', 'update', ), - newIdentityResponse('known_user'), + newIdentityResponse('user:default/known_user'), ); expect(decision.result).toBe(AuthorizeResult.ALLOW); }); @@ -208,7 +258,7 @@ describe('RBACPermissionPolicy Tests', () => { 'test-resource', 'delete', ), - newIdentityResponse('known_user'), + newIdentityResponse('user:default/known_user'), ); expect(decision.result).toBe(AuthorizeResult.DENY); }); @@ -225,52 +275,459 @@ describe('RBACPermissionPolicy Tests', () => { ); expect(decision.result).toBe(AuthorizeResult.ALLOW); }); + }); +}); - // Tests for admin group added through app config - it('should allow access to permission resource for admin group added through app config', async () => { - const decision = await policy.handle( - newPolicyQueryWithResourcePermission( - 'policy-entity.read', - 'policy-entity', - 'read', - ), - newIdentityResponseWithGroup( - 'user:default/guest2', - 'group:default/guests', - ), - ); - expect(decision.result).toBe(AuthorizeResult.ALLOW); +describe('Policy checks for users and groups', () => { + let policy: RBACPermissionPolicy; + + beforeEach(async () => { + const adapter = new StringAdapter( + ` + # basic type permission policies + ### Let's deny 'use' action for 'test.resource' for group:default/data_admin + p, group:default/data_admin, test.resource, use, deny + + # case1: + # g, user:default/alice, group:default/data_admin + p, user:default/alice, test.resource, use, allow + + # case2: + # g, user:default/akira, group:default/data_admin + + # case3: + # g, user:default/antey, group:default/data_admin + p, user:default/antey, test.resource, use, deny + + ### Let's allow 'use' action for 'test.resource' for group:default/data_read_admin + p, group:default/data_read_admin, test.resource, use, allow + + # case4: + # g, user:default/julia, group:default/data_read_admin + p, user:default/julia, test.resource, use, allow + + # case5: + # g, user:default/mike, group:default/data_read_admin + + # case6: + # g, user:default/tom, group:default/data_read_admin + p, user:default/tom, test.resource, use, deny + + + # resource type permission policies + ### Let's deny 'read' action for 'test.resource' permission for group:default/data_admin + p, group:default/data_admin, test-resource, read, deny + + # case1: + # g, user:default/alice, group:default/data_admin + p, user:default/alice, test-resource, read, allow + + # case2: + # g, user:default/akira, group:default/data_admin + + # case3: + # g, user:default/antey, group:default/data_admin + p, user:default/antey, test-resource, read, deny + + ### Let's allow 'read' action for 'test-resource' permission for group:default/data_read_admin + p, group:default/data_read_admin, test-resource, read, allow + + # case4: + # g, user:default/julia, group:default/data_read_admin + p, user:default/julia, test-resource, read, allow + + # case5: + # g, user:default/mike, group:default/data_read_admin + + # case6: + # g, user:default/tom, group:default/data_read_admin + p, user:default/tom, test-resource, read, deny + + + # group inheritance: + # g, group:default/data-read-admin, group:default/data_parent_admin + # and we know case5: + # g, user:default/mike, data-read-admin + + p, group:default/data_parent_admin, test.resource.2, use, allow + p, group:default/data_parent_admin, test-resource, create, allow + `, + ); + const config = newConfigReader(); + const theModel = newModelFromString(MODEL); + const logger = getVoidLogger(); + const enf = await createEnforcer(theModel, adapter, logger); + + policy = await RBACPermissionPolicy.build(logger, config, enf); + + catalogApi.getEntities.mockReset(); + }); + + // User inherits permissions from groups and their parent groups. + // This behavior can be configured with `policy_effect` in the model. + // Also it can be customized using casbin function. + // Test suite table: + // +-------+---------+----------+-------+ + // | Group | User | result | case# | + // +-------+---------+----------+-------+ + // | deny | allow | deny | 1 | + + // | deny | - | deny | 2 | + + // | deny | deny | deny | 3 | + + // +-------+---------+----------+-------+ + // | allow | allow | allow | 4 | + + // | allow | - | allow | 5 | + + // | allow | deny | deny | 6 | + // +-------+---------+----------+-------+ + + // Basic type permissions + + // case1 + it('should deny access to basic permission for user Alice with "allow" "use" action, when her group "deny" this action', async () => { + const entityMock: Entity = { + apiVersion: 'v1', + kind: 'Group', + metadata: { + name: 'data_admin', + namespace: 'default', + }, + }; + catalogApi.getEntities.mockReturnValue({ items: [entityMock] }); + + const decision = await policy.handle( + newPolicyQueryWithBasicPermission('test.resource'), + newIdentityResponse('user:default/alice'), + ); + expect(decision.result).toBe(AuthorizeResult.DENY); + }); + + // case2 + it('should deny access to basic permission for user Akira without("-") "use" action definition, when his group "deny" this action', async () => { + const entityMock: Entity = { + apiVersion: 'v1', + kind: 'Group', + metadata: { + name: 'data_admin', + namespace: 'default', + }, + }; + catalogApi.getEntities.mockReturnValue({ items: [entityMock] }); + const decision = await policy.handle( + newPolicyQueryWithBasicPermission('test.resource'), + newIdentityResponse('user:default/akira'), + ); + expect(decision.result).toBe(AuthorizeResult.DENY); + }); + + // case3 + it('should deny access to basic permission for user Antey with "deny" "use" action definition, when his group "deny" this action', async () => { + const entityMock: Entity = { + apiVersion: 'v1', + kind: 'Group', + metadata: { + name: 'data_admin', + namespace: 'default', + }, + }; + catalogApi.getEntities.mockReturnValue({ items: [entityMock] }); + const decision = await policy.handle( + newPolicyQueryWithBasicPermission('test.resource'), + newIdentityResponse('user:default/antey'), + ); + expect(decision.result).toBe(AuthorizeResult.DENY); + }); + + // case4 + it('should allow access to basic permission for user Julia with "allow" "use" action, when her group "allow" this action', async () => { + const entityMock: Entity = { + apiVersion: 'v1', + kind: 'Group', + metadata: { + name: 'data_read_admin', + namespace: 'default', + }, + }; + catalogApi.getEntities.mockReturnValue({ items: [entityMock] }); + + const decision = await policy.handle( + newPolicyQueryWithBasicPermission('test.resource'), + newIdentityResponse('user:default/julia'), + ); + expect(decision.result).toBe(AuthorizeResult.ALLOW); + }); + + // case5 + it('should allow access to basic permission for user Mike without("-") "use" action definition, when his group "allow" this action', async () => { + const entityMock: Entity = { + apiVersion: 'v1', + kind: 'Group', + metadata: { + name: 'data_read_admin', + namespace: 'default', + }, + }; + catalogApi.getEntities.mockReturnValue({ items: [entityMock] }); + const decision = await policy.handle( + newPolicyQueryWithBasicPermission('test.resource'), + newIdentityResponse('user:default/mike'), + ); + expect(decision.result).toBe(AuthorizeResult.ALLOW); + }); + + // case6 + it('should deny access to basic permission for user Tom with "deny" "use" action definition, when his group "allow" this action', async () => { + const entityMock: Entity = { + apiVersion: 'v1', + kind: 'Group', + metadata: { + name: 'data_read_admin', + namespace: 'default', + }, + }; + catalogApi.getEntities.mockReturnValue({ items: [entityMock] }); + const decision = await policy.handle( + newPolicyQueryWithBasicPermission('test.resource'), + newIdentityResponse('user:default/tom'), + ); + expect(decision.result).toBe(AuthorizeResult.DENY); + }); + + // inheritance case + it('should allow access to basic permission to test.resource.2 for user Mike with "-" "use" action definition, when parent group of his group "allow" this action', async () => { + const groupMock: Entity = { + apiVersion: 'v1', + kind: 'Group', + metadata: { + name: 'data_read_admin', + namespace: 'default', + }, + spec: { + parent: 'group:default/data_parent_admin', + }, + }; + + const groupParentMock: Entity = { + apiVersion: 'v1', + kind: 'Group', + metadata: { + name: 'data_parent_admin', + namespace: 'default', + }, + }; + + catalogApi.getEntities.mockImplementation(arg => { + const hasMember = arg.filter['relations.hasMember']; + if (hasMember && hasMember[0] === 'user:default/mike') { + return { items: [groupMock] }; + } + const hasParent = arg.filter['relations.parentOf']; + if (hasParent && hasParent[0] === 'group:default/data_read_admin') { + return { items: [groupParentMock] }; + } + return { items: [] }; }); - // Test for group access permission - it('should allow access to a user in an authorized group', async () => { - const decision = await policy.handle( - newPolicyQueryWithBasicPermission('test.resource.deny'), - newIdentityResponseWithGroup('known_user', 'group'), - ); - expect(decision.result).toBe(AuthorizeResult.DENY); + const decision = await policy.handle( + newPolicyQueryWithBasicPermission('test.resource.2'), + newIdentityResponse('user:default/mike'), + ); + expect(decision.result).toBe(AuthorizeResult.ALLOW); + }); + + // Resource type permissions + + // case1 + it('should deny access to basic permission for user Alice with "allow" "read" action, when her group "deny" this action', async () => { + const entityMock: Entity = { + apiVersion: 'v1', + kind: 'Group', + metadata: { + name: 'data_admin', + namespace: 'default', + }, + }; + catalogApi.getEntities.mockReturnValue({ items: [entityMock] }); + + const decision = await policy.handle( + newPolicyQueryWithResourcePermission( + 'test.resource.read', + 'test-resource', + 'read', + ), + newIdentityResponse('user:default/alice'), + ); + expect(decision.result).toBe(AuthorizeResult.DENY); + }); + + // case2 + it('should deny access to basic permission for user Akira without("-") "read" action definition, when his group "deny" this action', async () => { + const entityMock: Entity = { + apiVersion: 'v1', + kind: 'Group', + metadata: { + name: 'data_admin', + namespace: 'default', + }, + }; + catalogApi.getEntities.mockReturnValue({ items: [entityMock] }); + const decision = await policy.handle( + newPolicyQueryWithResourcePermission( + 'test.resource.read', + 'test-resource', + 'read', + ), + newIdentityResponse('user:default/akira'), + ); + expect(decision.result).toBe(AuthorizeResult.DENY); + }); + + // case3 + it('should deny access to basic permission for user Antey with "deny" "read" action definition, when his group "deny" this action', async () => { + const entityMock: Entity = { + apiVersion: 'v1', + kind: 'Group', + metadata: { + name: 'data_admin', + namespace: 'default', + }, + }; + catalogApi.getEntities.mockReturnValue({ items: [entityMock] }); + const decision = await policy.handle( + newPolicyQueryWithResourcePermission( + 'test.resource.read', + 'test-resource', + 'read', + ), + newIdentityResponse('user:default/antey'), + ); + expect(decision.result).toBe(AuthorizeResult.DENY); + }); + + // case4 + it('should allow access to basic permission for user Julia with "allow" "read" action, when her group "allow" this action', async () => { + const entityMock: Entity = { + apiVersion: 'v1', + kind: 'Group', + metadata: { + name: 'data_read_admin', + namespace: 'default', + }, + }; + catalogApi.getEntities.mockReturnValue({ items: [entityMock] }); + + const decision = await policy.handle( + newPolicyQueryWithResourcePermission( + 'test.resource.read', + 'test-resource', + 'read', + ), + newIdentityResponse('user:default/julia'), + ); + expect(decision.result).toBe(AuthorizeResult.ALLOW); + }); + + // case5 + it('should allow access to basic permission for user Mike without("-") "read" action definition, when his group "allow" this action', async () => { + const entityMock: Entity = { + apiVersion: 'v1', + kind: 'Group', + metadata: { + name: 'data_read_admin', + namespace: 'default', + }, + }; + catalogApi.getEntities.mockReturnValue({ items: [entityMock] }); + const decision = await policy.handle( + newPolicyQueryWithResourcePermission( + 'test.resource.read', + 'test-resource', + 'read', + ), + newIdentityResponse('user:default/mike'), + ); + expect(decision.result).toBe(AuthorizeResult.ALLOW); + }); + + // case6 + it('should deny access to basic permission for user Tom with "deny" "read" action definition, when his group "allow" this action', async () => { + const entityMock: Entity = { + apiVersion: 'v1', + kind: 'Group', + metadata: { + name: 'data_read_admin', + namespace: 'default', + }, + }; + catalogApi.getEntities.mockReturnValue({ items: [entityMock] }); + const decision = await policy.handle( + newPolicyQueryWithResourcePermission( + 'test.resource.read', + 'test-resource', + 'read', + ), + newIdentityResponse('user:default/tom'), + ); + expect(decision.result).toBe(AuthorizeResult.DENY); + }); + + // inheritance case + it('should allow access to resource permission to test-resource for user Mike with "-" "write" action definition, when parent group of his group "allow" this action', async () => { + const groupMock: Entity = { + apiVersion: 'v1', + kind: 'Group', + metadata: { + name: 'data_read_admin', + namespace: 'default', + }, + spec: { + parent: 'group:default/data_parent_admin', + }, + }; + + const groupParentMock: Entity = { + apiVersion: 'v1', + kind: 'Group', + metadata: { + name: 'data_parent_admin', + namespace: 'default', + }, + }; + + catalogApi.getEntities.mockImplementation(arg => { + const hasMember = arg.filter['relations.hasMember']; + if (hasMember && hasMember[0] === 'user:default/mike') { + return { items: [groupMock] }; + } + const hasParent = arg.filter['relations.parentOf']; + if (hasParent && hasParent[0] === 'group:default/data_read_admin') { + return { items: [groupParentMock] }; + } + return { items: [] }; }); + + const decision = await policy.handle( + newPolicyQueryWithResourcePermission( + 'test.resource.create', + 'test-resource', + 'create', + ), + newIdentityResponse('user:default/mike'), + ); + expect(decision.result).toBe(AuthorizeResult.ALLOW); }); }); -function newPolicyQueryWithBasicPermission( - name: string, - action?: 'create' | 'read' | 'update' | 'delete' | undefined, -): PolicyQuery { +function newPolicyQueryWithBasicPermission(name: string): PolicyQuery { const mockPermission = createPermission({ name: name, attributes: {}, }); - if (action) { - mockPermission.attributes.action = action; - } return { permission: mockPermission }; } function newPolicyQueryWithResourcePermission( name: string, resource: string, - action?: 'create' | 'read' | 'update' | 'delete' | undefined, + action: 'create' | 'read' | 'update' | 'delete', ): PolicyQuery { const mockPermission = createPermission({ name: name, @@ -294,20 +751,6 @@ function newIdentityResponse(user: string): BackstageIdentityResponse { }; } -function newIdentityResponseWithGroup( - user: string, - group: string, -): BackstageIdentityResponse { - return { - identity: { - ownershipEntityRefs: [group], - type: 'user', - userEntityRef: user, - }, - token: '', - }; -} - function newConfigReader(): ConfigReader { return new ConfigReader({ permission: { diff --git a/plugins/rbac-backend/src/service/permission-policy.ts b/plugins/rbac-backend/src/service/permission-policy.ts index af9283e834..dcdeba2a3d 100644 --- a/plugins/rbac-backend/src/service/permission-policy.ts +++ b/plugins/rbac-backend/src/service/permission-policy.ts @@ -94,10 +94,10 @@ export class RBACPermissionPolicy implements PermissionPolicy { async handle( request: PolicyQuery, - user?: BackstageIdentityResponse | undefined, + identityResp?: BackstageIdentityResponse | undefined, ): Promise { this.logger.info( - `Policy check for ${user?.identity.userEntityRef} for permission ${request.permission.name}`, + `Policy check for ${identityResp?.identity.userEntityRef} for permission ${request.permission.name}`, ); try { let status = false; @@ -108,13 +108,13 @@ export class RBACPermissionPolicy implements PermissionPolicy { if (isResourcePermission(request.permission)) { status = await this.isAuthorized( - user?.identity, + identityResp?.identity, request.permission.resourceType, action, ); } else { status = await this.isAuthorized( - user?.identity, + identityResp?.identity, request.permission.name, action, ); @@ -122,7 +122,7 @@ export class RBACPermissionPolicy implements PermissionPolicy { const result = status ? AuthorizeResult.ALLOW : AuthorizeResult.DENY; this.logger.info( - `${user?.identity.userEntityRef} is ${result} for permission ${request.permission.name}`, + `${identityResp?.identity.userEntityRef} is ${result} for permission ${request.permission.name} and action ${action}`, ); return Promise.resolve({ result: result, @@ -139,27 +139,14 @@ export class RBACPermissionPolicy implements PermissionPolicy { identity: BackstageUserIdentity | undefined, resourceType: string, action: string, - ) => { - let status; - - // Check if the group has access first - const ownerStatus = await Promise.all( - identity?.ownershipEntityRefs.map(async entityRef => { - return await this.enforcer.enforce(entityRef, resourceType, action); - }) || [], - ); - - status = ownerStatus.includes(true); - - // Check if the user has access - if (!status) { - status = await this.enforcer.enforce( - identity?.userEntityRef, - resourceType, - action, - ); + ): Promise => { + if (!identity) { + // Allow access for backend plugins + return true; } - return status; + const entityRef = identity.userEntityRef; + + return await this.enforcer.enforce(entityRef, resourceType, action); }; } diff --git a/plugins/rbac-backend/src/service/policies-rest-api.ts b/plugins/rbac-backend/src/service/policies-rest-api.ts index 4f439d94ce..a93a81f04d 100644 --- a/plugins/rbac-backend/src/service/policies-rest-api.ts +++ b/plugins/rbac-backend/src/service/policies-rest-api.ts @@ -125,31 +125,29 @@ export class PolicesServer { response.json(this.transformPolicyArray(...policies)); }); - router.get( - '/policies/:kind/:namespace/:name', - async (request, response) => { - const decision = await this.authorize( - this.identity, - request, - this.permissions, - { - permission: policyEntityReadPermission, - }, - ); + router.get('/policies/:kind/:namespace/:name', async (req, response) => { + const decision = await this.authorize( + this.identity, + req, + this.permissions, + { + permission: policyEntityReadPermission, + }, + ); - if (decision.result === AuthorizeResult.DENY) { - throw new NotAllowedError(); // 403 - } + if (decision.result === AuthorizeResult.DENY) { + throw new NotAllowedError(); // 403 + } - const entityRef = this.getEntityReference(request); - const policy = await this.enforcer.getFilteredPolicy(0, entityRef); - if (policy.length !== 0) { - response.json(this.transformPolicyArray(...policy)); - } else { - throw new NotFoundError(); // 404 - } - }, - ); + const entityRef = this.getEntityReference(req); + + const policy = await this.enforcer.getFilteredPolicy(0, entityRef); + if (policy.length !== 0) { + response.json(this.transformPolicyArray(...policy)); + } else { + throw new NotFoundError(); // 404 + } + }); router.delete( '/policies/:kind/:namespace/:name', diff --git a/plugins/rbac-backend/src/service/policies-validation.ts b/plugins/rbac-backend/src/service/policies-validation.ts index c76de03073..516a0b93b6 100644 --- a/plugins/rbac-backend/src/service/policies-validation.ts +++ b/plugins/rbac-backend/src/service/policies-validation.ts @@ -61,6 +61,12 @@ export function validateEntityReference(entityRef?: string): Error | undefined { ); } + if (entityRefCompound.kind !== 'user' && entityRefCompound.kind !== 'group') { + return new Error( + `Unsupported kind ${entityRefCompound.kind}. List supported values ["user", "group"]`, + ); + } + return undefined; } diff --git a/plugins/rbac-backend/src/service/policy-builder.test.ts b/plugins/rbac-backend/src/service/policy-builder.test.ts index ca2df9ade4..548a5af09c 100644 --- a/plugins/rbac-backend/src/service/policy-builder.test.ts +++ b/plugins/rbac-backend/src/service/policy-builder.test.ts @@ -13,7 +13,10 @@ import { PolicyBuilder } from './policy-builder'; const mockEnforcer: Partial = { loadPolicy: jest.fn().mockImplementation(async () => {}), - enableAutoSave: jest.fn().mockImplementation((_enable: boolean) => {}), + enableAutoSave: jest.fn().mockImplementation(() => {}), + setRoleManager: jest.fn().mockImplementation(() => {}), + enableAutoBuildRoleLinks: jest.fn().mockImplementation(() => {}), + buildRoleLinks: jest.fn().mockImplementation(() => {}), }; jest.mock('casbin', () => { diff --git a/plugins/rbac-backend/src/service/policy-builder.ts b/plugins/rbac-backend/src/service/policy-builder.ts index f8ca1c10c1..79047dffb0 100644 --- a/plugins/rbac-backend/src/service/policy-builder.ts +++ b/plugins/rbac-backend/src/service/policy-builder.ts @@ -3,6 +3,7 @@ import { PluginEndpointDiscovery, resolvePackagePath, } from '@backstage/backend-common'; +import { CatalogClient } from '@backstage/catalog-client'; import { Config } from '@backstage/config'; import { IdentityApi } from '@backstage/plugin-auth-node'; import { RouterOptions } from '@backstage/plugin-permission-backend'; @@ -16,6 +17,7 @@ import { CasbinDBAdapterFactory } from './casbin-adapter-factory'; import { MODEL } from './permission-model'; import { RBACPermissionPolicy } from './permission-policy'; import { PolicesServer } from './policies-rest-api'; +import { BackstageRoleManager } from './role-manager'; export class PolicyBuilder { public static async build(env: { @@ -49,6 +51,12 @@ export class PolicyBuilder { await enf.loadPolicy(); enf.enableAutoSave(true); + const catalogClient = new CatalogClient({ discoveryApi: env.discovery }); + const rm = new BackstageRoleManager(catalogClient, env.logger); + enf.setRoleManager(rm); + enf.enableAutoBuildRoleLinks(false); + await enf.buildRoleLinks(); + const options: RouterOptions = { config: env.config, logger: env.logger, diff --git a/plugins/rbac-backend/src/service/role-manager.test.ts b/plugins/rbac-backend/src/service/role-manager.test.ts new file mode 100644 index 0000000000..7bfa5f7e9d --- /dev/null +++ b/plugins/rbac-backend/src/service/role-manager.test.ts @@ -0,0 +1,599 @@ +import { CatalogApi } from '@backstage/catalog-client'; +import { Entity } from '@backstage/catalog-model'; + +import { Logger } from 'winston'; + +import { BackstageRoleManager } from './role-manager'; + +describe('BackstageRoleManager', () => { + const catalogApiMock: any = { + getEntities: jest + .fn() + .mockImplementation(() => Promise.resolve({ items: [] })), + }; + + const loggerMock: any = { + warn: jest.fn().mockImplementation(), + debug: jest.fn().mockImplementation(), + }; + + const roleManager = new BackstageRoleManager( + catalogApiMock as CatalogApi, + loggerMock as Logger, + ); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('unimplemented methods', () => { + it('should throw an error for addLink', async () => { + await expect( + roleManager.addLink('user:default/role1', 'user:default/role2'), + ).rejects.toThrow('Method "addLink" not implemented.'); + }); + + it('should throw an error for deleteLink', async () => { + await expect( + roleManager.deleteLink('user:default/role1', 'user:default/role2'), + ).rejects.toThrow('Method "deleteLink" not implemented.'); + }); + + it('should throw an error for syncedHasLink', () => { + expect(() => + roleManager.syncedHasLink!('user:default/role1', 'user:default/role2'), + ).toThrow('Method "syncedHasLink" not implemented.'); + }); + + it('should throw an error for getRoles', async () => { + await expect(roleManager.getRoles('name')).rejects.toThrow( + 'Method "getRoles" not implemented.', + ); + }); + + it('should throw an error for getUsers', async () => { + await expect(roleManager.getUsers('name')).rejects.toThrow( + 'Method "getUsers" not implemented.', + ); + }); + }); + + describe('hasLink tests', () => { + it('should throw an error for unsupported domain', async () => { + await expect( + roleManager.hasLink( + 'user:default/mike', + 'group:default/somegroup', + 'someDomain', + ), + ).rejects.toThrow('domain argument is not supported.'); + }); + + it('should return true for hasLink when names are the same', async () => { + const result = await roleManager.hasLink( + 'user:default/mike', + 'user:default/mike', + ); + expect(result).toBe(true); + }); + + it('should return false for hasLink when name2 has a user kind', async () => { + const result = await roleManager.hasLink( + 'user:default/mike', + 'user:default/some-user', + ); + expect(result).toBe(false); + }); + + // user:default/mike should not inherits from group:default/somegroup + // + // Hierarchy: + // + // user:default/mike -> user without group + // + it('should return false for hasLink when user without group', async () => { + const result = await roleManager.hasLink( + 'user:default/mike', + 'group:default/somegroup', + ); + expect(catalogApiMock.getEntities).toHaveBeenCalledWith({ + filter: { + kind: 'Group', + 'relations.hasMember': ['user:default/mike'], + }, + fields: [ + 'metadata.name', + 'kind', + 'metadata.namespace', + 'spec.parent', + 'spec.children', + ], + }); + expect(result).toBeFalsy(); + }); + + // user:default/mike should inherits from group:default/somegroup + // + // Hierarchy: + // + // group:default/somegroup + // | + // user:default/mike + // + it('should return true for hasLink when user:default/mike inherits from group:default/somegroup', async () => { + const entityMock = createGroupEntity('somegroup', undefined, []); + catalogApiMock.getEntities.mockReturnValue({ items: [entityMock] }); + + const result = await roleManager.hasLink( + 'user:default/mike', + 'group:default/somegroup', + ); + expect(result).toBeTruthy(); + }); + + // user:default/mike should not inherits from group:default/somegroup + // + // Hierarchy: + // + // group:default/not-matched-group + // | + // user:default/mike + // + it('should return false for hasLink when user:default/mike does not inherits group:default/somegroup', async () => { + const entityMock = createGroupEntity('not-matched-group', undefined, []); + catalogApiMock.getEntities.mockReturnValue({ items: [entityMock] }); + + const result = await roleManager.hasLink( + 'user:default/mike', + 'group:default/somegroup', + ); + expect(result).toBeFalsy(); + }); + + // user:default/mike should inherits from group:default/team-a + // + // Hierarchy: + // + // group:default/team-a + // | + // group:default/team-b + // | + // user:default/mike + // + it('should return true for hasLink, when user:default/mike inherits from group:default/team-a', async () => { + const groupMock = createGroupEntity('team-b', 'team-a', []); + const groupParentMock = createGroupEntity('team-a', undefined, [ + 'team-b', + ]); + + catalogApiMock.getEntities.mockImplementation((arg: any) => { + const hasMember = arg.filter['relations.hasMember']; + if (hasMember && hasMember[0] === 'user:default/mike') { + return { items: [groupMock] }; + } + const hasParent = arg.filter['relations.parentOf']; + if (hasParent && hasParent[0] === 'group:default/team-b') { + return { items: [groupParentMock] }; + } + return { items: [] }; + }); + + const result = await roleManager.hasLink( + 'user:default/mike', + 'group:default/team-a', + ); + expect(result).toBeTruthy(); + }); + + // user:default/mike should inherits from group:default/team-b. + // + // Hierarchy: + // + // |---------group:default/team-a---------| + // | | | + // user:default/team-c group:default/team-b group:default/team-d + // | | | + // user:default/tom user:default/mike user:default:john + // + it('should return true for hasLink, when user:default/mike inherits from group:default/team-b', async () => { + const groupAMock = createGroupEntity('team-a', undefined, [ + 'team-b', + 'team-c', + 'team-d', + ]); + const groupBMock = createGroupEntity('team-b', 'team-a', []); + const groupCMock = createGroupEntity('team-c', 'team-a', []); + const groupDMock = createGroupEntity('team-d', 'team-a', []); + + catalogApiMock.getEntities.mockImplementation((arg: any) => { + const hasMember = arg.filter['relations.hasMember']; + if (hasMember && hasMember[0] === 'user:default/mike') { + return { items: [groupBMock] }; + } + if (hasMember && hasMember[0] === 'user:default/tom') { + return { items: [groupCMock] }; + } + if (hasMember && hasMember[0] === 'user:default/john') { + return { items: [groupDMock] }; + } + + const hasParent = arg.filter['relations.parentOf']; + if (hasParent && hasParent[0] === 'group:default/team-b') { + return { items: [groupAMock] }; + } + if (hasParent && hasParent[0] === 'group:default/team-c') { + return { items: [groupAMock] }; + } + if (hasParent && hasParent[0] === 'group:default/team-d') { + return { items: [groupAMock] }; + } + return { items: [] }; + }); + + const result = await roleManager.hasLink( + 'user:default/mike', + 'group:default/team-a', + ); + expect(result).toBeTruthy(); + }); + + // user:default/mike should not inherits from group:default/team-c + // + // Hierarchy: + // + // group:default/team-a + // | + // group:default/team-b + // | + // user:default/mike + // + it('should return false for hasLink, when user:default/mike does not inherits from group:default/team-c', async () => { + const groupBMock = createGroupEntity('team-b', 'team-a', []); + const groupAMock = createGroupEntity('team-a', undefined, ['team-b']); + + catalogApiMock.getEntities.mockImplementation((arg: any) => { + const hasMember = arg.filter['relations.hasMember']; + if (hasMember && hasMember[0] === 'user:default/mike') { + return { items: [groupBMock] }; + } + const hasParent = arg.filter['relations.parentOf']; + if (hasParent && hasParent[0] === 'group:default/team-b') { + return { items: [groupAMock] }; + } + return { items: [] }; + }); + + const result = await roleManager.hasLink( + 'user:default/mike', + 'group:default/team-c', + ); + expect(result).toBeFalsy(); + }); + + // user:default/mike should inherits from group:default/team-a + // + // Hierarchy: + // + // group:default/team-a group:default/team-b + // | | + // group:default/team-c group:default/team-d + // | | + // user:default/mike + // + it('should return true for hasLink, when user:default/mike inherits group tree with group:default/team-a', async () => { + const groupCMock = createGroupEntity('team-c', 'team-a', []); + const groupDMock = createGroupEntity('team-d', 'team-b', []); + const groupAMock = createGroupEntity('team-a', undefined, ['team-c']); + const groupBMock = createGroupEntity('team-b', undefined, ['team-d']); + + catalogApiMock.getEntities.mockImplementation((arg: any) => { + const hasMember = arg.filter['relations.hasMember']; + if (hasMember && hasMember[0] === 'user:default/mike') { + return { items: [groupCMock, groupDMock] }; + } + const hasParent = arg.filter['relations.parentOf']; + if ( + hasParent && + hasParent[0] === 'group:default/team-c' && + hasParent[1] === 'group:default/team-d' + ) { + return { items: [groupAMock, groupBMock] }; + } + return { items: [] }; + }); + + const result = await roleManager.hasLink( + 'user:default/mike', + 'group:default/team-a', + ); + expect(result).toBeTruthy(); + }); + + // user:default/mike should not inherits from group:default/team-e + // + // Hierarchy: + // + // group:default/team-a group:default/team-b + // | | + // group:default/team-c group:default/team-d + // | | + // user:default/mike + // + it('should return false for hasLink, when user:default/mike inherits from group:default/team-e', async () => { + const groupCMock = createGroupEntity('team-c', 'team-a', []); + const groupDMock = createGroupEntity('team-d', 'team-b', []); + const groupAMock = createGroupEntity('team-a', undefined, ['team-c']); + const groupBMock = createGroupEntity('team-b', undefined, ['team-d']); + + catalogApiMock.getEntities.mockImplementation((arg: any) => { + const hasMember = arg.filter['relations.hasMember']; + if (hasMember && hasMember[0] === 'user:default/mike') { + return { items: [groupCMock, groupDMock] }; + } + const hasParent = arg.filter['relations.parentOf']; + if ( + hasParent && + hasParent[0] === 'group:default/team-c' && + hasParent[1] === 'group:default/team-d' + ) { + return { items: [groupAMock, groupBMock] }; + } + return { items: [] }; + }); + + const result = await roleManager.hasLink( + 'user:default/mike', + 'group:default/team-e', + ); + expect(result).toBeFalsy(); + }); + + // user:default/mike should inherits from group:default/team-b and group:default/team-a, but we have cycle dependency. + // So return false on call hasLink. + // + // Hierarchy: + // + // group:default/team-a + // ↓ ↑ + // group:default/team-b + // ↓ + // user:default/mike + // + it('should return false for hasLink, when user:default/mike inherits from group:default/team-a and group:default/team-b, but we have cycle dependency', async () => { + const groupBMock = createGroupEntity('team-b', 'team-a', []); + const groupAMock = createGroupEntity('team-a', 'team-b', ['team-b']); + + catalogApiMock.getEntities.mockImplementation((arg: any) => { + const hasMember = arg.filter['relations.hasMember']; + if (hasMember && hasMember[0] === 'user:default/mike') { + return { items: [groupBMock] }; + } + const hasParent = arg.filter['relations.parentOf']; + if (hasParent && hasParent[0] === 'group:default/team-b') { + return { items: [groupAMock] }; + } + return { items: [] }; + }); + + let result = await roleManager.hasLink( + 'user:default/mike', + 'group:default/team-b', + ); + expect(result).toBeFalsy(); + expect(loggerMock.warn).toHaveBeenCalledWith( + 'Detected cycle dependencies in the Group graph: [["group:default/team-a","group:default/team-b"]]. Admin/(catalog owner) have to fix it to make RBAC permission evaluation correct for group: group:default/team-b', + ); + + result = await roleManager.hasLink( + 'user:default/mike', + 'group:default/team-a', + ); + expect(result).toBeFalsy(); + expect(loggerMock.warn).toHaveBeenCalledWith( + 'Detected cycle dependencies in the Group graph: [["group:default/team-a","group:default/team-b"]]. Admin/(catalog owner) have to fix it to make RBAC permission evaluation correct for group: group:default/team-a', + ); + }); + + // user:default/mike should inherits from group:default/team-a, group:default/team-b, group:default/team-c, but we have cycle dependency. + // So return false on call hasLink. + // + // Hierarchy: + // + // group:default/team-a + // ↓ ↑ + // group:default/team-b + // ↓ + // group:default/team-c + // ↓ + // user:default/mike + // + it('should return false for hasLink, when user:default/mike inherits from group:default/team-a, group:default/team-b, group:default/team-c, but we have cycle dependency', async () => { + const groupAMock = createGroupEntity('team-a', 'team-b', ['team-b']); + const groupBMock = createGroupEntity('team-b', 'team-a', []); + const groupCMock = createGroupEntity('team-c', 'team-b', []); + + catalogApiMock.getEntities.mockImplementation((arg: any) => { + const hasMember = arg.filter['relations.hasMember']; + if (hasMember && hasMember[0] === 'user:default/mike') { + return { items: [groupCMock] }; + } + const hasParent = arg.filter['relations.parentOf']; + if (hasParent && hasParent[0] === 'group:default/team-c') { + return { items: [groupBMock] }; + } + if (hasParent && hasParent[0] === 'group:default/team-b') { + return { items: [groupAMock] }; + } + return { items: [] }; + }); + + let result = await roleManager.hasLink( + 'user:default/mike', + 'group:default/team-c', + ); + expect(result).toBeFalsy(); + expect(loggerMock.warn).toHaveBeenCalledWith( + 'Detected cycle dependencies in the Group graph: [["group:default/team-a","group:default/team-b"]]. Admin/(catalog owner) have to fix it to make RBAC permission evaluation correct for group: group:default/team-c', + ); + + result = await roleManager.hasLink( + 'user:default/mike', + 'group:default/team-b', + ); + expect(result).toBeFalsy(); + expect(loggerMock.warn).toHaveBeenCalledWith( + 'Detected cycle dependencies in the Group graph: [["group:default/team-a","group:default/team-b"]]. Admin/(catalog owner) have to fix it to make RBAC permission evaluation correct for group: group:default/team-b', + ); + + result = await roleManager.hasLink( + 'user:default/mike', + 'group:default/team-a', + ); + expect(result).toBeFalsy(); + expect(loggerMock.warn).toHaveBeenCalledWith( + 'Detected cycle dependencies in the Group graph: [["group:default/team-a","group:default/team-b"]]. Admin/(catalog owner) have to fix it to make RBAC permission evaluation correct for group: group:default/team-a', + ); + }); + + // user:default/mike should inherits from group:default/team-a, but we have cycle dependency: team-a -> team-c. + // So return false on call hasLink. + // + // Hierarchy: + // + // group:default/team-a group:default/team-b + // ↓ ↑ ↓ + // group:default/team-c group:default/team-d + // ↓ ↓ + // user:default/mike + // + it('should return false for hasLink, when user:default/mike inherits group tree with group:default/team-a, but we cycle dependency', async () => { + const groupCMock = createGroupEntity('team-c', 'team-a', ['mike']); + const groupDMock = createGroupEntity('team-d', 'team-b', ['mike']); + const groupAMock = createGroupEntity('team-a', 'team-c', ['team-c']); + const groupBMock = createGroupEntity('team-b', undefined, ['team-d']); + + catalogApiMock.getEntities.mockImplementation((arg: any) => { + const hasMember = arg.filter['relations.hasMember']; + // first iteration + if (hasMember && hasMember[0] === 'user:default/mike') { + return { items: [groupCMock, groupDMock] }; + } + const hasParent = arg.filter['relations.parentOf']; + // second iteration + if ( + hasParent && + hasParent[0] === 'group:default/team-c' && + hasParent[1] === 'group:default/team-d' + ) { + return { items: [groupAMock, groupBMock] }; + } + return { items: [] }; + }); + + const result = await roleManager.hasLink( + 'user:default/mike', + 'group:default/team-a', + ); + expect(result).toBeFalsy(); + expect(loggerMock.warn).toHaveBeenCalledWith( + 'Detected cycle dependencies in the Group graph: [["group:default/team-a","group:default/team-c"]]. Admin/(catalog owner) have to fix it to make RBAC permission evaluation correct for group: group:default/team-a', + ); + }); + + // user:default/mike should inherits from group:default/team-a, but we have cycle dependency: team-a -> team-c. + // So return false on call hasLink. + // + // user:default/tom should inherits from group:default/team-b. Cycle dependency in the neighbor subgraph, should + // not affect evaluation user:default/tom inheritance. + // + // Hierarchy: + // + // group:default/root + // ↓ ↓ + // group:default/team-a group:default/team-b + // ↓ ↑ ↓ + // group:default/team-c group:default/team-d + // ↓ ↓ + // user:default/mike user:default/tom + // + it('should return false for hasLink for user:default/mike and group:default/team-a(cycle dependency), but should be true for user:default/tom and group:default/team-b', async () => { + const groupRootMock = createGroupEntity('root', undefined, [ + 'team-a', + 'team-b', + ]); + const groupCMock = createGroupEntity('team-c', 'team-a', ['team-a']); + const groupDMock = createGroupEntity('team-d', 'team-b', []); + const groupAMock = createGroupEntity('team-a', 'root', ['team-c']); + const groupBMock = createGroupEntity('team-b', 'root', ['team-d']); + + catalogApiMock.getEntities.mockImplementation((arg: any) => { + const hasMember = arg.filter['relations.hasMember']; + + if (hasMember && hasMember[0] === 'user:default/mike') { + return { items: [groupCMock] }; + } + if (hasMember && hasMember[0] === 'user:default/tom') { + return { items: [groupDMock] }; + } + + const hasParent = arg.filter['relations.parentOf']; + + if (hasParent && hasParent[0] === 'group:default/team-c') { + return { items: [groupAMock] }; + } + + if (hasParent && hasParent[0] === 'group:default/team-d') { + return { items: [groupBMock] }; + } + + if ( + (hasParent && hasParent[0] === 'group:default/team-b') || + hasParent[0] === 'group:default/team-a' + ) { + return { items: [groupRootMock] }; + } + return { items: [] }; + }); + + let result = await roleManager.hasLink( + 'user:default/mike', + 'group:default/team-a', + ); + expect(result).toBeFalsy(); + expect(loggerMock.warn).toHaveBeenCalledWith( + 'Detected cycle dependencies in the Group graph: [["group:default/team-a","group:default/team-c"]]. Admin/(catalog owner) have to fix it to make RBAC permission evaluation correct for group: group:default/team-a', + ); + + result = await roleManager.hasLink( + 'user:default/tom', + 'group:default/team-b', + ); + expect(result).toBeTruthy(); + }); + }); + function createGroupEntity( + name: string, + parent?: string, + children?: string[], + ): Entity { + const entity: Entity = { + apiVersion: 'v1', + kind: 'Group', + metadata: { + name, + namespace: 'default', + }, + spec: {}, + }; + + if (children) { + entity.spec!.children = children; + } + + if (parent) { + entity.spec!.parent = parent; + } + + return entity; + } +}); diff --git a/plugins/rbac-backend/src/service/role-manager.ts b/plugins/rbac-backend/src/service/role-manager.ts index 43b149d2bb..ebc9a8199b 100644 --- a/plugins/rbac-backend/src/service/role-manager.ts +++ b/plugins/rbac-backend/src/service/role-manager.ts @@ -1,40 +1,241 @@ +import { CatalogApi } from '@backstage/catalog-client'; +import { parseEntityRef } from '@backstage/catalog-model'; + +import { alg, Graph } from '@dagrejs/graphlib'; import { RoleManager } from 'casbin'; +import { Logger } from 'winston'; -export class BackstageRoleManager implements RoleManager { - clear(): Promise { - throw new Error('Method not implemented.'); +type FilterRelations = 'relations.hasMember' | 'relations.parentOf'; + +// AncestorSearchMemo - should be used to build group hierarchy graph for User entity reference. +// It supports search group entity reference link in the graph. +// Also AncestorSearchMemo supports detection cycle dependencies between groups in the graph. +// +// Notice: this class should be used like cache object in the nearest feature. +// This cache can be implemented with time expiration and it can be map: Map. +class AncestorSearchMemo { + private filterEntityRefs: Set; + + private graph: Graph; + private fr: FilterRelations; + + constructor(userEntityRef: string) { + this.filterEntityRefs = new Set([userEntityRef]); + this.graph = new Graph({ directed: true }); + this.fr = 'relations.hasMember'; + } + + setFilterEntityRefs(entityRefs: Set) { + this.filterEntityRefs = entityRefs; + } + + getFilterEntityRefs(): Set { + return this.filterEntityRefs; + } + + setFilterRelations(fr: FilterRelations): void { + this.fr = fr; + } + + getFilterRelations(): FilterRelations { + return this.fr; + } + + isAcyclic(): boolean { + return alg.isAcyclic(this.graph); + } + + findCycles(): string[][] { + return alg.findCycles(this.graph); + } + + setEdge(parentEntityRef: string, childEntityRef: string) { + this.graph.setEdge(parentEntityRef, childEntityRef); + } + + setNode(entityRef: string): void { + this.graph.setNode(entityRef); + } + + hasEntityRef(groupRef: string): boolean { + return this.graph.hasNode(groupRef); + } + + debugNodesAndEdges(log: Logger, userEntity: string): void { + log.debug( + `SubGraph edges: ${JSON.stringify(this.graph.edges())} for ${userEntity}`, + ); + log.debug( + `SubGraph nodes: ${JSON.stringify(this.graph.nodes())} for ${userEntity}`, + ); } - addLink(_name1: string, _name2: string, ..._domain: string[]): Promise { - throw new Error('Method not implemented.'); +} + +export class BackstageRoleManager implements RoleManager { + constructor( + private readonly catalogApi: CatalogApi, + private readonly log: Logger, + ) {} + + /** + * clear clears all stored data and resets the role manager to the initial state. + */ + async clear(): Promise { + // do nothing } - deleteLink( + + /** + * addLink adds the inheritance link between role: name1 and role: name2. + * aka role: name1 inherits role: name2. + * domain is a prefix to the roles. + */ + async addLink( _name1: string, _name2: string, ..._domain: string[] ): Promise { - throw new Error('Method not implemented.'); + throw new Error('Method "addLink" not implemented.'); } - hasLink( + + /** + * deleteLink deletes the inheritance link between role: name1 and role: name2. + * aka role: name1 does not inherit role: name2 any more. + * domain is a prefix to the roles. + */ + async deleteLink( _name1: string, _name2: string, ..._domain: string[] + ): Promise { + throw new Error('Method "deleteLink" not implemented.'); + } + + /** + * hasLink determines whether role: name1 inherits role: name2. + * domain is a prefix to the roles. + */ + async hasLink( + name1: string, + name2: string, + ...domain: string[] ): Promise { - throw new Error('Method not implemented.'); + if (domain.length > 0) { + throw new Error('domain argument is not supported.'); + } + + if (name1 === name2) { + return true; + } + + // name1 is always user in our case. + // name2 is user or group. + // user(name1) couldn't inherit user(name2). + // We can use this fact for optimization. + const { kind } = parseEntityRef(name2); + if (kind.toLocaleLowerCase() === 'user') { + return false; + } + + const memo = new AncestorSearchMemo(name1); + await this.findAncestorGroups(memo); + memo.debugNodesAndEdges(this.log, name1); + if (!memo.isAcyclic()) { + const cycles = memo.findCycles(); + + memo.hasEntityRef(name2); + + this.log.warn( + `Detected cycle ${ + cycles.length > 0 ? 'dependencies' : 'dependency' + } in the Group graph: ${JSON.stringify( + cycles, + )}. Admin/(catalog owner) have to fix it to make RBAC permission evaluation correct for group: ${name2}`, + ); + + return false; + } + return memo.hasEntityRef(name2); } + + /** + * syncedHasLink determines whether role: name1 inherits role: name2. + * domain is a prefix to the roles. + */ syncedHasLink?( _name1: string, _name2: string, ..._domain: string[] ): boolean { - throw new Error('Method not implemented.'); + throw new Error('Method "syncedHasLink" not implemented.'); } - getRoles(_name: string, ..._domain: string[]): Promise { - throw new Error('Method not implemented.'); + + /** + * getRoles gets the roles that a subject inherits. + * domain is a prefix to the roles. + */ + async getRoles(_name: string, ..._domain: string[]): Promise { + throw new Error('Method "getRoles" not implemented.'); + } + + /** + * getUsers gets the users that inherits a subject. + * domain is an unreferenced parameter here, may be used in other implementations. + */ + async getUsers(_name: string, ..._domain: string[]): Promise { + throw new Error('Method "getUsers" not implemented.'); } - getUsers(_name: string, ..._domain: string[]): Promise { - throw new Error('Method not implemented.'); + + /** + * printRoles prints all the roles to log. + */ + async printRoles(): Promise { + // do nothing } - printRoles(): Promise { - throw new Error('Method not implemented.'); + + private async findAncestorGroups(memo: AncestorSearchMemo): Promise { + const { items } = await this.catalogApi.getEntities({ + filter: { + kind: 'Group', + [memo.getFilterRelations()]: Array.from(memo.getFilterEntityRefs()), + }, + // Save traffic with only required information for us + fields: [ + 'metadata.name', + 'kind', + 'metadata.namespace', + 'spec.parent', + 'spec.children', + ], + }); + + const groupsRefs = new Set(); + for (const item of items) { + const groupRef = `group:default/${item.metadata.name.toLocaleLowerCase()}`; + + memo.setNode(groupRef); + for (const child of (item.spec?.children as string[]) ?? []) { + const childEntityRef = `group:default/${child.toLocaleLowerCase()}`; + if ( + memo.getFilterEntityRefs().has(childEntityRef) || + memo.getFilterRelations() === 'relations.hasMember' + ) { + memo.setEdge(groupRef, childEntityRef); + } + } + + if (item.spec?.parent) { + const parentRef = `group:default/${( + item.spec?.parent as string + ).toLocaleLowerCase()}`; + memo.setEdge(parentRef, groupRef); + groupsRefs.add(groupRef); + } + } + + if (groupsRefs.size > 0 && memo.isAcyclic()) { + memo.setFilterEntityRefs(groupsRefs); + memo.setFilterRelations('relations.parentOf'); + await this.findAncestorGroups(memo); + } } } diff --git a/yarn.lock b/yarn.lock index 05b70b67d5..e18702e106 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5832,6 +5832,11 @@ enabled "2.0.x" kuler "^2.0.0" +"@dagrejs/graphlib@^2.1.13": + version "2.1.13" + resolved "https://registry.yarnpkg.com/@dagrejs/graphlib/-/graphlib-2.1.13.tgz#7d0481966e67226d0a864484524f0db933ffc753" + integrity sha512-calbMa7Gcyo+/t23XBaqQqon8LlgE9regey4UVoikoenKBXvUnCUL3s9RP6USCxttfr0XWVICtYUuKMdehKqMw== + "@date-io/core@1.x", "@date-io/core@^1.3.13": version "1.3.13" resolved "https://registry.yarnpkg.com/@date-io/core/-/core-1.3.13.tgz#90c71da493f20204b7a972929cc5c482d078b3fa"