-
Notifications
You must be signed in to change notification settings - Fork 196
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: resolve undefined credential_definition.type in credentialRequest #1989
fix: resolve undefined credential_definition.type in credentialRequest #1989
Conversation
|
@@ -32,7 +32,7 @@ export type OpenId4VciIssuerMetadata = OpenId4VciIssuerMetadataV1Draft11 | OpenI | |||
export type OpenId4VciIssuerMetadataDisplay = MetadataDisplay | |||
|
|||
export type OpenId4VciCredentialRequest = | |||
| CredentialRequestJwtVcJson | |||
| (CredentialRequestJwtVcJson & { credential_definition: { type: string[] } }) |
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.
I think we forgot to update the credential request type when adding support for V13.
I think we can do:
export type OpenId4VciCredentialRequest = UniformCredentialRequest
UniformCredentialRequest contains both draft 11 and draft 13 types. And for the others we can then do:
export type OpenId4VciCredentialRequestJwtVcJson = CredentialRequestJwtVcJson | CredentialRequestJwtVcJsonV1_0_13
and also for all the other format specific ones
/* | ||
Issue: https://github.com/openwallet-foundation/credo-ts/issues/1963 | ||
Handling `credentialRequest.types` by checking if `credentialRequest.credential_definition` is not `undefined`. | ||
`credentialRequest.credential_definition` requires a `type` attribute which does not currently exist. | ||
Therefore, adding the `type` attribute at line 41 in `packages/openid4vc/src/shared/models/index.ts`. | ||
*/ | ||
|
||
credentialRequest.types = credentialRequest.credential_definition | ||
? credentialRequest.credential_definition.type | ||
: credentialRequest.types | ||
|
||
return equalsIgnoreOrder(offeredCredential.credential_definition.type ?? [], credentialRequest.types) |
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.
/* | |
Issue: https://github.com/openwallet-foundation/credo-ts/issues/1963 | |
Handling `credentialRequest.types` by checking if `credentialRequest.credential_definition` is not `undefined`. | |
`credentialRequest.credential_definition` requires a `type` attribute which does not currently exist. | |
Therefore, adding the `type` attribute at line 41 in `packages/openid4vc/src/shared/models/index.ts`. | |
*/ | |
credentialRequest.types = credentialRequest.credential_definition | |
? credentialRequest.credential_definition.type | |
: credentialRequest.types | |
return equalsIgnoreOrder(offeredCredential.credential_definition.type ?? [], credentialRequest.types) | |
// credential_definition.type is draft 13 and .types is draft 11 | |
const types = credentialRequest.credential_definition | |
? credentialRequest.credential_definition.type | |
: credentialRequest.types | |
return equalsIgnoreOrder(offeredCredential.credential_definition.type ?? [], types) |
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.
Thanks for the PR @parametprame! I left some comments / suggestions, as we can use the types form the sphereon library
I run into same problem playing with the demo, however although this PR seem to fix the original problem, it seems I am still hitting some subsequent outstanding issue. Didn't have time to investigate, so I am just reporting this finding. ISSUER:
HOLDER:
|
@Patrik-Stas Did you selected Accept the credential offer. more than once? |
Hey @parametprame it seems your commit hasn't been signed off. Can you follow https://github.com/hyperledger/aries-rfcs/blob/main/contributing.md#signing-off-commits-dco? |
Signed-off-by: Paramet Kongjaroen <parametprame2@gmail.com>
Signed-off-by: Paramet Kongjaroen <parametprame2@gmail.com>
ee50738
to
1aa1623
Compare
@TimoGlastra I've signed it. Thanks for letting me know. |
This fixes the issue reported in (#1963).