public key cred - fixes #406 #432

equalsJeffH · 2017-05-01T16:06:22Z

"scoped cred" -> "public key cred" - fixes #406

whatchay'all think?

jyasskin

I double-checked https://api.csswg.org/bikeshed/?url=https://raw.githubusercontent.com/w3c/webauthn/jeffh-publicKeyCred-406/index.bs for any remaining uses of "scoped", and they all make sense after the rename.

jyasskin · 2017-05-01T16:25:53Z

index.bs

-    ::  The {{ScopedCredential}} [=interface object=]'s {{Credential/[[type]]}} [=internal slot=]'s value is the string
-        "`scoped`".
+    ::  The {{PublicKeyCredential}} [=interface object=]'s {{Credential/[[type]]}} [=internal slot=]'s value is the string
+        "`publickey`".


I believe https://w3ctag.github.io/design-principles/#casing-rules recommends "public-key" here.

yeah, it did seem to me that casing-rules can be interpreted to apply here, but it sorta seems an "internal slot" is only implicitly an enum. If up to me I'd use "publickey" rather than "public-key", but it's not a big deal. this was sort of an experiment to see if anyone would catch it :)

:) The only things that just concatenate lower-case words are events and HTML elements and attributes, and this definitely isn't one of those.

jyasskin · 2017-05-01T16:28:01Z

index.bs

-        or length of this identifier, except that it must be sufficient for the platform to uniquely select a key. For example,
-        an authenticator without on-board storage may create identifiers containing a [=credential private key=] wrapped with a
-        symmetric key that is burned into the authenticator.
+    ::  This [=internal slot=] contains an identifier for the credential, chosen by the platform with help from the


I think the file is mostly wrapped to 130 characters. To avoid churn, maybe don't rewrap to less than that when you're not otherwise changing the paragraph?

@vijaybh wrapped it to 128 chars when initially converting to BS format: https://lists.w3.org/Archives/Public/public-webauthn/2016Apr/0116.html

personally, I'd much prefer 80-char lines, but can live with it as-is.

Also, I tried hard-wrap+indent of header lines "#..." and BS complained so did not wrap them.

jyasskin · 2017-05-01T16:32:17Z

index.bs

