Merge pull request #149 from topcoder-platform/dev

Fix for wallet-admin switch for admin vs. engagement approver

Author: jmgasper

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<string, unknown>) =>
+        Promise.resolve({
+          ...req,
+          admin: true,
+        }),
+      );
+    const engagementApproverApplyFilter = jest
+      .fn()
+      .mockImplementation((_userId, req: Record<string, unknown>) =>
+        Promise.resolve({
+          ...req,
+          category: 'ENGAGEMENT_PAYMENT',
+        }),
+      );
+    const paymentAdminProvider: RoleAccessProvider<Record<string, unknown>> = {
+      roleName: Role.PaymentAdmin,
+      applyFilter: paymentAdminApplyFilter,
+    };
+    const engagementApproverProvider: RoleAccessProvider<
+      Record<string, unknown>
+    > = {
+      roleName: Role.EngagementPaymentApprover,
+      applyFilter: engagementApproverApplyFilter,
+    };
+
+    service.register(paymentAdminProvider);
+    service.register(engagementApproverProvider);
+
+    const result = await service.applyFilters<Record<string, unknown>>(
+      '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<string, unknown>) =>
+        Promise.resolve({
+          ...req,
+          category: 'ENGAGEMENT_PAYMENT',
+        }),
+      );
+    const engagementApproverProvider: RoleAccessProvider<
+      Record<string, unknown>
+    > = {
+      roleName: Role.EngagementPaymentApprover,
+      applyFilter: engagementApproverApplyFilter,
+    };
+
+    service.register(engagementApproverProvider);
+
+    const result = await service.applyFilters<Record<string, unknown>>(
+      '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();
+  });
+});

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<string> {
+    return new Set(
+      (roles || []).map((role) => role?.trim().toLowerCase()).filter(Boolean),
+    );
+  }
+
+  private shouldSkipProvider(
+    providerRole: string,
+    normalizedRoles: Set<string>,
+  ): boolean {
+    return (
+      providerRole === Role.EngagementPaymentApprover.trim().toLowerCase() &&
+      normalizedRoles.has(Role.PaymentAdmin.trim().toLowerCase())
+    );
+  }
+
   async applyFilters<T>(
     userId: string,
     roles: string[] = [],
     req: any,
   ): Promise<T> {
     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);
       }