Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New PublicKeyCredential methods for JSON (de)serialization #1703

Merged
merged 27 commits into from
Jun 29, 2022
Merged

New PublicKeyCredential methods for JSON (de)serialization #1703

Changes from 26 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
59ecbf3
Add sections for new API methods
MasterKale Feb 23, 2022
493095f
Migrate toJSON to instance method defs
MasterKale Mar 2, 2022
e87c96d
Add notes
MasterKale Mar 2, 2022
bec86b5
Add placeholder IDL for optionsFromJSON
MasterKale Mar 2, 2022
002d0f5
Take a stab at toJSON IDL definitions
MasterKale Mar 2, 2022
a476667
Clarify toJSON behavior
MasterKale Mar 2, 2022
161e102
Group related IDL defs together
MasterKale Mar 2, 2022
16aab08
Define IDLs for optionsFromJSON()
MasterKale Mar 2, 2022
d264b61
Finish a thought
MasterKale Mar 2, 2022
c1aef27
Mark arguments required
MasterKale Mar 2, 2022
e11e518
Remove notes rationalizing new API existence
MasterKale Mar 23, 2022
91161a2
Try to take care of fatal build errors
MasterKale Mar 23, 2022
1161328
Define AuthenticationExtensionsClientInputsJSON
MasterKale Mar 23, 2022
f13cb35
Add required and align properties
MasterKale Mar 23, 2022
bfa21b0
Split up optionsFromJSON for simplicity's sake
MasterKale Mar 23, 2022
4a904af
Refine definition of `toJSON()` method
MasterKale Mar 23, 2022
078e432
Move JSON xmp out of PublicKeyCredential dl
MasterKale Mar 23, 2022
c0c9454
Remove required from property with default value
MasterKale Mar 23, 2022
22a796c
Split up options methods into two sections
MasterKale Mar 23, 2022
6fa0da7
Return authenticatorAttachment and transports
MasterKale Apr 17, 2022
592271d
Define clientExtensionResults in toJSON()
MasterKale Apr 17, 2022
408b323
Try to support all extension inputs
MasterKale Apr 18, 2022
582b2e2
Incorporate Emil's feedback
MasterKale Jun 9, 2022
ae3a7d4
Rename options methods from `get-` to `parse-`
MasterKale Jun 9, 2022
b1b5fd1
Incorporate @emlun's copy advice
MasterKale Jun 10, 2022
88bd8fb
Attempt to define rules for when encoding fails
MasterKale Jun 10, 2022
12e5071
Incorporate feedback on wording for exceptions
MasterKale Jun 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
166 changes: 163 additions & 3 deletions index.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1407,6 +1407,7 @@ that are returned to the caller when a new credential is created, or a new asser
[SameObject] readonly attribute AuthenticatorResponse response;
[SameObject] readonly attribute DOMString? authenticatorAttachment;
AuthenticationExtensionsClientOutputs getClientExtensionResults();
PublicKeyCredentialJSON toJSON();
};
</xmp>
<dl dfn-type="attribute" dfn-for="PublicKeyCredential">
Expand All @@ -1424,7 +1425,7 @@ that are returned to the caller when a new credential is created, or a new asser
{{CredentialsContainer/create()}}, this attribute's value will be an {{AuthenticatorAttestationResponse}}, otherwise,
the {{PublicKeyCredential}} was created in response to {{CredentialsContainer/get()}}, and this attribute's value
will be an {{AuthenticatorAssertionResponse}}.

: <dfn>authenticatorAttachment</dfn>
:: This attribute reports the [=authenticator attachment modality=] in effect at the time the
{{CredentialsContainer/create()|navigator.credentials.create()}} or
Expand All @@ -1450,6 +1451,67 @@ that are returned to the caller when a new credential is created, or a new asser
[=extension identifier=] → [=client extension output=] entries produced by the extension's
[=client extension processing=].

