Fixing Re-authentication with passkeys - #10
Open
ShashankFC wants to merge 2 commits into
Open
Conversation
closes #41242 closes #41008 Signed-off-by: mposolda <mposolda@gmail.com>
This change modifies the method signature to require a UserModel parameter for proper user context validation during conditional passkey checks.
Author
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
|
|
||
| protected boolean isConditionalPasskeysEnabled(UserModel user) { | ||
| return webauthnAuth != null && webauthnAuth.isPasskeysEnabled() && user != null; | ||
| } |
There was a problem hiding this comment.
Passkeys not shown during initial login due to null-user check
High Severity
isConditionalPasskeysEnabled requires user != null, but the old code set up passkeys form data when user was null (initial login). This prevents fillContextForm from being called during initial login, so the enableWebAuthnConditionalUI template attribute is never set, and passkeys UI (conditional autocomplete and the webauthn form) won't render on the initial login page. Multiple existing and new tests expect passkeys to be present during initial login.
Additional Locations (2)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Test 1nnn## Summary by CodeRabbitnn* Bug Fixesn * Enhanced reauthentication error handling to properly manage form state during passkey-based login flows.n * Improved conditional passkey logic to prevent unnecessary form skipping and ensure correct authentication pathway.nn* Testsn * Added comprehensive test coverage for passkey-based reauthentication scenarios across multiple authentication flows.nn✏️ Tip: You can customize this high-level summary in your review settings.nnn---nReplicated from ai-code-review-evaluation/keycloak-coderabbit#1
Note
Medium Risk
Touches core login/reauthentication and passkey UI behavior; regressions could affect which login options appear or how errors are rendered, but changes are scoped and backed by new/updated integration tests.
Overview
Fixes re-authentication flows (
prompt=loginand similar) so conditional passkeys remain available when a user is already identified, rather than skipping username steps or dropping WebAuthn UI when transitioning to the username/password form.Centralizes re-auth error form setup in
AuthenticatorUtils.setupReauthenticationInUsernamePasswordFormErrorand applies it consistently for bad-password and passkey error cases, while adjusting WebAuthn conditional UI to not list authenticators and adding a dedicatedshouldShowWebAuthnAuthenticatorshook.Expands/updates passkey integration tests to cover re-authentication with discoverable keys and external keys, verify WebAuthn UI persistence after failed password attempts, and ensure passkeys are not shown when disabled for the realm.
Written by Cursor Bugbot for commit 3214b18. Configure here.