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

equalsJeffH · 2018-07-23T23:54:24Z

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..

the CredentialCreationOptions and CredentialRequestOptions dictionary extensions' member name of publicKey to be public-key, or
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

The text was updated successfully, but these errors were encountered:

equalsJeffH · 2018-07-23T23:55:10Z

@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 · 2018-07-23T23:55:57Z

@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 · 2018-07-25T16:29:31Z

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

equalsJeffH · 2018-07-25T17:50:20Z

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

bzbarsky · 2018-07-25T18:16:39Z

How Gecko is dealing as follows:

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).
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 · 2018-08-01T17:29:44Z

on webauthn call 1-Aug-2018:

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

agl · 2018-08-02T16:40:43Z

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 · 2018-08-02T17:45:33Z

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 · 2018-08-03T18:37:16Z

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 · 2018-08-03T19:15:42Z

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 · 2018-08-08T16:35:53Z

(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 · 2018-08-08T16:37:27Z

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

equalsJeffH · 2018-08-15T17:44:38Z

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 · 2020-06-24T22:21:23Z

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

equalsJeffH added type:technical subtype:algorithms/WebIDL stat:breaking labels Jul 23, 2018

equalsJeffH assigned equalsJeffH and emlun Jul 23, 2018

equalsJeffH mentioned this issue Jul 24, 2018

CredentialRequestOptions make otherwise valid values invalid in an undesirable way #750

Closed

samuelweiler added this to the PropRec milestone Aug 1, 2018

nadalin added the subtype:credman label Aug 22, 2018

YubicoDemo modified the milestones: PropRec, REC Sep 19, 2018

akshayku modified the milestones: REC, L2-WD-00 Feb 6, 2019

equalsJeffH modified the milestones: L2-WD-01, L2-WD-02 May 1, 2019

equalsJeffH added the stat:OnGoing label Jun 12, 2019

equalsJeffH modified the milestones: L2-WD-02, L2-WD-03 Oct 2, 2019

equalsJeffH modified the milestones: L2-WD-03 Feature Complete , L3-WD-01 Jun 24, 2020

equalsJeffH mentioned this issue Jun 24, 2020

publicKey member identifier in Options should be "public-key", or vice-versa? w3c/webappsec-credential-management#147

Closed

equalsJeffH closed this as completed Jun 24, 2020

equalsJeffH mentioned this issue Nov 24, 2021

Map options member identifiers and credential types: public-key vs publicKey w3c/webappsec-credential-management#181

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

equalsJeffH commented Jul 23, 2018

equalsJeffH commented Jul 23, 2018

equalsJeffH commented Jul 23, 2018

akshayku commented Jul 25, 2018

equalsJeffH commented Jul 25, 2018 •

edited

bzbarsky commented Jul 25, 2018 •

edited

equalsJeffH commented Aug 1, 2018

agl commented Aug 2, 2018

bzbarsky commented Aug 2, 2018 •

edited by equalsJeffH

agl commented Aug 3, 2018 •

edited by equalsJeffH

bzbarsky commented Aug 3, 2018 •

edited

agl commented Aug 8, 2018 •

edited by equalsJeffH

bzbarsky commented Aug 8, 2018

equalsJeffH commented Aug 15, 2018

equalsJeffH commented Jun 24, 2020

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

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

Comments

equalsJeffH commented Jul 23, 2018

equalsJeffH commented Jul 23, 2018

equalsJeffH commented Jul 23, 2018

akshayku commented Jul 25, 2018

equalsJeffH commented Jul 25, 2018 • edited

bzbarsky commented Jul 25, 2018 • edited

equalsJeffH commented Aug 1, 2018

agl commented Aug 2, 2018

bzbarsky commented Aug 2, 2018 • edited by equalsJeffH

agl commented Aug 3, 2018 • edited by equalsJeffH

bzbarsky commented Aug 3, 2018 • edited

agl commented Aug 8, 2018 • edited by equalsJeffH

bzbarsky commented Aug 8, 2018

equalsJeffH commented Aug 15, 2018

equalsJeffH commented Jun 24, 2020

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

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

equalsJeffH commented Jul 25, 2018 •

edited

bzbarsky commented Jul 25, 2018 •

edited

bzbarsky commented Aug 2, 2018 •

edited by equalsJeffH

agl commented Aug 3, 2018 •

edited by equalsJeffH

bzbarsky commented Aug 3, 2018 •

edited

agl commented Aug 8, 2018 •

edited by equalsJeffH