From 57972d85d37869172124a27ab333b98c52c94209 Mon Sep 17 00:00:00 2001 From: mposolda Date: Fri, 18 Jul 2025 13:36:51 +0200 Subject: [PATCH] Update per feedback review Signed-off-by: mposolda --- .../authenticators/browser/PasswordForm.java | 7 ++ .../authenticators/browser/UsernameForm.java | 2 +- .../browser/UsernamePasswordForm.java | 9 ++- .../browser/WebAuthnAuthenticator.java | 11 +-- .../WebAuthnConditionalUIAuthenticator.java | 5 -- ...asskeysOrganizationAuthenticationTest.java | 4 + .../PasskeysUsernameFormTest.java | 77 ++++++++++++++++++- .../PasskeysUsernamePasswordFormTest.java | 15 +--- .../resources/theme/base/login/passkeys.ftl | 7 ++ .../keycloak.v2/login/login-password.ftl | 2 + 10 files changed, 106 insertions(+), 33 deletions(-) diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/PasswordForm.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/PasswordForm.java index 7213dee34da..232ae014a25 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/PasswordForm.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/PasswordForm.java @@ -47,6 +47,12 @@ public class PasswordForm extends UsernamePasswordForm implements CredentialVali context.success(); return; } + + // setup webauthn data when passkeys enabled + if (isConditionalPasskeysEnabled(context.getUser())) { + webauthnAuth.fillContextForm(context); + } + Response challengeResponse = context.form().createLoginPassword(); context.challenge(challengeResponse); } @@ -54,6 +60,7 @@ public class PasswordForm extends UsernamePasswordForm implements CredentialVali @Override public boolean configuredFor(KeycloakSession session, RealmModel realm, UserModel user) { return user.credentialManager().isConfiguredFor(getCredentialProvider(session).getType()) + || (isConditionalPasskeysEnabled(user)) || alreadyAuthenticatedUsingPasswordlessCredential(session.getContext().getAuthenticationSession()); } diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernameForm.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernameForm.java index 91c2a242c99..b8fc0eafd68 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernameForm.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernameForm.java @@ -44,7 +44,7 @@ public final class UsernameForm extends UsernamePasswordForm { @Override public void authenticate(AuthenticationFlowContext context) { - if (context.getUser() != null && !isConditionalPasskeysEnabled()) { + if (context.getUser() != null) { // We can skip the form when user is re-authenticating. Unless current user has some IDP set, so he can re-authenticate with that IDP if (!this.hasLinkedBrokers(context)) { context.success(); diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernamePasswordForm.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernamePasswordForm.java index ce874028745..159cc885d10 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernamePasswordForm.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernamePasswordForm.java @@ -112,7 +112,7 @@ public class UsernamePasswordForm extends AbstractUsernameFormAuthenticator impl } } // setup webauthn data when passkeys enabled - if (isConditionalPasskeysEnabled()) { + if (isConditionalPasskeysEnabled(context.getUser())) { webauthnAuth.fillContextForm(context); } Response challengeResponse = challenge(context, formData); @@ -134,7 +134,7 @@ public class UsernamePasswordForm extends AbstractUsernameFormAuthenticator impl @Override protected Response challenge(AuthenticationFlowContext context, String error, String field) { - if (isConditionalPasskeysEnabled()) { + if (isConditionalPasskeysEnabled(context.getUser())) { // setup webauthn data when possible webauthnAuth.fillContextForm(context); } @@ -157,8 +157,9 @@ public class UsernamePasswordForm extends AbstractUsernameFormAuthenticator impl } - protected boolean isConditionalPasskeysEnabled() { - return webauthnAuth != null && webauthnAuth.isPasskeysEnabled(); + protected boolean isConditionalPasskeysEnabled(UserModel currentUser) { + return webauthnAuth != null && webauthnAuth.isPasskeysEnabled() && + (currentUser == null || currentUser.credentialManager().isConfiguredFor(webauthnAuth.getCredentialType())); } } diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/WebAuthnAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/WebAuthnAuthenticator.java index 9b421fcb3e6..f1b53fc2d42 100644 --- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/WebAuthnAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/WebAuthnAuthenticator.java @@ -97,8 +97,7 @@ public class WebAuthnAuthenticator implements Authenticator, CredentialValidator UserModel user = context.getUser(); boolean isUserIdentified = false; - - if (shouldShowWebAuthnAuthenticators(context)) { + if (user != null) { // in 2 Factor Scenario where the user has already been identified WebAuthnAuthenticatorsBean authenticators = new WebAuthnAuthenticatorsBean(context.getSession(), context.getRealm(), user, getCredentialType()); if (authenticators.getAuthenticators().isEmpty()) { @@ -121,14 +120,6 @@ public class WebAuthnAuthenticator implements Authenticator, CredentialValidator return form; } - /** - * @param context authentication context - * @return true if the available webauthn authenticators should be shown on the screen. Typically during 2-factor authentication for example - */ - protected boolean shouldShowWebAuthnAuthenticators(AuthenticationFlowContext context) { - return context.getUser() != null; - } - protected WebAuthnPolicy getWebAuthnPolicy(AuthenticationFlowContext context) { return context.getRealm().getWebAuthnPolicy(); } diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/WebAuthnConditionalUIAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/WebAuthnConditionalUIAuthenticator.java index 3f94338b082..3b72cc862ae 100644 --- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/WebAuthnConditionalUIAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/WebAuthnConditionalUIAuthenticator.java @@ -60,9 +60,4 @@ public class WebAuthnConditionalUIAuthenticator extends WebAuthnPasswordlessAuth return Profile.isFeatureEnabled(Profile.Feature.PASSKEYS) && Boolean.TRUE.equals(session.getContext().getRealm().getWebAuthnPolicyPasswordless().isPasskeysEnabled()); } - - // Do not show authenticators during login with conditional passkeys (For example during username/password) - protected boolean shouldShowWebAuthnAuthenticators(AuthenticationFlowContext context) { - return false; - } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/webauthn/passwordless/PasskeysOrganizationAuthenticationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/webauthn/passwordless/PasskeysOrganizationAuthenticationTest.java index 007c4df7db6..626516afade 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/webauthn/passwordless/PasskeysOrganizationAuthenticationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/webauthn/passwordless/PasskeysOrganizationAuthenticationTest.java @@ -307,6 +307,10 @@ public class PasskeysOrganizationAuthenticationTest extends AbstractWebAuthnVirt .open(); WaitUtils.waitForPageToLoad(); + loginPage.assertCurrent(); + MatcherAssert.assertThat(loginPage.isPasswordInputPresent(), Matchers.is(true)); + MatcherAssert.assertThat(driver.findElement(By.xpath("//form[@id='webauth']")), Matchers.notNullValue()); + webAuthnLoginPage.clickAuthenticate(); appPage.assertCurrent(); events.expectLogin() diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/webauthn/passwordless/PasskeysUsernameFormTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/webauthn/passwordless/PasskeysUsernameFormTest.java index 82faa8572d7..58a9c315947 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/webauthn/passwordless/PasskeysUsernameFormTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/webauthn/passwordless/PasskeysUsernameFormTest.java @@ -42,6 +42,7 @@ import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.testsuite.Assert; import org.keycloak.testsuite.admin.AbstractAdminTest; +import org.keycloak.testsuite.admin.ApiUtil; import org.keycloak.testsuite.arquillian.annotation.EnableFeature; import org.keycloak.testsuite.arquillian.annotation.IgnoreBrowserDriver; import org.keycloak.testsuite.util.WaitUtils; @@ -51,6 +52,9 @@ import org.openqa.selenium.By; import org.openqa.selenium.NoSuchElementException; import org.openqa.selenium.firefox.FirefoxDriver; +import static org.hamcrest.Matchers.nullValue; +import static org.junit.Assert.assertEquals; + /** * * @author rmartinc @@ -66,6 +70,7 @@ public class PasskeysUsernameFormTest extends AbstractWebAuthnVirtualTest { makePasswordlessRequiredActionDefault(realmRepresentation); switchExecutionInBrowser(realmRepresentation); + configureTestRealm(realmRepresentation); testRealms.add(realmRepresentation); } @@ -198,8 +203,10 @@ public class PasskeysUsernameFormTest extends AbstractWebAuthnVirtualTest { // login OK now loginPage.loginUsername(USERNAME); loginPage.assertCurrent(); + // Passkeys available on password-form as well. Allows to login only with the passkey of current user MatcherAssert.assertThat(loginPage.getAttemptedUsername(), Matchers.is(USERNAME)); - Assert.assertThrows(NoSuchElementException.class, () -> driver.findElement(By.xpath("//form[@id='webauth']"))); + MatcherAssert.assertThat(loginPage.isPasswordInputPresent(), Matchers.is(true)); + MatcherAssert.assertThat(driver.findElement(By.xpath("//form[@id='webauth']")), Matchers.notNullValue()); loginPage.login(getPassword(USERNAME)); appPage.assertCurrent(); events.expectLogin() @@ -307,6 +314,11 @@ public class PasskeysUsernameFormTest extends AbstractWebAuthnVirtualTest { .open(); WaitUtils.waitForPageToLoad(); + loginPage.assertCurrent(); + MatcherAssert.assertThat(loginPage.isPasswordInputPresent(), Matchers.is(true)); + MatcherAssert.assertThat(driver.findElement(By.xpath("//form[@id='webauth']")), Matchers.notNullValue()); + webAuthnLoginPage.clickAuthenticate(); + appPage.assertCurrent(); events.expectLogin() @@ -319,4 +331,67 @@ public class PasskeysUsernameFormTest extends AbstractWebAuthnVirtualTest { logout(); } } + + + @Test + public void passwordLogin_reauthenticationOfUserWithoutPasskey() throws Exception { + // use a default resident key which is not shown in conditional UI + getVirtualAuthManager().useAuthenticator(DefaultVirtualAuthOptions.DEFAULT_RESIDENT_KEY.getOptions()); + + // set passwordless policy for discoverable keys + try (Closeable c = getWebAuthnRealmUpdater() + .setWebAuthnPolicyRpEntityName("localhost") + .setWebAuthnPolicyRequireResidentKey(Constants.WEBAUTHN_POLICY_OPTION_YES) + .setWebAuthnPolicyUserVerificationRequirement(Constants.WEBAUTHN_POLICY_OPTION_REQUIRED) + .setWebAuthnPolicyPasskeysEnabled(Boolean.TRUE) + .update()) { + + // Login with password + oauth.openLoginForm(); + WaitUtils.waitForPageToLoad(); + + // WebAuthn elements available, user is not yet known. Password not available as on username-form + loginPage.assertCurrent(); + MatcherAssert.assertThat(loginPage.isPasswordInputPresent(), Matchers.is(false)); + MatcherAssert.assertThat(driver.findElement(By.xpath("//form[@id='webauth']")), Matchers.notNullValue()); + + // Login with password. WebAuthn elements not available on password screen as user does not have passkeys + loginPage.loginUsername("test-user@localhost"); + Assert.assertThrows(NoSuchElementException.class, () -> driver.findElement(By.xpath("//form[@id='webauth']"))); + loginPage.login(getPassword("test-user@localhost")); + appPage.assertCurrent(); + + events.clear(); + + // Re-authentication now with prompt=login. Passkeys login should not be available on the page as this user does not have passkey + oauth.loginForm() + .prompt(OIDCLoginProtocol.PROMPT_VALUE_LOGIN) + .open(); + WaitUtils.waitForPageToLoad(); + + loginPage.assertCurrent(); + assertEquals("Please re-authenticate to continue", loginPage.getInfoMessage()); + Assert.assertThrows(NoSuchElementException.class, () -> driver.findElement(By.xpath("//form[@id='webauth']"))); + + // Incorrect password (password of different user) + loginPage.login(getPassword("john-doh@localhost")); + MatcherAssert.assertThat(loginPage.getPasswordInputError(), Matchers.is("Invalid password.")); + + events.clear(); + + // Login with password + loginPage.login(getPassword("test-user@localhost")); + appPage.assertCurrent(); + + UserRepresentation testUser = ApiUtil.findUserByUsernameId(testRealm(), "test-user@localhost").toRepresentation(); + + events.expectLogin() + .user(testUser.getId()) + .detail(Details.USERNAME, testUser.getUsername()) + .detail(WebAuthnConstants.USER_VERIFICATION_CHECKED, nullValue()) + .assertEvent(); + + logout(); + } + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/webauthn/passwordless/PasskeysUsernamePasswordFormTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/webauthn/passwordless/PasskeysUsernamePasswordFormTest.java index e089a9e875d..900deff93c5 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/webauthn/passwordless/PasskeysUsernamePasswordFormTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/webauthn/passwordless/PasskeysUsernamePasswordFormTest.java @@ -46,6 +46,7 @@ import org.keycloak.testsuite.util.WaitUtils; import org.keycloak.testsuite.webauthn.AbstractWebAuthnVirtualTest; import org.keycloak.testsuite.webauthn.authenticators.DefaultVirtualAuthOptions; import org.openqa.selenium.By; +import org.openqa.selenium.NoSuchElementException; import org.openqa.selenium.firefox.FirefoxDriver; import static org.hamcrest.Matchers.nullValue; @@ -288,12 +289,7 @@ public class PasskeysUsernamePasswordFormTest extends AbstractWebAuthnVirtualTes // WebAuthn elements not available loginPage.assertCurrent(); - try { - MatcherAssert.assertThat(driver.findElement(By.xpath("//form[@id='webauth']")), nullValue()); - fail("Not expected to have webauthn button"); - } catch (Exception nsee) { - // expected - } + Assert.assertThrows(NoSuchElementException.class, () -> driver.findElement(By.xpath("//form[@id='webauth']"))); // Login with password loginPage.login("test-user@localhost", getPassword("test-user@localhost")); @@ -309,12 +305,7 @@ public class PasskeysUsernamePasswordFormTest extends AbstractWebAuthnVirtualTes loginPage.assertCurrent(); assertEquals("Please re-authenticate to continue", loginPage.getInfoMessage()); - try { - MatcherAssert.assertThat(driver.findElement(By.xpath("//form[@id='webauth']")), nullValue()); - fail("Not expected to have webauthn button"); - } catch (Exception nsee) { - // expected - } + Assert.assertThrows(NoSuchElementException.class, () -> driver.findElement(By.xpath("//form[@id='webauth']"))); // Login with password loginPage.login(getPassword("test-user@localhost")); diff --git a/themes/src/main/resources/theme/base/login/passkeys.ftl b/themes/src/main/resources/theme/base/login/passkeys.ftl index 5c9914a9356..b86be07190d 100644 --- a/themes/src/main/resources/theme/base/login/passkeys.ftl +++ b/themes/src/main/resources/theme/base/login/passkeys.ftl @@ -8,6 +8,13 @@ + <#if authenticators??> +
+ <#list authenticators.authenticators as authenticator> + + +
+