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(anoncreds): unqualified revocation registry processing #1833

Conversation

genaris
Copy link
Contributor

@genaris genaris commented Apr 13, 2024

I ran into issues issues while receiving legacy Indy credentials with revocation support, e.g. receiving the following error when using BC Digital Trust Showcase:

CredoError: QEquAHkM35w4XVT3Ku5yat:4:QEquAHkM35w4XVT3Ku5yat:3:CL:567943:student_card:CL_ACCUM:dd6cecd9-a71b-4af4-a8f6-26a3fee2e073 is not a valid did:indy did\n at parseIndyDid at getUnQualifiedDidIndyDid at getW3cRecordAnonCredsTags

I found that record metadata was populated with revocationRegistryId from the W3C credential object, which in this case would be an unqualified identifier. As getUnQualifiedDidIndyDid expected a qualified identifier, this fails. In addition, the utility method getQualifiedDidIndyDid was not taking into account credentialDefinitionTag while generating the identifier, resulting also in an error.

In this PR this is fixed by explicitly specifying the revocationRegistryId when calling AnonCredsRsHolderService.storeW3cCredential rather than taking it from the W3C credential. I'm not sure if this is the most elegant solution though, so any suggestion on that will be more than appreciated.

Another a bit controversial thing I changed here is to delay the w3cCredentialService.storeCredential() call to the last minute, in order to prevent any possible error to be thrown that could leave a W3cCredentialRecord without tags and metadata.

Signed-off-by: Ariel Gentile <gentilester@gmail.com>
Copy link
Contributor

@TimoGlastra TimoGlastra left a comment

Choose a reason for hiding this comment

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

Great fixes 👍

@TimoGlastra
Copy link
Contributor

@auer-martin this might be related to openwallet-foundation/bifold-wallet#1115 (comment)?

@auer-martin
Copy link
Contributor

Yes, that probably is the issue 👍🏻

@TimoGlastra TimoGlastra merged commit edc5735 into openwallet-foundation:main Apr 18, 2024
12 checks passed
@genaris genaris deleted the fix/anoncreds-unqualified-revregid branch April 18, 2024 15:26
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.

3 participants