From 2f196556cffc61c83239721b1cd51d6a2c64eee7 Mon Sep 17 00:00:00 2001 From: Patrick Knight Date: Fri, 10 May 2024 10:54:42 -0400 Subject: [PATCH] fix(rbac)!: remove token manager for auth service (#1632) --- plugins/rbac-backend/README.md | 1 - .../src/file-permissions/csv.test.ts | 31 ++----------------- plugins/rbac-backend/src/plugin.ts | 3 -- .../role-manager/ancestor-search-memo.test.ts | 11 ------- .../src/role-manager/ancestor-search-memo.ts | 9 +++--- .../src/role-manager/role-manager.test.ts | 10 ------ .../src/role-manager/role-manager.ts | 4 --- .../src/service/enforcer-delegate.test.ts | 8 ----- .../src/service/permission-policy.test.ts | 27 ++-------------- .../src/service/policy-builder.test.ts | 10 ------ .../src/service/policy-builder.ts | 3 -- 11 files changed, 10 insertions(+), 107 deletions(-) diff --git a/plugins/rbac-backend/README.md b/plugins/rbac-backend/README.md index 233692a7d2..88652dc642 100644 --- a/plugins/rbac-backend/README.md +++ b/plugins/rbac-backend/README.md @@ -62,7 +62,6 @@ export default async function createPlugin( discovery: env.discovery, identity: env.identity, permissions: env.permissions, - tokenManager: env.tokenManager, }, pluginIdProvider, ); diff --git a/plugins/rbac-backend/src/file-permissions/csv.test.ts b/plugins/rbac-backend/src/file-permissions/csv.test.ts index 384beeaccc..21fdb30d19 100644 --- a/plugins/rbac-backend/src/file-permissions/csv.test.ts +++ b/plugins/rbac-backend/src/file-permissions/csv.test.ts @@ -1,4 +1,3 @@ -import { TokenManager } from '@backstage/backend-common'; import { mockServices } from '@backstage/backend-test-utils'; import { ConfigReader } from '@backstage/config'; @@ -96,13 +95,6 @@ const policyMetadataStorageMock: PolicyMetadataStorage = { removePolicyMetadata: jest.fn().mockImplementation(), }; -const tokenManagerMock = { - getToken: jest.fn().mockImplementation(async () => { - return Promise.resolve({ token: 'some-token' }); - }), - authenticate: jest.fn().mockImplementation(), -}; - const loggerMock: any = { warn: jest.fn().mockImplementation(), debug: jest.fn().mockImplementation(), @@ -114,7 +106,6 @@ async function createEnforcer( theModel: Model, adapter: Adapter, log: Logger, - tokenManager: TokenManager, ): Promise { const catalogDBClient = Knex.knex({ client: MockClient }); const enf = await newEnforcer(theModel, adapter); @@ -124,7 +115,6 @@ async function createEnforcer( const rm = new BackstageRoleManager( catalogApi, log, - tokenManager, catalogDBClient, config, mockAuthService, @@ -176,12 +166,7 @@ describe('CSV file', () => { const adapter = new FileAdapter(csvPermFile); const stringModel = newModelFromString(MODEL); - enf = await createEnforcer( - stringModel, - adapter, - loggerMock, - tokenManagerMock, - ); + enf = await createEnforcer(stringModel, adapter, loggerMock); knex = Knex.knex({ client: MockClient }); @@ -557,12 +542,7 @@ describe('CSV file', () => { const adapter = new FileAdapter(csvPermFile); const stringModel = newModelFromString(MODEL); - enf = await createEnforcer( - stringModel, - adapter, - loggerMock, - tokenManagerMock, - ); + enf = await createEnforcer(stringModel, adapter, loggerMock); const knex = Knex.knex({ client: MockClient }); @@ -837,12 +817,7 @@ describe('CSV file', () => { ); const stringModel = newModelFromString(MODEL); - enf = await createEnforcer( - stringModel, - adapter, - loggerMock, - tokenManagerMock, - ); + enf = await createEnforcer(stringModel, adapter, loggerMock); const knex = Knex.knex({ client: MockClient }); diff --git a/plugins/rbac-backend/src/plugin.ts b/plugins/rbac-backend/src/plugin.ts index d4015ed30e..d07bec768b 100644 --- a/plugins/rbac-backend/src/plugin.ts +++ b/plugins/rbac-backend/src/plugin.ts @@ -41,7 +41,6 @@ export const rbacPlugin = createBackendPlugin({ discovery: coreServices.discovery, identity: coreServices.identity, permissions: coreServices.permissions, - tokenManager: coreServices.tokenManager, auth: coreServices.auth, httpAuth: coreServices.httpAuth, }, @@ -52,7 +51,6 @@ export const rbacPlugin = createBackendPlugin({ discovery, identity, permissions, - tokenManager, auth, httpAuth, }) { @@ -66,7 +64,6 @@ export const rbacPlugin = createBackendPlugin({ discovery, identity, permissions, - tokenManager, auth, httpAuth, }, diff --git a/plugins/rbac-backend/src/role-manager/ancestor-search-memo.test.ts b/plugins/rbac-backend/src/role-manager/ancestor-search-memo.test.ts index d146ad2757..e5d0080226 100644 --- a/plugins/rbac-backend/src/role-manager/ancestor-search-memo.test.ts +++ b/plugins/rbac-backend/src/role-manager/ancestor-search-memo.test.ts @@ -66,19 +66,11 @@ describe('ancestor-search-memo', () => { const catalogDBClient = Knex.knex({ client: MockClient }); - const tokenManagerMock = { - getToken: jest.fn().mockImplementation(async () => { - return Promise.resolve({ token: 'some-token' }); - }), - authenticate: jest.fn().mockImplementation(), - }; - let asm: AncestorSearchMemo; beforeEach(() => { asm = new AncestorSearchMemo( 'user:default/adam', - tokenManagerMock, catalogApiMock, catalogDBClient, mockAuthService, @@ -201,7 +193,6 @@ describe('ancestor-search-memo', () => { it('should build the graph but stop based on the maxDepth', async () => { const asmMaxDepth = new AncestorSearchMemo( 'user:default/adam', - tokenManagerMock, catalogApiMock, catalogDBClient, mockAuthService, @@ -268,7 +259,6 @@ describe('ancestor-search-memo', () => { it('should build the graph but stop based on the maxDepth', async () => { const asmMaxDepth = new AncestorSearchMemo( 'user:default/adam', - tokenManagerMock, catalogApiMock, catalogDBClient, mockAuthService, @@ -300,7 +290,6 @@ describe('ancestor-search-memo', () => { const asmUserGraph = new AncestorSearchMemo( 'user:default/adam', - tokenManagerMock, catalogApiMock, catalogDBClient, mockAuthService, diff --git a/plugins/rbac-backend/src/role-manager/ancestor-search-memo.ts b/plugins/rbac-backend/src/role-manager/ancestor-search-memo.ts index 874db8be69..24f193ee25 100644 --- a/plugins/rbac-backend/src/role-manager/ancestor-search-memo.ts +++ b/plugins/rbac-backend/src/role-manager/ancestor-search-memo.ts @@ -1,4 +1,3 @@ -import { TokenManager } from '@backstage/backend-common'; import { AuthService } from '@backstage/backend-plugin-api'; import { CatalogApi } from '@backstage/catalog-client'; import { Entity } from '@backstage/catalog-model'; @@ -21,7 +20,6 @@ export type ASMGroup = Relation | Entity; export class AncestorSearchMemo { private graph: Graph; - private tokenManager: TokenManager; private catalogApi: CatalogApi; private catalogDBClient: Knex; private auth: AuthService; @@ -31,7 +29,6 @@ export class AncestorSearchMemo { constructor( userEntityRef: string, - tokenManager: TokenManager, catalogApi: CatalogApi, catalogDBClient: Knex, auth: AuthService, @@ -39,7 +36,6 @@ export class AncestorSearchMemo { ) { this.graph = new Graph({ directed: true }); this.userEntityRef = userEntityRef; - this.tokenManager = tokenManager; this.catalogApi = catalogApi; this.catalogDBClient = catalogDBClient; this.auth = auth; @@ -115,7 +111,10 @@ export class AncestorSearchMemo { } async getUserGroups(): Promise { - const { token } = await this.tokenManager.getToken(); + const { token } = await this.auth.getPluginRequestToken({ + onBehalfOf: await this.auth.getOwnServiceCredentials(), + targetPluginId: 'catalog', + }); const { items } = await this.catalogApi.getEntities( { filter: { kind: 'Group', 'relations.hasMember': this.userEntityRef }, diff --git a/plugins/rbac-backend/src/role-manager/role-manager.test.ts b/plugins/rbac-backend/src/role-manager/role-manager.test.ts index d302bbb5e0..49a76271f2 100644 --- a/plugins/rbac-backend/src/role-manager/role-manager.test.ts +++ b/plugins/rbac-backend/src/role-manager/role-manager.test.ts @@ -1,4 +1,3 @@ -import { TokenManager } from '@backstage/backend-common'; import { mockServices } from '@backstage/backend-test-utils'; import { CatalogApi } from '@backstage/catalog-client'; import { Entity } from '@backstage/catalog-model'; @@ -21,13 +20,6 @@ describe('BackstageRoleManager', () => { debug: jest.fn().mockImplementation(), }; - const tokenManagerMock = { - getToken: jest.fn().mockImplementation(async () => { - return Promise.resolve({ token: 'some-token' }); - }), - authenticate: jest.fn().mockImplementation(), - }; - const mockAuthService = mockServices.auth(); let roleManager: BackstageRoleManager; @@ -41,7 +33,6 @@ describe('BackstageRoleManager', () => { roleManager = new BackstageRoleManager( catalogApiMock as CatalogApi, loggerMock as Logger, - tokenManagerMock as TokenManager, catalogDBClient, config, mockAuthService, @@ -1023,7 +1014,6 @@ describe('BackstageRoleManager', () => { const roleManagerMaxDepth = new BackstageRoleManager( catalogApiMock as CatalogApi, loggerMock as Logger, - tokenManagerMock as TokenManager, catalogDBClient, config, mockAuthService, diff --git a/plugins/rbac-backend/src/role-manager/role-manager.ts b/plugins/rbac-backend/src/role-manager/role-manager.ts index 95256f79ed..f1fea91f58 100644 --- a/plugins/rbac-backend/src/role-manager/role-manager.ts +++ b/plugins/rbac-backend/src/role-manager/role-manager.ts @@ -1,4 +1,3 @@ -import { TokenManager } from '@backstage/backend-common'; import { AuthService } from '@backstage/backend-plugin-api'; import { CatalogApi } from '@backstage/catalog-client'; import { parseEntityRef } from '@backstage/catalog-model'; @@ -17,7 +16,6 @@ export class BackstageRoleManager implements RoleManager { constructor( private readonly catalogApi: CatalogApi, private readonly log: Logger, - private readonly tokenManager: TokenManager, private readonly catalogDBClient: Knex, private readonly config: Config, private readonly auth: AuthService, @@ -108,7 +106,6 @@ export class BackstageRoleManager implements RoleManager { const memo = new AncestorSearchMemo( name1, - this.tokenManager, this.catalogApi, this.catalogDBClient, this.auth, @@ -186,7 +183,6 @@ export class BackstageRoleManager implements RoleManager { if (kind === 'user') { const memo = new AncestorSearchMemo( name, - this.tokenManager, this.catalogApi, this.catalogDBClient, this.auth, diff --git a/plugins/rbac-backend/src/service/enforcer-delegate.test.ts b/plugins/rbac-backend/src/service/enforcer-delegate.test.ts index 6801dfa2aa..2a187673d8 100644 --- a/plugins/rbac-backend/src/service/enforcer-delegate.test.ts +++ b/plugins/rbac-backend/src/service/enforcer-delegate.test.ts @@ -64,13 +64,6 @@ const policyMetadataStorageMock: PolicyMetadataStorage = { removePolicyMetadata: jest.fn().mockImplementation(), }; -const tokenManagerMock = { - getToken: jest.fn().mockImplementation(async () => { - return Promise.resolve({ token: 'some-token' }); - }), - authenticate: jest.fn().mockImplementation(), -}; - const dbManagerMock: DatabaseService = { getClient: jest.fn().mockImplementation(), }; @@ -174,7 +167,6 @@ describe('EnforcerDelegate', () => { const rm = new BackstageRoleManager( catalogApi, logger, - tokenManagerMock, catalogDBClient, config, mockAuthService, diff --git a/plugins/rbac-backend/src/service/permission-policy.test.ts b/plugins/rbac-backend/src/service/permission-policy.test.ts index 9a94eb709e..db054b1200 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, TokenManager } from '@backstage/backend-common'; +import { getVoidLogger } from '@backstage/backend-common'; import { DatabaseService } from '@backstage/backend-plugin-api'; import { mockServices } from '@backstage/backend-test-utils'; import { Entity } from '@backstage/catalog-model'; @@ -61,13 +61,6 @@ const catalogApi = { getLocationByEntity: jest.fn().mockImplementation(), }; -const tokenManagerMock = { - getToken: jest.fn().mockImplementation(async () => { - return Promise.resolve({ token: 'some-token' }); - }), - authenticate: jest.fn().mockImplementation(), -}; - const conditionalStorage: ConditionalStorage = { filterConditions: jest.fn().mockImplementation(() => []), createCondition: jest.fn().mockImplementation(), @@ -1805,13 +1798,7 @@ describe('Policy checks for conditional policies', () => { const config = newConfigReader(); const theModel = newModelFromString(MODEL); const logger = getVoidLogger(); - const enf = await createEnforcer( - theModel, - adapter, - logger, - tokenManagerMock, - config, - ); + const enf = await createEnforcer(theModel, adapter, logger, config); const enfDelegate = new EnforcerDelegate( enf, @@ -2132,7 +2119,6 @@ async function createEnforcer( theModel: Model, adapter: Adapter, logger: Logger, - tokenManager: TokenManager, config: ConfigReader, ): Promise { const catalogDBClient = Knex.knex({ client: MockClient }); @@ -2141,7 +2127,6 @@ async function createEnforcer( const rm = new BackstageRoleManager( catalogApi, logger, - tokenManager, catalogDBClient, config, mockAuthService, @@ -2163,13 +2148,7 @@ async function newEnforcerDelegate( const theModel = newModelFromString(MODEL); const logger = getVoidLogger(); - const enf = await createEnforcer( - theModel, - adapter, - logger, - tokenManagerMock, - config, - ); + const enf = await createEnforcer(theModel, adapter, logger, config); if (storedPolicies) { await enf.addPolicies(storedPolicies); diff --git a/plugins/rbac-backend/src/service/policy-builder.test.ts b/plugins/rbac-backend/src/service/policy-builder.test.ts index 1e86e090a2..4dcfe9c68e 100644 --- a/plugins/rbac-backend/src/service/policy-builder.test.ts +++ b/plugins/rbac-backend/src/service/policy-builder.test.ts @@ -107,11 +107,6 @@ describe('PolicyBuilder', () => { getExternalBaseUrl: jest.fn(), }; - const tokenManagerMock = { - getToken: jest.fn().mockImplementation(), - authenticate: jest.fn().mockImplementation(), - }; - const backendPluginIDsProviderMock = { getPluginIds: jest.fn().mockImplementation(() => { return []; @@ -147,7 +142,6 @@ describe('PolicyBuilder', () => { discovery: mockDiscovery, identity: mockIdentityClient, permissions: mockPermissionEvaluator, - tokenManager: tokenManagerMock, }, backendPluginIDsProviderMock, ); @@ -184,7 +178,6 @@ describe('PolicyBuilder', () => { discovery: mockDiscovery, identity: mockIdentityClient, permissions: mockPermissionEvaluator, - tokenManager: tokenManagerMock, }, backendPluginIDsProviderMock, ); @@ -224,7 +217,6 @@ describe('PolicyBuilder', () => { discovery: mockDiscovery, identity: mockIdentityClient, permissions: mockPermissionEvaluator, - tokenManager: tokenManagerMock, }, pluginIdProvider, ); @@ -266,7 +258,6 @@ describe('PolicyBuilder', () => { discovery: mockDiscovery, identity: mockIdentityClient, permissions: mockPermissionEvaluator, - tokenManager: tokenManagerMock, }, pluginIdProvider, ); @@ -306,7 +297,6 @@ describe('PolicyBuilder', () => { discovery: mockDiscovery, identity: mockIdentityClient, permissions: mockPermissionEvaluator, - 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 272117996f..d43de012e6 100644 --- a/plugins/rbac-backend/src/service/policy-builder.ts +++ b/plugins/rbac-backend/src/service/policy-builder.ts @@ -2,7 +2,6 @@ import { createLegacyAuthAdapters, DatabaseManager, PluginEndpointDiscovery, - TokenManager, } from '@backstage/backend-common'; import { AuthService, HttpAuthService } from '@backstage/backend-plugin-api'; import { CatalogClient } from '@backstage/catalog-client'; @@ -36,7 +35,6 @@ export class PolicyBuilder { discovery: PluginEndpointDiscovery; identity: IdentityApi; permissions: PermissionEvaluator; - tokenManager: TokenManager; auth?: AuthService; httpAuth?: HttpAuthService; }, @@ -73,7 +71,6 @@ export class PolicyBuilder { const rm = new BackstageRoleManager( catalogClient, env.logger, - env.tokenManager, catalogDBClient, env.config, auth,