feat(journey-client): add hasPasskeyAutocompleteValues to WebAuthn#720
feat(journey-client): add hasPasskeyAutocompleteValues to WebAuthn#720vatsalparikh wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: 5472d71 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 |
|
View your CI Pipeline Execution ↗ for commit 5472d71
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗ ☁️ Nx Cloud last updated this comment at |
vatsalparikh
requested review from
SteinGabriel,
ancheetah,
cerebrl and
ryanbas21
📝 WalkthroughWalkthroughAdds an optional ChangesWebAuthn Mediation Override and Manual Passkey Button
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant WebAuthnStep
participant WebAuthn
participant AbortController
WebAuthnStep->>WebAuthn: isConditionalMediationSupported()
WebAuthnStep->>AbortController: create controller
WebAuthnStep->>WebAuthn: authenticate(step, signal) [background]
alt background succeeds
WebAuthn-->>WebAuthnStep: success
WebAuthnStep->>WebAuthnStep: submitForm()
else user clicks manual button
User->>WebAuthnStep: click "Sign in with a passkey"
WebAuthnStep->>AbortController: abort()
WebAuthnStep->>WebAuthn: authenticate(step, undefined, 'optional')
WebAuthn-->>WebAuthnStep: result
WebAuthnStep->>WebAuthnStep: submitForm() or setError()
end
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/journey-client/src/lib/webauthn/webauthn.ts (1)
190-233: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAbort the prior conditional request before the modal fallback.
mediationOverridecan send a secondauthenticate()call down the non-conditional path whilethis.conditionalAbortControlleris still pending, so the original conditionalnavigator.credentials.get()keeps running alongside the new modal request. Abort the stored controller here too.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/journey-client/src/lib/webauthn/webauthn.ts` around lines 190 - 233, The authentication flow in `getAuthenticationOutcome` should also cancel any in-flight conditional request when `mediationOverride` routes a retry into the non-conditional/modal path. Update the logic around `this.conditionalAbortController`, `mediationOverride`, and `getAuthenticationCredential` so the stored controller is aborted before starting the fallback request, not only in the `mediation === 'conditional'` branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/journey-app/components/webauthn-step.ts`:
- Around line 68-72: The WebAuthn error handler in WebAuthn.authenticate should
not surface an error for the intentional conditional-request abort. Update the
catch branch in webauthn-step’s submit flow to inspect the thrown error and skip
setError when it is an AbortError, while still showing the existing failure
message for real authentication problems so the manual fallback can continue
cleanly.
---
Outside diff comments:
In `@packages/journey-client/src/lib/webauthn/webauthn.ts`:
- Around line 190-233: The authentication flow in `getAuthenticationOutcome`
should also cancel any in-flight conditional request when `mediationOverride`
routes a retry into the non-conditional/modal path. Update the logic around
`this.conditionalAbortController`, `mediationOverride`, and
`getAuthenticationCredential` so the stored controller is aborted before
starting the fallback request, not only in the `mediation === 'conditional'`
branch.
🪄 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: 60f2d951-4056-456f-ace4-cd7b14d14fb0
📒 Files selected for processing (9)
.changeset/webauthn-mediation-override.mde2e/journey-app/components/text-input.tse2e/journey-app/components/webauthn-step.tse2e/journey-suites/src/webauthn-device.test.tspackages/journey-client/api-report/journey-client.webauthn.api.mdpackages/journey-client/src/lib/webauthn/interfaces.tspackages/journey-client/src/lib/webauthn/webauthn.mock.data.tspackages/journey-client/src/lib/webauthn/webauthn.test.tspackages/journey-client/src/lib/webauthn/webauthn.ts
💤 Files with no reviewable changes (1)
- e2e/journey-app/components/text-input.ts
| void WebAuthn.authenticate(step, 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.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the relevant files and show compact structure first.
git ls-files | rg '(^|/)e2e/journey-app/components/webauthn-step\.ts$|(^|/)webauthn|(^|/)passkey|(^|/)auth' || true
echo '--- outline: e2e/journey-app/components/webauthn-step.ts ---'
ast-grep outline e2e/journey-app/components/webauthn-step.ts --view expanded || true
# Read the relevant slice with line numbers.
echo '--- webauthn-step.ts (selected lines) ---'
sed -n '1,180p' e2e/journey-app/components/webauthn-step.ts | cat -n
# Search for the controller abort and authenticate implementation/usages.
echo '--- search: controller.abort / AbortError / WebAuthn.authenticate ---'
rg -n "controller\.abort|AbortError|WebAuthn\.authenticate|mediation === 'conditional'|conditional" e2e/journey-app . --glob '!**/node_modules/**' || true
# If WebAuthn.authenticate is in a nearby file, inspect it.
AUTH_FILE=$(rg -l "class WebAuthn|function authenticate|const WebAuthn|authenticate\\(" e2e/journey-app . --glob '!**/node_modules/**' | head -n 20 | tr '\n' ' ')
echo "--- candidate auth files: ${AUTH_FILE} ---"
for f in $AUTH_FILE; do
echo "--- $f ---"
sed -n '1,260p' "$f" | cat -n | sed -n '1,260p'
doneRepository: ForgeRock/ping-javascript-sdk
Length of output: 50386
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- packages/journey-client/src/lib/webauthn/webauthn.ts (relevant slices) ---'
sed -n '160,270p' packages/journey-client/src/lib/webauthn/webauthn.ts | cat -n
echo
sed -n '610,670p' packages/journey-client/src/lib/webauthn/webauthn.ts | cat -n
echo '--- e2e/journey-suites/src/webauthn-device.test.ts (conditional/passkey cases) ---'
sed -n '300,380p' e2e/journey-suites/src/webauthn-device.test.ts | cat -nRepository: ForgeRock/ping-javascript-sdk
Length of output: 11725
Ignore AbortError from the background conditional request
e2e/journey-app/components/webauthn-step.ts:68-72 will show “WebAuthn failed or was cancelled…” when the passkey button intentionally aborts the in-flight conditional request. Skip setError(...) for AbortError here so the manual flow can continue cleanly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/journey-app/components/webauthn-step.ts` around lines 68 - 72, The
WebAuthn error handler in WebAuthn.authenticate should not surface an error for
the intentional conditional-request abort. Update the catch branch in
webauthn-step’s submit flow to inspect the thrown error and skip setError when
it is an AbortError, while still showing the existing failure message for real
authentication problems so the manual fallback can continue cleanly.
@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: |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (23.16%) 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 #720 +/- ##
==========================================
+ Coverage 18.07% 23.16% +5.09%
==========================================
Files 155 161 +6
Lines 24398 25612 +1214
Branches 1203 1617 +414
==========================================
+ Hits 4410 5934 +1524
+ Misses 19988 19678 -310
🚀 New features to boost your workflow:
|
|
Deployed b308f69 to https://ForgeRock.github.io/ping-javascript-sdk/pr-720/b308f69db4859ff302f25d7d63f29af7e4702677 branch gh-pages in ForgeRock/ping-javascript-sdk |
📦 Bundle Size Analysis📦 Bundle Size Analysis🚨 Significant Changes🔻 @forgerock/device-client - 0.0 KB (-10.0 KB, -100.0%) 📊 Minor Changes📉 @forgerock/device-client - 10.0 KB (-0.0 KB) ➖ No Changes➖ @forgerock/oidc-client - 35.3 KB 14 packages analyzed • Baseline from latest Legend🆕 New package ℹ️ How bundle sizes are calculated
🔄 Updated automatically on each push to this PR |
…r passkey autofill
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
At least one additional CI pipeline execution has run since the conclusion below was written and it may no longer be applicable.
Nx Cloud is proposing a fix for your failed CI:
We updated the "Authenticate with the registered WebAuthn device" test step to align with the PR's new non-blocking WebAuthn behaviour, where authenticate() fires on render and the virtual authenticator auto-resolves it, calling submitForm() in the background. The old test was manually filling the username and clicking Submit — a pattern from the previous sequential flow — which raced with the background auto-resolve, causing the form to be submitted without the WebAuthn outcome set. Removing the manual interactions and simply waiting for the Logout button matches what all four new conditional-autofill tests (000–110) already do.
Warning
❌ We could not verify this fix.
diff --git a/e2e/journey-suites/src/webauthn-device.test.ts b/e2e/journey-suites/src/webauthn-device.test.ts
index 52538a0..9941523 100644
--- a/e2e/journey-suites/src/webauthn-device.test.ts
+++ b/e2e/journey-suites/src/webauthn-device.test.ts
@@ -92,9 +92,8 @@ test.describe('WebAuthn register, authenticate, and delete device', () => {
await expect(page.getByRole('button', { name: 'Submit' })).toBeVisible();
await navigate(authenticationUrl);
- await expect(page.getByLabel('User Name')).toBeVisible();
- await page.getByLabel('User Name').fill(username);
- await clickButton('Submit', '/authenticate');
+ // WebAuthn fires automatically on render. The virtual authenticator auto-resolves it,
+ // calling submitForm() in the background — no manual form interaction is needed.
await expect(page.getByRole('button', { name: 'Logout' })).toBeVisible();
});
Or Apply changes locally with:
npx nx-cloud apply-locally vWFv-39tY
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
vatsalparikh
changed the title
SteinGabriel
left a comment
SteinGabriel
left a comment
There was a problem hiding this comment.
Changes look good. There's a stale copyright date in webauthn.mock.data.ts, but other than that it looks good to merge.
3 participants
Summary
Adds passkey-autofill support to the journey-app WebAuthn flow, backed by two new public helpers on the
WebAuthnclass and an optionalmediationOverrideparameter onauthenticate().The journey-app now fires WebAuthn once, non-blocking, on every authentication step. The SDK decides silent (conditional mediation) vs. modal prompt internally from the step metadata, so the app never has to branch on mediation. A manual "Sign in with a passkey" button is rendered only for passkey-autofill steps, and clicking it forces a modal prompt via
mediationOverride.SDK changes (
@forgerock/journey-client)WebAuthn.hasPasskeyAutocompleteValues(step)— returnstruewhen aNameCallbackcarries bothusernameandwebauthnautocomplete values (AM's signal that the step supports passkey autofill).WebAuthn.hasAuthenticationButton(step)— returnstruewhen the metadata setsmanualButtonEnabled.WebAuthn.authenticate(step, signal?, mediationOverride?)— new optionalmediationOverride. When provided it takes precedence over the metadata mediation (mediationOverride ?? meta.mediation), letting the manual button force a modal (non-conditional) prompt even when the server requested conditional mediation. Server behavior is unchanged when the argument is omitted.The three AM config flags
Passkey-autofill behavior is driven by three server-controlled signals in the authentication step:
NameCallbackautocompleteValuescontainsusername+webauthnautocomplete="username webauthn". Detected viahasPasskeyAutocompleteValues.mediation: 'conditional'navigator.credentials.get()with conditional mediation → silent autofill (no modal), surfaced inline in the username field.manualButtonEnabled: truehasAuthenticationButton.autocompletegates the whole passkey path: without it, the input is not decorated and the button is never rendered, regardless of the other two flags. Note also that AM only sendsmanualButtonEnabled: truetogether with conditional mediation — the two are coupled server-side.Behavior across all 8 combinations
authenticate()is fired on render for every authentication step, non-blocking. The SDK then decides silent vs. modal from the metadata mediation:mediation === 'conditional'runs a silent conditional request; anything else runs a modalnavigator.credentials.get(). There is no submit-triggered prompt — the WebAuthn request always starts on render.isConditionalSupportedis a separate browser capability check, orthogonal to these three AM flags; the table below assumes a browser that supports conditional mediation.The autofill input is decorated exactly when
autocompleteis present, so it is not shown as a separate column.manualButtonEnabled: falsewithout conditional mediationKey invariants:
authenticate()is fired withoutawaitand the handler returns immediately.autocompleteis present —manualButtonEnabledalone is insufficient (see 001 / 011).mediationOverride: 'optional'), so the user can explicitly pick a passkey.Tests
e2e/journey-suites/src/webauthn-device.test.tscovers all 8 cases plus two variants of the full-feature (111) case:mediationOverridepath end to end.The autofill cases fire WebAuthn on render, which the auto-resolving virtual authenticator would otherwise complete before the transient UI can be asserted. A
setAutomaticPresenceSimulation(false)helper holds the request open so the rendered input/button can be checked deterministically. Silent conditional completion depends on real autofill-dropdown interaction that the virtual authenticator cannot drive, so the autofill cases assert the rendered UI rather than final login.Unit tests in
packages/journey-client/src/lib/webauthn/webauthn.test.tscoverhasPasskeyAutocompleteValuesandhasAuthenticationButton.🤖 Generated with Claude Code