Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(openid4vc): several fixes and improvements #1795

Conversation

TimoGlastra
Copy link
Contributor

Enable issuanceMetadata to be provided during offer creation, to add some context to an issuance session. Can be used to already provide the credential data, store an user id so the data can be fetched at time of issuance, or other metadata.

This was already supported, but just not exposed in the API.

Non breaking and can be released as 0.5.1

@TimoGlastra TimoGlastra changed the title feat(openid4vc): allow to set issuance metadata fix(openid4vc): several fixes and improvements Mar 17, 2024
Signed-off-by: Timo Glastra <[email protected]>
Copy link
Contributor

@berendsliedrecht berendsliedrecht left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of minor points, LGTM!

packages/core/src/utils/transformers.ts Show resolved Hide resolved
type: openBadgeCredential.types,
issuer: new W3cIssuer({ id: issuerDid }),
credentialSubject: new W3cCredentialSubject({ id: holderDid }),
issuanceDate: w3cDate(Date.now()),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not related to your PR, but looking at all the W3cXXX classes, it might be worth considering to make the W3cDate a class as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah maybe, or we can make it a date instance and do the transformation for the user

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to address this in a separate PR, as it will require quite some changes across the codebase 😊

Signed-off-by: Timo Glastra <[email protected]>
@TimoGlastra TimoGlastra enabled auto-merge (squash) March 18, 2024 10:59
@TimoGlastra TimoGlastra merged commit b83c517 into openwallet-foundation:main Mar 18, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants