Skip to content

Commit 57972d8

Browse files
mposoldaabstractj
mposolda
authored and
abstractj
committed
Update per feedback review
Signed-off-by: mposolda <[email protected]>
1 parent bba869b commit 57972d8

File tree

10 files changed

+106
-33
lines changed

10 files changed

+106
-33
lines changed

‎services/src/main/java/org/keycloak/authentication/authenticators/browser/PasswordForm.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,13 +47,20 @@ public void authenticate(AuthenticationFlowContext context) {
4747
context.success();
4848
return;
4949
}
50+
51+
// setup webauthn data when passkeys enabled
52+
if (isConditionalPasskeysEnabled(context.getUser())) {
53+
webauthnAuth.fillContextForm(context);
54+
}
55+
5056
Response challengeResponse = context.form().createLoginPassword();
5157
context.challenge(challengeResponse);
5258
}
5359

5460
@Override
5561
public boolean configuredFor(KeycloakSession session, RealmModel realm, UserModel user) {
5662
return user.credentialManager().isConfiguredFor(getCredentialProvider(session).getType())
63+
|| (isConditionalPasskeysEnabled(user))
5764
|| alreadyAuthenticatedUsingPasswordlessCredential(session.getContext().getAuthenticationSession());
5865
}
5966

‎services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernameForm.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public UsernameForm(KeycloakSession session) {
4444

4545
@Override
4646
public void authenticate(AuthenticationFlowContext context) {
47-
if (context.getUser() != null && !isConditionalPasskeysEnabled()) {
47+
if (context.getUser() != null) {
4848
// 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
4949
if (!this.hasLinkedBrokers(context)) {
5050
context.success();

‎services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernamePasswordForm.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ public void authenticate(AuthenticationFlowContext context) {
112112
}
113113
}
114114
// setup webauthn data when passkeys enabled
115-
if (isConditionalPasskeysEnabled()) {
115+
if (isConditionalPasskeysEnabled(context.getUser())) {
116116
webauthnAuth.fillContextForm(context);
117117
}
118118
Response challengeResponse = challenge(context, formData);
@@ -134,7 +134,7 @@ protected Response challenge(AuthenticationFlowContext context, MultivaluedMap<S
134134

135135
@Override
136136
protected Response challenge(AuthenticationFlowContext context, String error, String field) {
137-
if (isConditionalPasskeysEnabled()) {
137+
if (isConditionalPasskeysEnabled(context.getUser())) {
138138
// setup webauthn data when possible
139139
webauthnAuth.fillContextForm(context);
140140
}
@@ -157,8 +157,9 @@ public void close() {
157157

158158
}
159159

160-
protected boolean isConditionalPasskeysEnabled() {
161-
return webauthnAuth != null && webauthnAuth.isPasskeysEnabled();
160+
protected boolean isConditionalPasskeysEnabled(UserModel currentUser) {
161+
return webauthnAuth != null && webauthnAuth.isPasskeysEnabled() &&
162+
(currentUser == null || currentUser.credentialManager().isConfiguredFor(webauthnAuth.getCredentialType()));
162163
}
163164

164165
}

‎services/src/main/java/org/keycloak/authentication/authenticators/browser/WebAuthnAuthenticator.java‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,7 @@ public LoginFormsProvider fillContextForm(AuthenticationFlowContext context) {
9797

9898
UserModel user = context.getUser();
9999
boolean isUserIdentified = false;
100-
101-
if (shouldShowWebAuthnAuthenticators(context)) {
100+
if (user != null) {
102101
// in 2 Factor Scenario where the user has already been identified
103102
WebAuthnAuthenticatorsBean authenticators = new WebAuthnAuthenticatorsBean(context.getSession(), context.getRealm(), user, getCredentialType());
104103
if (authenticators.getAuthenticators().isEmpty()) {
@@ -121,14 +120,6 @@ public LoginFormsProvider fillContextForm(AuthenticationFlowContext context) {
121120
return form;
122121
}
123122

124-
/**
125-
* @param context authentication context
126-
* @return true if the available webauthn authenticators should be shown on the screen. Typically during 2-factor authentication for example
127-
*/
128-
protected boolean shouldShowWebAuthnAuthenticators(AuthenticationFlowContext context) {
129-
return context.getUser() != null;
130-
}
131-
132123
protected WebAuthnPolicy getWebAuthnPolicy(AuthenticationFlowContext context) {
133124
return context.getRealm().getWebAuthnPolicy();
134125
}

‎services/src/main/java/org/keycloak/authentication/authenticators/browser/WebAuthnConditionalUIAuthenticator.java‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,4 @@ public boolean isPasskeysEnabled() {
6060
return Profile.isFeatureEnabled(Profile.Feature.PASSKEYS) &&
6161
Boolean.TRUE.equals(session.getContext().getRealm().getWebAuthnPolicyPasswordless().isPasskeysEnabled());
6262
}
63-
64-
// Do not show authenticators during login with conditional passkeys (For example during username/password)
65-
protected boolean shouldShowWebAuthnAuthenticators(AuthenticationFlowContext context) {
66-
return false;
67-
}
6863
}

‎testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/webauthn/passwordless/PasskeysOrganizationAuthenticationTest.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,10 @@ public void webauthnLoginWithDiscoverableKey_reauthentication() throws IOExcepti
307307
.open();
308308
WaitUtils.waitForPageToLoad();
309309

310+
loginPage.assertCurrent();
311+
MatcherAssert.assertThat(loginPage.isPasswordInputPresent(), Matchers.is(true));
312+
MatcherAssert.assertThat(driver.findElement(By.xpath("//form[@id='webauth']")), Matchers.notNullValue());
313+
webAuthnLoginPage.clickAuthenticate();
310314
appPage.assertCurrent();
311315

312316
events.expectLogin()

‎testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/webauthn/passwordless/PasskeysUsernameFormTest.java‎

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import org.keycloak.representations.idm.UserRepresentation;
4343
import org.keycloak.testsuite.Assert;
4444
import org.keycloak.testsuite.admin.AbstractAdminTest;
45+
import org.keycloak.testsuite.admin.ApiUtil;
4546
import org.keycloak.testsuite.arquillian.annotation.EnableFeature;
4647
import org.keycloak.testsuite.arquillian.annotation.IgnoreBrowserDriver;
4748
import org.keycloak.testsuite.util.WaitUtils;
@@ -51,6 +52,9 @@
5152
import org.openqa.selenium.NoSuchElementException;
5253
import org.openqa.selenium.firefox.FirefoxDriver;
5354

55+
import static org.hamcrest.Matchers.nullValue;
56+
import static org.junit.Assert.assertEquals;
57+
5458
/**
5559
*
5660
* @author rmartinc
@@ -66,6 +70,7 @@ public void addTestRealms(List<RealmRepresentation> testRealms) {
6670
makePasswordlessRequiredActionDefault(realmRepresentation);
6771
switchExecutionInBrowser(realmRepresentation);
6872

73+
configureTestRealm(realmRepresentation);
6974
testRealms.add(realmRepresentation);
7075
}
7176

@@ -198,8 +203,10 @@ public void passwordLoginWithNonDiscoverableKey() throws IOException {
198203
// login OK now
199204
loginPage.loginUsername(USERNAME);
200205
loginPage.assertCurrent();
206+
// Passkeys available on password-form as well. Allows to login only with the passkey of current user
201207
MatcherAssert.assertThat(loginPage.getAttemptedUsername(), Matchers.is(USERNAME));
202-
Assert.assertThrows(NoSuchElementException.class, () -> driver.findElement(By.xpath("//form[@id='webauth']")));
208+
MatcherAssert.assertThat(loginPage.isPasswordInputPresent(), Matchers.is(true));
209+
MatcherAssert.assertThat(driver.findElement(By.xpath("//form[@id='webauth']")), Matchers.notNullValue());
203210
loginPage.login(getPassword(USERNAME));
204211
appPage.assertCurrent();
205212
events.expectLogin()
@@ -307,6 +314,11 @@ public void webauthnLoginWithDiscoverableKey_reauthentication() throws IOExcepti
307314
.open();
308315
WaitUtils.waitForPageToLoad();
309316

317+
loginPage.assertCurrent();
318+
MatcherAssert.assertThat(loginPage.isPasswordInputPresent(), Matchers.is(true));
319+
MatcherAssert.assertThat(driver.findElement(By.xpath("//form[@id='webauth']")), Matchers.notNullValue());
320+
webAuthnLoginPage.clickAuthenticate();
321+
310322
appPage.assertCurrent();
311323

312324
events.expectLogin()
@@ -319,4 +331,67 @@ public void webauthnLoginWithDiscoverableKey_reauthentication() throws IOExcepti
319331
logout();
320332
}
321333
}
334+
335+
336+
@Test
337+
public void passwordLogin_reauthenticationOfUserWithoutPasskey() throws Exception {
338+
// use a default resident key which is not shown in conditional UI
339+
getVirtualAuthManager().useAuthenticator(DefaultVirtualAuthOptions.DEFAULT_RESIDENT_KEY.getOptions());
340+
341+
// set passwordless policy for discoverable keys
342+
try (Closeable c = getWebAuthnRealmUpdater()
343+
.setWebAuthnPolicyRpEntityName("localhost")
344+
.setWebAuthnPolicyRequireResidentKey(Constants.WEBAUTHN_POLICY_OPTION_YES)
345+
.setWebAuthnPolicyUserVerificationRequirement(Constants.WEBAUTHN_POLICY_OPTION_REQUIRED)
346+
.setWebAuthnPolicyPasskeysEnabled(Boolean.TRUE)
347+
.update()) {
348+
349+
// Login with password
350+
oauth.openLoginForm();
351+
WaitUtils.waitForPageToLoad();
352+
353+
// WebAuthn elements available, user is not yet known. Password not available as on username-form
354+
loginPage.assertCurrent();
355+
MatcherAssert.assertThat(loginPage.isPasswordInputPresent(), Matchers.is(false));
356+
MatcherAssert.assertThat(driver.findElement(By.xpath("//form[@id='webauth']")), Matchers.notNullValue());
357+
358+
// Login with password. WebAuthn elements not available on password screen as user does not have passkeys
359+
loginPage.loginUsername("test-user@localhost");
360+
Assert.assertThrows(NoSuchElementException.class, () -> driver.findElement(By.xpath("//form[@id='webauth']")));
361+
loginPage.login(getPassword("test-user@localhost"));
362+
appPage.assertCurrent();
363+
364+
events.clear();
365+
366+
// Re-authentication now with prompt=login. Passkeys login should not be available on the page as this user does not have passkey
367+
oauth.loginForm()
368+
.prompt(OIDCLoginProtocol.PROMPT_VALUE_LOGIN)
369+
.open();
370+
WaitUtils.waitForPageToLoad();
371+
372+
loginPage.assertCurrent();
373+
assertEquals("Please re-authenticate to continue", loginPage.getInfoMessage());
374+
Assert.assertThrows(NoSuchElementException.class, () -> driver.findElement(By.xpath("//form[@id='webauth']")));
375+
376+
// Incorrect password (password of different user)
377+
loginPage.login(getPassword("john-doh@localhost"));
378+
MatcherAssert.assertThat(loginPage.getPasswordInputError(), Matchers.is("Invalid password."));
379+
380+
events.clear();
381+
382+
// Login with password
383+
loginPage.login(getPassword("test-user@localhost"));
384+
appPage.assertCurrent();
385+
386+
UserRepresentation testUser = ApiUtil.findUserByUsernameId(testRealm(), "test-user@localhost").toRepresentation();
387+
388+
events.expectLogin()
389+
.user(testUser.getId())
390+
.detail(Details.USERNAME, testUser.getUsername())
391+
.detail(WebAuthnConstants.USER_VERIFICATION_CHECKED, nullValue())
392+
.assertEvent();
393+
394+
logout();
395+
}
396+
}
322397
}

‎testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/webauthn/passwordless/PasskeysUsernamePasswordFormTest.java‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
import org.keycloak.testsuite.webauthn.AbstractWebAuthnVirtualTest;
4747
import org.keycloak.testsuite.webauthn.authenticators.DefaultVirtualAuthOptions;
4848
import org.openqa.selenium.By;
49+
import org.openqa.selenium.NoSuchElementException;
4950
import org.openqa.selenium.firefox.FirefoxDriver;
5051

5152
import static org.hamcrest.Matchers.nullValue;
@@ -288,12 +289,7 @@ public void reauthenticationOfUserWithoutPasskey() throws Exception {
288289

289290
// WebAuthn elements not available
290291
loginPage.assertCurrent();
291-
try {
292-
MatcherAssert.assertThat(driver.findElement(By.xpath("//form[@id='webauth']")), nullValue());
293-
fail("Not expected to have webauthn button");
294-
} catch (Exception nsee) {
295-
// expected
296-
}
292+
Assert.assertThrows(NoSuchElementException.class, () -> driver.findElement(By.xpath("//form[@id='webauth']")));
297293

298294
// Login with password
299295
loginPage.login("test-user@localhost", getPassword("test-user@localhost"));
@@ -309,12 +305,7 @@ public void reauthenticationOfUserWithoutPasskey() throws Exception {
309305

310306
loginPage.assertCurrent();
311307
assertEquals("Please re-authenticate to continue", loginPage.getInfoMessage());
312-
try {
313-
MatcherAssert.assertThat(driver.findElement(By.xpath("//form[@id='webauth']")), nullValue());
314-
fail("Not expected to have webauthn button");
315-
} catch (Exception nsee) {
316-
// expected
317-
}
308+
Assert.assertThrows(NoSuchElementException.class, () -> driver.findElement(By.xpath("//form[@id='webauth']")));
318309

319310
// Login with password
320311
loginPage.login(getPassword("test-user@localhost"));

‎themes/src/main/resources/theme/base/login/passkeys.ftl‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,13 @@
88
<input type="hidden" id="userHandle" name="userHandle"/>
99
<input type="hidden" id="error" name="error"/>
1010
</form>
11+
<#if authenticators??>
12+
<form id="authn_select" class="${properties.kcFormClass!}">
13+
<#list authenticators.authenticators as authenticator>
14+
<input type="hidden" name="authn_use_chk" value="${authenticator.credentialId}"/>
15+
</#list>
16+
</form>
17+
</#if>
1118
<script type="module">
1219
import { authenticateByWebAuthn } from "${url.resourcesPath}/js/webauthnAuthenticate.js";
1320
import { initAuthenticate } from "${url.resourcesPath}/js/passkeysConditionalAuth.js";

‎themes/src/main/resources/theme/keycloak.v2/login/login-password.ftl‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
<#import "template.ftl" as layout>
22
<#import "field.ftl" as field>
33
<#import "buttons.ftl" as buttons>
4+
<#import "passkeys.ftl" as passkeys>
45
<@layout.registrationLayout displayMessage=!messagesPerField.existsError('password'); section>
56
<!-- template: login-password.ftl -->
67
<#if section = "header">
@@ -14,6 +15,7 @@
1415
</form>
1516
</div>
1617
</div>
18+
<@passkeys.conditionalUIData />
1719
</#if>
1820

1921
</@layout.registrationLayout>

0 commit comments

Comments
 (0)