API aesthetic updates #142
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -1410,10 +1410,12 @@ authenticator. Since all extensions are optional, this will not cause a function | |||
| Extensions are identified by a string, chosen by the extension author. Extension identifiers should aim to be globally unique, | |||
| e.g., by using reverse domain-name of the defining entity such as `com.example.webauthn.myextension`. | |||
|
|
|||
| Note: Use of dot-separated notation here does not imply an object hierarchy. | |||
There was a problem hiding this comment.
Could we go further and just strip the dots out of all the examples and predefined extensions? We could advise camel-casing, e.g. WebAuthnGeo or webauthnTxauthSimple. That seems better than straddling two boats as we'd be doing here.
There was a problem hiding this comment.
Sure, that sounds great. Added in 67bf014
The HTML5 `window` object represents an open window in the browser, whereas the `navigator` object represents the browser itself. Since authenticators belong to the whole system, not one window, the object model makes more sense to be a property attached to `navigator`.
Generally it's frowned upon to name a Web API "web-". I considered using "authn", but I feel the verbosity here is justified. Most users will assign the object to a local variable for ease of use anyway, just as our example code does.
In response to Vijay's note about extension names[1], I've renamed the example extensions (correcting a CBOR mismatch in the process). I also noted in section 7.1, Extension identifiers, that "Use of dot-separated notation here does not imply an object hierarchy.", and provided a counterexample of a differently- separated, versioned id: `mycompany- myextension_v01`. None of the pre-defined extensions were modified. 1) https://lists.w3.org/Archives/Public/public-webauthn/2016Jul/0046.html
Per the phone call on 13 July 2016, change extension IDs to a format of being
{defining body}_{extension name}<_{version}>.
|
Merging this now as discussed earlier since there have been no more comments. |
WebAuthnBot
pushed a commit
that referenced
this pull request
Jul 16, 2016
WebAuthnBot
pushed a commit
that referenced
this pull request
Jul 18, 2016
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These changes are also aesthetic, similar to PR #135, but in this case they are all related to API aesthetics. They are still, for the most part, editorial in nature. It'd be good to get some care in the review of the
makeCredential()updates per the removal ofScopedCredentialParameters, though.First, per Vijay's comment on issue 60, moving the interface from the global window namespace to the navigator namespace to reduce pollution of the global namespace. This also makes semantic sense, as the HTML5
windowobject represents an open window in the browser, whereas thenavigatorobject represents the browser itself. Since authenticators belong to the whole system, not one window, the object model makes more sense to be a property attached tonavigator.Second, I've renamed the factory object from
navigator.webauthntonavigator.authentication. In general, web APIs shouldn't use the word "web" in their name. It's implicit. (I considered "authn" but I feel the verbosity here is justified -- most of the users will assign the object to a local variable for ease of use anyway, like our example code does.)Third, I changed the type of
ScopedCredentialInfo.publicKeyfromanytoCryptoKey, referencing the WebCryptoAPI for the type information. This feels like a fairly substantial change, but it really isn't: javascript gets flexibility to ask for the pubkey in a variety of formats (including JsonWebKey), and we get a tighter (and more expected) type.Fourth, I've removed the
ScopedCredentialParameterstuple by unpairing thetypeandalgorithmfields.ScopedCredentialParametersis used in the procedure for normalizing algorithms and types, but said procedure doesn't actually use the types! This changes the makeCredential() method to instead have a list of supported types and a list of supported algorithms, and perform its procedure that way.Keeping these together in an object gives a kind of flexibility, but after consideration, I don't see much actual purpose in said flexibility. A RP can either handle an algorithm or not, and is either willing to handle an authenticator type or not -- particular combinations seem an unnecessary burden.
Fifth, in response to [Vijay's note about extension names](https://lists.w3.org/Archives/Public /public- webauthn/2016Jul/0046.html), I've renamed the example extensions (correcting a CBOR mismatch in the process). I also noted in section 7.1, Extension identifiers, that "Use of dot-separated notation here does not imply an object hierarchy.", and provided a counterexample of a differently- separated, versioned id:
mycompany-myextension_v01. None of the pre-defined extensions were modified.