fix: We didn't merge verification options set on an object and passed…

… in as arguments to a method properly as pointed out in Sphereon-Opensource/SSI-SDK#125
Sphereon-Opensource · Sep 28, 2023 · 973238a · 973238a
1 parent 4d1b1dc
commit 973238a
Show file tree

Hide file tree

Showing 14 changed files with 501 additions and 116 deletions.
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@sphereon/did-auth-siop",
-  "version": "0.3.2-unstable.8",
+  "version": "0.3.2-unstable.9",
   "source": "src/index.ts",
   "main": "dist/index.js",
   "types": "dist/index.d.ts",
@@ -29,9 +29,9 @@
   },
   "dependencies": {
     "@sphereon/did-uni-client": "^0.6.0",
-    "@sphereon/pex": "^2.1.0",
+    "@sphereon/pex": "^2.1.2",
     "@sphereon/pex-models": "^2.0.3",
-    "@sphereon/ssi-types": "^0.14.1",
+    "@sphereon/ssi-types": "^0.15.1",
     "@sphereon/wellknown-dids-client": "^0.1.3",
     "@astronautlabs/jsonpath": "^1.1.2",
     "cross-fetch": "^3.1.8",
@@ -47,8 +47,8 @@
   "devDependencies": {
     "@digitalcredentials/did-method-key": "^2.0.3",
     "@digitalcredentials/ed25519-signature-2020": "^3.0.2",
-    "@digitalcredentials/jsonld-signatures": "^9.3.1",
-    "@digitalcredentials/vc": "^5.0.0",
+    "@digitalcredentials/jsonld-signatures": "^9.3.2",
+    "@digitalcredentials/vc": "^6.0.0",
     "@types/jest": "^29.5.3",
     "@types/language-tags": "^1.0.1",
     "@types/uuid": "^9.0.1",
@@ -80,7 +80,7 @@
     "typescript": "4.9.5"
   },
   "resolutions": {
-    "isomorphic-webcrypto": "npm:@sphereon/isomorphic-webcrypto@^2.4.0-unstable.1"
+    "isomorphic-webcrypto": "npm:@sphereon/isomorphic-webcrypto@^2.4.0-unstable.4"
   },
   "files": [
     "dist"

diff --git a/src/authorization-request/AuthorizationRequest.ts b/src/authorization-request/AuthorizationRequest.ts
@@ -119,12 +119,14 @@ export class AuthorizationRequest {
     const jwt = await this.requestObjectJwt();
     if (jwt) {
       parseJWT(jwt);
+      const resolver = getResolver(opts.verification.resolveOpts);
       const options: JWTVerifyOptions = {
         ...opts.verification?.resolveOpts?.jwtVerifyOpts,
+        resolver,
         audience: getAudience(jwt),
       };
 
-      verifiedJwt = await verifyDidJWT(jwt, getResolver(opts.verification.resolveOpts), options);
+      verifiedJwt = await verifyDidJWT(jwt, resolver, options);
       if (!verifiedJwt || !verifiedJwt.payload) {
         throw Error(SIOPErrors.ERROR_VERIFYING_SIGNATURE);
       }

diff --git a/src/authorization-request/Opts.ts b/src/authorization-request/Opts.ts
@@ -1,5 +1,5 @@
 import { assertValidRequestObjectOpts } from '../request-object/Opts';
-import { isExternalVerification, isInternalVerification, SIOPErrors } from '../types';
+import { ExternalVerification, InternalVerification, isExternalVerification, isInternalVerification, SIOPErrors } from '../types';
 
 import { assertValidRequestRegistrationOpts } from './RequestRegistration';
 import { CreateAuthorizationRequestOpts, VerifyAuthorizationRequestOpts } from './types';
@@ -20,3 +20,48 @@ export const assertValidAuthorizationRequestOpts = (opts: CreateAuthorizationReq
   assertValidRequestObjectOpts(opts.requestObject, false);
   assertValidRequestRegistrationOpts(opts['registration'] ? opts['registration'] : opts.clientMetadata);
 };
+
+export const mergeVerificationOpts = (
+  classOpts: {
+    verification?: InternalVerification | ExternalVerification;
+  },
+  requestOpts: {
+    correlationId: string;
+    verification?: InternalVerification | ExternalVerification;
+  }
+) => {
+  const resolver = requestOpts.verification?.resolveOpts?.resolver ?? classOpts.verification?.resolveOpts?.resolver;
+  const wellknownDIDVerifyCallback = requestOpts.verification?.wellknownDIDVerifyCallback ?? classOpts.verification?.wellknownDIDVerifyCallback;
+  const presentationVerificationCallback =
+    requestOpts.verification?.presentationVerificationCallback ?? classOpts.verification?.presentationVerificationCallback;
+  const replayRegistry = requestOpts.verification?.replayRegistry ?? classOpts.verification?.replayRegistry;
+  return {
+    ...classOpts.verification,
+    ...requestOpts.verification,
+    ...(wellknownDIDVerifyCallback && { wellknownDIDVerifyCallback }),
+    ...(presentationVerificationCallback && { presentationVerificationCallback }),
+    ...(replayRegistry && { replayRegistry }),
+    resolveOpts: {
+      ...classOpts.verification?.resolveOpts,
+      ...requestOpts.verification?.resolveOpts,
+      ...(resolver && { resolver }),
+      jwtVerifyOpts: {
+        ...classOpts.verification?.resolveOpts?.jwtVerifyOpts,
+        ...requestOpts.verification?.resolveOpts?.jwtVerifyOpts,
+        ...(resolver && { resolver }),
+        policies: {
+          ...classOpts.verification?.resolveOpts?.jwtVerifyOpts?.policies,
+          ...requestOpts.verification?.resolveOpts?.jwtVerifyOpts?.policies,
+          aud: false, // todo: check why we are setting this. Probably needs a PR upstream in DID-JWT
+        },
+      },
+    },
+    revocationOpts: {
+      ...classOpts.verification?.revocationOpts,
+      ...requestOpts.verification?.revocationOpts,
+      revocationVerificationCallback:
+        requestOpts.verification?.revocationOpts?.revocationVerificationCallback ??
+        classOpts?.verification?.revocationOpts?.revocationVerificationCallback,
+    },
+  };
+};
diff --git a/src/authorization-request/Payload.ts b/src/authorization-request/Payload.ts
@@ -31,14 +31,12 @@ export const createPresentationDefinitionClaimsProperties = (opts: ClaimPayloadO
 
   return {
     ...(opts.id_token ? { id_token: opts.id_token } : {}),
-    ...(opts.vp_token.presentation_definition || opts.vp_token.presentation_definition_uri
-      ? {
-          vp_token: {
-            ...(!opts.vp_token.presentation_definition_uri ? { presentation_definition: opts.vp_token.presentation_definition } : {}),
-            ...(opts.vp_token.presentation_definition_uri ? { presentation_definition_uri: opts.vp_token.presentation_definition_uri } : {}),
-          },
-        }
-      : {}),
+    ...((opts.vp_token.presentation_definition || opts.vp_token.presentation_definition_uri) && {
+      vp_token: {
+        ...(!opts.vp_token.presentation_definition_uri && { presentation_definition: opts.vp_token.presentation_definition }),
+        ...(opts.vp_token.presentation_definition_uri && { presentation_definition_uri: opts.vp_token.presentation_definition_uri }),
+      },
+    }),
   };
 };
 

diff --git a/src/authorization-request/types.ts b/src/authorization-request/types.ts
@@ -52,7 +52,6 @@ interface AuthorizationRequestCommonOpts<CT extends ClaimPayloadCommonOpts> {
   // Yes, this includes common payload properties both at the payload level as well as in the requestObject.payload property. That is to support OAuth2 with or without a signed OpenID requestObject
 
   version: SupportedVersion;
-
   clientMetadata?: ClientMetadataOpts; // this maps to 'registration' for older SIOPv2 specs! OPTIONAL. This parameter is used by the RP to provide information about itself to a Self-Issued OP that would normally be provided to an OP during Dynamic RP Registration, as specified in {#rp-registration-parameter}.
   payload?: AuthorizationRequestPayloadOpts<CT>;
   requestObject: RequestObjectOpts<CT>;
@@ -63,10 +62,7 @@ interface AuthorizationRequestCommonOpts<CT extends ClaimPayloadCommonOpts> {
 export type AuthorizationRequestOptsVID1 = AuthorizationRequestCommonOpts<ClaimPayloadOptsVID1>;
 
 export interface AuthorizationRequestOptsVD11 extends AuthorizationRequestCommonOpts<ClaimPayloadCommonOpts> {
-  // clientMetadata?: ClientMetadataOpts; // from openid-connect-self-issued-v2-1_0-11 look at https://openid.net/specs/openid-connect-registration-1_0.html
-  // clientMetadataUri?: string; // from openid-connect-self-issued-v2-1_0-11
   idTokenType?: string; // OPTIONAL. Space-separated string that specifies the types of ID token the RP wants to obtain, with the values appearing in order of preference. The allowed individual values are subject_signed and attester_signed (see Section 8.2). The default value is attester_signed.
-  // claims?: ClaimPayloadCommonOpts;
 }
 
 export type CreateAuthorizationRequestOpts = AuthorizationRequestOptsVID1 | AuthorizationRequestOptsVD11;
@@ -80,8 +76,6 @@ export interface VerifyAuthorizationRequestOpts {
   state?: string; // If provided the state in the request needs to match
 
   supportedVersions?: SupportedVersion[];
-  // redirectUri?: string;
-  // wellknownDIDverifyCallback?: WellknownDIDVerifyCallback;
 }
 
 /**

diff --git a/src/did/DIDResolution.ts b/src/did/DIDResolution.ts
@@ -6,13 +6,16 @@ import { DIDDocument, ResolveOpts, SIOPErrors, SubjectIdentifierType, SubjectSyn
 import { getMethodFromDid, toSIOPRegistrationDidMethod } from './index';
 
 export function getResolver(opts: ResolveOpts): Resolvable {
-  if (opts && opts.resolver) {
+  if (opts?.resolver && typeof opts.resolver === 'object') {
     return opts.resolver;
   }
   if (!opts || !opts.subjectSyntaxTypesSupported) {
     if (opts?.noUniversalResolverFallback) {
       throw Error(`No subject syntax types nor did methods configured for DID resolution, but fallback to universal resolver has been disabled`);
     }
+    console.log(
+      `Falling back to universal resolver as not resolve opts have been provided, or no subject syntax types supported are provided. It is wise to fix this`
+    );
     return new UniResolver();
   }
 
@@ -33,6 +36,9 @@ export function getResolver(opts: ResolveOpts): Resolvable {
     if (opts?.noUniversalResolverFallback) {
       throw Error(`No subject syntax types nor did methods configured for DID resolution, but fallback to universal resolver has been disabled`);
     }
+    console.log(
+      `Falling back to universal resolver as not resolve opts have been provided, or no subject syntax types supported are provided. It is wise to fix this`
+    );
     return new UniResolver();
   }
 }

diff --git a/src/did/DidJWT.ts b/src/did/DidJWT.ts
@@ -56,7 +56,7 @@ import {
  *  @return   {Promise<Object, Error>}                  a promise which resolves with a response object or rejects with an error
  */
 export async function verifyDidJWT(jwt: string, resolver: Resolvable, options: JWTVerifyOptions): Promise<VerifiedJWT> {
-  return verifyJWT(jwt, { resolver, ...options });
+  return verifyJWT(jwt, { ...options, resolver });
 }
 
 /**

diff --git a/src/op/OP.ts b/src/op/OP.ts
@@ -4,6 +4,7 @@ import { IIssuerId } from '@sphereon/ssi-types/src/types/vc';
 import { v4 as uuidv4 } from 'uuid';
 
 import { AuthorizationRequest, URI, VerifyAuthorizationRequestOpts } from '../authorization-request';
+import { mergeVerificationOpts } from '../authorization-request/Opts';
 import {
   AuthorizationResponse,
   AuthorizationResponseOpts,
@@ -74,27 +75,8 @@ export class OP {
         throw error;
       });
 
-    const verification = {
-      ...requestOpts?.verification,
-      ...{
-        resolveOpts: {
-          ...requestOpts?.verification?.resolveOpts,
-          ...{
-            jwtVerifyOpts: {
-              ...requestOpts?.verification?.resolveOpts?.jwtVerifyOpts,
-              ...{
-                policies: {
-                  ...requestOpts?.verification?.resolveOpts?.jwtVerifyOpts?.policies,
-                  aud: false,
-                },
-              },
-            },
-          },
-        },
-      },
-    };
     return authorizationRequest
-      .verify(this.newVerifyAuthorizationRequestOpts({ ...requestOpts, verification, correlationId }))
+      .verify(this.newVerifyAuthorizationRequestOpts({ ...requestOpts, correlationId }))
       .then((verifiedAuthorizationRequest: VerifiedAuthorizationRequest) => {
         this.emitEvent(AuthorizationEvents.ON_AUTH_REQUEST_VERIFIED_SUCCESS, {
           correlationId,
@@ -182,10 +164,7 @@ export class OP {
     ) {
       throw new Error(SIOPErrors.BAD_PARAMS);
     }
-    /*const request = response.authorizationRequest;
-    if (!request) {
-      throw Error('Cannot submit an authorization response without a request present');
-    }*/
+
     const payload = await response.payload;
     const idToken = await response.idToken?.payload();
     const redirectURI = authorizationResponse.redirectURI || idToken?.aud;
@@ -242,30 +221,33 @@ export class OP {
     if (!issuer) {
       throw Error(`No issuer value present. Either use IDv1, JWT VC Presentation profile version, or provide a DID as issuer value`);
     }
+    // We are taking the whole presentationExchange object from a certain location
+    const presentationExchange = opts.presentationExchange ?? this._createResponseOptions.presentationExchange;
     return {
       ...this._createResponseOptions,
+      ...opts,
+      signature: {
+        ...this._createResponseOptions?.signature,
+        ...opts.signature,
+      },
+      ...(presentationExchange && { presentationExchange }),
       registration: { ...this._createResponseOptions?.registration, issuer },
-      ...(opts?.audience ? { redirectUri: opts.audience } : {}),
-      ...(opts?.presentationExchange ? { presentationExchange: opts.presentationExchange } : {}),
-      ...(opts?.signature
-        ? { signature: opts.signature }
-        : this._createResponseOptions?.signature
-        ? { signature: this._createResponseOptions.signature }
-        : {}),
+      redirectUri: opts.audience ?? this._createResponseOptions.redirectUri,
     };
   }
 
-  private newVerifyAuthorizationRequestOpts(opts: {
+  private newVerifyAuthorizationRequestOpts(requestOpts: {
     correlationId: string;
     verification?: InternalVerification | ExternalVerification;
-    // verifyCallback?: VerifyCallback;
   }): VerifyAuthorizationRequestOpts {
-    return {
+    const verification: VerifyAuthorizationRequestOpts = {
       ...this._verifyRequestOptions,
-      correlationId: opts.correlationId,
-      verification: { ...this._verifyRequestOptions.verification, ...opts?.verification },
-      // wellknownDIDverifyCallback: opts?.verifyCallback,
+      ...requestOpts,
+      verification: mergeVerificationOpts(this._verifyRequestOptions, requestOpts),
+      correlationId: requestOpts.correlationId,
     };
+
+    return verification;
   }
 
   private async emitEvent(

diff --git a/src/rp/InMemoryRPSessionManager.ts b/src/rp/InMemoryRPSessionManager.ts
@@ -205,6 +205,7 @@ export class InMemoryRPSessionManager implements IRPSessionManager {
         this.authorizationResponses[event.correlationId] = eventState as AuthorizationResponseState;
       }
     } catch (error: unknown) {
+      console.log(`Error in update state happened: ${error}`);
       // TODO VDX-166 handle error
     }
   }

diff --git a/src/rp/RP.ts b/src/rp/RP.ts
@@ -11,6 +11,7 @@ import {
   RequestPropertyWithTargets,
   URI,
 } from '../authorization-request';
+import { mergeVerificationOpts } from '../authorization-request/Opts';
 import { AuthorizationResponse, PresentationDefinitionWithLocation, VerifyAuthorizationResponseOpts } from '../authorization-response';
 import { getNonce, getState } from '../helpers';
 import {
@@ -282,9 +283,9 @@ export class RP {
       presentationDefinitions?: PresentationDefinitionWithLocation | PresentationDefinitionWithLocation[];
     }
   ): Promise<VerifyAuthorizationResponseOpts> {
-    let correlationId = opts?.correlationId || this._verifyResponseOptions.correlationId;
-    let state = opts?.state || this._verifyResponseOptions.state;
-    let nonce = opts?.nonce || this._verifyResponseOptions.nonce;
+    let correlationId = opts?.correlationId ?? this._verifyResponseOptions.correlationId;
+    let state = opts?.state ?? this._verifyResponseOptions.state;
+    let nonce = opts?.nonce ?? this._verifyResponseOptions.nonce;
     if (this.sessionManager) {
       const resNonce = (await authorizationResponse.getMergedProperty('nonce', false)) as string;
       const resState = (await authorizationResponse.getMergedProperty('state', false)) as string;
@@ -305,16 +306,17 @@ export class RP {
     }
     return {
       ...this._verifyResponseOptions,
+      ...opts,
       correlationId,
       audience:
-        opts?.audience ||
-        this._verifyResponseOptions.audience ||
-        this._verifyResponseOptions.verification.resolveOpts.jwtVerifyOpts.audience ||
+        opts?.audience ??
+        this._verifyResponseOptions.audience ??
+        this._verifyResponseOptions.verification.resolveOpts.jwtVerifyOpts.audience ??
         this._createRequestOptions.payload.client_id,
       state,
       nonce,
-      verification: opts?.verification || this._verifyResponseOptions.verification,
-      presentationDefinitions: opts?.presentationDefinitions || this._verifyResponseOptions.presentationDefinitions,
+      verification: mergeVerificationOpts(this._verifyResponseOptions, opts),
+      presentationDefinitions: opts?.presentationDefinitions ?? this._verifyResponseOptions.presentationDefinitions,
     };
   }
 

diff --git a/src/schemas/AuthorizationRequestPayloadVD11.schema.ts b/src/schemas/AuthorizationRequestPayloadVD11.schema.ts
@@ -81,7 +81,7 @@ export const AuthorizationRequestPayloadVD11SchemaObj = {
           "type": "string"
         },
         "response_mode": {
-          "$ref": "#/definitions/ResponseMode"
+          "type": "string"
         },
         "request": {
           "type": "string"
@@ -324,15 +324,6 @@ export const AuthorizationRequestPayloadVD11SchemaObj = {
       ],
       "additionalProperties": false
     },
-    "ResponseMode": {
-      "type": "string",
-      "enum": [
-        "fragment",
-        "form_post",
-        "post",
-        "query"
-      ]
-    },
     "ClaimPayloadCommon": {
       "type": "object",
       "additionalProperties": false

diff --git a/src/schemas/AuthorizationRequestPayloadVID1.schema.ts b/src/schemas/AuthorizationRequestPayloadVID1.schema.ts
@@ -78,7 +78,7 @@ export const AuthorizationRequestPayloadVID1SchemaObj = {
           "type": "string"
         },
         "response_mode": {
-          "$ref": "#/definitions/ResponseMode"
+          "type": "string"
         },
         "request": {
           "type": "string"
@@ -296,15 +296,6 @@ export const AuthorizationRequestPayloadVID1SchemaObj = {
       ],
       "additionalProperties": false
     },
-    "ResponseMode": {
-      "type": "string",
-      "enum": [
-        "fragment",
-        "form_post",
-        "post",
-        "query"
-      ]
-    },
     "ClaimPayloadVID1": {
       "type": "object",
       "properties": {