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

Aligning PublicKeyCredentialUserEntity with CTAP #757

Closed
ve7jtb opened this issue Jan 23, 2018 · 4 comments
Closed

Aligning PublicKeyCredentialUserEntity with CTAP #757

ve7jtb opened this issue Jan 23, 2018 · 4 comments
Assignees

Comments

@ve7jtb
Copy link
Contributor

ve7jtb commented Jan 23, 2018

Sec 5.4 lists id, displayName, and name

Sec 5.4.3 only lists id and displayName as required in the dictionary.

Should name be optional?

The CTAP spec states
This PublicKeyCredentialUserEntity data structure describes the user account to which the new public key credential will be associated at the RP. It contains an RP-specific user account identifier, (optionally) a user name, (optionally) a user display name, and (optionally) a URL pointing to an image (of a user avatar, for example). The authenticator associates the created public key credential with the account identifier, and MAY also associate any or all of the user name, user display name, and image data (pointed to by the URL, if any).

The CTAP example is:
var user = {
id: Uint8Array.from(window.atob("MIIBkzCCATigAwIBAjCCAZMwggE4oAMCAQIwggGTMII="), c=>c.charCodeAt(0)),
icon: "https://pics.acme.com/00/p/aBjjjpqPb.png",
name: "johnpsmith@example.com",
displayName: "John P. Smith"
};

icon is not mentioned at all in this spec.

We need to clarify name and icon if we expect browsers to pass these through to the authenticator and or display these to the user. Otherwise they should be removed from CTAP or appropriate comments added that they won't be passed through the browser.

@selfissued
Copy link
Contributor

5.4.3 https://w3c.github.io/webauthn/#sctn-user-credential-params defines PublicKeyCredentialUserEntity, which is a derived class that inherits from PublicKeyCredentialEntity, which is defined in 5.4.1 https://w3c.github.io/webauthn/#dictdef-publickeycredentialentity. "name" is required in PublicKeyCredentialEntity. "id" and "displayName" are required in PublicKeyCredentialUserEntity. Therefore, all are present and required in PublicKeyCredentialUserEntity.

"icon" is defined in PublicKeyCredentialEntity and is not marked as being required. Browsers can (and should) pass it through to CTAP when provided.

Therefore, I believe that WebAuthn and CTAP are aligned in their treatments of these values. If you concur with my analysis, I think that this issue can be closed. Thanks for sweating the details.

@emlun
Copy link
Member

emlun commented Jan 24, 2018

I agree with @selfissued. For the record, whether these fields should be required or optional was recently discussed in #666, starting here: #666 (comment) . The conclusion of that discussion was (#666 (comment)) that changing that would be a breaking change which we shouldn't do at this time.

@nadalin nadalin added this to the CR milestone Jan 26, 2018
@nadalin
Copy link
Contributor

nadalin commented Jan 26, 2018

@selfissued so can we close this ?

@selfissued
Copy link
Contributor

After review, it appears that there isn't an actual problem here.

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

4 participants