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.

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

Include the "Easily accessing credential data" fields in JSON. #1887

Merged
merged 4 commits into from Jun 14, 2023

Conversation

agl
Copy link
Contributor

@agl agl commented May 4, 2023

The WebAuthn API provides accessors to get the SPKI-formatted public key and authenticator data without needing to parse CBOR or handle COSE. However, the JSON structures, prior to this change, didn't include these values giving users an unfortunate choice: either use the accessors and do the JSON encoding yourself, or use the provided toJSON function.

But we can have both!


Preview | Diff

The WebAuthn API provides accessors to get the SPKI-formatted public key
and authenticator data without needing to parse CBOR or handle COSE.
However, the JSON structures, prior to this change, didn't include these
values giving users an unfortunate choice: either use the accessors and
do the JSON encoding yourself, or use the provided `toJSON` function.

But we can have both!
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
Copy link
Contributor

@MasterKale MasterKale left a comment

Choose a reason for hiding this comment

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

lgmt lgtm :shipit:

Copy link
Member

@emlun emlun left a comment

Choose a reason for hiding this comment

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

LGTM, with or without the below suggestion.

index.bs Outdated Show resolved Hide resolved
Co-authored-by: Emil Lundberg <emil@yubico.com>
@agl
Copy link
Contributor Author

agl commented May 17, 2023

(from the call of 2023-05-17: good to merge when checks are happy.)

@MasterKale MasterKale merged commit e0aedf3 into json Jun 14, 2023
2 checks passed
@MasterKale MasterKale deleted the jsoneasy branch June 14, 2023 19:52
@emlun
Copy link
Member

emlun commented Jun 14, 2023

I realize belatedly that we probably also need instructions on how to populate these new fields, like we have for clientExtensionResults and transports:

webauthn/index.bs

Lines 1592 to 1602 in 6dfbdba

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]].
The {{AuthenticatorAttestationResponseJSON/transports|AuthenticatorAttestationResponseJSON.transports}}
member MUST be set to the output of {{AuthenticatorAttestationResponse/getTransports()}}.

I'll open a follow-up PR.

@emlun
Copy link
Member

emlun commented Jun 14, 2023

I'll fix the above issue in PR #1906.

MasterKale added a commit that referenced this pull request Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants