From 8ad348029cec4eabf605c7065e76a5305be3cac8 Mon Sep 17 00:00:00 2001 From: Oleksandr Andriienko Date: Tue, 24 Oct 2023 05:42:30 +0300 Subject: [PATCH] fix(rbac): use token manager for catalog requests (#866) Signed-off-by: Oleksandr Andriienko --- packages/backend/src/plugins/permissions.ts | 1 + .../src/service/permission-policy.test.ts | 40 ++++++++++++++--- .../src/service/permission-policy.ts | 3 +- .../src/service/policy-builder.test.ts | 7 +++ .../src/service/policy-builder.ts | 8 +++- .../src/service/role-manager.test.ts | 45 ++++++++++++------- .../rbac-backend/src/service/role-manager.ts | 32 +++++++------ 7 files changed, 97 insertions(+), 39 deletions(-) diff --git a/packages/backend/src/plugins/permissions.ts b/packages/backend/src/plugins/permissions.ts index 19dd4b387d..03f331fdf8 100644 --- a/packages/backend/src/plugins/permissions.ts +++ b/packages/backend/src/plugins/permissions.ts @@ -14,5 +14,6 @@ export default async function createPlugin( identity: env.identity, permissions: env.permissions, database: env.database, + tokenManager: env.tokenManager, }); } diff --git a/plugins/rbac-backend/src/service/permission-policy.test.ts b/plugins/rbac-backend/src/service/permission-policy.test.ts index 8216cdaf98..6543af152d 100644 --- a/plugins/rbac-backend/src/service/permission-policy.test.ts +++ b/plugins/rbac-backend/src/service/permission-policy.test.ts @@ -1,4 +1,4 @@ -import { getVoidLogger } from '@backstage/backend-common'; +import { getVoidLogger, TokenManager } from '@backstage/backend-common'; import { Entity } from '@backstage/catalog-model'; import { ConfigReader } from '@backstage/config'; import { BackstageIdentityResponse } from '@backstage/plugin-auth-node'; @@ -40,14 +40,22 @@ const catalogApi = { validateEntity: jest.fn().mockImplementation(), }; +const tokenManagerMock = { + getToken: jest.fn().mockImplementation(async () => { + return Promise.resolve({ token: 'some-token' }); + }), + authenticate: jest.fn().mockImplementation(), +}; + async function createEnforcer( theModel: Model, adapter: Adapter, logger: Logger, + tokenManager: TokenManager, ): Promise { const enf = await newEnforcer(theModel, adapter); - const rm = new BackstageRoleManager(catalogApi, logger); + const rm = new BackstageRoleManager(catalogApi, logger, tokenManager); enf.setRoleManager(rm); enf.enableAutoBuildRoleLinks(false); await enf.buildRoleLinks(); @@ -63,7 +71,12 @@ describe('RBACPermissionPolicy Tests', () => { const config = newConfigReader(); const theModel = newModelFromString(MODEL); const logger = getVoidLogger(); - const enf = await createEnforcer(theModel, adapter, logger); + const enf = await createEnforcer( + theModel, + adapter, + logger, + tokenManagerMock, + ); const policy = await RBACPermissionPolicy.build(logger, config, enf); @@ -89,7 +102,12 @@ describe('RBACPermissionPolicy Tests', () => { }); const theModel = newModelFromString(MODEL); const logger = getVoidLogger(); - const enf = await createEnforcer(theModel, adapter, logger); + const enf = await createEnforcer( + theModel, + adapter, + logger, + tokenManagerMock, + ); policy = await RBACPermissionPolicy.build(logger, config, enf); @@ -150,7 +168,12 @@ describe('RBACPermissionPolicy Tests', () => { const config = newConfigReader(); const theModel = newModelFromString(MODEL); const logger = getVoidLogger(); - const enf = await createEnforcer(theModel, adapter, logger); + const enf = await createEnforcer( + theModel, + adapter, + logger, + tokenManagerMock, + ); policy = await RBACPermissionPolicy.build(logger, config, enf); @@ -356,7 +379,12 @@ describe('Policy checks for users and groups', () => { const config = newConfigReader(); const theModel = newModelFromString(MODEL); const logger = getVoidLogger(); - const enf = await createEnforcer(theModel, adapter, logger); + const enf = await createEnforcer( + theModel, + adapter, + logger, + tokenManagerMock, + ); policy = await RBACPermissionPolicy.build(logger, config, enf); diff --git a/plugins/rbac-backend/src/service/permission-policy.ts b/plugins/rbac-backend/src/service/permission-policy.ts index dcdeba2a3d..5e7388c616 100644 --- a/plugins/rbac-backend/src/service/permission-policy.ts +++ b/plugins/rbac-backend/src/service/permission-policy.ts @@ -141,8 +141,7 @@ export class RBACPermissionPolicy implements PermissionPolicy { action: string, ): Promise => { if (!identity) { - // Allow access for backend plugins - return true; + return false; } const entityRef = identity.userEntityRef; diff --git a/plugins/rbac-backend/src/service/policy-builder.test.ts b/plugins/rbac-backend/src/service/policy-builder.test.ts index 548a5af09c..782bae01d8 100644 --- a/plugins/rbac-backend/src/service/policy-builder.test.ts +++ b/plugins/rbac-backend/src/service/policy-builder.test.ts @@ -109,6 +109,11 @@ describe('PolicyBuilder', () => { getExternalBaseUrl: jest.fn(), }; + const tokenManagerMock = { + getToken: jest.fn().mockImplementation(), + authenticate: jest.fn().mockImplementation(), + }; + beforeEach(async () => { jest.clearAllMocks(); }); @@ -131,6 +136,7 @@ describe('PolicyBuilder', () => { identity: mockIdentityClient, permissions: mockPermissionEvaluator, database: mockDatabaseManager, + tokenManager: tokenManagerMock, }); expect(FileAdapter).toHaveBeenCalled(); @@ -167,6 +173,7 @@ describe('PolicyBuilder', () => { identity: mockIdentityClient, permissions: mockPermissionEvaluator, database: mockDatabaseManager, + tokenManager: tokenManagerMock, }); expect(CasbinDBAdapterFactory).toHaveBeenCalled(); expect(mockEnforcer.loadPolicy).toHaveBeenCalled(); diff --git a/plugins/rbac-backend/src/service/policy-builder.ts b/plugins/rbac-backend/src/service/policy-builder.ts index 79047dffb0..63bb360c3e 100644 --- a/plugins/rbac-backend/src/service/policy-builder.ts +++ b/plugins/rbac-backend/src/service/policy-builder.ts @@ -2,6 +2,7 @@ import { PluginDatabaseManager, PluginEndpointDiscovery, resolvePackagePath, + TokenManager, } from '@backstage/backend-common'; import { CatalogClient } from '@backstage/catalog-client'; import { Config } from '@backstage/config'; @@ -27,6 +28,7 @@ export class PolicyBuilder { identity: IdentityApi; permissions: PermissionEvaluator; database: PluginDatabaseManager; + tokenManager: TokenManager; }): Promise { let adapter; const databaseEnabled = env.config.getOptionalBoolean( @@ -52,7 +54,11 @@ export class PolicyBuilder { enf.enableAutoSave(true); const catalogClient = new CatalogClient({ discoveryApi: env.discovery }); - const rm = new BackstageRoleManager(catalogClient, env.logger); + const rm = new BackstageRoleManager( + catalogClient, + env.logger, + env.tokenManager, + ); enf.setRoleManager(rm); enf.enableAutoBuildRoleLinks(false); await enf.buildRoleLinks(); diff --git a/plugins/rbac-backend/src/service/role-manager.test.ts b/plugins/rbac-backend/src/service/role-manager.test.ts index 7bfa5f7e9d..eb357f77c5 100644 --- a/plugins/rbac-backend/src/service/role-manager.test.ts +++ b/plugins/rbac-backend/src/service/role-manager.test.ts @@ -1,3 +1,4 @@ +import { TokenManager } from '@backstage/backend-common'; import { CatalogApi } from '@backstage/catalog-client'; import { Entity } from '@backstage/catalog-model'; @@ -17,13 +18,20 @@ describe('BackstageRoleManager', () => { debug: jest.fn().mockImplementation(), }; - const roleManager = new BackstageRoleManager( - catalogApiMock as CatalogApi, - loggerMock as Logger, - ); + const tokenManagerMock = { + getToken: jest.fn().mockImplementation(async () => { + return Promise.resolve({ token: 'some-token' }); + }), + authenticate: jest.fn().mockImplementation(), + }; + let roleManager: BackstageRoleManager; beforeEach(() => { - jest.clearAllMocks(); + roleManager = new BackstageRoleManager( + catalogApiMock as CatalogApi, + loggerMock as Logger, + tokenManagerMock as TokenManager, + ); }); describe('unimplemented methods', () => { @@ -96,19 +104,22 @@ describe('BackstageRoleManager', () => { 'user:default/mike', 'group:default/somegroup', ); - expect(catalogApiMock.getEntities).toHaveBeenCalledWith({ - filter: { - kind: 'Group', - 'relations.hasMember': ['user:default/mike'], + expect(catalogApiMock.getEntities).toHaveBeenCalledWith( + { + filter: { + kind: 'Group', + 'relations.hasMember': ['user:default/mike'], + }, + fields: [ + 'metadata.name', + 'kind', + 'metadata.namespace', + 'spec.parent', + 'spec.children', + ], }, - fields: [ - 'metadata.name', - 'kind', - 'metadata.namespace', - 'spec.parent', - 'spec.children', - ], - }); + { token: 'some-token' }, + ); expect(result).toBeFalsy(); }); diff --git a/plugins/rbac-backend/src/service/role-manager.ts b/plugins/rbac-backend/src/service/role-manager.ts index ebc9a8199b..cad710cfe5 100644 --- a/plugins/rbac-backend/src/service/role-manager.ts +++ b/plugins/rbac-backend/src/service/role-manager.ts @@ -1,3 +1,4 @@ +import { TokenManager } from '@backstage/backend-common'; import { CatalogApi } from '@backstage/catalog-client'; import { parseEntityRef } from '@backstage/catalog-model'; @@ -75,6 +76,7 @@ export class BackstageRoleManager implements RoleManager { constructor( private readonly catalogApi: CatalogApi, private readonly log: Logger, + private readonly tokenManager: TokenManager, ) {} /** @@ -193,20 +195,24 @@ export class BackstageRoleManager implements RoleManager { } private async findAncestorGroups(memo: AncestorSearchMemo): Promise { - const { items } = await this.catalogApi.getEntities({ - filter: { - kind: 'Group', - [memo.getFilterRelations()]: Array.from(memo.getFilterEntityRefs()), + const { token } = await this.tokenManager.getToken(); + 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', + ], }, - // Save traffic with only required information for us - fields: [ - 'metadata.name', - 'kind', - 'metadata.namespace', - 'spec.parent', - 'spec.children', - ], - }); + { token }, + ); const groupsRefs = new Set(); for (const item of items) {