fix: improve code as per cristian feedback on the PR

hyperledger · Sep 20, 2024 · 4556ce4 · 4556ce4
1 parent 3b4e14b
commit 4556ce4
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 67 deletions.
diff --git a/src/domain/buildingBlocks/Pollux.ts b/src/domain/buildingBlocks/Pollux.ts
@@ -137,11 +137,13 @@ export namespace Pollux {
       }
       export interface JWT {
         presentationDefinitionRequest: PresentationDefinitionRequest<CredentialType.JWT>,
+        challenge?: string,
+        domain?: string
       }
       export interface SDJWT {
         issuer: DID,
         presentationDefinitionRequest: PresentationDefinitionRequest<CredentialType.SDJWT>,
-        requiredClaims: string[]
+        requiredClaims?: string[]
       }
     }
   }
@@ -157,7 +159,7 @@ export namespace Pollux {
       }
       export interface SDJWT {
         privateKey: PrivateKey;
-        presentationFrame: string[]
+        presentationFrame: Record<string, boolean>
       }
     }
   }

diff --git a/src/domain/models/VerifiableCredential.ts b/src/domain/models/VerifiableCredential.ts
@@ -171,25 +171,11 @@ export type PresentationExchangeDefinitionRequest = {
   }
 }
 
-export type PresentationExchangeDefinitionRequestOptions = {
-  options?: {
-    challenge: string,
-    domain: string
-  }
-}
-
-export type PresentationExchangeDefinitionSDJWTRequestOptions = {
-  options?: {
-    presentationFrame: PresentationFrame<any>
-  }
-}
-
-export type PresentationSDJWTRequest = any
 
 export type PresentationDefinitionData = {
   [CredentialType.AnonCreds]: PresentationAnoncredsRequest;
-  [CredentialType.JWT]: PresentationExchangeDefinitionRequest & PresentationExchangeDefinitionRequestOptions;
-  [CredentialType.SDJWT]: PresentationExchangeDefinitionRequest & PresentationExchangeDefinitionSDJWTRequestOptions;
+  [CredentialType.JWT]: PresentationExchangeDefinitionRequest;
+  [CredentialType.SDJWT]: PresentationExchangeDefinitionRequest;
   [CredentialType.Unknown]: any;
   [CredentialType.W3C]: any;
 };
