Skip to content

Conversation

@AlfioEmanueleFresta
Copy link
Member

⚠️ AI-Assisted PR: This pull request was developed with the assistance of an AI coding agent (Claude Opus 4.5).


Summary

This PR is the second in a series (follows #138) and adds JSON serialization for WebAuthn responses - the inverse of the existing JSON request parsing functionality.

Changes

New Response Serialization Infrastructure

  • Add WebAuthnIDLResponse trait - Provides to_json() and to_inner_model() methods for converting responses to JSON format
  • Add JsonFormat enum - Supports both minified and prettified JSON output
  • Add ResponseSerializationError - Error type for serialization failures

New JSON Response Models (per WebAuthn Level 3 spec)

  • RegistrationResponseJSON - For MakeCredentialResponse serialization
  • AuthenticationResponseJSON - For Assertion serialization
  • AuthenticatorAttestationResponseJSON - Attestation response details
  • AuthenticatorAssertionResponseJSON - Assertion response details
  • AuthenticationExtensionsClientOutputsJSON - Extension output serialization

Request Structure Refactoring

Refactored MakeCredentialRequest and GetAssertionRequest to enable client data generation on-the-fly:

  • Replaced hash: Vec<u8> with challenge: Vec<u8> - Store raw challenge instead of pre-computed hash
  • Added origin: String field - Store origin explicitly
  • Added cross_origin: Option<bool> field - Optional cross-origin flag
  • Removed client_data: ClientData field - No longer needed as separate field
  • Added helper methods:
    • client_data() - Builds ClientData internally
    • client_data_hash() - Computes SHA-256 hash on demand
    • client_data_json() - Returns JSON bytes for response serialization

Implementation Details

  • Implemented WebAuthnIDLResponse for MakeCredentialResponse with full attestation object CBOR serialization
  • Implemented WebAuthnIDLResponse for Assertion with signature and authenticator data handling
  • Support for extension outputs: credProps, hmacCreateSecret, hmacGetSecret, largeBlob, prf
  • Added Serialize derives to attestation statement types for CBOR encoding

Updated Files

  • All example files updated to use new request structure
  • All test files updated
  • CTAP2 model conversions updated to use client_data_hash()
  • U2F fallback code updated
  • WebAuthn preflight logic updated

Testing

  • All 85 existing tests pass
  • New unit tests added for response serialization
  • Updated webauthn_json_hid.rs example to demonstrate JSON response output

Example Usage

use libwebauthn::ops::webauthn::{WebAuthnIDLResponse, JsonFormat};

// After a MakeCredential operation
let response = channel.webauthn_make_credential(&request).await?;
let json = response.to_json(&request, JsonFormat::Prettified)?;
println!("{}", json);

// After a GetAssertion operation  
for assertion in &response.assertions {
    let json = assertion.to_json(&request, JsonFormat::Minified)?;
    println!("{}", json);
}

@AlfioEmanueleFresta AlfioEmanueleFresta changed the title feat(idl): Add WebAuthn response JSON serialization Web IDL support 2/N: response JSON serialization Dec 19, 2025
@AlfioEmanueleFresta AlfioEmanueleFresta changed the base branch from master to json December 19, 2025 00:08
@AlfioEmanueleFresta
Copy link
Member Author

@msirringhaus — Following up on your review comment on PR #138 regarding clientExtensionResults handling:

I investigated the WebAuthn Level 3 specification and found that clientExtensionResults is marked as required in both response types:

This matches the behavior observed on demo.yubico.com, which always returns "clientExtensionResults": {} even when no extensions produce output.

So the current implementation in this PR is correct — we always include clientExtensionResults (as an empty object {} if no extensions produced output). The two cases you mentioned:

  1. Extensions absent in request → Response includes "clientExtensionResults": {}
  2. Extensions present but empty (extensions: {}) → Response includes "clientExtensionResults": {}

Both cases result in the same output per the spec, since clientExtensionResults is always required.


This comment was written by GitHub Copilot (Claude) operating on @AlfioEmanueleFresta's instructions.

This adds the inverse of JSON request parsing - serializing WebAuthn
responses back to JSON format per WebAuthn Level 3 specification.

Changes:
- Add WebAuthnIDLResponse trait for response-to-JSON conversion
- Add RegistrationResponseJSON and AuthenticationResponseJSON models
- Implement to_json() for MakeCredentialResponse and Assertion
- Refactor requests to store challenge/origin/cross_origin separately
- Add client_data_hash() and client_data_json() helper methods
- Update all examples and tests to use new request structure
- Add unit tests for response serialization
Copy link
Collaborator

@msirringhaus msirringhaus left a comment

Choose a reason for hiding this comment

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

Looking good to me. Some minor comments and questions inline.

impl ClientData {
pub fn hash(&self) -> Vec<u8> {
/// Returns the canonical JSON representation of the client data.
pub fn to_json_bytes(&self) -> Vec<u8> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if it would be more versatile to make this to_json() and return a String. Those who need the bytes can then call into_bytes() on the result.

hash: Vec::from(challenge),
challenge: Vec::from(challenge),
origin: "example.org".to_string(),
cross_origin: None,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we have any tests where cross_origin is not None?

self.client_data().hash()
}

pub fn client_data_json(&self) -> Vec<u8> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same question as above. More so here: The naming client_data_json() would lead me to expect a String.

let parsed: serde_json::Value = serde_json::from_str(&json_str).unwrap();

// Verify required fields
assert!(parsed.get("id").is_some());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to also check the contents of the fields, not just if they are Some()?


/// JSON output format options.
#[derive(Debug, Clone, Copy, Default)]
pub enum JsonFormat {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it intentional to 'reimplement' this, instead of exposing a serde_json-Value, which the user can then use and format according to whatever serde_json offers?
I don't have a strong preference for either way, just wanting to understand the reasoning better, if there is any.

request: &Self::Context,
) -> Result<Self::InnerModel, ResponseSerializationError> {
// Get credential ID from attested credential data
let credential_id_bytes = self
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same comment as for the GetAssertion-case regarding crential IDs.

},
authenticator_attachment: None,
client_extension_results,
r#type: "public-key".to_string(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm ok with this for now, as we do it in lots of places, but somehow my engineering mind would feel more at ease, if we could make this a transparent enum instead of a hardcoded string in a bunch of places, to avoid typos. But this doesn't have to be in this PR.

let parsed: serde_json::Value = serde_json::from_str(&json_str).unwrap();

// Verify required fields
assert!(parsed.get("id").is_some());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as with GetAssertion: Can we check the contents as well for those that are easy/obvious?

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.

3 participants