CCM-10422: download authorizer works with client ownership (#626)

Author: alexnuttall

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

@@ -107,24 +107,31 @@ describe('LambdaCognitoAuthorizer', () => {
     expect(mockLogger.logMessages).toEqual([]);
   });
 
-  test('returns success on valid token without notify-client-id', async () => {
+  test('returns success on valid token when expected resource owner is specified', 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': 'nhs-notify-client-id',
       },
       'key',
       {
         keyid: 'key-id',
       }
     );
 
-    const res = await authorizer.authorize(userPoolId, userPoolClientId, jwt);
+    const res = await authorizer.authorize(
+      userPoolId,
+      userPoolClientId,
+      jwt,
+      'nhs-notify-client-id'
+    );
 
     expect(res).toEqual({
       success: true,
       subject: 'sub',
+      clientId: 'nhs-notify-client-id',
     });
     expect(mockLogger.logMessages).toEqual([]);
   });
@@ -152,7 +159,7 @@ describe('LambdaCognitoAuthorizer', () => {
         token_use: 'access',
         client_id: 'user-pool-client-id',
         iss: 'https://cognito-idp.eu-west-2.amazonaws.com/user-pool-id',
-        clientId: 'nhs-notify-client-id',
+        'nhs-notify:client-id': 'nhs-notify-client-id',
       },
       'key'
     );
@@ -174,7 +181,7 @@ describe('LambdaCognitoAuthorizer', () => {
         token_use: 'access',
         client_id: 'user-pool-client-id-2',
         iss: 'https://cognito-idp.eu-west-2.amazonaws.com/user-pool-id',
-        clientId: 'nhs-notify-client-id',
+        'nhs-notify:client-id': 'nhs-notify-client-id',
       },
       'key',
       {
@@ -200,7 +207,7 @@ describe('LambdaCognitoAuthorizer', () => {
         token_use: 'access',
         client_id: 'user-pool-client-id',
         iss: 'https://cognito-idp.eu-west-2.amazonaws.com/user-pool-id-2',
-        clientId: 'nhs-notify-client-id',
+        'nhs-notify:client-id': 'nhs-notify-client-id',
       },
       'key',
       {
@@ -226,7 +233,7 @@ describe('LambdaCognitoAuthorizer', () => {
         token_use: 'id',
         client_id: 'user-pool-client-id',
         iss: 'https://cognito-idp.eu-west-2.amazonaws.com/user-pool-id',
-        clientId: 'nhs-notify-client-id',
+        'nhs-notify:client-id': 'nhs-notify-client-id',
       },
       'key',
       {
@@ -245,6 +252,36 @@ describe('LambdaCognitoAuthorizer', () => {
     );
   });
 
+  test('returns failure when no NHS Notify client ID is present in the access token', 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',
+      },
+      '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: 'Invalid input: expected string, received undefined',
+          }),
+        ]),
+      })
+    );
+  });
+
   test('returns failure on Cognito not validating the token', async () => {
     const cognitoErrorUserPool = 'user-pool-id-cognito-error';
 
@@ -253,7 +290,7 @@ describe('LambdaCognitoAuthorizer', () => {
         token_use: 'access',
         client_id: 'user-pool-client-id',
         iss: 'https://cognito-idp.eu-west-2.amazonaws.com/user-pool-id-cognito-error',
-        clientId: 'nhs-notify-client-id',
+        'nhs-notify:client-id': 'nhs-notify-client-id',
       },
       'key',
       {
@@ -285,7 +322,7 @@ describe('LambdaCognitoAuthorizer', () => {
         token_use: 'access',
         client_id: 'user-pool-client-id',
         iss: `https://cognito-idp.eu-west-2.amazonaws.com/${iss}`,
-        clientId: 'nhs-notify-client-id',
+        'nhs-notify:client-id': 'nhs-notify-client-id',
       },
       'key',
       {
@@ -312,7 +349,7 @@ describe('LambdaCognitoAuthorizer', () => {
         token_use: 'access',
         client_id: 'user-pool-client-id',
         iss: 'https://cognito-idp.eu-west-2.amazonaws.com/user-pool-id-cognito-no-sub',
-        clientId: 'nhs-notify-client-id',
+        'nhs-notify:client-id': 'nhs-notify-client-id',
       },
       'key',
       {
@@ -335,13 +372,13 @@ describe('LambdaCognitoAuthorizer', () => {
     );
   });
 
-  test('returns failure when sub on Cognito UserAttributes does not match expected subject', async () => {
+  test('returns failure when expected resource owner does not match notify client id from Cognito', 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',
-        clientId: 'nhs-notify-client-id',
+        'nhs-notify:client-id': 'nhs-notify-client-id',
       },
       'key',
       {
@@ -360,7 +397,7 @@ describe('LambdaCognitoAuthorizer', () => {
     expect(mockLogger.logMessages).toContainEqual(
       expect.objectContaining({
         level: 'warn',
-        message: 'Subject path does not match expected subject',
+        message: 'clientId does not match expected resource owner',
       })
     );
   });
@@ -372,7 +409,7 @@ describe('LambdaCognitoAuthorizer', () => {
         client_id: 'user-pool-client-id',
         iss: 'https://cognito-idp.eu-west-2.amazonaws.com/user-pool-id',
         exp: 1_640_995_200,
-        clientId: 'nhs-notify-client-id',
+        'nhs-notify:client-id': 'nhs-notify-client-id',
       },
       'key',
       {

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().optional(),
+  'nhs-notify:client-id': z.string(),
 });
 
 export class LambdaCognitoAuthorizer {
@@ -25,7 +25,7 @@ export class LambdaCognitoAuthorizer {
     userPoolId: string,
     userPoolClientId: string,
     jwt: string,
-    expectedSubject?: string
+    expectedResourceOwner?: string
   ): Promise<
     { success: true; subject: string; clientId?: string } | { success: false }
   > {
@@ -90,14 +90,23 @@ export class LambdaCognitoAuthorizer {
         return { success: false };
       }
 
-      if (expectedSubject !== undefined && expectedSubject !== sub) {
-        this.logger.warn('Subject path does not match expected subject');
+      if (
+        expectedResourceOwner !== undefined &&
+        expectedResourceOwner !== notifyClientId
+      ) {
+        this.logger.warn('clientId does not match expected resource owner');
         return { success: false };
       }
 
       return { success: true, subject: sub, clientId: notifyClientId };
     } catch (error) {
-      this.logger.error('Failed to authorize:', error);
+      this.logger
+        .child(
+          error instanceof Error && 'issues' in error
+            ? { issues: error.issues }
+            : {}
+        )
+        .error('Failed to authorize:', error);
       return { success: false };
     }
   }