@@ -464,19 +450,16 @@ export class SDJWPresentationOptions {
   public name: string;
   public purpose: string;
   public sdjwt?: PresentationJWTOptions
-  public requiredClaims: PresentationFrame<any>
 
   constructor(
     options: {
       name?: string,
       purpose?: string,
-      jwt?: PresentationJWTOptions,
-      requiredClaims?: PresentationFrame<any>
+      jwt?: PresentationJWTOptions
     }
   ) {
     this.name = options.name ?? "Presentation";
     this.purpose = options.purpose ?? "Verifying Credentials";
-    this.requiredClaims = options.requiredClaims ?? {}
     this.sdjwt = options.jwt ?? {
       jwtAlg: [JWT_ALG.ES256K],
     };

diff --git a/src/pollux/Pollux.ts b/src/pollux/Pollux.ts
@@ -9,6 +9,7 @@ import { base64, base64url } from "multiformats/bases/base64";
 import { AnoncredsLoader } from "./AnoncredsLoader";
 import * as pako from 'pako';
 import wasmBuffer from "jwe-wasm/jwe_rust_bg.wasm"
+import type { DisclosureFrame, Extensible, PresentationFrame } from '@sd-jwt/types';
 
 import {
   CredentialRequestOptions,
@@ -379,10 +380,15 @@ export default class Pollux implements IPollux {
   private async createJWTPresentationSubmission(
     presentationDefinitionRequest: any,
     credential: Credential,
-    privateKey: PrivateKey
+    privateKey: PrivateKey,
+    options?: {
+      presentationFrame?: PresentationFrame<any>,
+      domain?: string,
+      challenge?: string
+    }
   ): Promise<PresentationSubmission<CredentialType.JWT | CredentialType.SDJWT>> {
 
-    const { presentation_definition, options } = presentationDefinitionRequest;
+    const { presentation_definition } = presentationDefinitionRequest;
     const inputDescriptors = presentation_definition.input_descriptors ?? [];
 
     if (credential.isCredentialType<JWTCredential>(CredentialType.JWT)) {
@@ -421,6 +427,7 @@ export default class Pollux implements IPollux {
         nbf: nbf,
         vp: credential.presentation(),
       }
+
       const challenge = options && "challenge" in options && options?.challenge;
       const domain = options && "domain" in options && options?.domain;
 
@@ -441,21 +448,14 @@ export default class Pollux implements IPollux {
 
     } else if (credential.isCredentialType<SDJWTCredential>(CredentialType.SDJWT)) {
 
-      //TODO: improve this
-      const options: {
-        presentationFrame?: { [name: string]: boolean }
-      } = presentationDefinitionRequest?.options ?? {};
+      const presentationFrame = options && "presentationFrame" in options ?
+        options.presentationFrame :
+        undefined;
 
-      const presentationFrame =
-        options &&
-          "presentationFrame" in options ?
-          options.presentationFrame :
-          {};
-
-      jws = await this.SDJWT.createPresentationFor({
+      jws = await this.SDJWT.createPresentationFor<any>({
         jws: credential.id,
         privateKey,
-        frame: presentationFrame
+        presentationFrame: presentationFrame
       })
     } else {
       throw new PolluxError.InvalidCredentialError("Expected JWT or SDJWT credential")
@@ -499,7 +499,13 @@ export default class Pollux implements IPollux {
   >(
     presentationDefinitionRequest: PresentationDefinitionRequest<Type>,
     credential: Credential,
-    privateKey?: PrivateKey | LinkSecret
+    privateKey?: PrivateKey | LinkSecret,
+    options?: {
+      presentationFrame?: PresentationFrame<any>,
+      domain?: string,
+      challenge?: string
+    }
+
   ): Promise<PresentationSubmission<Type>> {
 
     if (
@@ -514,7 +520,8 @@ export default class Pollux implements IPollux {
       return this.createJWTPresentationSubmission(
         presentationDefinitionRequest,
         credential,
-        privateKey
+        privateKey,
+        options
       )
     }
 
@@ -849,7 +856,7 @@ export default class Pollux implements IPollux {
           JWTCredential.fromJWS(jws);
 
         const issuer = presentation.issuer
-        const presentationDefinitionOptions = presentationDefinitionRequest.options ?? {};
+        const presentationDefinitionOptions = options.presentationDefinitionRequest;
 
         if ("challenge" in presentationDefinitionOptions && "domain" in presentationDefinitionOptions) {
           const challenge = presentationDefinitionOptions?.challenge;
@@ -877,6 +884,10 @@ export default class Pollux implements IPollux {
             issuerDID: issuer,
             jws
           })
+
+          if (!credentialValid) {
+            throw new InvalidVerifyCredentialError(jws, "Invalid Holder Presentation JWS Signature");
+          }
         } else {
           const requiredClaims = "requiredClaims" in options && Array.isArray(options.requiredClaims) ?
             options.requiredClaims :
@@ -889,14 +900,13 @@ export default class Pollux implements IPollux {
 
         }
 
+
         let vc: string;
         let verifiableCredentialPropsMapper: DescriptorPath;
         const verifiablePresentation = presentation;
 
         if (descriptorItem.format === DescriptorItemFormat.JWT_VP) {
-          if (!credentialValid) {
-            throw new InvalidVerifyCredentialError(jws, "Invalid Holder Presentation JWS Signature");
-          }
+
           const nestedPath = descriptorItem.path_nested;
           if (!nestedPath) {
             throw new InvalidVerifyFormatError(
@@ -932,6 +942,7 @@ export default class Pollux implements IPollux {
           }
           verifiableCredentialPropsMapper = new DescriptorPath(verifiableCredential);
         } else {
+
           const sdjwtPresentation = presentation as SDJWTCredential;
           const claims = await this.SDJWT.reveal(
             sdjwtPresentation.core.jwt?.payload ?? {},
@@ -1315,14 +1326,15 @@ export default class Pollux implements IPollux {
       credential.isCredentialType<SDJWTCredential>(CredentialType.SDJWT)
       && presentationRequest.isType(AttachmentFormats.SDJWT)
       && "privateKey" in options
-    ) {
 
-      const presentationJSON = presentationRequest.toJSON();
-      const frame = presentationJSON.options?.presentationFrame ?? {}
+    ) {
+      const presentationFrame = "presentationFrame" in options ?
+        options.presentationFrame :
+        {};
       const presentationJWS = await this.SDJWT.createPresentationFor<any>(
         {
           jws: credential.id,
-          frame,
+          presentationFrame,
           privateKey: options.privateKey
         }
       )

diff --git a/src/pollux/utils/SDJWT.ts b/src/pollux/utils/SDJWT.ts
@@ -98,10 +98,10 @@ export class SDJWT extends JWTCore {
   async createPresentationFor<E extends Extensible>(options: {
     jws: string,
     privateKey: Domain.PrivateKey,
-    frame?: PresentationFrame<E> | undefined
+    presentationFrame?: PresentationFrame<E>
   }) {
     const sdjwt = new SDJwtVcInstance(this.getSKConfig(options.privateKey));
-    return sdjwt.present<E>(options.jws, options.frame)
+    return sdjwt.present<E>(options.jws, options.presentationFrame)
   }
 
 

diff --git a/tests/pollux/Pollux.test.ts b/tests/pollux/Pollux.test.ts
@@ -11,6 +11,7 @@ import { AttachmentDescriptor, AttachmentFormats, Claims, Credential, Credential
 import { JWTCredential } from "../../src/pollux/models/JWTVerifiableCredential";
 import Castor from "../../src/castor/Castor";
 import Apollo from "../../src/apollo/Apollo";
+import { CredentialOfferPayloads, CredentialOfferTypes, Pollux as IPollux, ProcessedCredentialOfferPayloads } from "../../src/domain/buildingBlocks/Pollux";
 
 import { InvalidJWTString } from "../../src/domain/models/errors/Pollux";
 import Pollux from "../../src/pollux/Pollux";
@@ -36,6 +37,7 @@ const jwtString = jwtParts.join(".");
 
 type JWTVerificationTestCase = {
   challenge?: string,
+  domain?: string,
   apollo: Apollo,
   castor: Castor,
   jwt: JWT,
@@ -85,7 +87,6 @@ async function createAnoncredsVerificationTestCase(options: AnoncredsVerificatio
 
 async function createSDJWTVerificationTestCase<T extends SdJwtVcPayload>(
   options: {
-    challenge?: string,
     apollo: Apollo,
     castor: Castor,
     jwt: SDJWT,
@@ -108,7 +109,8 @@ async function createSDJWTVerificationTestCase<T extends SdJwtVcPayload>(
     jwt,
     payload,
     disclosure,
-    claims
+    claims,
+    presentationFrame
   } = options;
   const kid = await (pollux as any).getSigningKid(issuer, issuerPrv);
   const signedJWT = await jwt.sign<typeof payload>({
@@ -134,14 +136,12 @@ async function createSDJWTVerificationTestCase<T extends SdJwtVcPayload>(
   expect(Object.keys(disclosed).length).to.gte(1);
   const presentationSubmissionJSON = await pollux.createPresentationSubmission
     <CredentialType.SDJWT>(
-      {
-        ...presentationDefinition,
-        options: {
-          presentationFrame: options.presentationFrame
-        }
-      },
+      presentationDefinition,
       jwtCredential,
-      holderPrv
+      holderPrv,
+      {
+        presentationFrame
+      }
     );
   return {
     presentationDefinition,
@@ -161,7 +161,8 @@ async function createJWTVerificationTestCase(options: JWTVerificationTestCase) {
     jwt,
     subject,
     claims,
-    challenge = 'sign this'
+    challenge = 'sign this',
+    domain
   } = options;
 
   const currentDate = new Date();
@@ -926,7 +927,7 @@ describe("Pollux", () => {
         const presentation = await sdjwt.createPresentationFor<typeof claims>(
           {
             jws: credential,
-            frame: { firstname: true, id: true },
+            presentationFrame: { firstname: true, id: true },
             privateKey: sk
           }
         );
@@ -977,7 +978,7 @@ describe("Pollux", () => {
         const presentation = await sdjwt.createPresentationFor<typeof claims>(
           {
             jws: credential,
-            frame: { firstname: true, id: true },
+            presentationFrame: { firstname: true, id: true },
             privateKey: sk
           }
         );
@@ -1209,13 +1210,12 @@ describe("Pollux", () => {
             }
           },
         });
-        // At verification level, the verifier can choose which fields it requires to be included in the presentation and also disclosed fields
-        const requiredClaims = ['vc.credentialSubject.email'];
         // Fails despite the verifier asked for the email, the holder rejected disclosing it
         expect(pollux.verifyPresentationSubmission(presentationSubmissionJSON, {
           presentationDefinitionRequest: presentationDefinition,
           issuer: issuerDID,
-          requiredClaims
+          // At verification level, the verifier can choose which fields it requires to be included in the presentation and also disclosed fields
+          requiredClaims: ['vc.credentialSubject.email']
         })).to.eventually.be.rejectedWith(
           "Invalid Claim: Expected one of the paths $.vc.credentialSubject.email, $.credentialSubject.email, $.email to exist."
         );
@@ -2148,9 +2148,16 @@ describe("Pollux", () => {
         }
       });
 
-      expect(pollux.verifyPresentationSubmission(presentationSubmissionJSON, {
-        presentationDefinitionRequest: presentationDefinition
-      })).to.eventually.be.rejectedWith(
+      expect(
+        pollux.verifyPresentationSubmission(
+          presentationSubmissionJSON,
+          {
+            presentationDefinitionRequest: presentationDefinition,
+            challenge,
+            domain: 'n/a'
+          }
+        )
+      ).to.eventually.be.rejectedWith(
         `Verification failed for credential (${issuedJWS.slice(0, 10)}...): reason -> Invalid Holder Presentation JWS Signature`
       );
     });
@@ -2205,7 +2212,8 @@ describe("Pollux", () => {
       });
 
       expect(pollux.verifyPresentationSubmission(presentationSubmissionJSON, {
-        presentationDefinitionRequest: presentationDefinition
+        presentationDefinitionRequest: presentationDefinition,
+
       })).to.eventually.be.rejectedWith(
         `Verification failed for credential (${issuedJWS.slice(0, 10)}...): reason -> Invalid Claim: Expected the $.credentialSubject.course field to be "Identus Training course Certification 2024" but got "Identus Training course Certification 2023"`
       );