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.

Sign up for GitHub

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

Jump to bottom

Fix #622: Clarify PublicKeyCredentialEntity name descriptions #666

Merged
merged 9 commits into from
Jan 3, 2018
Merged

Fix #622: Clarify PublicKeyCredentialEntity name descriptions #666

Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8cdeac8
Clarify PublicKeyCredentialEntity name descriptions
emlun Oct 31, 2017
be5143f
Drop user.name uniqueness recommendation
emlun Oct 31, 2017
343b89d
Merge branch 'master' into issue-622
emlun Nov 9, 2017
819b77a
Partially address review comments
emlun Nov 26, 2017
1f8e10c
Change link target for [=username=]
emlun Dec 7, 2017
7508d60
Address review comments
emlun Dec 6, 2017
64b728e
Extract formal definition of Human Palatability
emlun Dec 7, 2017
1904e73
Reference EduPerson for definition of Human Palatability
emlun Dec 20, 2017
432e531
Merge branch 'master' into issue-622
emlun Jan 3, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 24 additions & 20 deletions index.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1343,8 +1343,7 @@ optionally evidence of [=user consent=] to a specific transaction.
: <dfn>rp</dfn>
:: This member contains data about the [=[RP]=] responsible for the request.

Its value's {{PublicKeyCredentialEntity/name}} member contains the friendly name of the [=[RP]=] (e.g. "Acme Corporation",
"Widgets, Inc.", or "Awesome Site".
Its value's {{PublicKeyCredentialEntity/name}} member is required.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RP's "name" is optional according to CTAP spec. See below comment.


Its value's {{PublicKeyCredentialRpEntity/id}} member specifies the [=relying party identifier=] with which the credential
should be associated. If omitted, its value will be the {{CredentialsContainer}} object's [=relevant
Expand All @@ -1353,14 +1352,8 @@ optionally evidence of [=user consent=] to a specific transaction.
: <dfn>user</dfn>
:: This member contains data about the user account for which the [=[RP]=] is requesting attestation.

Its value's {{PublicKeyCredentialEntity/name}} member contains a name for the user account (e.g.,
"john.p.smith@example.com" or "+14255551234").

Its value's {{PublicKeyCredentialUserEntity/displayName}} member contains a friendly name for the user account (e.g.,
"John P. Smith").

Its value's {{PublicKeyCredentialUserEntity/id}} member contains the [=user handle=] for the account, specified by the
[=[RP]=].
Its value's {{PublicKeyCredentialEntity/name}}, {{PublicKeyCredentialUserEntity/displayName}} and
Copy link
Contributor

@akshayku akshayku Dec 29, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

user's "name" and "displayName" are not required according to CTAP spec. Only user's "id" is required.
For very security conscious RPs, who don't want to store any user identifiable information on the FIDO device, they will only set "id" (which is supposed to be random bytes). There is no UI requirement for single account per RP case.

On FIDO device, RP's "id" and users "id" makes a unique combination to identify a credentials. Rest of the fields are helper details which are optional.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For an authenticator with a display (or, for example, an authenticator app on a phone), needs this information to show a useful message. "Do you want to register at example.com with Alex P. Mueller?" is a lot more informative (and, I would argue, security conscious) than "Do you want to register?".

Storing this information on the authenticator is more privacy than security sensitive, especially considering that the device is already entrusted with the keys. Perhaps I am biased but I assume authenticator vendors are security and privacy conscious enough to encrypt this additional information. If not, don't buy their products.

For getAssertion, I am fine with making this optional. If the device needs the information, it can store it.

{{PublicKeyCredentialUserEntity/id}} members are required.

: <dfn>challenge</dfn>
:: This member contains a challenge intended to be used for generating the newly created credential's [=attestation
Expand Down Expand Up @@ -1405,10 +1398,18 @@ associated.
</xmp>
<div dfn-type="dict-member" dfn-for="PublicKeyCredentialEntity">
: <dfn>name</dfn>
:: A human-friendly identifier for the entity. For example, this could be a company name for a [=[RP]=], or a
user's name. This identifier is intended for display. [=Authenticators=] MUST accept and store a 64 byte minimum length
for a name members's value. Authenticators MAY truncate a
name member's value to a length equal to or greater than 64 bytes.
:: A human-readable name for the entity. Its function depends on what the {{PublicKeyCredentialEntity}} represents:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would s/-readable/-palatable/. And add this note:
"Note: An identifier that is human-palatable is intended to be rememberable and reproducible by typical human users, in contrast to identifiers that are, for example, randomly generated sequences of bits."