-    enum ScopedCredentialType {
-        "scoped"
+    enum PublicKeyCredentialType {
+        "publickey"


Probably "public-key" per https://w3ctag.github.io/design-principles/#casing-rules.

ja, shur, u betcha, see above.

jyasskin · 2017-05-01T16:33:07Z

index.bs

    This enumeration defines the valid credential types. It is an extension point; values may be added to it in the future, as
    more credential types are defined. The values of this enumeration are used for versioning the Authentication Assertion and
    attestation structures according to the type of the authenticator.

-    Currently one credential type is defined, namely "<dfn>scoped</dfn>".
+    Currently one credential type is defined, namely "<dfn>publickey</dfn>".


And here, so you don't have to re-find this use.

jyasskin · 2017-05-01T16:36:17Z

index.bs

@@ -2443,7 +2466,7 @@ The entry key is the [=extension identifier=] and the value is the [=client exte

 <pre class="example" highlight="js">
    var assertionPromise = navigator.credentials.get({
-        scoped: {
+        publickey: {


publicKey

jyasskin · 2017-05-01T16:44:07Z

index.bs

@@ -3060,11 +3086,11 @@ The sample code for generating and registering a new key follows:
      // prefers an ES256 credential.
      parameters: [
        {
-          type: "scoped",
+          type: "publickey",


public-key

jyasskin · 2017-05-01T16:47:28Z

index.bs

@@ -3076,7 +3102,7 @@ The sample code for generating and registering a new key follows:
    };

    // Note: The following call will cause the authenticator to display UI.
-    navigator.credentials.create({ scoped })
+    navigator.credentials.create({ publickey })


publicKey

jyasskin · 2017-05-01T16:47:40Z

index.bs


    var options = {
                    challenge: new TextEncoder().encode("climb a mountain"),
                    timeout: 60000,  // 1 minute
-                    allowList: [{ type: "scoped" }]
+                    allowList: [{ type: "publickey" }]


public-key

jyasskin · 2017-05-01T16:47:47Z

index.bs

                  };

-    navigator.credentials.get({ "scoped": options })
+    navigator.credentials.get({ "publickey": options })


publicKey

jyasskin · 2017-05-01T16:47:58Z

index.bs


    var encoder = new TextEncoder();
    var acceptableCredential1 = {
-        type: "scoped",
+        type: "publickey",


jcjones

I'm supportive of these changes; they compiled fine locally, and while I didn't re-read the whole spec, I did some grep-ing and checked over the algorithms.

I like just calling it a public key. 👍

selfissued · 2017-05-02T00:25:29Z

I agree that we shouldn't re-wrap lines unless we're actually changing them to prevent unnecessary merge conflicts.

selfissued

Merge this one soon. The edits are so extensive that there are likely to be continual merge conflicts until this is in.

selfissued · 2017-05-02T00:30:02Z

BTW, I'm not suggesting unwrapping any lines that were already re-wrapped by this set of edits. What's done is done.

equalsJeffH · 2017-05-02T16:00:24Z

@selfissued said

we shouldn't re-wrap lines unless we're actually changing them to prevent unnecessary merge conflicts.

fyi/fwiw: AFAIK simply re-hard-wrapping lines does not itself cause merge conflicts -- e.g. it did not when I merged master into this branch, nor when I've done prior line-wrapping cleanup in prior PRs.

equalsJeffH · 2017-05-02T16:07:35Z

Ok folks -- given the three positive reviews so far, unless anyone objects by noon PT, I have it good authority that @selfissued is gonna merge this :)

AngeloKai · 2017-05-02T17:47:07Z

@selfissued @equalsJeffH I still need to review them. I may submit a formal objection later today.

nadalin · 2017-05-02T19:31:30Z

I really would like to hear from all the implementers on this one before it is just merged @jcjones and @AngeloKai

AngeloKai · 2017-05-02T20:15:39Z

@equalsJeffH My main concern with the change is that the new name, "PublicKeyCredentials", isn't easier to understand for web developers than the old name "ScopedCredential". The former implies the credential has a public/private key pair, which is a common concept found in every crypto API. The latter implies the credential is scoped to something, which most web dev immediately think of scoping to origin. I took an informal vote around my office full of web developers and none of them knows what either means. I am afraid we are tinkering without real feedbacks.

leshi · 2017-05-02T20:27:07Z

My 2 cents is that the spec is SO complex that there is no way anyone is going to understand it enough to implement anything useful by skimming it. The developers at the very least will have to understand how to verify assertions. IMHO, that will be easier if they have the "asymmetric key" big picture rather than the "scoped to something" big picture.

I support this change.

jyasskin · 2017-05-02T21:08:07Z

"Scoped" will never survive TAG review because it doesn't distinguish this kind of credential from any other kind of web credential. PublicKey might not survive either, due to some of the other public-key-ish systems mentioned in #406 (comment), but it at least has a chance. We should get feedback on a name that has a chance, rather than getting feedback on one that doesn't.

I'm not wedded to "PublicKey" if @AngeloKai has another suggestion, but we do need a concrete suggestion.

AngeloKai · 2017-05-02T22:25:34Z

@jyasskin @leshi How about DeviceBoundCredentials or AuthenticatorBoundCredentials? One of the biggest value add of the API is that credentials are now bound with the device, ensuring at least security boundary. Also, "scoped" already survived the TAG review: w3ctag/design-reviews#97

jyasskin · 2017-05-02T22:53:44Z

@AngeloKai In your informal vote, what did your coworkers think a device- or authenticator-bound credential would imply? What did they think a public-key credential would imply?

IIUC, with self-attested keys, there's not actually a guarantee that the key is bound to the client? Whereas there is always a public/private key pair involved?

(None of the ScopedCredential interface survived the first TAG review: they said to merge it into Credential Manager.)

jyasskin · 2017-05-02T23:03:17Z

"DeviceBoundCredential" would be acceptable to me. I prefer "PublicKeyCredential" because it gives developers more hint about how they need to use the credential, and I don't think @AngeloKai has given an argument against it ("implies the credential has a public/private key pair" is correct), but "DeviceBound" does distinguish this from other kinds of credentials on the platform.

rlin1 · 2017-05-03T11:00:52Z

I like publicKeyCredential (as it is more specific than ScopedCredential). Good to be merged.

equalsJeffH · 2017-05-03T18:10:36Z

decision on the webauthn call today is to merge this for WD-05:
https://www.w3.org/2017/05/03-webauthn-minutes.html

…Info These were added in #1742, but none of these interfaces are in Chrome, Firefox or Safari, or appear in Chromium, Gecko or WebKit source code. (There are matches in Gecko and WebKit, but for internal things, not web-exposed interfaces.) `AuthenticationAssertion` was renamed to `AuthenticatorAssertionResponse` in w3c/webauthn#397. `ScopedCredential` was renamed to `PublicKeyCredential` in w3c/webauthn#432. `ScopedCredentialInfo` was renamed to `AuthenticatorResponse` in w3c/webauthn#397. All of the new names are already in BCD.

…Info (#9398) These were added in #1742, but none of these interfaces are in Chrome, Firefox or Safari, or appear in Chromium, Gecko or WebKit source code. (There are matches in Gecko and WebKit, but for internal things, not web-exposed interfaces.) `AuthenticationAssertion` was renamed to `AuthenticatorAssertionResponse` in w3c/webauthn#397. `ScopedCredential` was renamed to `PublicKeyCredential` in w3c/webauthn#432. `ScopedCredentialInfo` was renamed to `AuthenticatorResponse` in w3c/webauthn#397. All of the new names are already in BCD.

JeffH added 3 commits May 1, 2017 08:19

'scoped cred' -> 'public key cred', autolink 'origin'

3abeaed

complete autolinking 'origin'

065f864

cleanup blank spaces & lines

fb2b593

equalsJeffH added type:editorial type:technical labels May 1, 2017

equalsJeffH added this to the WD-05 milestone May 1, 2017

equalsJeffH self-assigned this May 1, 2017

equalsJeffH requested review from jyasskin, jcjones, leshi, AngeloKai, selfissued and vijaybh May 1, 2017 16:06

equalsJeffH mentioned this pull request May 1, 2017

Rename ScopedCredential to PublicKeyCredential #406

Closed

jyasskin suggested changes May 1, 2017

View reviewed changes

JeffH added 3 commits May 1, 2017 11:37

fixup publickey -> public-key|publicKey per jyasskin-the-enforcer ;-)

866e882

merge from master and fix conflicts

254bb0a

fix conflicts fer real this time (i hope)

371c876

jyasskin approved these changes May 1, 2017

View reviewed changes

jcjones approved these changes May 1, 2017

View reviewed changes

selfissued approved these changes May 2, 2017

View reviewed changes

selfissued added the priority:implementation label May 2, 2017

leshi merged commit d7b1ece into master May 3, 2017

WebAuthnBot pushed a commit that referenced this pull request May 3, 2017

Built by Travis-CI: d7b1ece public key cred - fixes #406 (#432)

74154b2

jyasskin mentioned this pull request May 3, 2017

"ScopedCred" enum should be "scoped-cred" #328

Closed

jyasskin mentioned this pull request May 4, 2017

Why is the only value of ScopedCredentialType "ScopedCred" as opposed to "ScopedCredential"? #291

Closed

emlun deleted the jeffh-publicKeyCred-406 branch June 12, 2019 11:26

foolip mentioned this pull request Mar 9, 2021

Remove AuthenticationAssertion, ScopedCredential and ScopedCredentialInfo mdn/browser-compat-data#9398

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

public key cred - fixes #406 #432

public key cred - fixes #406 #432

equalsJeffH commented May 1, 2017

jyasskin left a comment

jyasskin May 1, 2017

equalsJeffH May 1, 2017

jyasskin May 1, 2017

jyasskin May 1, 2017

equalsJeffH May 1, 2017

equalsJeffH May 1, 2017

jyasskin May 1, 2017

equalsJeffH May 1, 2017

jyasskin May 1, 2017

jyasskin May 1, 2017

jyasskin May 1, 2017

jyasskin May 1, 2017

jyasskin May 1, 2017

jyasskin May 1, 2017

jyasskin May 1, 2017

jcjones left a comment •

edited

Loading

selfissued commented May 2, 2017

selfissued left a comment

selfissued commented May 2, 2017

equalsJeffH commented May 2, 2017

equalsJeffH commented May 2, 2017

AngeloKai commented May 2, 2017

nadalin commented May 2, 2017

AngeloKai commented May 2, 2017

leshi commented May 2, 2017

jyasskin commented May 2, 2017

AngeloKai commented May 2, 2017 •

edited

Loading

jyasskin commented May 2, 2017

jyasskin commented May 2, 2017

rlin1 commented May 3, 2017

equalsJeffH commented May 3, 2017

public key cred - fixes #406 #432

public key cred - fixes #406 #432

Conversation

equalsJeffH commented May 1, 2017

jyasskin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcjones left a comment • edited Loading

Choose a reason for hiding this comment

selfissued commented May 2, 2017

selfissued left a comment

Choose a reason for hiding this comment

selfissued commented May 2, 2017

equalsJeffH commented May 2, 2017

equalsJeffH commented May 2, 2017

AngeloKai commented May 2, 2017

nadalin commented May 2, 2017

AngeloKai commented May 2, 2017

leshi commented May 2, 2017

jyasskin commented May 2, 2017

AngeloKai commented May 2, 2017 • edited Loading

jyasskin commented May 2, 2017

jyasskin commented May 2, 2017

rlin1 commented May 3, 2017

equalsJeffH commented May 3, 2017

jcjones left a comment •

edited

Loading

AngeloKai commented May 2, 2017 •

edited

Loading