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

Define error handling #130

Open
RByers opened this issue Jun 21, 2024 · 4 comments · May be fixed by #156
Open

Define error handling #130

RByers opened this issue Jun 21, 2024 · 4 comments · May be fixed by #156
Assignees
Labels

Comments

@RByers
Copy link
Member

RByers commented Jun 21, 2024

From discussion in F2F meeting today, we have a few goals:

  • No silent information disclosure (eg. can't reveal no matching credential as distinct from user cancelled)
  • Developers have a path to diagnose deployment issues

Our conclusions were:

  • Most errors will come from the higher level protocol. In that case the wallet should return a protocol-specific object describing the error and the API will technically be a success. It's up to the wallet to ensure whatever information returned fits with their privacy model based on what consent has been collected from the user.
  • We will have a few errors at our API level. Once the user has given permission to share a credential, we believe it's OK for us to reveal to the verifier whether there was a matching credential or not (even though they may not have provided consent to the wallet to share any information in particular).
  • We want to re-use existing DOMException error names and don't believe it should be necessary to define any new ones.
  • Wallets will also have the ability to just return a "user cancelled" error which will just return to the credential selector screen.

So the work now is probably twofold:

  1. Add spec text (or a note) that describes that API success doesn't necessarily. mean a credential is being returned - there may be a protocol error.
  2. Identify appropriate errors for the API to return.
@RByers
Copy link
Member Author

RByers commented Aug 12, 2024

From discussion in the call today, we (Android/Chrome) are arguing that these two cases must be indistinguishable (consistent with WebAuthn):

  • User declines to share a credential they have which matches the criteria
  • User declines to share any credential because they have none (and opted not to use the cross-device flow)

But once a user has said they're willing to share a credential, then in general it's fine to propagate rich error information (but that's all likely at the protocol level).

It sounds like this bit isn't controversial? We just need to formalize the details in the spec.

Another point from the call today: will front-ends parse some of the protocol response or leave that entirely to their back-end (who have the keys to decrypt the credential presentation)? Perhaps we should provide a mechanism for wallets to be able to trigger a specific DOMException, to be used for front-end error handling? Alternately, should we encourage protocols to have an unencrypted portion intended to be handled in the front-end like for error handling?

@marcoscaceres
Copy link
Collaborator

marcoscaceres commented Aug 13, 2024

@RByers, I think #156 and #143 will address this (together with what we already have in the spec)... but basically, we have the order in which stuff gets checked and errors get thrown now.

Can you confirm?... if so, I'll say that #156 closes this.

@marcoscaceres marcoscaceres linked a pull request Aug 13, 2024 that will close this issue
8 tasks
@RByers
Copy link
Member Author

RByers commented Aug 20, 2024

Hmm, I'm not sure if #156 helps or hurts actually. Is it implicit that all such validation rules must be stateless (not influenced by the contents of the user's wallet)? If not then there's possibly some risk of information leakage based on whether a given request throws a TypeError or not.

I think addressing this issue is really in the algorithm for getting the credential presentation. Like perhaps we need a step something like "Get explicit user permission for allowing this website to communicate with the selected wallet application now". Before that step all errors must reveal nothing about the user's personal state, while after that step it's up to the wallet what's revealed. Presumably we won't actually specify anything about the credential store, just about how the user agent / OS communicates with a wallet application, right? If so then I think this issue is addressed by a combination of that architecture and a statement saying users must give permission before information is exchanged with the wallet on each and every invocation.

@npm1
Copy link

npm1 commented Sep 26, 2024

At TPAC we had a breakout where we tried to dig into this but we didn't. I'm not familiar with the DigitalCredentials errors being exposed right now but I can explains what we are proposing in w3c-fedid/FedCM#498 to see what kind of coordination we would like to have. Filing this issue to perhaps get the interested folks in a call at some point to flesh this out? Note: surfacing new errors / discussions on the privacy implications of doing so is out of scope for this issue, I'm more interested in looking at the current status quo of each spec so that we can confidently merge the relevant FedCM PR.

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

Successfully merging a pull request may close this issue.

4 participants