Skip to content

Inform user of revoked credentials #765

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

Closed
wants to merge 5 commits into from

Conversation

Gavinok
Copy link
Contributor

@Gavinok Gavinok commented May 12, 2025

This PR resolves #704

I have added a new "Revoked" state to auth sessions that will be displayed as seen below when the provided credential has been revoked

image

In addition, the failed state has now had the revoked possibility removed from the list of possible errors. Something we should consider looking into is if the other possible errors would make sense. "Expired" credentials could mean a failed requested_predicate which we should specify.

I also am not sure what we could really mean by a "missing" credential. Is this for when a wallet holds a credential which is not on the ledger? This should not be possible with the BC wallet

Gavin Jaeger-Freeborn added 3 commits May 9, 2025 11:09
Signed-off-by: Gavin Jaeger-Freeborn <gavinfreeborn@google.com>
Signed-off-by: Gavin Jaeger-Freeborn <gavinfreeborn@google.com>
@Gavinok Gavinok requested review from esune and loneil May 12, 2025 19:52
@coveralls
Copy link

coveralls commented May 12, 2025

Pull Request Test Coverage Report for Build 15006433657

Details

  • 12 of 12 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 88.02%

Totals Coverage Status
Change from base Build 14889016103: 0.2%
Covered Lines: 720
Relevant Lines: 818

💛 - Coveralls

@Gavinok Gavinok marked this pull request as draft May 12, 2025 21:15
Signed-off-by: Gavin Jaeger-Freeborn <gavinfreeborn@google.com>
@Gavinok Gavinok marked this pull request as ready for review May 13, 2025 18:00
Copy link
Member

@esune esune left a comment

Choose a reason for hiding this comment

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

Thinking about the message in the UI: can we surface which credential is revoked, or just that "a credential" is revoked? In scenarios where the proof-request includes more than one credential restriction (e.g.: Person credential and LSBC credential for BCGov's lawyer use-case) it would be helpful to know which credential did not pass the revocation check. If this is not possible, we may just want to update the message to indicate that one or more crdentials that are part of the response to the proof-request ae revoked.

Signed-off-by: Gavin Jaeger-Freeborn <gavinfreeborn@google.com>
@Gavinok
Copy link
Contributor Author

Gavinok commented May 13, 2025

Thinking about the message in the UI: can we surface which credential is revoked, or just that "a credential" is revoked? In scenarios where the proof-request includes more than one credential restriction (e.g.: Person credential and LSBC credential for BCGov's lawyer use-case) it would be helpful to know which credential did not pass the revocation check. If this is not possible, we may just want to update the message to indicate that one or more crdentials that are part of the response to the proof-request ae revoked.

Ya we can get the cred_def_id so from there we can get the tag. Would that be enough or is there something else we would need?

@esune
Copy link
Member

esune commented May 13, 2025

For missing: I do not think that use-case is currently possible, at least not with BC Wallet, as you would be prevented from submitting the response if you don;t hold all the credentials that are required. In general, it could be that the proof-request requires one or more credentials and the response does not send the correct ones in the response - unsure on how likely this scenario is though.

Thinking about the message in the UI: can we surface which credential is revoked, or just that "a credential" is revoked? In scenarios where the proof-request includes more than one credential restriction (e.g.: Person credential and LSBC credential for BCGov's lawyer use-case) it would be helpful to know which credential did not pass the revocation check. If this is not possible, we may just want to update the message to indicate that one or more crdentials that are part of the response to the proof-request ae revoked.

Ya we can get the cred_def_id so from there we can get the tag. Would that be enough or is there something else we would need?

For troubleshooting purposes knowing the cred_def_id would certainly be helpful as it would allow us to figure out which credential is causing the verification to fail.

@Gavinok
Copy link
Contributor Author

Gavinok commented May 14, 2025

I realized that the revoked field I am using is unfortunately not only filled for revoked fields based on further testing. Jamie has suggested trying to use the /issued/details endpoint so I will try migrating the is_revoked method to using it

@Gavinok
Copy link
Contributor Author

Gavinok commented May 15, 2025

What should be informing us about the revoked credential is the verified_msgs we get back which seem to be empty. I have opened an issue to track the handling of this openwallet-foundation/acapy#3724

For more context see https://github.com/openwallet-foundation/acapy/blob/main/open-api/swagger.json#L13336

@Gavinok Gavinok closed this May 15, 2025
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.

If a completed present proof 2.0 webhook is not verified (failed state) see what we can log and/or bring to the UI
3 participants