Skip to content

Commit b0c4913

Browse files
mabartosmabartos
authored
KEYCLOAK-12177 KEYCLOAK-12178 WebAuthn: Improve usability (keycloak#6710)
1 parent 42fdc12 commit b0c4913

File tree

16 files changed

+214
-131
lines changed

16 files changed

+214
-131
lines changed

‎server-spi-private/src/main/java/org/keycloak/forms/login/LoginFormsPages.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public enum LoginFormsPages {
2424

2525
LOGIN, LOGIN_USERNAME, LOGIN_PASSWORD, LOGIN_TOTP, LOGIN_CONFIG_TOTP, LOGIN_WEBAUTHN, LOGIN_VERIFY_EMAIL,
2626
LOGIN_IDP_LINK_CONFIRM, LOGIN_IDP_LINK_EMAIL,
27-
OAUTH_GRANT, LOGIN_RESET_PASSWORD, LOGIN_UPDATE_PASSWORD, LOGIN_SELECT_AUTHENTICATOR, REGISTER, INFO, ERROR, LOGIN_UPDATE_PROFILE,
27+
OAUTH_GRANT, LOGIN_RESET_PASSWORD, LOGIN_UPDATE_PASSWORD, LOGIN_SELECT_AUTHENTICATOR, REGISTER, INFO, ERROR, ERROR_WEBAUTHN, LOGIN_UPDATE_PROFILE,
2828
LOGIN_PAGE_EXPIRED, CODE, X509_CONFIRM, SAML_POST_FORM;
2929

3030
}

‎server-spi-private/src/main/java/org/keycloak/forms/login/LoginFormsProvider.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,8 @@ public interface LoginFormsProvider extends Provider {
8282

8383
Response createErrorPage(Response.Status status);
8484

85+
Response createWebAuthnErrorPage();
86+
8587
Response createOAuthGrant();
8688

8789
Response createSelectAuthenticator();

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

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import com.webauthn4j.data.client.challenge.DefaultChallenge;
2323
import com.webauthn4j.server.ServerProperty;
2424
import com.webauthn4j.util.exception.WebAuthnException;
25-
2625
import org.jboss.logging.Logger;
2726
import org.keycloak.WebAuthnConstants;
2827
import org.keycloak.authentication.AuthenticationFlowContext;
@@ -53,13 +52,16 @@
5352
import java.util.Collections;
5453
import java.util.List;
5554

55+
import static org.keycloak.services.messages.Messages.*;
56+
5657
/**
5758
* Authenticator for WebAuthn authentication, which will be typically used when WebAuthn is used as second factor.
5859
*/
5960
public class WebAuthnAuthenticator implements Authenticator, CredentialValidator<WebAuthnCredentialProvider> {
6061

6162
private static final Logger logger = Logger.getLogger(WebAuthnAuthenticator.class);
6263
private KeycloakSession session;
64+
private WebAuthnAuthenticatorsBean authenticators;
6365

6466
public WebAuthnAuthenticator(KeycloakSession session) {
6567
this.session = session;
@@ -82,7 +84,7 @@ public void authenticate(AuthenticationFlowContext context) {
8284
boolean isUserIdentified = false;
8385
if (user != null) {
8486
// in 2 Factor Scenario where the user has already been identified
85-
WebAuthnAuthenticatorsBean authenticators = new WebAuthnAuthenticatorsBean(context.getSession(), context.getRealm(), user, getCredentialType());
87+
authenticators = new WebAuthnAuthenticatorsBean(context.getSession(), context.getRealm(), user, getCredentialType());
8688
if (authenticators.getAuthenticators().isEmpty()) {
8789
// require the user to register webauthn authenticator
8890
return;
@@ -119,7 +121,7 @@ public void action(AuthenticationFlowContext context) {
119121
// receive error from navigator.credentials.get()
120122
String errorMsgFromWebAuthnApi = params.getFirst(WebAuthnConstants.ERROR);
121123
if (errorMsgFromWebAuthnApi != null && !errorMsgFromWebAuthnApi.isEmpty()) {
122-
setErrorResponse(context, ERR_WEBAUTHN_API_GET, errorMsgFromWebAuthnApi);
124+
setErrorResponse(context, WEBAUTHN_ERROR_API_GET, errorMsgFromWebAuthnApi);
123125
return;
124126
}
125127

@@ -154,7 +156,7 @@ public void action(AuthenticationFlowContext context) {
154156
context.getEvent()
155157
.detail("first_authenticated_user_id", firstAuthenticatedUserId)
156158
.detail("web_authn_authenticator_authenticated_user_id", userId);
157-
setErrorResponse(context, ERR_DIFFERENT_USER_AUTHENTICATED, null);
159+
setErrorResponse(context, WEBAUTHN_ERROR_DIFFERENT_USER, null);
158160
return;
159161
}
160162
} else {
@@ -181,7 +183,7 @@ public void action(AuthenticationFlowContext context) {
181183
try {
182184
result = session.userCredentialManager().isValid(context.getRealm(), user, cred);
183185
} catch (WebAuthnException wae) {
184-
setErrorResponse(context, ERR_WEBAUTHN_VERIFICATION_FAIL, wae.getMessage());
186+
setErrorResponse(context, WEBAUTHN_ERROR_AUTH_VERIFICATION, wae.getMessage());
185187
return;
186188
}
187189
String encodedCredentialID = Base64Url.encode(credentialId);
@@ -197,7 +199,7 @@ public void action(AuthenticationFlowContext context) {
197199
context.getEvent()
198200
.detail("web_authn_authenticated_user_id", userId)
199201
.detail("public_key_credential_id", encodedCredentialID);
200-
setErrorResponse(context, ERR_WEBAUTHN_AUTHENTICATED_USER_NOT_FOUND, null);
202+
setErrorResponse(context, WEBAUTHN_ERROR_USER_NOT_FOUND, null);
201203
context.cancelLogin();
202204
}
203205
}
@@ -232,64 +234,62 @@ public WebAuthnCredentialProvider getCredentialProvider(KeycloakSession session)
232234

233235
private static final String ERR_LABEL = "web_authn_authentication_error";
234236
private static final String ERR_DETAIL_LABEL = "web_authn_authentication_error_detail";
235-
private static final String ERR_NO_AUTHENTICATORS_REGISTERED = "No WebAuthn Authenticator registered.";
236-
private static final String ERR_WEBAUTHN_API_GET = "Failed to authenticate by the WebAuthn Authenticator";
237-
private static final String ERR_DIFFERENT_USER_AUTHENTICATED = "First authenticated user is not the one authenticated by the WebAuthn authenticator.";
238-
private static final String ERR_WEBAUTHN_VERIFICATION_FAIL = "WebAuthn Authentication result is invalid.";
239-
private static final String ERR_WEBAUTHN_AUTHENTICATED_USER_NOT_FOUND = "Unknown user authenticated by the WebAuthn Authenticator";
240237

241238
private void setErrorResponse(AuthenticationFlowContext context, final String errorCase, final String errorMessage) {
242239
Response errorResponse = null;
243240
switch (errorCase) {
244-
case ERR_NO_AUTHENTICATORS_REGISTERED:
241+
case WEBAUTHN_ERROR_REGISTRATION:
245242
logger.warn(errorCase);
246243
context.getEvent()
247244
.detail(ERR_LABEL, errorCase)
248245
.error(Errors.INVALID_USER_CREDENTIALS);
249-
errorResponse = context.form()
250-
.setError(errorCase)
251-
.createErrorPage(Response.Status.BAD_REQUEST);
246+
errorResponse = createErrorResponse(context, errorCase);
252247
context.failure(AuthenticationFlowError.INVALID_CREDENTIALS, errorResponse);
253248
break;
254-
case ERR_WEBAUTHN_API_GET:
249+
case WEBAUTHN_ERROR_API_GET:
255250
logger.warnv("error returned from navigator.credentials.get(). {0}", errorMessage);
256251
context.getEvent()
257252
.detail(ERR_LABEL, errorCase)
258253
.detail(ERR_DETAIL_LABEL, errorMessage)
259254
.error(Errors.NOT_ALLOWED);
260-
errorResponse = context.form()
261-
.setError(errorCase)
262-
.createErrorPage(Response.Status.BAD_REQUEST);
255+
errorResponse = createErrorResponse(context, errorCase);
263256
context.failure(AuthenticationFlowError.INVALID_USER, errorResponse);
264257
break;
265-
case ERR_DIFFERENT_USER_AUTHENTICATED:
258+
case WEBAUTHN_ERROR_DIFFERENT_USER:
266259
logger.warn(errorCase);
267260
context.getEvent()
268261
.detail(ERR_LABEL, errorCase)
269262
.error(Errors.DIFFERENT_USER_AUTHENTICATED);
270-
errorResponse = context.form()
271-
.setError(errorCase)
272-
.createErrorPage(Response.Status.BAD_REQUEST);
263+
errorResponse = createErrorResponse(context, errorCase);
273264
context.failure(AuthenticationFlowError.USER_CONFLICT, errorResponse);
274265
break;
275-
case ERR_WEBAUTHN_VERIFICATION_FAIL:
266+
case WEBAUTHN_ERROR_AUTH_VERIFICATION:
276267
logger.warnv("WebAuthn API .get() response validation failure. {0}", errorMessage);
277268
context.getEvent()
278269
.detail(ERR_LABEL, errorCase)
279270
.detail(ERR_DETAIL_LABEL, errorMessage)
280271
.error(Errors.INVALID_USER_CREDENTIALS);
281-
errorResponse = context.form()
282-
.setError(errorCase)
283-
.createErrorPage(Response.Status.BAD_REQUEST);
272+
errorResponse = createErrorResponse(context, errorCase);
284273
context.failure(AuthenticationFlowError.INVALID_USER, errorResponse);
285274
break;
286-
case ERR_WEBAUTHN_AUTHENTICATED_USER_NOT_FOUND:
275+
case WEBAUTHN_ERROR_USER_NOT_FOUND:
287276
logger.warn(errorCase);
288-
context.getEvent().detail(ERR_LABEL, errorCase);
289-
context.getEvent().error(Errors.USER_NOT_FOUND);
277+
context.getEvent()
278+
.detail(ERR_LABEL, errorCase)
279+
.error(Errors.USER_NOT_FOUND);
280+
errorResponse = createErrorResponse(context, errorCase);
281+
context.failure(AuthenticationFlowError.UNKNOWN_USER, errorResponse);
290282
break;
291283
default:
292284
// NOP
293285
}
294286
}
287+
288+
private Response createErrorResponse(AuthenticationFlowContext context, final String errorCase) {
289+
LoginFormsProvider provider = context.form().setError(errorCase);
290+
if (authenticators != null && authenticators.getAuthenticators() != null) {
291+
provider.setAttribute(WebAuthnConstants.ALLOWED_AUTHENTICATORS, authenticators);
292+
}
293+
return provider.createWebAuthnErrorPage();
294+
}
295295
}

‎services/src/main/java/org/keycloak/authentication/requiredactions/WebAuthnRegister.java‎

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -64,17 +64,20 @@
6464
import com.webauthn4j.validator.attestation.statement.tpm.TPMAttestationStatementValidator;
6565
import com.webauthn4j.validator.attestation.statement.u2f.FIDOU2FAttestationStatementValidator;
6666
import com.webauthn4j.validator.attestation.trustworthiness.certpath.CertPathTrustworthinessValidator;
67-
import com.webauthn4j.validator.attestation.trustworthiness.certpath.NullCertPathTrustworthinessValidator;
6867
import com.webauthn4j.validator.attestation.trustworthiness.ecdaa.DefaultECDAATrustworthinessValidator;
6968
import com.webauthn4j.validator.attestation.trustworthiness.self.DefaultSelfAttestationTrustworthinessValidator;
7069
import org.keycloak.models.credential.WebAuthnCredentialModel;
7170

71+
import static org.keycloak.services.messages.Messages.*;
72+
7273
/**
7374
* Required action for register WebAuthn 2-factor credential for the user
7475
*/
7576
public class WebAuthnRegister implements RequiredActionProvider, CredentialRegistrator {
7677

78+
private static final String WEB_AUTHN_TITLE_ATTR = "webAuthnTitle";
7779
private static final Logger logger = Logger.getLogger(WebAuthnRegister.class);
80+
7881
private KeycloakSession session;
7982
private CertPathTrustworthinessValidator certPathtrustValidator;
8083

@@ -167,7 +170,7 @@ public void processAction(RequiredActionContext context) {
167170
// receive error from navigator.credentials.create()
168171
String errorMsgFromWebAuthnApi = params.getFirst(WebAuthnConstants.ERROR);
169172
if (errorMsgFromWebAuthnApi != null && !errorMsgFromWebAuthnApi.isEmpty()) {
170-
setErrorResponse(context, ERR_WEBAUTHN_API_CREATE, errorMsgFromWebAuthnApi);
173+
setErrorResponse(context, WEBAUTHN_ERROR_REGISTER_VERIFICATION, errorMsgFromWebAuthnApi);
171174
return;
172175
}
173176

@@ -218,11 +221,11 @@ public void processAction(RequiredActionContext context) {
218221
context.success();
219222
} catch (WebAuthnException wae) {
220223
if (logger.isDebugEnabled()) logger.debug(wae.getMessage(), wae);
221-
setErrorResponse(context, ERR_WEBAUTHN_API_CREATE, wae.getMessage());
224+
setErrorResponse(context, WEBAUTHN_ERROR_REGISTRATION, wae.getMessage());
222225
return;
223226
} catch (Exception e) {
224227
if (logger.isDebugEnabled()) logger.debug(e.getMessage(), e);
225-
setErrorResponse(context, ERR_WEBAUTHN_API_CREATE, e.getMessage());
228+
setErrorResponse(context, WEBAUTHN_ERROR_REGISTRATION, e.getMessage());
226229
return;
227230
}
228231
}
@@ -326,44 +329,35 @@ public void evaluateTriggers(RequiredActionContext context) {
326329

327330
private static final String ERR_LABEL = "web_authn_registration_error";
328331
private static final String ERR_DETAIL_LABEL = "web_authn_registration_error_detail";
329-
private static final String ERR_WEBAUTHN_API_CREATE = "Failed to authenticate by the WebAuthn Authenticator";
330-
private static final String ERR_WEBAUTHN_VERIFICATION_FAIL = "WetAuthn Registration result is invalid.";
331-
private static final String ERR_REGISTRATION_FAIL = "Failed to register your WebAuthn Authenticator.";
332+
private static final String REGISTRATION_ATTR = "webAuthnRegistration";
332333

333334
private void setErrorResponse(RequiredActionContext context, final String errorCase, final String errorMessage) {
334335
Response errorResponse = null;
335336
switch (errorCase) {
336-
case ERR_WEBAUTHN_API_CREATE:
337-
logger.warnv("error returned from navigator.credentials.create(). {0}", errorMessage);
338-
context.getEvent()
339-
.detail(ERR_LABEL, errorCase)
340-
.detail(ERR_DETAIL_LABEL, errorMessage)
341-
.error(Errors.NOT_ALLOWED);
342-
errorResponse = context.form()
343-
.setError(errorCase)
344-
.createErrorPage(Response.Status.BAD_REQUEST);
345-
context.challenge(errorResponse);
346-
break;
347-
case ERR_WEBAUTHN_VERIFICATION_FAIL:
337+
case WEBAUTHN_ERROR_REGISTER_VERIFICATION:
348338
logger.warnv("WebAuthn API .create() response validation failure. {0}", errorMessage);
349339
context.getEvent()
350340
.detail(ERR_LABEL, errorCase)
351341
.detail(ERR_DETAIL_LABEL, errorMessage)
352342
.error(Errors.INVALID_USER_CREDENTIALS);
353343
errorResponse = context.form()
354344
.setError(errorCase)
355-
.createErrorPage(Response.Status.BAD_REQUEST);
345+
.setAttribute(WEB_AUTHN_TITLE_ATTR, WEBAUTHN_REGISTER_TITLE)
346+
.setAttribute(REGISTRATION_ATTR,true)
347+
.createWebAuthnErrorPage();
356348
context.challenge(errorResponse);
357349
break;
358-
case ERR_REGISTRATION_FAIL:
350+
case WEBAUTHN_ERROR_REGISTRATION:
359351
logger.warn(errorCase);
360352
context.getEvent()
361353
.detail(ERR_LABEL, errorCase)
362354
.detail(ERR_DETAIL_LABEL, errorMessage)
363355
.error(Errors.INVALID_REGISTRATION);
364356
errorResponse = context.form()
365357
.setError(errorCase)
366-
.createErrorPage(Response.Status.BAD_REQUEST);
358+
.setAttribute(WEB_AUTHN_TITLE_ATTR, WEBAUTHN_REGISTER_TITLE)
359+
.setAttribute(REGISTRATION_ATTR,true)
360+
.createWebAuthnErrorPage();
367361
context.challenge(errorResponse);
368362
break;
369363
default:

‎services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -547,6 +547,11 @@ public Response createErrorPage(Response.Status status) {
547547
return createResponse(LoginFormsPages.ERROR);
548548
}
549549

550+
@Override
551+
public Response createWebAuthnErrorPage() {
552+
return createResponse(LoginFormsPages.ERROR_WEBAUTHN);
553+
}
554+
550555
@Override
551556
public Response createOAuthGrant() {
552557
return createResponse(LoginFormsPages.OAUTH_GRANT);

‎services/src/main/java/org/keycloak/forms/login/freemarker/Templates.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ public static String getTemplate(LoginFormsPages page) {
5858
return "info.ftl";
5959
case ERROR:
6060
return "error.ftl";
61+
case ERROR_WEBAUTHN:
62+
return "webauthn-error.ftl";
6163
case LOGIN_UPDATE_PROFILE:
6264
return "login-update-profile.ftl";
6365
case CODE:

‎services/src/main/java/org/keycloak/services/messages/Messages.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,4 +240,16 @@ public class Messages {
240240
public static final String DELEGATION_FAILED = "delegationFailedMessage";
241241
public static final String DELEGATION_FAILED_HEADER = "delegationFailedHeader";
242242

243+
// WebAuthn
244+
public static final String WEBAUTHN_REGISTER_TITLE = "webauthn-registration-title";
245+
public static final String WEBAUTHN_LOGIN_TITLE = "webauthn-login-title";
246+
public static final String WEBAUTHN_ERROR_TITLE = "webauthn-error-title";
247+
248+
// WebAuthn Error
249+
public static final String WEBAUTHN_ERROR_REGISTRATION = "webauthn-error-registration";
250+
public static final String WEBAUTHN_ERROR_API_GET = "webauthn-error-api-get";
251+
public static final String WEBAUTHN_ERROR_DIFFERENT_USER = "webauthn-error-different-user";
252+
public static final String WEBAUTHN_ERROR_AUTH_VERIFICATION = "webauthn-error-auth-verification";
253+
public static final String WEBAUTHN_ERROR_REGISTER_VERIFICATION = "webauthn-error-register-verification";
254+
public static final String WEBAUTHN_ERROR_USER_NOT_FOUND = "webauthn-error-user-not-found";
243255
}

‎testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/webauthn/WebAuthnErrorPage.java‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
package org.keycloak.testsuite.pages.webauthn;
2+
3+
import org.keycloak.testsuite.pages.LanguageComboboxAwarePage;
4+
import org.openqa.selenium.By;
5+
import org.openqa.selenium.NoSuchElementException;
6+
import org.openqa.selenium.WebElement;
7+
import org.openqa.selenium.support.FindBy;
8+
9+
/**
10+
* @author <a href="mailto:[email protected]">Martin Bartos</a>
11+
*/
12+
public class WebAuthnErrorPage extends LanguageComboboxAwarePage {
13+
14+
@FindBy(id = "kc-try-again")
15+
private WebElement tryAgainButton;
16+
17+
public void clickTryAgain() {
18+
tryAgainButton.click();
19+
}
20+
21+
@Override
22+
public boolean isCurrent() {
23+
try {
24+
driver.findElement(By.id("kc-try-again"));
25+
driver.findElement(By.id("kc-error-credential-form"));
26+
return true;
27+
} catch (NoSuchElementException e) {
28+
return false;
29+
}
30+
}
31+
32+
@Override
33+
public void open() {
34+
throw new UnsupportedOperationException();
35+
}
36+
}

‎testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/webauthn/WebAuthnLoginPage.java‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,20 +18,12 @@
1818
package org.keycloak.testsuite.pages.webauthn;
1919

2020
import org.keycloak.testsuite.pages.LanguageComboboxAwarePage;
21-
import org.openqa.selenium.By;
2221

2322
/**
2423
* Page shown during WebAuthn login. Page is useful with Chrome testing API
2524
*/
2625
public class WebAuthnLoginPage extends LanguageComboboxAwarePage {
2726

28-
// After click the button, the "navigator.credentials.get" will be called on the browser side, which should automatically
29-
// login user with the chrome testing API
30-
public void confirmWebAuthnLogin() {
31-
driver.findElement(By.cssSelector("input[type=\"button\"]")).click();
32-
}
33-
34-
3527
public boolean isCurrent() {
3628
return driver.getPageSource().contains("navigator.credentials.get");
3729
}

0 commit comments

Comments
 (0)