feat: custom verification callback for the authorization request on the holder side for verification #2303

berendsliedrecht · 2025-05-27T12:03:40Z

Signed-off-by: Berend Sliedrecht berend@animo.id

…the holder side for verification Signed-off-by: Berend Sliedrecht <berend@animo.id>

changeset-bot · 2025-05-27T12:03:44Z

🦋 Changeset detected

Latest commit: 03b0011

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 15 packages

Name	Type
@credo-ts/openid4vc	Patch
@credo-ts/action-menu	Patch
@credo-ts/anoncreds	Patch
@credo-ts/askar	Patch
@credo-ts/cheqd	Patch
@credo-ts/core	Patch
@credo-ts/didcomm	Patch
@credo-ts/drpc	Patch
@credo-ts/indy-sdk-to-askar-migration	Patch
@credo-ts/indy-vdr	Patch
@credo-ts/node	Patch
@credo-ts/question-answer	Patch
@credo-ts/react-native	Patch
@credo-ts/redis-cache	Patch
@credo-ts/tenants	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

TimoGlastra · 2025-05-27T12:21:57Z

Chore is generally for non feature related changes (e.g. preparing for a release). This is more a feature (as it allows you to do something that wasn't possible before)

TimoGlastra · 2025-05-27T12:23:31Z

packages/openid4vc/src/openid4vc-holder/OpenId4vpHolderService.ts

+      if (!result) {
+        throw new CredoError('verificationAuthorizationCallback returned false. User-provided validation failed.')
+      }


This will swallow any errors. I think it might be better to make the inner method throw. Or we at least return the error so we can add it as cause. i think requiring the callback to throw has my preference

TimoGlastra · 2025-05-27T12:26:07Z

can you add a changeset (just know i will keep commenting this on every PR, so might be good to just add it to every PR by default 😅)

TimoGlastra · 2025-05-27T12:27:48Z

packages/openid4vc/src/openid4vc-holder/OpenId4vpHolderServiceOptions.ts

 export interface ResolveOpenId4VpAuthorizationRequestOptions {
  trustedCertificates?: EncodedX509Certificate[]
  origin?: string
+  verifyAuthorizationRequestCallback?: VerifyAuthorizationRequestCallback


Soo looking at it like this, what does the callback provide over just doing

resolve

call custom callback method

The callback just adds another layer of indirection.

For the verifier side the callback makes more sense since we automatically send a http response (and you might want to block it).

also thinking whether the name of the callback may mislead into thinking it will override the verification (which it doesn't).

To continue on my previous comment. The input to the verify is just the authorization request, which is not always the full context (which you get returned from the method). So I'd actually lean to not merge this, and just call our custom callback after calling the credo resolve method

It enforces it slightly more, the callback is optional so it is not enforced strictly. The API will be extended before merging to get more values next to the authorizationRequest in the callback. I just need to find out everything that is required or can be nice to have in the verification context.

berendsliedrecht · 2025-05-27T12:49:05Z

can you add a changeset (just know i will keep commenting this on every PR, so might be good to just add it to every PR by default 😅)

Haha yes, I will!

…s the holder to do additional validation on the authorization request when resolving it Signed-off-by: Berend Sliedrecht <berend@animo.id>

…sed on the registration_certificate Signed-off-by: Berend Sliedrecht <berend@animo.id>

berendsliedrecht · 2025-05-28T10:54:19Z

packages/openid4vc/tests/openid4vc.e2e.test.ts

+    const holderTenant = await holder.agent.modules.tenants.getTenantAgent({ tenantId: holder1.tenantId })
+
+    await expect(
+      holderTenant.modules.openId4VcHolder.resolveOpenId4VpAuthorizationRequest(


Main interesting code, i.e. validation the registration certificate, happens here.

TimoGlastra · 2025-05-28T11:08:35Z

packages/openid4vc/src/openid4vc-holder/OpenId4vpHolderServiceOptions.ts


+export type VerifyAuthorizationRequestOptions = {
+  authorizationRequest: Openid4vpAuthorizationRequest | Openid4vpAuthorizationRequestDcApi
+  jwsService: JwsService


I think it makes more sense to pass the agentContext as the first param to the callback. The user can then inject anything they need. Attestations could be other things than JWTs as well, so it's too focused on a specific case.

If the context is passed the user can just do agentContext.resolve(JwsService)

Yes, makes sense

TimoGlastra · 2025-05-28T11:11:14Z

packages/openid4vc/tests/openid4vc.e2e.test.ts

+            for (const va of authorizationRequest.verifier_attestations) {
+              // Here we verify it as a registration certificate according to
+              // https://bmi.usercontent.opencode.de/eudi-wallet/eidas-2.0-architekturkonzept/flows/Wallet-Relying-Party-Authentication/#registration-certificate
+              if (va.format === 'jwt') {
+                if (typeof va.data !== 'string') {
+                  throw new Error('Only inline JWTs are supported')
+                }
+
+                const { isValid } = await jwsService.verifyJws(holder.agent.context, { jws: va.data })
+                const jwt = Jwt.fromSerializedJwt(va.data)
+
+                if (!isValid) {
+                  throw new Error('Invalid signature on JWT provided in the verifier_attestations')
+                }
+
+                if (jwt.header.typ !== 'rc-rp+jwt') {
+                  throw new Error(`only 'rc-rp+jwt' is supported as header typ. Request included: ${jwt.header.typ}`)
+                }
+
+                const registrationCertificateHeaderSchema = z
+                  .object({
+                    typ: z.literal('rc-rp+jwt'),
+                    alg: z.string(),
+                    // sprin-d did not define this
+                    x5u: z.string().url().optional(),
+                    // sprin-d did not define this
+                    'x5t#s256': z.string().optional(),
+                  })
+                  .passthrough()
+
+                // TODO: does not support intermediaries
+                const registrationCertificatePayloadSchema = z
+                  .object({
+                    credentials: z.array(
+                      z.object({
+                        id: z.string().optional(),
+                        format: z.string(),
+                        multiple: z.boolean().default(false),


I think this is too complex for what's being tested. We're testing the callback, and this adds the very complex structure of a registration certificate. I would like to keep this test simple, and add the EUDI specific validations to a new repository.

I agree that we are not testing what is relevant here. I can move it to another repo, that is fine.

TimoGlastra · 2025-05-28T11:14:12Z

packages/openid4vc/src/openid4vc-holder/OpenId4vpHolderServiceOptions.ts

 // TODO: export from oid4vp
 export type ParsedTransactionDataEntry = NonNullable<ResolvedOpenid4vpAuthorizationRequest['transactionData']>[number]

+export type VerifyAuthorizationRequestOptions = {


I feel we're still missing relevant information from the OpenId4VpResolvedAuthorizationRequest interface here.

Also we should try to stay as consistent as possible with OpenId4VpResolvedAuthorizationRequest.

So

authorizationRequestPayload: OpenId4VpAuthorizationRequestPayload

and ideally:

/** * Metadata about the signed authorization request. * * Only present if the authorization request was signed */ signedAuthorizationRequest?: { signer: VerifiedJarRequest['signer'] payload: VerifiedJarRequest['jwt']['payload'] header: VerifiedJarRequest['jwt']['header'] }

also add the origin (in case of DC-API):

/** * Origin of the request, to be used with Digital Credentials API */ origin?: string

Maybe we can just pass the same interface? Or do we call this before the DCQL and Presentation Exchange Queries are run? (in that case we can omit these from the interface). But i would rather not add client here if it's not also available in the resolved value.

https://github.com/openwallet-foundation/credo-ts/blob/main/packages/openid4vc/src/openid4vc-holder/OpenId4vpHolderServiceOptions.ts#L22

TimoGlastra · 2025-05-28T11:20:18Z

packages/openid4vc/src/openid4vc-holder/OpenId4vpHolderService.ts

+        })
+      } catch (e) {
+        throw new CredoError(
+          `verificationAuthorizationCallback returned false. User-provided validation failed. cause: ${e}`,


The method does not return false anymore

Suggested change

`verificationAuthorizationCallback returned false. User-provided validation failed. cause: ${e}`,

`error during call to user-provided verifyAuthorizationRequestCallback. cause: ${e}`,

… request Signed-off-by: Berend Sliedrecht <berend@animo.id>

Signed-off-by: Berend Sliedrecht <berend@animo.id>

TimoGlastra

One remaining comment

TimoGlastra · 2025-05-30T12:43:22Z

packages/openid4vc/src/openid4vc-holder/OpenId4vpHolderServiceOptions.ts


+export type VerifyAuthorizationRequestOptions = {
+  agentContext: AgentContext
+} & OpenId4VpResolvedAuthorizationRequest


transactionData, pex and dcql will never be defined, so just using the interface is misleading. Can we do a Pick on the interface?

Signed-off-by: Berend Sliedrecht <berend@animo.id>

…he holder side for verification (openwallet-foundation#2303) Signed-off-by: Berend Sliedrecht <berend@animo.id>

…he holder side for verification (openwallet-foundation#2303) Signed-off-by: Berend Sliedrecht <berend@animo.id> Signed-off-by: Ariel Gentile <gentilester@gmail.com>

chore: custom verification callback for the authorization request on …

798d673

…the holder side for verification Signed-off-by: Berend Sliedrecht <berend@animo.id>

berendsliedrecht requested a review from a team as a code owner May 27, 2025 12:03

TimoGlastra reviewed May 27, 2025

View reviewed changes

berendsliedrecht changed the title ~~chore: custom verification callback for the authorization request on the holder side for verification~~ feat: custom verification callback for the authorization request on the holder side for verification May 27, 2025

berendsliedrecht added 2 commits May 27, 2025 14:52

docs(changeset): Added feature to add a verifyAuthorizationCallback a…

8eff971

…s the holder to do additional validation on the authorization request when resolving it Signed-off-by: Berend Sliedrecht <berend@animo.id>

tests: added better callback for verifier_attestation verification ba…

ae03ac5

…sed on the registration_certificate Signed-off-by: Berend Sliedrecht <berend@animo.id>

berendsliedrecht force-pushed the authorization-request-verification-callback branch from 45d4b54 to ae03ac5 Compare May 28, 2025 10:53

berendsliedrecht commented May 28, 2025

View reviewed changes

TimoGlastra reviewed May 28, 2025

View reviewed changes

tests: add test for registration certificate for valid and overasking…

cc916d4

… request Signed-off-by: Berend Sliedrecht <berend@animo.id>

berendsliedrecht force-pushed the authorization-request-verification-callback branch from 775bf07 to cc916d4 Compare May 28, 2025 13:36

chore: found a matching, if more meta+format match

d4b82f9

Signed-off-by: Berend Sliedrecht <berend@animo.id>

TimoGlastra approved these changes May 30, 2025

View reviewed changes

chore: removed eudi specific functionality

03b0011

Signed-off-by: Berend Sliedrecht <berend@animo.id>

berendsliedrecht force-pushed the authorization-request-verification-callback branch from f36d781 to 03b0011 Compare May 30, 2025 12:46

berendsliedrecht merged commit 66d3384 into openwallet-foundation:main Jun 2, 2025
34 of 38 checks passed

berendsliedrecht deleted the authorization-request-verification-callback branch June 2, 2025 09:34

berendsliedrecht added a commit to animo/credo-ts that referenced this pull request Jun 4, 2025

feat: custom verification callback for the authorization request on t…

d88a0ed

…he holder side for verification (openwallet-foundation#2303) Signed-off-by: Berend Sliedrecht <berend@animo.id>

	`verificationAuthorizationCallback returned false. User-provided validation failed. cause: ${e}`,
	`error during call to user-provided verifyAuthorizationRequestCallback. cause: ${e}`,

feat: custom verification callback for the authorization request on the holder side for verification #2303

feat: custom verification callback for the authorization request on the holder side for verification #2303

Uh oh!

Conversation

berendsliedrecht commented May 27, 2025

Uh oh!

changeset-bot bot commented May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

TimoGlastra commented May 27, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TimoGlastra commented May 27, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

berendsliedrecht commented May 27, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TimoGlastra left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

changeset-bot bot commented May 27, 2025 •

edited

Loading