MOSIP fixes

[sanderPostma]

include presentationSubmission in auth response
include nonce in AuthorizationResponsePayload

[sanderPostma]

assertValidVerifiablePresentations has this check

} else if (args.presentationDefinitions && !args.opts.presentationSubmission) {
  throw new Error(`No presentation submission present. Please use presentationSubmission opt argument!`);
}

So we need opts.presentationSubmission

[sanderPostma]

In AuthenticationResponse verify():

    const nonce = merged.nonce ?? verifiedIdToken?.payload.nonce ?? oid4vp.nonce;
    ...
    } else if (oid4vp.presentationDefinitions.length > 0 && !nonce) {
      throw Error('Nonce is required when using OID4VP');
    }

In my case, if I don't return the nonce it will run into "Nonce is required when using OID4VP"

[nklomp]

Yes that is according to spec

[nklomp]

So this PR is wrong

[nklomp]

PR is wrong. if there is a vp token in the request, you are required to have a presentation submission

[nklomp]

This is wrong! If the above hasVpToken is true, there needs to be a submission. So either the logic to determine the hasVpToken is wrong, or something else is happening. This PR is wrong for sure

[sanderPostma]

I found the submission in authorizationResponsePayload.presentation_submission not in presentationExchange. So this needs to be fixed elsewhere then.
So that that makes this PR is not to merge, just for diagnostics. I will revert the change in my branch as soon as presentationSubmission appears in presentationExchange

[nklomp]

No you are likely simply misunderstanding how it works. Presentation Exchange is a service that provides the submission, but only when a vp_token is needed

[sanderPostma]

I have found the problem, in the SSI SDK version I am using with RSA support, presentationSubmission in OPSession is not loaded yet.

But this line still has to be in opts

          ...(presentationSubmission ? { presentationSubmission: presentationSubmission } : {}),

It's asserted in assertValidVerifiablePresentations

  } else if (args.presentationDefinitions && !args.opts.presentationSubmission) {
    throw new Error(`No presentation submission present. Please use presentationSubmission opt argument!`);
  } else if (args.presentationDefinitions && presentationsWithFormat) {

[nklomp]

No that line should not be in opts

[nklomp]

Please reread from the top. Whenever a vp_token is requested by the RP a PE definition needs to be present. Then we require the PresentationExchange service to be present. That one can create the Presentation Submission using the definition from the RP and the VCs provided to it.
There is no situation where there is no submission or an empty submission when a vp_token is requested and thus a PE definition being present.

That is literally why those guards are in place

[sanderPostma]

Ah I see now it has the ... before responseOpts.presentationExchange so it should be in opts already. So the problem was my OPSession which was from before October 13th and I patched before it up by getting it from elsewhere.
I reverted that code

[nklomp]

Nonce should not endup in authorization response. That should be either in the id token or vp token

[sanderPostma]

const oid4vp = await verifyPresentations(this, verifyOpts);
const nonce = merged.nonce ?? verifiedIdToken?.payload.nonce ?? oid4vp.nonce;

When I remove nonce from AuthorizationResponsePayload it will not be in merged.nonce, I don't have idToken so I will need to debug verifyPresentations .

[nklomp]

As mentioned it needs to end either in the id token and/or vp token depending om which token(s) are requested. Since this comes from the OID request object it should never end up in oauth2 response payload directly

[nklomp]

So if there is no request object then there indeed is no nonce typically

[sanderPostma]

I reverted this. After retest with newer libraries this appears to be working again.

[sanderPostma]

Closing this PR, the left hand develop branch does not match what's actually in that branch atm