Skip to content

Fixing Re-authentication with passkeys#10

Open
ShashankFC wants to merge 2 commits into
improve-auth-user-experiencefrom
enhance-passkey-authentication-flow
Open

Fixing Re-authentication with passkeys#10
ShashankFC wants to merge 2 commits into
improve-auth-user-experiencefrom
enhance-passkey-authentication-flow

Conversation

@ShashankFC

@ShashankFC ShashankFC commented Feb 13, 2026

Copy link
Copy Markdown

User description

Test 1

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced reauthentication error handling to properly manage form state during passkey-based login flows.
    • Improved conditional passkey logic to prevent unnecessary form skipping and ensure correct authentication pathway.
  • Tests

    • Added comprehensive test coverage for passkey-based reauthentication scenarios across multiple authentication flows.

✏️ Tip: You can customize this high-level summary in your review settings.


Replicated from ai-code-review-evaluation/keycloak-coderabbit#1


CodeAnt-AI Description

Fix re-authentication with passkeys

What Changed

  • Passkey (WebAuthn) UI is now shown and usable during re-authentication prompts (e.g., OIDC prompt=login), so users can re-authenticate with passkeys as they do for initial login.
  • When re-authentication fails (bad password) the username is preserved and the page remains configured for re-authentication (username hidden, registration disabled) so passkey options stay available without losing context.
  • Conditional passkey checks are tied to the current user context to avoid skipping the username form incorrectly; several tests added/updated to cover passkey and password re-authentication scenarios (including discoverable and external keys).

Impact

✅ Passkeys available during re-authentication
✅ Re-authentication preserves username and passkey UI after failed login
✅ Test coverage for passkey re-authentication scenarios

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

mposolda and others added 2 commits July 22, 2025 10:00
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.
@ShashankFC

Copy link
Copy Markdown
Author

@CodeAnt-AI: review

@codeant-ai

codeant-ai Bot commented Feb 13, 2026

Copy link
Copy Markdown

CodeAnt AI is running the review.

@codeant-ai codeant-ai Bot added the size:L This PR changes 100-499 lines, ignoring generated files label Feb 13, 2026
@codeant-ai

codeant-ai Bot commented Feb 13, 2026

Copy link
Copy Markdown

Sequence Diagram

Shows how the authenticator now conditionally prepares passkey UI for username/password flows and ensures re-authentication errors keep the form in "re-authentication" state (username hidden) so passkey-based re-authentication works correctly.

sequenceDiagram
    participant User
    participant Frontend as "Login Page"
    participant Backend as "Username/Password Authenticator"
    participant WebAuthn as "WebAuthn Module"
    participant Utils as "AuthenticatorUtils"

    User->>Frontend: Open login / re-authentication page
    Frontend->>Backend: Render username/password form
    Backend->>Backend: Check isConditionalPasskeysEnabled(user)
    alt Passkeys enabled for this context
        Backend->>WebAuthn: fillContextForm(context)   // prepare challenge & authenticators
        WebAuthn-->>Frontend: Passkey UI data
        Frontend->>User: Show passkey option alongside form
        User->>Frontend: Use passkey to authenticate
        Frontend->>WebAuthn: Submit passkey assertion
        WebAuthn-->>Backend: Verify assertion
        Backend-->>User: Authentication success
    else Passkeys not enabled
        Backend-->>Frontend: Render regular username/password form
    end

    Note over Backend,Utils: On re-authentication errors
    Backend->>Utils: setupReauthenticationInUsernamePasswordFormError(context)
    Utils-->>Frontend: Form attributes set (username hidden, registration disabled)
Loading

Generated by CodeAnt AI

@codeant-ai

codeant-ai Bot commented Feb 13, 2026

Copy link
Copy Markdown

Nitpicks 🔍

🔒 No security issues identified
⚡ Recommended areas for review

  • Reauthentication UI handling
    The authenticator now forces conditional UI and calls the new setup helper when an error occurs. Confirm that the USER_SET_BEFORE_USERNAME_PASSWORD_AUTH note is always set appropriately in the flows where re-authentication is expected; otherwise the helper will not enable re-authentication mode and passkey UI may be lost after errors.

  • Method call mismatch
    The new condition calls isConditionalPasskeysEnabled() with no arguments, but the concrete implementation added in UsernamePasswordForm expects a UserModel parameter (isConditionalPasskeysEnabled(UserModel user)). This looks like an oversight and may cause the wrong overload to be called or rely on an unexpected method; verify the intended call and fix to ensure the conditional behaves as expected (likely isConditionalPasskeysEnabled(context.getUser())).

  • Possible NPE
    The new helper setupReauthenticationInUsernamePasswordFormError accesses context.getAuthenticationSession() and context.form() without null checks. In some edge or unit-test flows the authentication session or form may be null, leading to NPEs. Validate callers always provide non-null context/session/form, or add defensive checks.

  • Flaky element presence checks
    The tests use driver.findElement(...) inside try/catch or assertThat(..., nullValue()) to verify absence of elements. findElement throws when element is not present and relying on exceptions leads to noisy tests and potential flakiness. Prefer non-throwing checks (findElements or explicit wait conditions) to make assertions deterministic and clearer.

  • Broad exception handling
    The tests catch generic Exception when verifying element absence, which can hide unrelated problems (e.g., driver failures). Narrow the exception type (NoSuchElementException) or avoid exception-driven logic altogether to avoid masking real issues.

@Override
public void authenticate(AuthenticationFlowContext context) {
if (context.getUser() != null) {
if (context.getUser() != null && !isConditionalPasskeysEnabled()) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The call to isConditionalPasskeysEnabled() in the authentication check omits the required user argument, so it cannot correctly determine whether passkeys are enabled for the current user and will not behave as intended; pass context.getUser() to align with the helper's contract and other call sites. [type error]

Severity Level: Critical 🚨
- ❌ Services module fails to compile due to missing method.
- ❌ Browser login flows using UsernameForm cannot be deployed.
- ⚠️ WebAuthn/passkeys tests using UsernameFormFactory cannot execute.
Suggested change
if (context.getUser() != null && !isConditionalPasskeysEnabled()) {
if (context.getUser() != null && !isConditionalPasskeysEnabled(context.getUser())) {
Steps of Reproduction ✅
1. Build the `services` module of this repo (e.g. run `mvn -pl services compile`), which
compiles `UsernameForm` as part of the browser authenticators.

2. During compilation, the Java compiler reaches
`services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernameForm.java:47`,
which calls `isConditionalPasskeysEnabled()` with no arguments.

3. The only available definition is `protected boolean
isConditionalPasskeysEnabled(UserModel user)` in `UsernamePasswordForm` at
`services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernamePasswordForm.java:160`,
and all other call sites (e.g. lines 115 and 137 in the same file) pass
`context.getUser()`.

4. Because there is no zero-argument overload in `UsernameForm`, `UsernamePasswordForm`,
or `AbstractUsernameFormAuthenticator` (verified via Grep/Read), the compiler reports a
"cannot find symbol: method isConditionalPasskeysEnabled()" error, causing the build for
the module (and thus the PR) to fail.
Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernameForm.java
**Line:** 47:47
**Comment:**
	*Type Error: The call to `isConditionalPasskeysEnabled()` in the authentication check omits the required user argument, so it cannot correctly determine whether passkeys are enabled for the current user and will not behave as intended; pass `context.getUser()` to align with the helper's contract and other call sites.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

@codeant-ai

codeant-ai Bot commented Feb 13, 2026

Copy link
Copy Markdown

CodeAnt AI finished running the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L This PR changes 100-499 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants