Merge pull request #932 from US-Trustee-Program/CAMS-446-adgroups-bug

CAMS-446 - Refactor ad_groups concatenation

backend/functions/lib/adapters/gateways/okta/HumbleVerifier.ts

@@ -52,13 +52,8 @@ export async function verifyAccessToken(
   const oktaJwtVerifier = new OktaJwtVerifier({ issuer });
   const oktaJwt: Jwt = await oktaJwtVerifier.verifyAccessToken(token, audience);
 
-  // DOJ Login Okta instances return a custom `AD_Groups` attribute on claims that does not
-  // appear on standard Okta claims. This line checks to see if it exists and if not
-  // appends an empty array for groups that will carry no permissions for the user.
-  const groups: string[] = oktaJwt.claims.AD_Groups ? (oktaJwt.claims.AD_Groups as string[]) : [];
-
   return {
-    claims: { ...oktaJwt.claims, groups },
+    claims: { ...oktaJwt.claims },
     header: { ...oktaJwt.header },
   };
 }

backend/functions/lib/adapters/gateways/okta/okta-gateway.test.ts

@@ -89,8 +89,10 @@ describe('Okta gateway tests', () => {
     const actual = await gateway.getUser(token);
     expect(actual).toEqual({
       user: { id: undefined, name: userInfo.name },
-      groups: ['groupA', 'groupB', 'groupC'],
-      jwt,
+      jwt: {
+        ...jwt,
+        claims: { ...jwt.claims, groups: expect.arrayContaining(['groupA', 'groupB', 'groupC']) },
+      },
     });
   });
 

backend/functions/lib/adapters/gateways/okta/okta-gateway.ts

@@ -44,9 +44,7 @@ async function verifyToken(token: string): Promise<CamsJwt> {
   }
 }
 
-async function getUser(
-  accessToken: string,
-): Promise<{ user: CamsUser; groups: string[]; jwt: CamsJwt }> {
+async function getUser(accessToken: string): Promise<{ user: CamsUser; jwt: CamsJwt }> {
   const { userInfoUri } = getAuthorizationConfig();
 
   try {
@@ -69,15 +67,20 @@ async function getUser(
         name: oktaUser.name,
       };
 
-      type DojLoginUnifiedGroupClaims = {
-        ad_groups?: string[];
-        groups?: string[];
-      };
-
-      const claims = jwt.claims as unknown as DojLoginUnifiedGroupClaims;
-      const groups = Array.from(new Set<string>([].concat(claims.ad_groups, claims.groups)));
+      // DOJ Login Okta instances return a custom `AD_Groups` attribute on claims that does not
+      // appear on standard Okta claims. This line checks to see if it exists and if not
+      // appends an empty array for groups that will carry no permissions for the user.
+      jwt.claims.groups = Array.from(
+        new Set<string>(
+          [].concat(
+            jwt.claims.groups ?? [],
+            jwt.claims.AD_Groups ?? [],
+            jwt.claims.ad_groups ?? [],
+          ),
+        ),
+      );
 
-      return { user, groups, jwt };
+      return { user, jwt };
     } else {
       throw new Error('Failed to retrieve user info from Okta.');
     }

backend/functions/lib/adapters/gateways/storage/local-storage-gateway.ts

@@ -65,7 +65,7 @@ function getRoleMapping(): Map<string, CamsRole> {
   if (!roleMapping) {
     const roleArray = ROLE_MAPPING.split('\n');
     roleMapping = roleArray.reduce((roleMap, roleString, idx) => {
-      if (idx === 0) return roleMap;
+      if (idx === 0 || !roleString.length) return roleMap;
       const roleInfo = roleString.split(',');
 
       roleMap.set(roleInfo[1], CamsRole[roleInfo[2]]);

backend/functions/lib/adapters/types/authorization.ts

@@ -9,7 +9,7 @@ export type AuthorizationConfig = {
 };
 
 export interface OpenIdConnectGateway {
-  getUser: (accessToken: string) => Promise<{ user: CamsUser; groups: string[]; jwt: CamsJwt }>;
+  getUser: (accessToken: string) => Promise<{ user: CamsUser; jwt: CamsJwt }>;
 }
 
 export interface UserGroupGateway {

backend/functions/lib/use-cases/user-session/user-session.test.ts

@@ -60,7 +60,7 @@ describe('user-session.gateway test', () => {
     jest.spyOn(Verifier, 'verifyAccessToken').mockResolvedValue(camsJwt);
     jest
       .spyOn(MockOpenIdConnectGateway, 'getUser')
-      .mockResolvedValue({ user: mockUser, groups: [], jwt: camsJwt });
+      .mockResolvedValue({ user: mockUser, jwt: camsJwt });
   });
 
   afterEach(() => {
@@ -115,7 +115,6 @@ describe('user-session.gateway test', () => {
     });
     jest.spyOn(MockOpenIdConnectGateway, 'getUser').mockResolvedValue({
       user: mockUser,
-      groups: [],
       jwt: null,
     });
     await expect(gateway.lookup(context, jwtString, provider)).rejects.toThrow(UnauthorizedError);
@@ -127,7 +126,6 @@ describe('user-session.gateway test', () => {
     });
     jest.spyOn(MockOpenIdConnectGateway, 'getUser').mockResolvedValue({
       user: mockUser,
-      groups: [],
       jwt: undefined,
     });
     await expect(gateway.lookup(context, jwtString, provider)).rejects.toThrow(UnauthorizedError);

backend/functions/lib/use-cases/user-session/user-session.ts

@@ -89,9 +89,9 @@ export class UserSessionUseCase {
         });
       }
 
-      const { user, groups, jwt } = await authGateway.getUser(token);
-      user.roles = getRoles(groups);
-      user.offices = await getOffices(context, groups);
+      const { user, jwt } = await authGateway.getUser(token);
+      user.roles = getRoles(jwt.claims.groups);
+      user.offices = await getOffices(context, jwt.claims.groups);
 
       // Simulate the legacy behavior by appending roles and Manhattan office to the user
       // if the 'restrict-case-assignment' feature flag is not set.

common/src/cams/jwt.ts

@@ -11,7 +11,7 @@ export type CamsJwtClaims = {
   nbf?: number;
   iat?: number;
   jti?: string;
-  groups: string[];
+  groups?: string[];
   [key: string]: unknown;
 };