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

Added credProps.authenticatorDisplayName #363

Merged
merged 4 commits into from
Jul 19, 2024

Conversation

fdennis
Copy link
Contributor

@fdennis fdennis commented Jun 13, 2024

No description provided.

@fdennis fdennis requested a review from emlun June 13, 2024 12:42
Copy link

github-actions bot commented Jun 13, 2024

Test Results

2 218 tests  +12   2 210 ✅ +12   1m 31s ⏱️ +33s
   45 suites ± 0       8 💤 ± 0 
   45 files   ± 0       0 ❌ ± 0 

Results for commit a1a3e2f. ± Comparison against base commit 563e5c3.

♻️ This comment has been updated with latest results.

@warchildmd
Copy link

When will this be merged? 1Password is sending this field and it's failing because of FAIL_ON_UNKNOWN_PROPERTIES

@JDussaussois
Copy link

@warchildmd : Same here, i'm interested about a merge 👍

@emlun
Copy link
Member

emlun commented Jul 9, 2024

@warchildmd Which version of webauthn-server-core are you using? That issue should have been fixed in release 2.5.2.

@JDussaussois
Copy link

@emlun : I'm actually using release 2.5.2 and the error is still here :
UnrecognizedPropertyException: Unrecognized field "authenticatorDisplayName"

@emlun
Copy link
Member

emlun commented Jul 9, 2024

Ok, we can fast track a bug fix for that then. Could you please open a new issue and include the full stack trace of the exception?

@JDussaussois
Copy link

My bad, after a deep check, i'm actually using 2.5.1 in my deployed env, i'll update to 2.5.2 and check, thanks for the feedback

@emlun
Copy link
Member

emlun commented Jul 11, 2024

@JDussaussois Did the upgrade fix your problem?

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.

Looking good! I tweaked the JavaDocs a bit, and while doing that I realized that the new version of the extension is also applicable to assertions, so we need to parse and expose credProps in ClientAssertionExtensionOutputs and AssertionResult too. I'll push those changes on top of this, they're fully analogous to what's here already.

@emlun emlun force-pushed the credProps-authenticatorDisplayName branch from 35b4880 to a1a3e2f Compare July 11, 2024 13:53
@fdennis fdennis merged commit 362f1b4 into main Jul 19, 2024
20 checks passed
@fdennis fdennis deleted the credProps-authenticatorDisplayName branch July 19, 2024 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants