feat: batch issuance#346
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds initial support for batch credential issuance by allowing multiple proofs/keys to be processed and returning multiple credentials in a single OpenID4VCI credential response.
Changes:
- Extend proof parsing to extract multiple JWKs (including multiple
attested_keysfrom key attestations). - Update issuer credential handler to issue one credential per extracted JWK and return a
credentials[]array. - Add unit tests for batch JWK extraction/counting and basic batch request structuring.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/openid4vci/proof_attestation.go | Adds ExtractAllJWKs() to return all attested keys; refactors ExtractJWK() to reuse it. |
| pkg/openid4vci/credential.go | Adds Proofs.ExtractAllJWKs() and Proofs.Count() to support batch flows. |
| pkg/openid4vci/credential_test.go | Adds tests for ExtractAllJWKs() and Proofs.Count(). |
| internal/apigw/apiv1/handlers_issuer.go | Implements batch issuance loop and batch-size validation; refactors issue helpers to return credential strings. |
| internal/apigw/apiv1/handlers_issuer_test.go | Adds tests around batch proof extraction and batch request structure. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| t.Run("single JWT proof", func(t *testing.T) { | ||
| proofs := &Proofs{JWT: []ProofJWTToken{mockProofJWT}} | ||
| jwks, err := proofs.ExtractAllJWKs() | ||
| assert.NoError(t, err) | ||
| assert.Len(t, jwks, 1) |
There was a problem hiding this comment.
TestExtractAllJWKs only exercises the JWT branch. Since ExtractAllJWKs() now has separate paths for DIVP and Attestation, add subtests that cover those branches (including an attestation with multiple attested_keys) to prevent regressions in batch issuance behavior.
…o get the batch size. - `Proofs.Count()` would need to extract all JWKs to get the correct count for attestations, and at that point it makes more sense to not use it and simply check the length of the already extracted JWKs
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
smncd
force-pushed
the
feat/wip-batch-issuance
branch
from
f2bc06d to
f761ded
Compare
| var jwks []*apiv1_issuer.Jwk | ||
| if req.Proof != nil { | ||
| jwk, err = req.Proof.ExtractJWK() | ||
| jwk, err := req.Proof.ExtractJWK() |
There was a problem hiding this comment.
i think proof is an compromise from when wallet did not support proofs, not sure however. If wallet supports proofs then this could be removed.
There was a problem hiding this comment.
My understanding is Proofs is the way forward in the spec.
There was a problem hiding this comment.
can we skip the support for Proof now?
There was a problem hiding this comment.
We can remove it I think
- issueVC20 drops the jwks parameter entirely — it was never consumed. - issueVC20 rejects proofs that are not JWTs
…ch statement to decide which type is to be used
| var jwks []*apiv1_issuer.Jwk | ||
| if req.Proof != nil { | ||
| jwk, err = req.Proof.ExtractJWK() | ||
| jwk, err := req.Proof.ExtractJWK() |
| } | ||
|
|
||
| // Save credential subject information to the registry for status management. | ||
| func (c *Client) saveCredentialSubjects(ctx context.Context, document *model.CompleteDocument, entries []statusEntry) { |
There was a problem hiding this comment.
So, exit the loop if a save fails and return the error?
There was a problem hiding this comment.
yes, it's a major fault if a credential not getting saved in registry. If that happens the status of that credential won't be able to change its status.
There was a problem hiding this comment.
Noted, sounds good. Will push a fix asap.
| } | ||
|
|
||
| // AssertSingleProofType checks that only one proof type is used per request. | ||
| func (p *Proofs) AssertSingleProofType() (string, error) { |
There was a problem hiding this comment.
nope, use validator v10, pkg/helpers
There was a problem hiding this comment.
There was a problem hiding this comment.
That could work, I wonder if it would work in a situation where two different proof types are given. Since we want to make sure only one is present.
Since the docs for this says:
The field under validation must be present and not empty only when any of the other specified fields are not present
There was a problem hiding this comment.
yes, from manual: Usage: required_without=Field1 Field2
means that the fieldd using the obove struct tag munst not be present if any of Field1 or Field2 are not nil.
if that work we dont need a custom validaton function and that's a goal in it self :)
|
A quick and messy implementation of batch issuance of credentials. I did this for a unrelated demo that needed batch-issued creds, putting it here in case it turns out it's useful. Probably needs work.