CCM-11870: refactor authorizer client id claim check

Author: harrim91

lambdas/authorizer/src/__tests__/index.test.ts

@@ -101,58 +101,6 @@ test('returns Allow policy on valid token with clientId', async () => {
   );
 });
 
-test('returns Deny policy when authorisation succeeds but clientId is missing', async () => {
-  lambdaCognitoAuthorizer.authorize.mockResolvedValue({
-    success: true,
-    subject: 'sub',
-    clientId: undefined,
-  });
-
-  const res = await handler(
-    mock<APIGatewayRequestAuthorizerEvent>({
-      requestContext,
-      headers: { Authorization: 'jwt' },
-      type: 'REQUEST',
-    }),
-    mock<Context>(),
-    jest.fn()
-  );
-
-  expect(res).toEqual({
-    ...denyPolicy,
-    context: { user: 'sub' },
-  });
-  expect(mockLogger.warn).toHaveBeenCalledWith(
-    'Authorisation denied: missing clientId'
-  );
-});
-
-test('returns Deny policy when authorisation succeeds but clientId is empty/whitespace', async () => {
-  lambdaCognitoAuthorizer.authorize.mockResolvedValue({
-    success: true,
-    subject: 'sub',
-    clientId: '   ',
-  });
-
-  const res = await handler(
-    mock<APIGatewayRequestAuthorizerEvent>({
-      requestContext,
-      headers: { Authorization: 'jwt' },
-      type: 'REQUEST',
-    }),
-    mock<Context>(),
-    jest.fn()
-  );
-
-  expect(res).toEqual({
-    ...denyPolicy,
-    context: { user: 'sub' },
-  });
-  expect(mockLogger.warn).toHaveBeenCalledWith(
-    'Authorisation denied: missing clientId'
-  );
-});
-
 test('returns Deny policy on lambda misconfiguration', async () => {
   process.env.USER_POOL_ID = '';
 

lambdas/authorizer/src/index.ts

@@ -65,17 +65,10 @@ export const handler: APIGatewayRequestAuthorizerHandler = async (event) => {
   );
 
   if (authResult.success) {
-    const clientId = authResult.clientId;
-
-    if (!clientId || clientId.trim() === '') {
-      logger.warn('Authorisation denied: missing clientId');
-      return generatePolicy(methodArn, 'Deny', { user: authResult.subject });
-    } else {
-      return generatePolicy(methodArn, 'Allow', {
-        user: authResult.subject,
-        clientId,
-      });
-    }
+    return generatePolicy(methodArn, 'Allow', {
+      user: authResult.subject,
+      clientId: authResult.clientId,
+    });
   }
 
   return generatePolicy(methodArn, 'Deny');

utils/utils/src/__tests__/lambda-cognito-authorizer.test.ts

@@ -175,7 +175,7 @@ describe('LambdaCognitoAuthorizer', () => {
     );
   });
 
-  test('returns failure on token with incorrect client_id claim', async () => {
+  test('returns failure on token with incorrect cognito client_id claim', async () => {
     const jwt = sign(
       {
         token_use: 'access',
@@ -282,6 +282,68 @@ describe('LambdaCognitoAuthorizer', () => {
     );
   });
 
+  test('returns failure when NHS Notify client ID claim is empty string', async () => {
+    const jwt = sign(
+      {
+        token_use: 'access',
+        client_id: 'user-pool-client-id',
+        iss: 'https://cognito-idp.eu-west-2.amazonaws.com/user-pool-id',
+        'nhs-notify:client-id': '',
+      },
+      'key',
+      {
+        keyid: 'key-id',
+      }
+    );
+
+    const res = await authorizer.authorize(userPoolId, userPoolClientId, jwt);
+
+    expect(res).toEqual({ success: false });
+    expect(mockLogger.logMessages).toContainEqual(
+      expect.objectContaining({
+        level: 'error',
+        message: expect.stringContaining('Failed to authorize'),
+        issues: expect.arrayContaining([
+          expect.objectContaining({
+            path: ['nhs-notify:client-id'],
+            message: 'Too small: expected string to have >=1 characters',
+          }),
+        ]),
+      })
+    );
+  });
+
+  test('returns failure when NHS Notify client ID claim is whitespace', async () => {
+    const jwt = sign(
+      {
+        token_use: 'access',
+        client_id: 'user-pool-client-id',
+        iss: 'https://cognito-idp.eu-west-2.amazonaws.com/user-pool-id',
+        'nhs-notify:client-id': '    ',
+      },
+      'key',
+      {
+        keyid: 'key-id',
+      }
+    );
+
+    const res = await authorizer.authorize(userPoolId, userPoolClientId, jwt);
+
+    expect(res).toEqual({ success: false });
+    expect(mockLogger.logMessages).toContainEqual(
+      expect.objectContaining({
+        level: 'error',
+        message: expect.stringContaining('Failed to authorize'),
+        issues: expect.arrayContaining([
+          expect.objectContaining({
+            path: ['nhs-notify:client-id'],
+            message: 'Too small: expected string to have >=1 characters',
+          }),
+        ]),
+      })
+    );
+  });
+
   test('returns failure on Cognito not validating the token', async () => {
     const cognitoErrorUserPool = 'user-pool-id-cognito-error';
 

utils/utils/src/lambda-cognito-authorizer.ts

@@ -12,7 +12,7 @@ const $AccessToken = z.object({
   client_id: z.string(),
   iss: z.string(),
   token_use: z.string(),
-  'nhs-notify:client-id': z.string(),
+  'nhs-notify:client-id': z.string().trim().nonempty(),
 });
 
 export class LambdaCognitoAuthorizer {