-
Notifications
You must be signed in to change notification settings - Fork 47
feat(javascript-sdk): allow for json response outcome for webauthn #537
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: fc00259 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
View your CI Pipeline Execution ↗ for commit fc00259.
☁️ Nx Cloud last updated this comment at |
| } | ||
|
|
||
| hiddenCallback.setInputValue( | ||
| deviceName && deviceName.length > 0 ? `${outcome}::${deviceName}` : outcome, |
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.
I think that you will be missing the deviceName from the outcome in the case that the if (metadataCallback) { is true and you send the new format
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.
Ah, good catch. Will update now.
|
|
||
| if (metadataCallback) { | ||
| const meta = metadataCallback.getOutputValue('data') as WebAuthnAuthenticationMetadata; | ||
| if (meta?.supportsJsonResponse && credential && 'authenticatorAttachment' in credential) { |
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.
One minor I would suggest keys like authenticatorAttachment and data to be used from Constants files as they are re-used across files.
fe691bd to
9d2ce8c
Compare
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
9d2ce8c to
fc00259
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #537 +/- ##
===========================================
+ Coverage 58.47% 58.57% +0.10%
===========================================
Files 105 105
Lines 25307 25461 +154
Branches 1690 1690
===========================================
+ Hits 14799 14915 +116
- Misses 10508 10546 +38 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
JIRA Ticket
SDKS-3843
Description
This adds the optional outcome type to the HiddenValueCallback of JSON to provide
authenticatorAttachmentproperty.Did you add a changeset? Yes!