publicKey member name in CredentialCreationRequestOptions should be "public-key", or vice-versa?

[equalsJeffH]

forking the thread beginning here #750 (comment) into this new issue.

Here's the salient portions:

@bzbarsky wrote in #750 (comment):
Does the spec actually describe what should happen when the .publicKey dictionary is missing? As far as I can tell https://w3c.github.io/webauthn/#createCredential asserts it's present in the CredentialCreationOptions, but it's not clear to me what guarantees that, exactly... It's also not clear to me when the "publicKey" member of CredentialRequestOptions ever gets used.

As long as the spec completely describes behavior when the dictionaries involved are missing, it should be fine as things stand.

@emlun wrote in #750 (comment):
As far as I understand, the presence of the publicKey member is what triggers the client to invoke the WebAuthn methods instead of other Credential Management backends.

@emlun wrote in #750 (comment):
I think it happens here, where interfaces refers to here where options is the options argument object sent to the create() method and the [[type]] slot for the PublicKeyCredential interface contains "public-key".

...actually, does that mean that the publicKey member name in CredentialCreationRequestOptions should also be "public-key", or vice-versa?

@equalsJeffH wrote in #750 (comment):
Looking more closely at credman's magic relevant credential interface objects sub-algorithm, i'm now thinking you & @emlun are correct, due to steps 4.2 & 4.3 in said sub-alg:

4. For each object in interface objects:
    1. [ . . . ]
    2. Let key be object’s [[type]] slot’s value.   // "public-key"
    3. If options[key] exists, append object to 
        relevant interface objects.                 // options[public-key] throws (?)
                                                    // i.e., it ought to be 
                                                    // options[publicKey] given  
                                                    // how spec is currently written.

So, perhaps browser implementors can weigh-in on whether it matters if we change..

1. the CredentialCreationOptions and CredentialRequestOptions dictionary extensions' member name of publicKey to be public-key, or
2. the PublicKeyCredentialType's enumeration value to be "publicKey" (rather than the present "public-key").

..i.e., what are present implementations doing, would making this spec fix affect them at all?

cc @kpaulh @agl @akshayku @jcjones

[equalsJeffH]

@emlun noted in #750 (comment):
For what it's worth, (1) would require changes to RP code as well in addition to clients, while (2) would not. On the other hand, (2) looks like more idiomatic JS since a bare public-key is not a valid object key literal in JS (the string literal "public-key" is a valid key, though).

[equalsJeffH]

@equalsJeffH noted in #750 (comment):
hm. Plus there's the orange note in WebIDL S2.8. Enumerations that sez in part:

It is strongly suggested that enumeration values be all lowercase, and that multiple words be separated using dashes or not be separated at all, unless there is a specific reason to use another value naming scheme. For example, an enumeration value that indicates an object should be created could be named "createobject" or "create-object".

[akshayku]

(2) requires change for both RP as well as client. Its a breaking change.

[equalsJeffH]

IMPLEMENTORS -- please weigh-in on how you're ACTUALLY dealing with this ostensible impl-level issue (at the code level)?

[bzbarsky]

How Gecko is dealing as follows:

1. In Gecko right now, the publicKey member of CredentialRequestOptions is always present (and we throw if you try to call CredentialsContainer.create with an object without a publicKey property).

2. CredentialsContainer.create hardcodes the assumption that the only "relevant credential interface object" is PublicKeyCredential, since this is the only sort of credential we support and since we always have a publicKey.

Needless to say, this looks nothing like the spec. I ran into this issue when trying to make our behavior more like the spec...

[equalsJeffH]

on webauthn call 1-Aug-2018:

1. need Chrome and Edge to report how they are presently working around this.
2. are there any other options than (1) or (2) ?
3. need to assess impact of fix and choose (1) or (2) or ?

[agl]

In Chrome CredentialCreationOptions has three optional members and the code checks that exactly one of them is present and dispatches based on which it is.

When we create a Credential object the subclass sets the type member and the public-key credential sets it to public-key, as expected.

I don't understand why these two strings need to be same and thus why a change is being suggested here.

[bzbarsky]

and dispatches based on which it is

Right, the question is how that dispatch happens. Looking at the linked code, it looks like Chrome hardcodes the fact that the "publicKey" member maps to the PublicKeyCredential interface, right?

And then the PublicKeyCredential implementation is hardcoded to set "public-key" instead of having a [[type]] it gets from the interface object itself.

I don't understand why these two strings need to be same

The [CredMan] spec is trying to do dispatch for all this stuff without hardcoding a specific list of possible credentials. So it walks the list of all the interfaces looking for one whose [[type]] matches the dictionary member name and it also uses [[type]] for the value of the type member.

The [CredMan] spec could switch to having two different internal slots, or to defining a mapping from dictionary member names to [[type]] values or vice versa. Or it could explicitly list all the possible cases (which is what the Chrome code is doing, basically) and not have this problem at all, at the cost of making it harder for someone to add a new type of credential without changing the spec (arguably a good thing, actually).

[agl]

Ah, I see. It comes from this section of the CredMan spec. However, clearly no implementation actually works like that (otherwise they would have hit this issue).

Thus I would suggest option three: do nothing. This part of the CredMan spec was excessively abstract and we shouldn't suffer for it now.

If the group does not like option three, then option two would seem to be the least disruptive path. Lots of code probably doesn't check the type field at all, but it would all need changing if we altered the member name.

[bzbarsky]

I strongly object to "do nothing". That leaves the spec in a state that will cause problems for anyone who tries to actually implement what the spec says.

If we need to fix the credman spec, we should fix the credman spec. But leaving things in a known-broken state as a landmine for unwary implementors is unacceptable.

[agl]

(My ”do nothing“ was directed at webauthn specifically and I didn't intend to suggest that CredMan couldn't be updated. But it appears to me that nobody has implemented the CredMan spec as written and thus I think the CredMan spec should align with reality here rather than us changing webauthn to follow it.)

[bzbarsky]

That would be a perfectly reasonable course of action from my point of view, fwiw, as someone not directly involved in either spec.

[equalsJeffH]

on 15-aug webauthn call -- I'll open an issue on CredMan in regards to this and then we can note that here and close this issue.

[equalsJeffH]

per #1004 (comment) above, I've (finally) submitted w3c/webappsec-credential-management#147 and am closing this issue.