From 100e9a97d0c12f1b19d10b96a34524241844a9ee Mon Sep 17 00:00:00 2001 From: Justin Gasper Date: Thu, 26 Mar 2026 16:08:31 +1100 Subject: [PATCH] Fix for wallet-admin switch for admin vs. engagement approver --- .../access-control.service.spec.ts | 115 ++++++++++++++++++ .../access-control/access-control.service.ts | 39 +++++- 2 files changed, 152 insertions(+), 2 deletions(-) create mode 100644 src/shared/access-control/access-control.service.spec.ts diff --git a/src/shared/access-control/access-control.service.spec.ts b/src/shared/access-control/access-control.service.spec.ts new file mode 100644 index 0000000..9419bc3 --- /dev/null +++ b/src/shared/access-control/access-control.service.spec.ts @@ -0,0 +1,115 @@ +import { Role } from 'src/core/auth/auth.constants'; +import { AccessControlService } from './access-control.service'; +import { RoleAccessProvider } from './role-access.interface'; + +describe('AccessControlService', () => { + let service: AccessControlService; + + beforeEach(() => { + service = new AccessControlService(); + }); + + it('skips engagement approver filters when payment admin role is also present', async () => { + const paymentAdminApplyFilter = jest + .fn() + .mockImplementation((_userId, req: Record) => + Promise.resolve({ + ...req, + admin: true, + }), + ); + const engagementApproverApplyFilter = jest + .fn() + .mockImplementation((_userId, req: Record) => + Promise.resolve({ + ...req, + category: 'ENGAGEMENT_PAYMENT', + }), + ); + const paymentAdminProvider: RoleAccessProvider> = { + roleName: Role.PaymentAdmin, + applyFilter: paymentAdminApplyFilter, + }; + const engagementApproverProvider: RoleAccessProvider< + Record + > = { + roleName: Role.EngagementPaymentApprover, + applyFilter: engagementApproverApplyFilter, + }; + + service.register(paymentAdminProvider); + service.register(engagementApproverProvider); + + const result = await service.applyFilters>( + '88770025', + [Role.PaymentAdmin, Role.EngagementPaymentApprover], + { type: 'PAYMENT' }, + ); + + expect(result).toEqual({ + admin: true, + type: 'PAYMENT', + }); + expect(paymentAdminApplyFilter).toHaveBeenCalledTimes(1); + expect(engagementApproverApplyFilter).not.toHaveBeenCalled(); + }); + + it('applies engagement approver filters when payment admin role is absent', async () => { + const engagementApproverApplyFilter = jest + .fn() + .mockImplementation((_userId, req: Record) => + Promise.resolve({ + ...req, + category: 'ENGAGEMENT_PAYMENT', + }), + ); + const engagementApproverProvider: RoleAccessProvider< + Record + > = { + roleName: Role.EngagementPaymentApprover, + applyFilter: engagementApproverApplyFilter, + }; + + service.register(engagementApproverProvider); + + const result = await service.applyFilters>( + '88770025', + [Role.EngagementPaymentApprover], + { type: 'PAYMENT' }, + ); + + expect(result).toEqual({ + category: 'ENGAGEMENT_PAYMENT', + type: 'PAYMENT', + }); + expect(engagementApproverApplyFilter).toHaveBeenCalledTimes(1); + }); + + it('skips engagement approver resource checks when payment admin role is also present', async () => { + const paymentAdminVerifyAccess = jest.fn().mockResolvedValue(undefined); + const engagementApproverVerifyAccess = jest + .fn() + .mockRejectedValue(new Error('should not be called')); + const paymentAdminProvider: RoleAccessProvider = { + roleName: Role.PaymentAdmin, + verifyAccessToResource: paymentAdminVerifyAccess, + }; + const engagementApproverProvider: RoleAccessProvider = { + roleName: Role.EngagementPaymentApprover, + verifyAccessToResource: engagementApproverVerifyAccess, + }; + + service.register(paymentAdminProvider); + service.register(engagementApproverProvider); + + await expect( + service.verifyAccess('winning-id', '88770025', [ + Role.PaymentAdmin, + Role.EngagementPaymentApprover, + ]), + ).resolves.toBeUndefined(); + + expect(paymentAdminVerifyAccess).toHaveBeenCalledTimes(1); + expect(engagementApproverVerifyAccess).not.toHaveBeenCalled(); + }); +}); diff --git a/src/shared/access-control/access-control.service.ts b/src/shared/access-control/access-control.service.ts index 6e2fb41..7535d6e 100644 --- a/src/shared/access-control/access-control.service.ts +++ b/src/shared/access-control/access-control.service.ts @@ -1,4 +1,5 @@ import { Injectable } from '@nestjs/common'; +import { Role } from 'src/core/auth/auth.constants'; import { RoleAccessProvider } from './role-access.interface'; @Injectable() @@ -9,14 +10,39 @@ export class AccessControlService { this.providers.set(provider.roleName.trim().toLowerCase(), provider); } + private normalizeRoles(roles: string[] = []): Set { + return new Set( + (roles || []).map((role) => role?.trim().toLowerCase()).filter(Boolean), + ); + } + + private shouldSkipProvider( + providerRole: string, + normalizedRoles: Set, + ): boolean { + return ( + providerRole === Role.EngagementPaymentApprover.trim().toLowerCase() && + normalizedRoles.has(Role.PaymentAdmin.trim().toLowerCase()) + ); + } + async applyFilters( userId: string, roles: string[] = [], req: any, ): Promise { let out = { ...req }; + const normalizedRoles = this.normalizeRoles(roles); for (const r of roles || []) { - const p = this.providers.get(r?.trim().toLowerCase()); + const normalizedRole = r?.trim().toLowerCase(); + if ( + !normalizedRole || + this.shouldSkipProvider(normalizedRole, normalizedRoles) + ) { + continue; + } + + const p = this.providers.get(normalizedRole); if (p?.applyFilter) { out = await p.applyFilter(userId, out); } @@ -25,8 +51,17 @@ export class AccessControlService { } async verifyAccess(resourceId: string, userId: string, roles: string[] = []) { + const normalizedRoles = this.normalizeRoles(roles); for (const r of roles || []) { - const p = this.providers.get(r?.trim().toLowerCase()); + const normalizedRole = r?.trim().toLowerCase(); + if ( + !normalizedRole || + this.shouldSkipProvider(normalizedRole, normalizedRoles) + ) { + continue; + } + + const p = this.providers.get(normalizedRole); if (p?.verifyAccessToResource) { await p.verifyAccessToResource(resourceId, userId); }