: {{PublicKeyCredential/toJSON()}}
emlun marked this conversation as resolved.
Show resolved Hide resolved
:: This operation returns {{RegistrationResponseJSON}} or {{AuthenticationResponseJSON}},
which are [=JSON type=] representations mirroring {{PublicKeyCredential}}, suitable for submission to a
[=[RP]=] server as an `application/json` payload. The [=client=] is in charge of
[serializing values to JSON types as usual](https://webidl.spec.whatwg.org/#idl-tojson-operation),
but MUST take additional steps to first encode any {{ArrayBuffer}} values to {{DOMString}} values
using [=base64url encoding=].

The
{{RegistrationResponseJSON/clientExtensionResults|RegistrationResponseJSON.clientExtensionResults}} or
{{AuthenticationResponseJSON/clientExtensionResults|AuthenticationResponseJSON.clientExtensionResults}}
member MUST be set to the output of {{PublicKeyCredential/getClientExtensionResults()}},
with any {{ArrayBuffer}} values encoded to {{DOMString}} values using
[=base64url encoding=]. This MAY include {{ArrayBuffer}} values from extensions registered
in the IANA "WebAuthn Extension Identifiers" registry [[!IANA-WebAuthn-Registries]] but not
defined in [[#sctn-extensions]].
MasterKale marked this conversation as resolved.
Show resolved Hide resolved

The {{AuthenticatorAttestationResponseJSON/transports|AuthenticatorAttestationResponseJSON.transports}}
member MUST be set to the output of {{AuthenticatorAttestationResponse/getTransports()}}.
</dl>

<xmp class="idl">
typedef DOMString Base64URLString;
typedef (RegistrationResponseJSON or AuthenticationResponseJSON) PublicKeyCredentialJSON;

dictionary RegistrationResponseJSON {
Base64URLString id;
Base64URLString rawId;
AuthenticatorAttestationResponseJSON response;
DOMString? authenticatorAttachment;
AuthenticationExtensionsClientOutputsJSON clientExtensionResults;
DOMString type;
};

dictionary AuthenticatorAttestationResponseJSON {
Base64URLString clientDataJSON;
Base64URLString attestationObject;
sequence<DOMString> transports;
};

dictionary AuthenticationResponseJSON {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about, instead of separate RegistrationResponseJSON and AuthenticationResponseJSON type, we have a single PublicKeyCredentialJSON dictionary type with an AuthenticatorResponseJSON member, with typedef (AuthenticatorAttestationResponseJSON or AuthenticatorAssertionResponseJSON) AuthenticatorResponseJSON?

That would be more akin to how the regular PublicKeyCredential is defined.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's frustrating to me that PublicKeyCredential has been overloaded to mean two possible data structures, since response can be one of two types. I think this is a current developer pain point, that there aren't subclasses of PublicKeyCredential coming out of .create() and .get() to be uniquely typed. That's why I've been trying to emphasize the idea of "RegistrationResponse" and "AuthenticationResponse" here, to get to a 1:1 mapping between navigator.credentials method and return type.

To the suggestion that a PublicKeyCredentialJSON gets defined as a union type, is it a compromise to make it easier for the client to implement this feature? I personally want to see these additions err on the side of better developer usability, and necessarily see this as hoisting some of this complexity onto the client. The aim is to make it easier for devs to consume WebAuthn, and to achieve this I think we should be more straightforward/explicit with return types.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To the suggestion that a PublicKeyCredentialJSON gets defined as a union type, is it a compromise to make it easier for the client to implement this feature? I personally want to see these additions err on the side of better developer usability, and necessarily see this as hoisting some of this complexity onto the client.

@kreichgauer It's been a few weeks, what do you currently think about my response above to your PublicKeyCredentialJSON suggestion?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for the slow reply.

To the suggestion that a PublicKeyCredentialJSON gets defined as a union type, is it a compromise to make it easier for the client to implement this feature

No, I think it makes no difference implementation wise, and if it did that would not be a convincing argument as you point out.

I agree with the desire to aid developers as much as we can, and I agree that the structure of the API with create() and get() returning the same interface type but encapsulating two different response dictionary types is…awkward. (It's a side effect of having WebAuthn bolted on top of Credential Management, but our developers/users shouldn't care about that.) However, I'm kind of on the fence whether I find the separate inheritance hierarchy for JSON serialization more or less confusing. I guess it depends on whether you think the hypothetical WebAuthn developer would first look at the PublicKeyCredential interface IDL or not? If they did, comparing the two and realizing that they're different in weird subtle ways (beyond the binary->base64url conversion) I think could be a bit puzzling? If on the other hand, we'd assume the hypothetical developer would only ever see the JSON IDL, I agree what you propose would be simpler to grok.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it depends on whether you think the hypothetical WebAuthn developer would first look at the PublicKeyCredential interface IDL or not? If they did, comparing the two and realizing that they're different in weird subtle ways (beyond the binary->base64url conversion) I think could be a bit puzzling? If on the other hand, we'd assume the hypothetical developer would only ever see the JSON IDL, I agree what you propose would be simpler to grok.

@kreichgauer Can we leave comments in IDL definitions? I want to believe a developer who doesn't want to get too deep into things will jump to these JSON methods, but it's probably more realistic that they'd stumble on PublicKeyCredential first and then the JSON IDL. If we could leave comments in the JSON IDL explaining why there are split types then perhaps we can have the best of both worlds.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, IDL can contain comments. The syntax is the same as in JavaScript.

Base64URLString id;
Base64URLString rawId;
AuthenticatorAssertionResponseJSON response;
DOMString? authenticatorAttachment;
AuthenticationExtensionsClientOutputsJSON clientExtensionResults;
DOMString type;
};

dictionary AuthenticatorAssertionResponseJSON {
Base64URLString clientDataJSON;
Base64URLString authenticatorData;
Base64URLString signature;
Base64URLString? userHandle;
};

dictionary AuthenticationExtensionsClientOutputsJSON {
};
</xmp>

<dl dfn-type="attribute" dfn-for="PublicKeyCredential">
: <dfn>\[[type]]</dfn>
:: The {{PublicKeyCredential}} [=interface object=]'s {{Credential/[[type]]}} [=internal slot=]'s value is the string
"`public-key`".
Expand Down Expand Up @@ -2387,6 +2449,104 @@ Note: Invoking this method from a [=browsing context=] where the [=Web Authentic

</div>

### Deserialize Registration ceremony options - PublicKeyCredential's `parseCreationOptionsFromJSON()` Method ### {#sctn-parseCreationOptionsFromJSON}

<div link-for-hint="WebAuthentication/parseCreationOptionsFromJSON">

[=[WRPS]=] use this method to convert [=JSON type=] representations of options for
{{CredentialsContainer/create()|navigator.credentials.create()}} into
{{PublicKeyCredentialCreationOptions}}.

Upon invocation, the [=client=] MUST convert the
{{PublicKeyCredential/parseCreationOptionsFromJSON(options)/options}} argument into a new,
identically-structured {{PublicKeyCredentialCreationOptions}} object, using [=base64url encoding=]
to decode any {{DOMString}} attributes in {{PublicKeyCredentialCreationOptionsJSON}} that correspond
to [=buffer source type=] attributes in {{PublicKeyCredentialCreationOptions}}. This conversion MUST
also apply to any [=client extension inputs=] processed by the [=client=].

{{AuthenticationExtensionsClientInputsJSON}} MAY include extensions registered in the IANA
"WebAuthn Extension Identifiers" registry [[!IANA-WebAuthn-Registries]] but not defined in
[[#sctn-extensions]].

If the [=client=] encounters any issues parsing any of the [=JSON type=] representations then it
MUST return an error code equivalent to "{{EncodingError}}" with a description of the incompatible
MasterKale marked this conversation as resolved.
Show resolved Hide resolved
value and terminate the operation.

<xmp class="idl">
partial interface PublicKeyCredential {
static PublicKeyCredentialCreationOptions parseCreationOptionsFromJSON(PublicKeyCredentialCreationOptionsJSON options);
};

dictionary PublicKeyCredentialCreationOptionsJSON {
required PublicKeyCredentialRpEntity rp;
required PublicKeyCredentialUserEntityJSON user;
required Base64URLString challenge;
required sequence<PublicKeyCredentialParameters> pubKeyCredParams;
unsigned long timeout;
sequence<PublicKeyCredentialDescriptorJSON> excludeCredentials = [];
AuthenticatorSelectionCriteria authenticatorSelection;
DOMString attestation = "none";
AuthenticationExtensionsClientInputsJSON extensions;
};

dictionary PublicKeyCredentialUserEntityJSON {
required Base64URLString id;
required DOMString name;
required DOMString displayName;
};

dictionary PublicKeyCredentialDescriptorJSON {
required Base64URLString id;
required DOMString type;
sequence<DOMString> transports;
};

dictionary AuthenticationExtensionsClientInputsJSON {
};
</xmp>

</div>

### Deserialize Authentication ceremony options - PublicKeyCredential's `parseRequestOptionsFromJSON()` Methods ### {#sctn-parseRequestOptionsFromJSON}

<div link-for-hint="WebAuthentication/parseRequestOptionsFromJSON">

[=[WRPS]=] use this method to convert [=JSON type=] representations of options for
{{CredentialsContainer/get()|navigator.credentials.get()}} into
{{PublicKeyCredentialRequestOptions}}.

Upon invocation, the [=client=] MUST convert the
{{PublicKeyCredential/parseRequestOptionsFromJSON(options)/options}} argument into a new,
identically-structured {{PublicKeyCredentialRequestOptions}} object, using [=base64url encoding=]
to decode any {{DOMString}} attributes in {{PublicKeyCredentialRequestOptionsJSON}} that correspond
to [=buffer source type=] attributes in {{PublicKeyCredentialRequestOptions}}. This conversion MUST
also apply to any [=client extension inputs=] processed by the [=client=].

{{AuthenticationExtensionsClientInputsJSON}} MAY include extensions registered in the IANA
"WebAuthn Extension Identifiers" registry [[!IANA-WebAuthn-Registries]] but not defined in
[[#sctn-extensions]].

If the [=client=] encounters any issues parsing any of the [=JSON type=] representations then it
MUST return an error code equivalent to "{{EncodingError}}" with a description of the incompatible
MasterKale marked this conversation as resolved.
Show resolved Hide resolved
value and terminate the operation.

MasterKale marked this conversation as resolved.
Show resolved Hide resolved
<xmp class="idl">
partial interface PublicKeyCredential {
static PublicKeyCredentialRequestOptions parseRequestOptionsFromJSON(PublicKeyCredentialRequestOptionsJSON options);
};

dictionary PublicKeyCredentialRequestOptionsJSON {
required Base64URLString challenge;
unsigned long timeout;
DOMString rpId;
sequence<PublicKeyCredentialDescriptorJSON> allowCredentials = [];
DOMString userVerification = "preferred";
AuthenticationExtensionsClientInputsJSON extensions;
};
</xmp>

</div>

## Authenticator Responses (interface <dfn interface>AuthenticatorResponse</dfn>) ## {#iface-authenticatorresponse}

[=Authenticators=] respond to [=[RP]=] requests by returning an object derived from the
Expand Down Expand Up @@ -3090,7 +3250,7 @@ Note: The {{CollectedClientData}} may be extended in the future. Therefore it's
required DOMString type;
required DOMString challenge;
required DOMString origin;
boolean crossOrigin;
boolean crossOrigin;
};

dictionary TokenBinding {
Expand Down Expand Up @@ -3122,7 +3282,7 @@ Note: The {{CollectedClientData}} may be extended in the future. Therefore it's
: \[RESERVED] <dfn dfn>tokenBinding</dfn>
:: This OPTIONAL member contains information about the state of the [=Token Binding=] protocol [[!TokenBinding]] used when communicating
with the [=[RP]=]. Its absence indicates that the client doesn't support token binding

Note: While [=Token Binding=] was present in Level 1 and Level 2 of WebAuthn, its use is not expected in Level 3. The {{CollectedClientData/tokenBinding}} field is reserved so that it will not be reused for a different purpose.

<div dfn-type="dict-member" dfn-for="TokenBinding">
Expand Down