c.f. https://www.internet2.edu/media/medialibrary/2013/09/04/internet2-mace-dir-eduperson-200604.html

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. I'll change to "palatable" in the ...UserEntity bullet point, but leave the top one as "readable" since that applies to both the User and Rp variants and Rp.name doesn't necessarily need to be reproducible. Does that sound good?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does copyright legalese say about flat-out copying that definition from the EduPerson spec? Should we reference the EduPerson spec for it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per the 2017-12-13 WG call, this is fair use and attribution is good practice.


- When inherited by {{PublicKeyCredentialRpEntity}} it is a human-friendly identifier for the [=[RP]=], intended only
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/identifier/name/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

for display. For example, "ACME Corporation", "Wonderful Widgets, Inc." or "Awesome Site".
- When inherited by {{PublicKeyCredentialUserEntity}}, it is a human-readable identifier for a user account. It is
intended only for display, and SHOULD allow the user to easily tell the difference between user accounts with similar
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, when inherited by {{PublicKeyCredentialUserEntity}}, an RP MAY use the name member's value to store an account identifier (aka a username), yes?

I'm thinking we should add a Note: wrt an RP's options when employing PublicKeyCredentialUserEntity.id and PublicKeyCredentialUserEntity.name.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean by your first paragraph. The member is required and won't be passed back to the RP, only (optionally) stored by the authenticator. The RP MAY use name for a username, yes, MUST use it, and SHOULD NOT use it for anything too functionally different from a conventional username.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@equalsJeffH Bump

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comments below...

{{PublicKeyCredentialUserEntity/displayName}}s. For example, "alexm", alex.p.mueller@acme.com" or "+14255551234". The
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/, alex.p.mueller/, "alex.p.mueller/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thanks!

[=[RP]=] SHOULD let the user choose this, and MAY restrict the choice as needed or appropriate.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest: s/ [=[RP]=] SHOULD /=[RP]=] MAY / because this is really up to the RP's deployment context.

Also, after "appropriate.", I would add: "For example, an [=[RP]=] may choose to map human-palatable [=username=] account identifiers to {{PublicKeyCredentialUserEntity}}'s {{PublicKeyCredentialUserEntity/name}}."

..where [=username=] perhaps should link to https://html.spec.whatwg.org/#attr-fe-autocomplete-username (AFAICT, it seems to be the most appropriate place to link "username" to in the greater Web specs space, for our purposes here).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently, [=username=] already links to https://url.spec.whatwg.org/#concept-url-username . Should we change that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we change that?

yeah, none of the autolinks for "username" amongst whatwg/w3c specs seemed very satisfying to me, so i was thinking that the link to https://html.spec.whatwg.org/#attr-fe-autocomplete-username would be the most clear for readers in terms of implying that we mean username in the sense of "unique-for-this-rp user account name". Though, I suppose we could just define it as such ourselves.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, done!


[=Authenticators=] MUST accept and store a 64 byte minimum length for a {{PublicKeyCredentialEntity/name}} members's
value. Authenticators MAY truncate a {{PublicKeyCredentialEntity/name}} member's value to a length equal to or greater
than 64 bytes.

: <dfn>icon</dfn>
:: A [=URL serializer|serialized=] URL which resolves to an image associated with the entity. For example, this could be
Expand Down Expand Up @@ -1451,9 +1452,12 @@ credential.
:: The [=user handle=] of the user account entity.

: <dfn>displayName</dfn>
:: A friendly name for the user account (e.g., "John P. Smith"). [=Authenticators=] MUST accept and store a 64 byte minimum
length for a displayName members's value. Authenticators MAY truncate a
displayName member's value to a length equal to or greater than 64 bytes.
:: A human-friendly name for the user account, intended only for display. For example, "Alex P. Müller" or "田中 倫". The
[=[RP]=] SHOULD let the user choose this, and SHOULD NOT restrict the choice more than necessary.

[=Authenticators=] MUST accept and store a 64 byte minimum length for a {{PublicKeyCredentialUserEntity/displayName}}
members's value. Authenticators MAY truncate a {{PublicKeyCredentialUserEntity/displayName}} member's value to a length
equal to or greater than 64 bytes.
</div>


Expand Down Expand Up @@ -3756,14 +3760,14 @@ The sample code for generating and registering a new key follows:

// Relying Party:
rp: {
name: "Acme"
name: "ACME Corporation"
},

// User:
user: {
id: Uint8Array.from(window.atob("MIIBkzCCATigAwIBAjCCAZMwggE4oAMCAQIwggGTMII="), c=>c.charCodeAt(0)),
name: "john.p.smith@example.com",
displayName: "John P. Smith",
name: "alex.p.mueller@example.com",
displayName: "Alex P. Müller",
icon: "https://pics.acme.com/00/p/aBjjjpqPb.png"
},

Expand Down