Skip to content

Add further details of how errors are returned from Browser API including a example error response #204

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
jogu opened this issue Jul 2, 2024 · 10 comments · Fixed by #556
Assignees
Milestone

Comments

@jogu
Copy link
Collaborator

jogu commented Jul 2, 2024

As per @awoie comment here https://github.com/openid/OpenID4VP/pull/155/files#r1652002661:

It would make sense to add an error response example as well.

and @marcoscaceres comment here https://github.com/openid/OpenID4VP/pull/155/files#r1660602655:

Errors should probably result in promise rejections, no?

@marcoscaceres
Copy link
Contributor

it would be good to discuss here and in the DC spec itself how we handle particular types of errors and response errors... we need to clarify what's "exceptional" (causing the promise to reject in the browser API), and what's expected as response data when there is some error.

@jogu
Copy link
Collaborator Author

jogu commented Jul 3, 2024

I agree @marcoscaceres - it would probably be good to have a discussion about errors in a WICG call. I had a look and this issue from Rick is probably a place to discuss it: w3c-fedid/digital-credentials#130

We may also need to update the example javascript call to the digital credential API in the VP spec depending on the outcome.

@jogu jogu added the dc-api label Jul 4, 2024
@Sakurann
Copy link
Collaborator

discussed during the WG call:

  • we need to clearly define how error codes currently defined in https://openid.github.io/OpenID4VP/openid-4-verifiable-presentations-wg-draft.html#section-6.4 are returned in the openid4vp response over the browser API. Current assumption is as members in the JS object.
  • need examples for the above.
  • out of the error codes defined in section 6.4, some probably don't apply in browser API profile, or can be used in both, like invalid_request? -> parallel discussion happening in W3C.
  • need a separate discussion if which error codes should not be returned from privacy perspective are different between vanilla openid4vp and browser api profile. -> please open a separate issue on this.

@jogu
Copy link
Collaborator Author

jogu commented Mar 27, 2025

I think wallet implementations are currently behaving inconsistently here, so I do think we need to clarify this.

It is definitely expected that an error before the user has actually selected a credential to return will result in an exception that has no detail (to avoid leaking any information without user interaction).

Once the user has selected a credential there's definitely an argument that a more detailed error can be returned. At least in the current Android implementation my understanding is that the only way to return a more detailed error to the verifier/caller is to return it in the same way you would return a successful response, i.e. the DC API would return a Credential object where data contains a OID4VP response that has error & potentially error_description.

@jogu jogu modified the milestones: 1.1, Final 1.0 Apr 5, 2025
@jogu jogu added the discuss label Apr 5, 2025
@jogu
Copy link
Collaborator Author

jogu commented Apr 5, 2025

This is causing some issues for the certification team as we have tests that deliberately generate errors and need to know how to process an error response.

Moved to 1.0 & added 'discuss' in the hope we might be able to quickly reach a consensus on this.

@jogu
Copy link
Collaborator Author

jogu commented Apr 5, 2025

My suggestion would be we add text saying that errors coming from the wallet (and hence after the user has indicated some willingness to share with the verifier) are returned as an OID4VP response and not as a promise rejection.

My main reluctance against promise rejection is that (at least in the google implementation I tried) it seems any information the wallet returns seems to be replaced with simply "Network error" so the developer doesn't get access to the actual error from the wallet. (Would be good to know if this is by design / specced by W3C like that / ...). I could be persuaded to go in that direction if the verifier does actually get error info that occurs post-credential selection & that wallet is willing to share

@bc-pi
Copy link
Member

bc-pi commented Apr 6, 2025

My suggestion would be we add text saying that errors coming from the wallet (and hence after the user has indicated some willingness to share with the verifier) are returned as an OID4VP response and not as a promise rejection.

This seems quite reasonable on the face of it.

@Sakurann
Copy link
Collaborator

Sakurann commented Apr 7, 2025

WG discussion:

  • privacy risks understood. nevertheless, direction seems to be to follow Joseph's suggestions, while giving a clear guidance of the risks.
    • definitely more guidance needed for promise rejection
  • but do an IIW session on it (@timcappalli to lead it) and do a PR on what has been roughly agreed on there.
  • we might be able to add this during WGLC or even 1.1 ..

@Sakurann Sakurann removed the discuss label Apr 7, 2025
@Sakurann
Copy link
Collaborator

Sakurann commented Apr 15, 2025

direction seems to be

  • Success at the DC API layer means you got a syntactically valid response (may be a protocol error) from a credential provider
  • Error at the DC API layer means you didn’t get a syntactically valid response from a credential provider (including the credential provider not being able to parse a request)

cc @timcappalli @marcoscaceres @leecam and notes

  • need to be careful with privacy considerations; especially say do not return errors when value matching was performed.

@Sakurann
Copy link
Collaborator

@timcappalli if you could do the PR, @GarethCOliver could help with the privacy considerations

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants