feat(journey-client): webauthn conditional mediation autofill passkey support …#581
feat(journey-client): webauthn conditional mediation autofill passkey support …#581vatsalparikh wants to merge 1 commit intomainfrom
Conversation
🦋 Changeset detectedLatest commit: 31044cf The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 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 |
📝 WalkthroughWalkthroughAdds WebAuthn conditional mediation support: extends WebAuthn APIs with optional Changes
Sequence DiagramsequenceDiagram
participant Client as Journey App
participant Handler as WebAuthn Handler
participant Support as Browser Capability<br/>(PublicKeyCredential.isConditionalMediationAvailable)
participant Credentials as navigator.credentials
participant Server as Backend
Client->>Handler: authenticate(step, 'conditional', signal)
Handler->>Support: isConditionalMediationSupported()
alt Conditional Mediation Unsupported
Support-->>Handler: false
Handler-->>Client: throw NotSupportedError / setError('unsupported')
else Conditional Mediation Supported
Support-->>Handler: true
alt AbortSignal Missing
Handler-->>Client: throw Error / setError(message)
else AbortSignal Provided
Handler->>Credentials: get({mediation:'conditional', signal, ...})
alt User Authenticates
Credentials-->>Handler: PublicKeyCredential
Handler->>Server: submitForm()
Server-->>Handler: Response
Handler-->>Client: return {callbacksRendered, didSubmit:true}
else Abort/Other Error
Credentials-->>Handler: AbortError / Error
Handler-->>Client: setError(message) / return didSubmit:false
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
vatsalparikh
requested review from
SteinGabriel,
ancheetah,
cerebrl and
ryanbas21
|
View your CI Pipeline Execution ↗ for commit 31044cf
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
e2e/journey-app/components/webauthn-step.ts (1)
68-69: Redundant conditional-mediation support check.
WebAuthn.authenticate(..., 'conditional', signal)already callsisConditionalMediationSupported()internally and throwsNotSupportedErrorif unsupported. Calling it here too means two async feature-detection round-trips per auth step. Consider caching the result, or relying onauthenticate()'s internal check and falling back on theNotSupportedErrorrejection.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/journey-app/components/webauthn-step.ts` around lines 68 - 69, The extra await WebAuthn.isConditionalMediationSupported() + if (isConditionalSupported && conditionalInput) is redundant and causes double feature-detection; remove that pre-check and always call WebAuthn.authenticate(..., 'conditional', signal) when conditionalInput is available, then catch the rejection and detect a NotSupportedError (or exception.name === 'NotSupportedError') to fall back to the non-conditional path. If you care about avoiding repeated detection calls, add a small cache (e.g., a module-level cachedConditionalSupported flag checked/updated when WebAuthn.isConditionalMediationSupported() is first called) and use it instead of awaiting every time. Ensure references to WebAuthn.authenticate, WebAuthn.isConditionalMediationSupported, and the conditionalInput handling are updated accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e/journey-app/components/webauthn-step.ts`:
- Around line 70-78: handleWebAuthnStep currently launches
WebAuthn.authenticate(step, 'conditional', controller.signal) with a local
AbortController that is never aborted, so the in-flight promise can later call
submitForm() or setError() against a stale step; fix by exposing and using an
abort mechanism (e.g., return or register the controller via a module-level
hook) so callers (main.ts and the submit handler) can call controller.abort()
whenever renderForm()/step changes, the form is submitted, or the component is
torn down; specifically ensure the AbortController created in handleWebAuthnStep
(and used in WebAuthn.authenticate) is aborted from main.ts before calling
journeyClient.next(...) and when swapping steps to prevent stale
submitForm()/setError() invocations.
- Around line 71-75: The catch block on WebAuthn.authenticate currently treats
every rejection as a user-facing error; change it so AbortError is ignored: in
the promise rejection handler for WebAuthn.authenticate(step, 'conditional',
controller.signal) check the thrown error (e.g., if (err?.name === 'AbortError')
return;), and only call setError('WebAuthn failed or was cancelled. Please try
again or use a different method.') for non-AbortError failures so cancellations
from the conditional flow (controller.signal/step changes) are not surfaced to
the user; keep successful flow calling submitForm() unchanged.
---
Nitpick comments:
In `@e2e/journey-app/components/webauthn-step.ts`:
- Around line 68-69: The extra await WebAuthn.isConditionalMediationSupported()
+ if (isConditionalSupported && conditionalInput) is redundant and causes double
feature-detection; remove that pre-check and always call
WebAuthn.authenticate(..., 'conditional', signal) when conditionalInput is
available, then catch the rejection and detect a NotSupportedError (or
exception.name === 'NotSupportedError') to fall back to the non-conditional
path. If you care about avoiding repeated detection calls, add a small cache
(e.g., a module-level cachedConditionalSupported flag checked/updated when
WebAuthn.isConditionalMediationSupported() is first called) and use it instead
of awaiting every time. Ensure references to WebAuthn.authenticate,
WebAuthn.isConditionalMediationSupported, and the conditionalInput handling are
updated accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8f68f8e4-60dd-4731-ad4c-0ede19ab8556
📒 Files selected for processing (8)
.changeset/ready-snakes-sell.mde2e/journey-app/components/text-input.tse2e/journey-app/components/validated-username.tse2e/journey-app/components/webauthn-step.tse2e/journey-app/main.tspackages/journey-client/api-report/journey-client.webauthn.api.mdpackages/journey-client/src/lib/webauthn/webauthn.test.tspackages/journey-client/src/lib/webauthn/webauthn.ts
| const controller = new AbortController(); | ||
| void WebAuthn.authenticate(step, 'conditional', controller.signal) | ||
| .then(() => submitForm()) | ||
| .catch(() => { | ||
| setError('WebAuthn failed or was cancelled. Please try again or use a different method.'); | ||
| }); | ||
|
|
||
| return { callbacksRendered: true, didSubmit: false }; | ||
| } |
There was a problem hiding this comment.
Stale in-flight conditional authenticate can submit or error against a newer step.
handleWebAuthnStep returns { callbacksRendered: true, didSubmit: false } while the WebAuthn.authenticate(step, 'conditional', controller.signal) promise keeps running in the background. The AbortController is created but never aborted, so if the user submits via a different method (e.g., username/password on the same step) or renderForm() is invoked again for a new step, the in-flight conditional request can still resolve later and call submitForm() on a now-stale step, or call setError() over the new UI. You should abort the controller when the step changes / form is submitted / component is torn down.
Suggested direction
Expose an abort hook (or module-level controller) so main.ts can cancel the in-flight conditional request before navigating to the next step, and abort it inside the submit handler in main.ts before calling journeyClient.next(...).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@e2e/journey-app/components/webauthn-step.ts` around lines 70 - 78,
handleWebAuthnStep currently launches WebAuthn.authenticate(step, 'conditional',
controller.signal) with a local AbortController that is never aborted, so the
in-flight promise can later call submitForm() or setError() against a stale
step; fix by exposing and using an abort mechanism (e.g., return or register the
controller via a module-level hook) so callers (main.ts and the submit handler)
can call controller.abort() whenever renderForm()/step changes, the form is
submitted, or the component is torn down; specifically ensure the
AbortController created in handleWebAuthnStep (and used in
WebAuthn.authenticate) is aborted from main.ts before calling
journeyClient.next(...) and when swapping steps to prevent stale
submitForm()/setError() invocations.
| void WebAuthn.authenticate(step, 'conditional', controller.signal) | ||
| .then(() => submitForm()) | ||
| .catch(() => { | ||
| setError('WebAuthn failed or was cancelled. Please try again or use a different method.'); | ||
| }); |
There was a problem hiding this comment.
Don't surface an error for AbortError in the conditional flow.
With conditional mediation, authenticate() intentionally rethrows AbortError without mutating the hidden outcome because cancellations are expected (user picked a different method, step changed, controller aborted). The blanket .catch(() => setError(...)) here turns every cancellation into a user-visible "WebAuthn failed or was cancelled" message, which will be noisy during normal autofill UX.
- void WebAuthn.authenticate(step, 'conditional', controller.signal)
- .then(() => submitForm())
- .catch(() => {
- setError('WebAuthn failed or was cancelled. Please try again or use a different method.');
- });
+ void WebAuthn.authenticate(step, 'conditional', controller.signal)
+ .then(() => submitForm())
+ .catch((err: unknown) => {
+ if (err instanceof Error && err.name === 'AbortError') return;
+ setError('WebAuthn failed or was cancelled. Please try again or use a different method.');
+ });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@e2e/journey-app/components/webauthn-step.ts` around lines 71 - 75, The catch
block on WebAuthn.authenticate currently treats every rejection as a user-facing
error; change it so AbortError is ignored: in the promise rejection handler for
WebAuthn.authenticate(step, 'conditional', controller.signal) check the thrown
error (e.g., if (err?.name === 'AbortError') return;), and only call
setError('WebAuthn failed or was cancelled. Please try again or use a different
method.') for non-AbortError failures so cancellations from the conditional flow
(controller.signal/step changes) are not surfaced to the user; keep successful
flow calling submitForm() unchanged.
Codecov Report❌ Patch coverage is
❌ Your project status has failed because the head coverage (16.05%) is below the target coverage (40.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #581 +/- ##
===========================================
- Coverage 70.90% 16.05% -54.85%
===========================================
Files 53 154 +101
Lines 2021 26700 +24679
Branches 377 1152 +775
===========================================
+ Hits 1433 4288 +2855
- Misses 588 22412 +21824
🚀 New features to boost your workflow:
|
@forgerock/davinci-client
@forgerock/device-client
@forgerock/journey-client
@forgerock/oidc-client
@forgerock/protect
@forgerock/sdk-types
@forgerock/sdk-utilities
@forgerock/iframe-manager
@forgerock/sdk-logger
@forgerock/sdk-oidc
@forgerock/sdk-request-middleware
@forgerock/storage
commit: |
|
Deployed 061c866 to https://ForgeRock.github.io/ping-javascript-sdk/pr-581/061c866bd360b2c21cf4a25c021e05d38d89eccc branch gh-pages in ForgeRock/ping-javascript-sdk |
📦 Bundle Size Analysis📦 Bundle Size Analysis🚨 Significant Changes🔻 @forgerock/device-client - 0.0 KB (-9.9 KB, -100.0%) 📊 Minor Changes📉 @forgerock/device-client - 9.9 KB (-0.0 KB) ➖ No Changes➖ @forgerock/oidc-client - 25.2 KB 14 packages analyzed • Baseline from latest Legend🆕 New package ℹ️ How bundle sizes are calculated
🔄 Updated automatically on each push to this PR |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/journey-client/src/lib/webauthn/webauthn.ts (1)
355-371: Optional: mirror the signal guard insidegetAuthenticationCredentialfor defense-in-depth.
authenticate()enforces thatmediation === 'conditional'requires anAbortSignal, butgetAuthenticationCredentialis apublic staticmethod on an exported class (surfaced injourney-client.webauthn.api.md). A caller invoking it directly withmediation: 'conditional'and no signal will get a browser-level rejection, which is acceptable, but a thin guard here would make the public API self-consistent and keep error messages uniform. Not blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/journey-client/src/lib/webauthn/webauthn.ts` around lines 355 - 371, Add a defensive guard in getAuthenticationCredential: before calling navigator.credentials.get, check if mediation === 'conditional' and signal is falsy, and if so throw an Error with a clear message like "AbortSignal required when mediation === 'conditional'" and set the error name to WebAuthnOutcomeType.InvalidStateError (mirroring the enforcement in authenticate); place this check in the getAuthenticationCredential method just after the PublicKeyCredential feature check and before the call to navigator.credentials.get so callers who invoke getAuthenticationCredential directly get a consistent, descriptive error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/journey-client/src/lib/webauthn/webauthn.ts`:
- Around line 355-371: Add a defensive guard in getAuthenticationCredential:
before calling navigator.credentials.get, check if mediation === 'conditional'
and signal is falsy, and if so throw an Error with a clear message like
"AbortSignal required when mediation === 'conditional'" and set the error name
to WebAuthnOutcomeType.InvalidStateError (mirroring the enforcement in
authenticate); place this check in the getAuthenticationCredential method just
after the PublicKeyCredential feature check and before the call to
navigator.credentials.get so callers who invoke getAuthenticationCredential
directly get a consistent, descriptive error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9f1390bc-a634-4fc6-9b70-94c33f98dafb
📒 Files selected for processing (8)
.changeset/ready-snakes-sell.mde2e/journey-app/components/text-input.tse2e/journey-app/components/validated-username.tse2e/journey-app/components/webauthn-step.tse2e/journey-app/main.tspackages/journey-client/api-report/journey-client.webauthn.api.mdpackages/journey-client/src/lib/webauthn/webauthn.test.tspackages/journey-client/src/lib/webauthn/webauthn.ts
✅ Files skipped from review due to trivial changes (2)
- e2e/journey-app/components/text-input.ts
- e2e/journey-app/components/validated-username.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- .changeset/ready-snakes-sell.md
- e2e/journey-app/main.ts
2 participants
JIRA Ticket
pingidentity.atlassian.net/browse/SDKS-4612
Description
Add WebAuthn conditional mediation (passkey autofill) support to @forgerock/journey-client.
This enables consumers to opt-in to conditional UI by passing mediation: 'conditional' and an AbortSignal through to navigator.credentials.get(...) with a helper to feature-detect browser support.
What changed
How to test
e2e/journey-appfolder and with commandspnpm buildfollowed bypnpm serveRecording
Screen.Recording.2026-04-23.at.11.53.07.AM.mov
Did you add a changeset?
Yes
Summary by CodeRabbit
New Features
Bug Fixes / UX
Tests