-
Notifications
You must be signed in to change notification settings - Fork 47
fix(javascript-sdk): prioritize displayName for WebAuthn name attribute #527
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
Conversation
🦋 Changeset detectedLatest commit: 5119dc0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
acb3012 to
03fca92
Compare
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 5119dc0. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 2 targetsSent with 💌 from NxCloud. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #527 +/- ##
========================================
Coverage 57.24% 57.25%
========================================
Files 123 123
Lines 26966 26967 +1
Branches 1718 1717 -1
========================================
+ Hits 15437 15439 +2
+ Misses 11529 11528 -1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update copyright
rodrigoareis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes looks good to me. Just missing copyright updates
george-bafaloukas-forgerock
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM please update copyright.
| console.log('Choose Passwordless login'); | ||
| const cb = step.getCallbackOfType('ChoiceCallback'); | ||
| cb.setChoiceIndex(0); | ||
| const cb = step.getCallbackOfType('ConfirmationCallback'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
has the test journey change and send a confirmation callback?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test journey was removed, so I duplicated a journey that Stoyan uses, and it had a Confirmation Callback instead. This is a manual test, and not automation, so changes here don't affect CI.
- this uses `displayName || userName` for assignment to the `name` attribute of the WebAuthn options object - This allows for a more readable value for when WebAuthn device is saved
03fca92 to
5119dc0
Compare
rodrigoareis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
JIRA Ticket
SDKS-3473
Description
In order to display a more user-friendly name when saving a WebAuthn/Passkey device to an account, we prioritized displayName over userName for assignment to the
nameproperty of the WebAuthn options object. This avoids the display of UUIDs for saved credentials.