diff --git a/plugins/rbac-backend/src/service/policies-rest-api.test.ts b/plugins/rbac-backend/src/service/policies-rest-api.test.ts index 7702223deb..7831260b8f 100644 --- a/plugins/rbac-backend/src/service/policies-rest-api.test.ts +++ b/plugins/rbac-backend/src/service/policies-rest-api.test.ts @@ -179,6 +179,7 @@ describe('REST policies api', () => { expect(result.status).toBe(200); expect(result.body).toEqual({ status: 'Authorized' }); + expect(mockIdentityClient.getIdentity).toHaveBeenCalledTimes(0); }); it('should return a status of Unauthorized', async () => { @@ -287,6 +288,7 @@ describe('REST policies api', () => { }); expect(result.statusCode).toBe(201); + expect(mockIdentityClient.getIdentity).toHaveBeenCalledTimes(1); }); it('should not be created permission policy, because it is has been already present', async () => { @@ -357,6 +359,7 @@ describe('REST policies api', () => { effect: 'allow', }, ]); + expect(mockIdentityClient.getIdentity).toHaveBeenCalledTimes(0); }); it('should be returned policies by user reference not found', async () => { mockEnforcer.getFilteredPolicy = jest @@ -423,6 +426,7 @@ describe('REST policies api', () => { effect: 'allow', }, ]); + expect(mockIdentityClient.getIdentity).toHaveBeenCalledTimes(0); }); it('should be returned list filtered policies', async () => { mockEnforcer.getFilteredPolicy = jest @@ -573,6 +577,7 @@ describe('REST policies api', () => { .send(); expect(result.statusCode).toEqual(204); + expect(mockIdentityClient.getIdentity).toHaveBeenCalledTimes(1); }); }); @@ -925,6 +930,7 @@ describe('REST policies api', () => { }); expect(result.statusCode).toEqual(200); + expect(mockIdentityClient.getIdentity).toHaveBeenCalledTimes(1); }); }); @@ -1012,6 +1018,7 @@ describe('REST policies api', () => { name: 'role:default/rbac_admin', }, ]); + expect(mockIdentityClient.getIdentity).toHaveBeenCalledTimes(0); }); it('should be returned roles by role reference not found', async () => { @@ -1129,6 +1136,7 @@ describe('REST policies api', () => { }); expect(result.statusCode).toBe(201); + expect(mockIdentityClient.getIdentity).toHaveBeenCalledTimes(1); }); it('should not be created role, because it is has been already present', async () => { @@ -1409,6 +1417,7 @@ describe('REST policies api', () => { }); expect(result.statusCode).toEqual(200); + expect(mockIdentityClient.getIdentity).toHaveBeenCalledTimes(1); }); it('should update role where newRole has multiple roles', async () => { @@ -1607,6 +1616,7 @@ describe('REST policies api', () => { .send(); expect(result.statusCode).toEqual(204); + expect(mockIdentityClient.getIdentity).toHaveBeenCalledTimes(1); }); it('should delete a role', async () => { @@ -1713,6 +1723,7 @@ describe('REST policies api', () => { const result = await request(app).get('/conditions').send(); expect(result.statusCode).toBe(200); expect(result.body).toEqual(conditions); + expect(mockIdentityClient.getIdentity).toHaveBeenCalledTimes(0); }); it('should be returned condition decision by pluginId', async () => { @@ -1827,6 +1838,7 @@ describe('REST policies api', () => { const result = await request(app).delete('/conditions/1').send(); expect(result.statusCode).toEqual(204); + expect(mockIdentityClient.getIdentity).toHaveBeenCalledTimes(1); }); it('should fail to delete condition decision by id', async () => { @@ -1899,6 +1911,7 @@ describe('REST policies api', () => { const result = await request(app).get('/conditions/1').send(); expect(result.statusCode).toBe(200); expect(result.body).toEqual(condition); + expect(mockIdentityClient.getIdentity).toHaveBeenCalledTimes(0); }); it('should return return 404', async () => { @@ -1962,6 +1975,7 @@ describe('REST policies api', () => { expect(result.statusCode).toBe(201); expect(result.body).toEqual({ id: 1 }); + expect(mockIdentityClient.getIdentity).toHaveBeenCalledTimes(1); }); }); @@ -2020,6 +2034,7 @@ describe('REST policies api', () => { 1, conditionDecision, ); + expect(mockIdentityClient.getIdentity).toHaveBeenCalledTimes(1); }); }); }); diff --git a/plugins/rbac-backend/src/service/policies-rest-api.ts b/plugins/rbac-backend/src/service/policies-rest-api.ts index 408041e201..1f47e0fe22 100644 --- a/plugins/rbac-backend/src/service/policies-rest-api.ts +++ b/plugins/rbac-backend/src/service/policies-rest-api.ts @@ -16,6 +16,7 @@ import { AuthorizeResult, ConditionalPolicyDecision, PermissionEvaluator, + PolicyDecision, QueryPermissionRequest, } from '@backstage/plugin-permission-common'; import { createPermissionIntegrationRouter } from '@backstage/plugin-permission-node'; @@ -55,21 +56,20 @@ export class PolicesServer { ) {} private async authorize( - identity: IdentityApi, request: Request, - permissionEvaluator: PermissionEvaluator, permission: QueryPermissionRequest, - ) { - const user = await identity.getIdentity({ request }); - if (!user) { - throw new NotAllowedError(); + ): Promise { + if (permission.permission !== policyEntityReadPermission) { + const user = await this.identity.getIdentity({ request }); + if (!user) { + throw new NotAllowedError('User not found'); + } } - const authHeader = request.header('authorization'); const token = getBearerTokenFromAuthorizationHeader(authHeader); const decision = ( - await permissionEvaluator.authorizeConditional([permission], { token }) + await this.permissions.authorizeConditional([permission], { token }) )[0]; return decision; @@ -85,14 +85,9 @@ export class PolicesServer { router.use(permissionsIntegrationRouter); router.get('/', async (request, response) => { - const decision = await this.authorize( - this.identity, - request, - this.permissions, - { - permission: policyEntityReadPermission, - }, - ); + const decision = await this.authorize(request, { + permission: policyEntityReadPermission, + }); if (decision.result === AuthorizeResult.DENY) { throw new NotAllowedError(); // 403 @@ -103,14 +98,9 @@ export class PolicesServer { // Policy CRUD router.get('/policies', async (request, response) => { - const decision = await this.authorize( - this.identity, - request, - this.permissions, - { - permission: policyEntityReadPermission, - }, - ); + const decision = await this.authorize(request, { + permission: policyEntityReadPermission, + }); if (decision.result === AuthorizeResult.DENY) { throw new NotAllowedError(); // 403 @@ -135,14 +125,9 @@ export class PolicesServer { router.get( '/policies/:kind/:namespace/:name', async (request, response) => { - const decision = await this.authorize( - this.identity, - request, - this.permissions, - { - permission: policyEntityReadPermission, - }, - ); + const decision = await this.authorize(request, { + permission: policyEntityReadPermission, + }); if (decision.result === AuthorizeResult.DENY) { throw new NotAllowedError(); // 403 @@ -162,14 +147,9 @@ export class PolicesServer { router.delete( '/policies/:kind/:namespace/:name', async (request, response) => { - const decision = await this.authorize( - this.identity, - request, - this.permissions, - { - permission: policyEntityDeletePermission, - }, - ); + const decision = await this.authorize(request, { + permission: policyEntityDeletePermission, + }); if (decision.result === AuthorizeResult.DENY) { throw new NotAllowedError(); // 403 @@ -203,14 +183,9 @@ export class PolicesServer { ); router.post('/policies', async (request, response) => { - const decision = await this.authorize( - this.identity, - request, - this.permissions, - { - permission: policyEntityCreatePermission, - }, - ); + const decision = await this.authorize(request, { + permission: policyEntityCreatePermission, + }); if (decision.result === AuthorizeResult.DENY) { throw new NotAllowedError(); // 403 @@ -240,14 +215,9 @@ export class PolicesServer { router.put( '/policies/:kind/:namespace/:name', async (request, response) => { - const decision = await this.authorize( - this.identity, - request, - this.permissions, - { - permission: policyEntityUpdatePermission, - }, - ); + const decision = await this.authorize(request, { + permission: policyEntityUpdatePermission, + }); if (decision.result === AuthorizeResult.DENY) { throw new NotAllowedError(); // 403 @@ -314,14 +284,9 @@ export class PolicesServer { // Role CRUD router.get('/roles', async (request, response) => { - const decision = await this.authorize( - this.identity, - request, - this.permissions, - { - permission: policyEntityReadPermission, - }, - ); + const decision = await this.authorize(request, { + permission: policyEntityReadPermission, + }); if (decision.result === AuthorizeResult.DENY) { throw new NotAllowedError(); // 403 @@ -333,14 +298,9 @@ export class PolicesServer { }); router.get('/roles/:kind/:namespace/:name', async (request, response) => { - const decision = await this.authorize( - this.identity, - request, - this.permissions, - { - permission: policyEntityReadPermission, - }, - ); + const decision = await this.authorize(request, { + permission: policyEntityReadPermission, + }); if (decision.result === AuthorizeResult.DENY) { throw new NotAllowedError(); // 403 @@ -360,14 +320,9 @@ export class PolicesServer { }); router.post('/roles', async (request, response) => { - const decision = await this.authorize( - this.identity, - request, - this.permissions, - { - permission: policyEntityCreatePermission, - }, - ); + const decision = await this.authorize(request, { + permission: policyEntityCreatePermission, + }); if (decision.result === AuthorizeResult.DENY) { throw new NotAllowedError(); // 403 @@ -397,14 +352,9 @@ export class PolicesServer { }); router.put('/roles/:kind/:namespace/:name', async (request, response) => { - const decision = await this.authorize( - this.identity, - request, - this.permissions, - { - permission: policyEntityUpdatePermission, - }, - ); + const decision = await this.authorize(request, { + permission: policyEntityUpdatePermission, + }); if (decision.result === AuthorizeResult.DENY) { throw new NotAllowedError(); // 403 @@ -481,14 +431,9 @@ export class PolicesServer { '/roles/:kind/:namespace/:name', async (request, response) => { let roles = []; - const decision = await this.authorize( - this.identity, - request, - this.permissions, - { - permission: policyEntityDeletePermission, - }, - ); + const decision = await this.authorize(request, { + permission: policyEntityDeletePermission, + }); if (decision.result === AuthorizeResult.DENY) { throw new NotAllowedError(); // 403 @@ -524,14 +469,9 @@ export class PolicesServer { ); router.get('/conditions', async (req, resp) => { - const decision = await this.authorize( - this.identity, - req, - this.permissions, - { - permission: policyEntityReadPermission, - }, - ); + const decision = await this.authorize(req, { + permission: policyEntityReadPermission, + }); if (decision.result === AuthorizeResult.DENY) { throw new NotAllowedError(); // 403 @@ -548,14 +488,9 @@ export class PolicesServer { }); router.post('/conditions', async (req, resp) => { - const decision = await this.authorize( - this.identity, - req, - this.permissions, - { - permission: policyEntityCreatePermission, - }, - ); + const decision = await this.authorize(req, { + permission: policyEntityCreatePermission, + }); if (decision.result === AuthorizeResult.DENY) { throw new NotAllowedError(); // 403 @@ -570,14 +505,9 @@ export class PolicesServer { }); router.get('/conditions/:id', async (req, resp) => { - const decision = await this.authorize( - this.identity, - req, - this.permissions, - { - permission: policyEntityReadPermission, - }, - ); + const decision = await this.authorize(req, { + permission: policyEntityReadPermission, + }); if (decision.result === AuthorizeResult.DENY) { throw new NotAllowedError(); // 403 @@ -597,14 +527,9 @@ export class PolicesServer { }); router.delete('/conditions/:id', async (req, resp) => { - const decision = await this.authorize( - this.identity, - req, - this.permissions, - { - permission: policyEntityDeletePermission, - }, - ); + const decision = await this.authorize(req, { + permission: policyEntityDeletePermission, + }); if (decision.result === AuthorizeResult.DENY) { throw new NotAllowedError(); // 403 @@ -620,14 +545,9 @@ export class PolicesServer { }); router.put('/conditions/:id', async (req, resp) => { - const decision = await this.authorize( - this.identity, - req, - this.permissions, - { - permission: policyEntityUpdatePermission, - }, - ); + const decision = await this.authorize(req, { + permission: policyEntityUpdatePermission, + }); if (decision.result === AuthorizeResult.DENY) { throw new NotAllowedError(); // 403