Skip to content

Commit

Permalink
fix(rbac): use token manager for catalog requests (#866)
Browse files Browse the repository at this point in the history
Signed-off-by: Oleksandr Andriienko <[email protected]>
  • Loading branch information
AndrienkoAleksandr authored Oct 24, 2023
1 parent eac14db commit 8ad3480
Show file tree
Hide file tree
Showing 7 changed files with 97 additions and 39 deletions.
1 change: 1 addition & 0 deletions packages/backend/src/plugins/permissions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,6 @@ export default async function createPlugin(
identity: env.identity,
permissions: env.permissions,
database: env.database,
tokenManager: env.tokenManager,
});
}
40 changes: 34 additions & 6 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 } 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';
Expand Down Expand Up @@ -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<Enforcer> {
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();
Expand All @@ -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);

Expand All @@ -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);

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

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

Expand Down
3 changes: 1 addition & 2 deletions plugins/rbac-backend/src/service/permission-policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,7 @@ export class RBACPermissionPolicy implements PermissionPolicy {
action: string,
): Promise<boolean> => {
if (!identity) {
// Allow access for backend plugins
return true;
return false;
}

const entityRef = identity.userEntityRef;
Expand Down
7 changes: 7 additions & 0 deletions plugins/rbac-backend/src/service/policy-builder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,11 @@ describe('PolicyBuilder', () => {
getExternalBaseUrl: jest.fn(),
};

const tokenManagerMock = {
getToken: jest.fn().mockImplementation(),
authenticate: jest.fn().mockImplementation(),
};

beforeEach(async () => {
jest.clearAllMocks();
});
Expand All @@ -131,6 +136,7 @@ describe('PolicyBuilder', () => {
identity: mockIdentityClient,
permissions: mockPermissionEvaluator,
database: mockDatabaseManager,
tokenManager: tokenManagerMock,
});

expect(FileAdapter).toHaveBeenCalled();
Expand Down Expand Up @@ -167,6 +173,7 @@ describe('PolicyBuilder', () => {
identity: mockIdentityClient,
permissions: mockPermissionEvaluator,
database: mockDatabaseManager,
tokenManager: tokenManagerMock,
});
expect(CasbinDBAdapterFactory).toHaveBeenCalled();
expect(mockEnforcer.loadPolicy).toHaveBeenCalled();
Expand Down
8 changes: 7 additions & 1 deletion plugins/rbac-backend/src/service/policy-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {
PluginDatabaseManager,
PluginEndpointDiscovery,
resolvePackagePath,
TokenManager,
} from '@backstage/backend-common';
import { CatalogClient } from '@backstage/catalog-client';
import { Config } from '@backstage/config';
Expand All @@ -27,6 +28,7 @@ export class PolicyBuilder {
identity: IdentityApi;
permissions: PermissionEvaluator;
database: PluginDatabaseManager;
tokenManager: TokenManager;
}): Promise<Router> {
let adapter;
const databaseEnabled = env.config.getOptionalBoolean(
Expand All @@ -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();
Expand Down
45 changes: 28 additions & 17 deletions plugins/rbac-backend/src/service/role-manager.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { TokenManager } from '@backstage/backend-common';
import { CatalogApi } from '@backstage/catalog-client';
import { Entity } from '@backstage/catalog-model';

Expand All @@ -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', () => {
Expand Down Expand Up @@ -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();
});

Expand Down
32 changes: 19 additions & 13 deletions plugins/rbac-backend/src/service/role-manager.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { TokenManager } from '@backstage/backend-common';
import { CatalogApi } from '@backstage/catalog-client';
import { parseEntityRef } from '@backstage/catalog-model';

Expand Down Expand Up @@ -75,6 +76,7 @@ export class BackstageRoleManager implements RoleManager {
constructor(
private readonly catalogApi: CatalogApi,
private readonly log: Logger,
private readonly tokenManager: TokenManager,
) {}

/**
Expand Down Expand Up @@ -193,20 +195,24 @@ export class BackstageRoleManager implements RoleManager {
}

private async findAncestorGroups(memo: AncestorSearchMemo): Promise<void> {
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<string>();
for (const item of items) {
Expand Down

0 comments on commit 8ad3480

Please sign in to comment.