Skip to content

Commit

Permalink
fix(rbac)!: remove token manager for auth service (#1632)
Browse files Browse the repository at this point in the history
  • Loading branch information
PatAKnight committed May 10, 2024
1 parent 089c2dd commit 2f19655
Show file tree
Hide file tree
Showing 11 changed files with 10 additions and 107 deletions.
1 change: 0 additions & 1 deletion plugins/rbac-backend/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ export default async function createPlugin(
discovery: env.discovery,
identity: env.identity,
permissions: env.permissions,
tokenManager: env.tokenManager,
},
pluginIdProvider,
);
Expand Down
31 changes: 3 additions & 28 deletions plugins/rbac-backend/src/file-permissions/csv.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { TokenManager } from '@backstage/backend-common';
import { mockServices } from '@backstage/backend-test-utils';
import { ConfigReader } from '@backstage/config';

Expand Down Expand Up @@ -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(),
Expand All @@ -114,7 +106,6 @@ async function createEnforcer(
theModel: Model,
adapter: Adapter,
log: Logger,
tokenManager: TokenManager,
): Promise<Enforcer> {
const catalogDBClient = Knex.knex({ client: MockClient });
const enf = await newEnforcer(theModel, adapter);
Expand All @@ -124,7 +115,6 @@ async function createEnforcer(
const rm = new BackstageRoleManager(
catalogApi,
log,
tokenManager,
catalogDBClient,
config,
mockAuthService,
Expand Down Expand Up @@ -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 });

Expand Down Expand Up @@ -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 });

Expand Down Expand Up @@ -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 });

Expand Down
3 changes: 0 additions & 3 deletions plugins/rbac-backend/src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand All @@ -52,7 +51,6 @@ export const rbacPlugin = createBackendPlugin({
discovery,
identity,
permissions,
tokenManager,
auth,
httpAuth,
}) {
Expand All @@ -66,7 +64,6 @@ export const rbacPlugin = createBackendPlugin({
discovery,
identity,
permissions,
tokenManager,
auth,
httpAuth,
},
Expand Down
11 changes: 0 additions & 11 deletions plugins/rbac-backend/src/role-manager/ancestor-search-memo.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -300,7 +290,6 @@ describe('ancestor-search-memo', () => {

const asmUserGraph = new AncestorSearchMemo(
'user:default/adam',
tokenManagerMock,
catalogApiMock,
catalogDBClient,
mockAuthService,
Expand Down
9 changes: 4 additions & 5 deletions plugins/rbac-backend/src/role-manager/ancestor-search-memo.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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;
Expand All @@ -31,15 +29,13 @@ export class AncestorSearchMemo {

constructor(
userEntityRef: string,
tokenManager: TokenManager,
catalogApi: CatalogApi,
catalogDBClient: Knex,
auth: AuthService,
maxDepth?: number,
) {
this.graph = new Graph({ directed: true });
this.userEntityRef = userEntityRef;
this.tokenManager = tokenManager;
this.catalogApi = catalogApi;
this.catalogDBClient = catalogDBClient;
this.auth = auth;
Expand Down Expand Up @@ -115,7 +111,10 @@ export class AncestorSearchMemo {
}

async getUserGroups(): Promise<ASMGroup[]> {
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 },
Expand Down
10 changes: 0 additions & 10 deletions plugins/rbac-backend/src/role-manager/role-manager.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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;
Expand All @@ -41,7 +33,6 @@ describe('BackstageRoleManager', () => {
roleManager = new BackstageRoleManager(
catalogApiMock as CatalogApi,
loggerMock as Logger,
tokenManagerMock as TokenManager,
catalogDBClient,
config,
mockAuthService,
Expand Down Expand Up @@ -1023,7 +1014,6 @@ describe('BackstageRoleManager', () => {
const roleManagerMaxDepth = new BackstageRoleManager(
catalogApiMock as CatalogApi,
loggerMock as Logger,
tokenManagerMock as TokenManager,
catalogDBClient,
config,
mockAuthService,
Expand Down
4 changes: 0 additions & 4 deletions plugins/rbac-backend/src/role-manager/role-manager.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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,
Expand Down Expand Up @@ -108,7 +106,6 @@ export class BackstageRoleManager implements RoleManager {

const memo = new AncestorSearchMemo(
name1,
this.tokenManager,
this.catalogApi,
this.catalogDBClient,
this.auth,
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 0 additions & 8 deletions plugins/rbac-backend/src/service/enforcer-delegate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
};
Expand Down Expand Up @@ -174,7 +167,6 @@ describe('EnforcerDelegate', () => {
const rm = new BackstageRoleManager(
catalogApi,
logger,
tokenManagerMock,
catalogDBClient,
config,
mockAuthService,
Expand Down
27 changes: 3 additions & 24 deletions plugins/rbac-backend/src/service/permission-policy.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -2132,7 +2119,6 @@ async function createEnforcer(
theModel: Model,
adapter: Adapter,
logger: Logger,
tokenManager: TokenManager,
config: ConfigReader,
): Promise<Enforcer> {
const catalogDBClient = Knex.knex({ client: MockClient });
Expand All @@ -2141,7 +2127,6 @@ async function createEnforcer(
const rm = new BackstageRoleManager(
catalogApi,
logger,
tokenManager,
catalogDBClient,
config,
mockAuthService,
Expand All @@ -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);
Expand Down
10 changes: 0 additions & 10 deletions plugins/rbac-backend/src/service/policy-builder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 [];
Expand Down Expand Up @@ -147,7 +142,6 @@ describe('PolicyBuilder', () => {
discovery: mockDiscovery,
identity: mockIdentityClient,
permissions: mockPermissionEvaluator,
tokenManager: tokenManagerMock,
},
backendPluginIDsProviderMock,
);
Expand Down Expand Up @@ -184,7 +178,6 @@ describe('PolicyBuilder', () => {
discovery: mockDiscovery,
identity: mockIdentityClient,
permissions: mockPermissionEvaluator,
tokenManager: tokenManagerMock,
},
backendPluginIDsProviderMock,
);
Expand Down Expand Up @@ -224,7 +217,6 @@ describe('PolicyBuilder', () => {
discovery: mockDiscovery,
identity: mockIdentityClient,
permissions: mockPermissionEvaluator,
tokenManager: tokenManagerMock,
},
pluginIdProvider,
);
Expand Down Expand Up @@ -266,7 +258,6 @@ describe('PolicyBuilder', () => {
discovery: mockDiscovery,
identity: mockIdentityClient,
permissions: mockPermissionEvaluator,
tokenManager: tokenManagerMock,
},
pluginIdProvider,
);
Expand Down Expand Up @@ -306,7 +297,6 @@ describe('PolicyBuilder', () => {
discovery: mockDiscovery,
identity: mockIdentityClient,
permissions: mockPermissionEvaluator,
tokenManager: tokenManagerMock,
});
expect(CasbinDBAdapterFactory).toHaveBeenCalled();
expect(mockEnforcer.loadPolicy).toHaveBeenCalled();
Expand Down

0 comments on commit 2f19655

Please sign in to comment.