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

Add explanation of why the account argument is useful #219

Closed
leshi opened this issue Sep 23, 2016 · 10 comments
Closed

Add explanation of why the account argument is useful #219

leshi opened this issue Sep 23, 2016 · 10 comments

Comments

@leshi
Copy link
Contributor

leshi commented Sep 23, 2016

Richard Barnes wrote:

I can't remember if we talked about this before. Would it make sense to move the account argument to makeCredential into the options dictionary? It seems like there are at least some credential types that don't require it (e.g., U2F credentials), and it makes the interface a bit simpler.

I agree.

@leshi leshi self-assigned this Sep 23, 2016
@leshi leshi added this to the WD-03 milestone Sep 23, 2016
leshi added a commit that referenced this issue Sep 23, 2016
@vijaybh
Copy link
Contributor

vijaybh commented Sep 23, 2016

Two concerns with this:

  1. What should an authenticator with onboard storage do if a caller creates a credential without this option, then asks for an assertion without specifying a credential ID? In that case the authenticator will have nothing to show in its chooser UI.
  2. Our solution to the "train in a tunnel" scenario of creating orphaned credentials was to say that the authenticator will not create multiple credentials for the same account ID. If we make the account ID optional this becomes a lot more complicated.

Would it just be better to always have this information from the caller, even if it was not always needed? It's not clear to me that going for argument-level minimalism here is worth the added complexity to both web developers and implementers from making this optional.

@equalsJeffH
Copy link
Contributor

equalsJeffH commented Sep 27, 2016

@vijaybh wrote:

What should an authenticator with onboard storage do if a caller creates a credential without this option, then asks for an assertion without specifying a credential ID?

by "without specifying a credential ID", you mean without specifying an options.allowList (which contains ScopedCredentialDescriptions containing cred ID), yes?

Overall, I'm tending to agree with @vijaybh's assessment.

@vijaybh
Copy link
Contributor

vijaybh commented Sep 28, 2016

Yes, thanks for the more precise statement @equalsJeffH - that's what I meant. In that case such a rich authenticator would normally be able to show a chooser for the user to pick a credential, but won't be able to fully populate the chooser if credentials were created with no account info.

@bifurcation
Copy link
Contributor

@vijaybh It seems like you're arguing that the web site has to make things pretty. The account info just tells the UA what to put in a chooser in order to make it easier for a user to understand (aside: note that this raises the possibility of misdirection). If the RP doesn't provide this information, the UA can still present a list of credentials associated with this origin (probably just one in practice) or choose to omit credentials that have no pretty UX info attached.

ISTM that your concern points more toward removing allowList from optional to required than to requiring account. Filed #221 for this.

@vijaybh
Copy link
Contributor

vijaybh commented Sep 28, 2016

@bifurcation I don't think this is true. The account info tells the authenticator what to put in a chooser - this is out of reach of the UA once the credential is created. This allows for roaming authenticators with local storage to do initial logon (as opposed to U2F-style password supplement).

Scenario is something like this: the user comes up to a new machine with his rich authenticator, plugs it in to USB for instance, and clicks the log in button on foo.com. That site calls getAssertion without a credential ID (i.e., without options.allowlist). The authenticator shows the user what credentials they have for foo.com so they can pick one. The user selects one, and that assertion gets generated. The site verifies the assertion, and the user is logged on.

In such situations one cannot require allowList because the site does not (and should not) yet know who the user is.

@leshi
Copy link
Contributor Author

leshi commented Sep 28, 2016

I agree with the sentiment that having this and not using this (e.g., U2F-like authenticators) is better than not having it and winding up in undefined behavior territory (e.g., rich, platform/built-in authenticator).

I'm now in favor of abandoning this PR. :)

@jcjones
Copy link
Contributor

jcjones commented Sep 28, 2016

I'll write up an alternative PR for some non-normative text to explain why the account argument is useful.

@equalsJeffH equalsJeffH assigned jcjones and unassigned leshi Oct 12, 2016
@bifurcation bifurcation self-assigned this Nov 2, 2016
@equalsJeffH equalsJeffH modified the milestones: WD-04, WD-03 Nov 9, 2016
@vijaybh vijaybh changed the title Move account argument to options Add explanation of why the account argument is useful Feb 15, 2017
@vijaybh
Copy link
Contributor

vijaybh commented Feb 15, 2017

Changing title to reflect the current purpose of this bug

@selfissued
Copy link
Contributor

If there's a chance that we're going to move the account argument to the options and therefore change the API, we should decide this one way or the other before declaring an Implementer's Draft. If we've decided not to ever do this, we should document this in a comment in the issue.

@jcjones
Copy link
Contributor

jcjones commented Apr 5, 2017

This is pretty much overcome-by-events in the alignment with Credential Management's account concepts in #384. Closing.

@jcjones jcjones closed this as completed Apr 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants