-
Notifications
You must be signed in to change notification settings - Fork 197
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
feat: vci13 support draft #1942
feat: vci13 support draft #1942
Conversation
🦋 Changeset detectedLatest commit: 6847141 The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 packages
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 |
Signed-off-by: Martin Auer <martin.auer97@gmail.com>
Signed-off-by: Martin Auer <martin.auer97@gmail.com>
641fac2
to
353440b
Compare
Signed-off-by: Martin Auer <martin.auer97@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great stuff on the update martin, and making sure we keep supporting both draft 11 and draft 13. The typing feels a bit off though, and it would be good if you can spend some time to make sure we are aligned, as I found several things that didn't seem to align with supporting both 11 and 13
packages/openid4vc/src/openid4vc-holder/OpenId4VciHolderService.ts
Outdated
Show resolved
Hide resolved
packages/openid4vc/src/openid4vc-holder/OpenId4VciHolderService.ts
Outdated
Show resolved
Hide resolved
packages/openid4vc/src/openid4vc-issuer/OpenId4VcIssuerService.ts
Outdated
Show resolved
Hide resolved
clientId, | ||
credentialIssuer: metadata.issuer, | ||
// eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
endpointMetadata: metadata as unknown as any, // will be v11 / v13 based on version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does it need to be unknown? I rather not use it outside of tests. We can probably do something with an if / else statement to get the correct typing witout having to cast?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried it out, not sure but typing the metadata according to the version does not work quite well, and probably requires casts anyways so I would keep this
packages/openid4vc/src/openid4vc-holder/OpenId4VciHolderService.ts
Outdated
Show resolved
Hide resolved
Can you also add a changeset? |
Signed-off-by: Martin Auer <martin.auer97@gmail.com>
…atibility Signed-off-by: Martin Auer <martin.auer97@gmail.com>
Signed-off-by: Martin Auer <martin.auer97@gmail.com>
52ecf48
to
701a978
Compare
Signed-off-by: Martin Auer <martin.auer97@gmail.com>
701a978
to
a5557eb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did another thorough review. Most of it looks good, but there's some loose ends to finish. I think most things are quite quick to resolve
packages/openid4vc/src/openid4vc-holder/OpenId4VciHolderServiceOptions.ts
Outdated
Show resolved
Hide resolved
@@ -197,14 +197,16 @@ describe('OpenId4Vc', () => { | |||
await issuerTenant1.modules.openId4VcIssuer.createCredentialOffer({ | |||
issuerId: openIdIssuerTenant1.issuerId, | |||
offeredCredentials: [universityDegreeCredentialSdJwt.id], | |||
preAuthorizedCodeFlowConfig: { userPinRequired: false }, | |||
preAuthorizedCodeFlowConfig: {}, // { txCode: { input_mode: 'numeric', length: 4 } }, // TODO: disable due to sphereon limitations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this resolved with Sphereon-Opensource/OID4VC#122?
What needs to happen here?
packages/openid4vc/src/openid4vc-issuer/OpenId4VcIssuerService.ts
Outdated
Show resolved
Hide resolved
packages/openid4vc/src/openid4vc-holder/OpenId4VciHolderService.ts
Outdated
Show resolved
Hide resolved
packages/openid4vc/src/openid4vc-holder/OpenId4VciHolderService.ts
Outdated
Show resolved
Hide resolved
packages/openid4vc/src/openid4vc-holder/OpenId4VciHolderService.ts
Outdated
Show resolved
Hide resolved
packages/openid4vc/src/openid4vc-holder/OpenId4VciHolderService.ts
Outdated
Show resolved
Hide resolved
Signed-off-by: Martin Auer <martin.auer97@gmail.com>
d60f945
to
a6181bf
Compare
Signed-off-by: Martin Auer <martin.auer97@gmail.com>
Signed-off-by: Martin Auer <martin.auer97@gmail.com>
af1e7a2
to
2cae342
Compare
Signed-off-by: Martin Auer <martin.auer97@gmail.com>
No description provided.