Fixing Re-authentication with passkeys #1
Conversation
closes #41242 closes #41008 Signed-off-by: mposolda <[email protected]>
This change modifies the method signature to require a UserModel parameter for proper user context validation during conditional passkey checks.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Summary
This PR implements a comprehensive fix for re-authentication flows when using passkeys (WebAuthn passwordless authentication) in Keycloak. The changes address scenarios where users need to re-authenticate after already being logged in, such as when the OIDC prompt=login parameter is used for step-up authentication.
The core issue being resolved is that during re-authentication scenarios, the authentication forms were not properly configured to handle the fact that a user was already identified. This led to inappropriate UI elements being shown (like username fields and registration options) and passkey authentication options not being available when they should be.
The solution involves several coordinated changes:
Code Refactoring and Utility Creation: A new utility method setupReauthenticationInUsernamePasswordFormError() was extracted to AuthenticatorUtils to centralize the logic for configuring forms during re-authentication error scenarios. This method hides the username field and disables registration when a user was already set before username/password authentication.
Authenticator Improvements: The UsernameForm and UsernamePasswordForm authenticators were modified to ensure passkey options remain available during re-authentication. The UsernameForm now prevents skipping the username form when conditional passkeys are enabled, ensuring users can access passkey authentication options. The UsernamePasswordForm was refactored to always setup WebAuthn form data when passkeys are enabled, regardless of whether a user is already present in the authentication context.
WebAuthn Architecture Enhancement: The WebAuthnAuthenticator was refactored to extract UI display logic into an overridable shouldShowWebAuthnAuthenticators() method, enabling better control over when authenticators are visually presented. The WebAuthnConditionalUIAuthenticator leverages this to hide authenticators during conditional UI flows while maintaining authentication functionality.
Test Coverage: Comprehensive test coverage was added across multiple test classes to verify that passkey re-authentication works correctly in various scenarios, including users with different credential combinations and realm configurations.
These changes integrate with Keycloak's existing authentication flow architecture and maintain backward compatibility while fixing the specific re-authentication issues with passkeys.
Confidence score: 4/5
• This PR appears safe to merge as it addresses a specific bug with comprehensive testing and follows established patterns in the codebase.
• The score reflects that while the changes are well-structured and tested, authentication flow modifications always carry inherent risk and the PR description is minimal.
• Files needing more attention: services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernamePasswordForm.java and services/src/main/java/org/keycloak/authentication/authenticators/browser/WebAuthnConditionalUIAuthenticator.java for their core authentication flow logic changes.
9 files reviewed, 3 comments
| loginPage.login(getPassword(USERNAME)); | ||
| appPage.assertCurrent(); | ||
|
|
||
| // Succesful event - password login |
There was a problem hiding this comment.
syntax: 'Succesful' is misspelled
| // Succesful event - password login | |
| // Successful event - password login |
| @Override | ||
| public void authenticate(AuthenticationFlowContext context) { | ||
| if (context.getUser() != null) { | ||
| if (context.getUser() != null && !isConditionalPasskeysEnabled()) { |
There was a problem hiding this comment.
syntax: The method isConditionalPasskeysEnabled() is called without parameters, but based on the parent class UsernamePasswordForm, it expects a UserModel parameter. This will cause a compilation error.
|
|
||
| if (shouldShowWebAuthnAuthenticators(context)) { | ||
| // in 2 Factor Scenario where the user has already been identified | ||
| WebAuthnAuthenticatorsBean authenticators = new WebAuthnAuthenticatorsBean(context.getSession(), context.getRealm(), user, getCredentialType()); |
There was a problem hiding this comment.
logic: this line still uses the user variable directly - ensure user is non-null when shouldShowWebAuthnAuthenticators() returns true
2 participants
Test 1