Introduce authenticator response interfaces. #397
Conversation
This patch adds an 'AuthenticatorResponse' interface, representing the generic attributes of responses from authenticators. It then redefines 'ScopedCredentialInfo' and 'AuthenticatorAssertion' to derive from this interface, and renames them to 'AuthenticatorAttestionResponse' and 'AuthenticatorAssertionResponse' respectively. These new interfaces are a drop-in replacement for the old interfaces, no normative changes are intended in this patch, other than the renaming.
|
This patch is an attempt to extract one point of agreement between @leshi's I used my names here, both because I an enamored with my own cleverness and because I could easily extract them from that existing commit. I don't think I'd be terribly sad if folks would prefer something more along the lines of @leshi's proposal. :) |
|
On-call, no one has any significant issues with this, but we want to get some detailed reviews before merging. Goal of merging on Friday. |
There was a problem hiding this comment.
Overall this looks fine to me. I have a couple of detail-level comments as well as taking the opportunity to polish/fix a couple paragraphs (that I'd noticed needed work and been waiting for opportunity to do it). submitted issue #401 along the way.
Also, i note that @mikewest's recent PRs introduce the..
: <term>
:: <definition>
notation into the spec. If we accept this PR (and the other two) as-is, we ought to reformat the other interface and dictionary expositions accordingly (if not now, then for CR -- i.e. submit an editorial issue about this upon merge).
index.bs
Outdated
| <div dfn-type="attribute" dfn-for="AuthenticatorResponse"> | ||
| : <dfn>clientDataJSON</dfn> | ||
| :: This attribute contains a [=JSON-serialized client data|JSON serialization=] of the [=client data=] passed to the | ||
| authenticator by the client in order to generate this response. |
There was a problem hiding this comment.
s/by the client in order to generate this response/by the client in its call to either {{makeCredential()}} or {{getAssertion()}}/
index.bs
Outdated
| public key=], and the attestation statement. It also contains any additional information that the [RP]'s | ||
| server requires to validate the attestation statement, as well as to decode and validate the bindings of both | ||
| the client and authenticator data. For more details, see [[#cred-attestation]] as well as | ||
| [Figure 3](#fig-attStructs). |
There was a problem hiding this comment.
might as well fix this entire parag now -- there's various issues with it. Here's a suggested rewrite:
This attribute contains an [=attestation object=], which is opaque to, and cryptographically protected against tampering by, the client. The [=attestation object=] contains both [=authenticator data=] and an attestation statement. The former contains the AAGUID, a unique credential ID, and the [=credential public key=]. The contents of the attestation statement are determined by the [=attestation statement format=] used by the [=authenticator=]. It also contains any additional information that the [RP]'s server requires to validate the attestation statement, as well as to decode and validate the [=authenticator data=] along with the [=JSON-serialized client data=]. For more details, see [[#cred-attestation]] as well as [Figure 3](#fig-attStructs).
index.bs
Outdated
| validate the attestation statement, as well as to decode and validate the bindings of both the client and authenticator | ||
| data. For more details, see [[#cred-attestation]]. | ||
| The {{AuthenticatorAttestationResponse}} interface represents the [=authenticator=]'s response to a client's request | ||
| to create a new [=scoped credential=]. It contains information about the credential that can be used to identify it |
There was a problem hiding this comment.
suggest:
s/request to create a new/request for the creation of a new/
s/about the credential/about the new credential/
index.bs
Outdated
|
|
||
| The {{AuthenticatorAssertionResponse}} interface represents that [=authenticator=]'s response to a client's request | ||
| to generate an [=authentication assertion=]. This response contains a cryptographic signature proving possession of | ||
| the [=credential private key=], and optionally evidence of [=user consent=] to a specific transaction. |
There was a problem hiding this comment.
suggest:
The {{AuthenticatorAssertionResponse}} interface represents an [=authenticator=]'s response to a client's request for generation of a new [=authentication assertion=] given the [=[RP]=]'s challenge and optional list of credentials it is aware of. This response contains a cryptographic signature proving possession of the [=credential private key=], and optionally evidence of [=user consent=] to a specific transaction.
| interface ScopedCredentialInfo { | ||
| readonly attribute ArrayBuffer clientDataJSON; | ||
| readonly attribute ArrayBuffer attestationObject; | ||
| interface AuthenticatorResponse { |
There was a problem hiding this comment.
<bikeshed>Maybe call this BaseResponse since this doesn't actually come from the Authenticator? And then call the other two RegistrationResponse and AuthenticationResponse or MakeCredentialRespone and GetAssertionResponse</bikeshed>
There was a problem hiding this comment.
I hereby officially don't care about this bikeshed :) . Thank you for considering it!
There was a problem hiding this comment.
No opinion on the overall direction of @leshi's suggestion, but in general be careful to keep interface names distinct from everything else that might ever appear on the web platform. So, "BaseResponse" is bad because it doesn't identify what kind of response. Including "Authenticator" or "PublicKeyCredential" somewhere in the name would fix it.
|
Merge after @equalsJeffH's changes. |
|
I think 3cdf1bf addresses the feedback. |
This was added in #1742, but there is no `AuthenticationAssertion` interface in Chromium, Gecko or WebKit source code. It was renamed to `AuthenticatorAssertionResponse` in w3c/webauthn#397 and that is already in BCD.
…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.
This patch adds an 'AuthenticatorResponse' interface, representing the
generic attributes of responses from authenticators. It then redefines
'ScopedCredentialInfo' and 'AuthenticatorAssertion' to derive from this
interface, and renames them to 'AuthenticatorAttestionResponse' and
'AuthenticatorAssertionResponse' respectively.
These new interfaces are a drop-in replacement for the old interfaces,
no normative changes are intended in this patch, other than the
renaming.