search

w3c / webauthn

Web Authentication: An API for accessing Public Key Credentials
https://w3c.github.io/webauthn/
Other
1.16k stars 166 forks source link

Aligning PublicKeyCredentialUserEntity with CTAP #757

Closed ve7jtb closed 6 years ago

ve7jtb commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

nadalin commented 6 years ago

@selfissued so can we close this ?

selfissued commented 6 years ago

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