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

Contradiction in whether user handle is required #720

Closed
emlun opened this issue Dec 8, 2017 · 7 comments · Fixed by #721 or #730
Closed

Contradiction in whether user handle is required #720

emlun opened this issue Dec 8, 2017 · 7 comments · Fixed by #721 or #730

Comments

@emlun
Copy link
Member

emlun commented Dec 8, 2017

The definition of Public Key Credential Source includes:

[...] A public key credential source has:

  • [...]
  • An optional user handle for the person who created this credential source.

However the user handle is required in PublicKeyCredentialUserEntity. We should drop the "optional" from the above bullet point.

@emlun emlun added this to the CR milestone Dec 8, 2017
@emlun emlun self-assigned this Dec 8, 2017
emlun added a commit that referenced this issue Dec 8, 2017
@Kieun
Copy link
Member

Kieun commented Dec 12, 2017

U2F authenticator does not store any user identifier (user handle) for credentials. Even though we send a user handle in PublicKeyCredentialUserEntity during registration, the U2F authenticator ignores it when generating a public key credential. So, I think we don't have to drop "optional" for user handle.

@emlun
Copy link
Member Author

emlun commented Dec 12, 2017

The RP is required to provide the user handle when creating a new credential: #558 (comment) So I do think it makes sense to drop "optional" here. U2F devices will ignore it, and as that comment points out they won't be expected to return it since they're always used in 2nd factor mode.

However, I think that also means we need to change authenticatorGetAssertion step 13 and up the stack to include this behaviour.

@Kieun
Copy link
Member

Kieun commented Dec 12, 2017

Yeah, user handle is required field for generating a credential. But it does not have to be required field to store credentials. Is that make sense that it is optional in the perspective of credential storage for 2nd factor authenticators?

@emlun
Copy link
Member Author

emlun commented Dec 12, 2017

In a way I think it does make some sense: the conceptual credential "has" a user handle, but since the authenticator will never return it it's fine if the authenticator as an implementation detail doesn't store the user handle. But yeah, that could be confusing.

@jyasskin @christiaanbrand @jeffh thoughts on this?

@emlun
Copy link
Member Author

emlun commented Dec 13, 2017

Reopening to track progress on the TODO stated in #558 (comment)

@emlun
Copy link
Member Author

emlun commented Dec 21, 2017

Closing this along with #730.

@emlun emlun closed this as completed Dec 21, 2017
@akshayku akshayku reopened this Dec 22, 2017
@emlun
Copy link
Member Author

emlun commented Dec 24, 2017

@akshayku Did we miss something?
EDIT: Never mind, I just saw your messages in #730.

@emlun emlun closed this as completed in #730 Jan 3, 2018
emlun added a commit that referenced this issue Jan 3, 2018
Fix #720: Align user handle management with CTAP
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment