IMN-522 Fixes in client-assertion-validation (#1084)

pagopa · Oct 10, 2024 · 96aa64e · 96aa64e
1 parent b54493c
commit 96aa64e
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 3 deletions.
diff --git a/packages/client-assertion-validation/src/types.ts b/packages/client-assertion-validation/src/types.ts
@@ -55,9 +55,8 @@ export const Key = z
     clientId: ClientId,
     consumerId: TenantId,
     kid: z.string(),
-    purposeId: PurposeId,
     publicKey: z.string().min(1),
-    algorithm: z.literal("RS256"),
+    algorithm: z.string(),
   })
   .strict();
 export type Key = z.infer<typeof Key>;

diff --git a/packages/client-assertion-validation/src/validation.ts b/packages/client-assertion-validation/src/validation.ts
@@ -24,6 +24,7 @@ import {
   validatePurposeId,
   validateSub,
   validatePlatformState,
+  ALLOWED_ALGORITHM,
 } from "./utils.js";
 import {
   ApiKey,
@@ -48,6 +49,7 @@ import {
   unexpectedClientAssertionPayload,
   invalidSignature,
   clientAssertionInvalidClaims,
+  algorithmNotAllowed,
 } from "./errors.js";
 
 export const validateRequestParameters = (
@@ -187,6 +189,10 @@ export const verifyClientAssertionSignature = (
   key: Key
 ): ValidationResult<JwtPayload> => {
   try {
+    if (key.algorithm !== ALLOWED_ALGORITHM) {
+      return failedValidation([algorithmNotAllowed(key.algorithm)]);
+    }
+
     const result = verify(clientAssertionJws, key.publicKey, {
       algorithms: [key.algorithm],
     });

diff --git a/packages/client-assertion-validation/test/utils.ts b/packages/client-assertion-validation/test/utils.ts
@@ -89,7 +89,6 @@ export const getMockApiKey = (): ApiKey => ({
   clientId: generateId<ClientId>(),
   consumerId: generateId<TenantId>(),
   kid: "kid",
-  purposeId: generateId<PurposeId>(),
   publicKey: crypto
     .generateKeyPairSync("rsa", {
       modulusLength: 2048,

diff --git a/packages/client-assertion-validation/test/validation.test.ts b/packages/client-assertion-validation/test/validation.test.ts
@@ -564,6 +564,43 @@ describe("validation test", () => {
       expect(errors).toBeUndefined();
     });
 
+    it("algorithmNotAllowed", () => {
+      const threeHourLater = new Date();
+      threeHourLater.setHours(threeHourLater.getHours() + 3);
+
+      const notAllowedAlg = "RS384";
+      const keySet = crypto.generateKeyPairSync("rsa", {
+        modulusLength: 2048,
+      });
+
+      const jws = getMockClientAssertion({
+        customHeader: {
+          alg: notAllowedAlg,
+        },
+        standardClaimsOverride: {
+          iat: new Date().getTime() / 1000,
+          exp: threeHourLater.getTime() / 1000,
+        },
+        customClaims: {},
+        keySet,
+      });
+      const publicKey = keySet.publicKey
+        .export({
+          type: "pkcs1",
+          format: "pem",
+        })
+        .toString();
+      const mockConsumerKey: ConsumerKey = {
+        ...getMockConsumerKey(),
+        publicKey,
+        algorithm: notAllowedAlg,
+      };
+      const { errors } = verifyClientAssertionSignature(jws, mockConsumerKey);
+      expect(errors).toBeDefined();
+      expect(errors).toHaveLength(1);
+      expect(errors![0]).toEqual(algorithmNotAllowed(notAllowedAlg));
+    });
+
     it.skip("invalidClientAssertionSignatureType", () => {
       // it's not clear when the result of the verify function is a string
       expect(1).toBe(1);