From 8d49409de1396c46aa4c1c90554bc2a59978e632 Mon Sep 17 00:00:00 2001 From: Marek Posolda Date: Tue, 14 Jan 2020 21:54:45 +0100 Subject: [PATCH] KEYCLOAK-12183 Refactor login screens. Introduce try-another-way link. Not show many credentials of same type in credential selector (#6591) --- .../keycloak/models/jpa/JpaUserProvider.java | 5 +- .../AuthenticationFlowContext.java | 13 - .../AuthenticationSelectionOption.java | 87 +----- .../keycloak/forms/login/LoginFormsPages.java | 2 +- .../forms/login/LoginFormsProvider.java | 2 + .../AuthenticationProcessor.java | 11 - .../AuthenticationSelectionResolver.java | 245 +++++++++++++++ .../DefaultAuthenticationFlow.java | 226 +++++++------- .../browser/OTPFormAuthenticator.java | 17 +- .../authenticators/browser/PasswordForm.java | 12 +- .../browser/UsernamePasswordForm.java | 15 +- .../browser/WebAuthnAuthenticator.java | 12 +- .../directgrant/ValidateOTP.java | 9 +- .../FreeMarkerLoginFormsProvider.java | 12 +- .../forms/login/freemarker/Templates.java | 2 + .../model/AuthenticationContextBean.java | 14 +- .../login/freemarker/model/TotpBean.java | 2 + .../login/freemarker/model/TotpLoginBean.java | 93 ++++++ .../auth/page/login/OneTimeCode.java | 8 - .../pages/CredentialsComboboxPage.java | 62 ---- .../pages/LanguageComboboxAwarePage.java | 20 ++ .../testsuite/pages/LoginTotpPage.java | 49 ++- .../testsuite/pages/PasswordPage.java | 2 +- .../pages/SelectAuthenticatorPage.java | 76 +++++ .../KcOidcFirstBrokerLoginNewAuthTest.java | 33 +- .../testsuite/forms/BrowserFlowTest.java | 161 +++------- .../forms/MultiFactorAuthenticationTest.java | 291 ++++++++++++++++++ .../ResetCredentialsAlternativeFlowsTest.java | 10 +- .../base/src/test/resources/testrealm.json | 1 + .../resources/theme/base/login/login-otp.ftl | 19 +- .../theme/base/login/login-password.ftl | 3 +- .../login/messages/messages_en.properties | 1 + .../theme/base/login/select-authenticator.ftl | 35 +++ .../resources/theme/base/login/select.ftl | 50 --- .../resources/theme/base/login/template.ftl | 11 + .../base/login/webauthn-authenticate.ftl | 2 +- 36 files changed, 1101 insertions(+), 512 deletions(-) create mode 100644 services/src/main/java/org/keycloak/authentication/AuthenticationSelectionResolver.java create mode 100644 services/src/main/java/org/keycloak/forms/login/freemarker/model/TotpLoginBean.java delete mode 100644 testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/CredentialsComboboxPage.java create mode 100644 testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/SelectAuthenticatorPage.java create mode 100644 testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/MultiFactorAuthenticationTest.java create mode 100644 themes/src/main/resources/theme/base/login/select-authenticator.ftl delete mode 100644 themes/src/main/resources/theme/base/login/select.ftl diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaUserProvider.java b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaUserProvider.java index 60a7cb6fee..41617d0e88 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaUserProvider.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaUserProvider.java @@ -57,6 +57,7 @@ import javax.persistence.criteria.Root; import javax.persistence.criteria.Subquery; import java.util.ArrayList; import java.util.Collection; +import java.util.Comparator; import java.util.HashSet; import java.util.LinkedList; import java.util.List; @@ -896,7 +897,9 @@ public class JpaUserProvider implements UserProvider, UserCredentialStore { if (userEntity != null) { // user already in persistence context, no need to execute a query - results = userEntity.getCredentials().stream().filter(it -> type.equals(it.getType())).collect(Collectors.toList()); + results = userEntity.getCredentials().stream().filter(it -> type.equals(it.getType())) + .sorted(Comparator.comparingInt(CredentialEntity::getPriority)) + .collect(Collectors.toList()); List rtn = new LinkedList<>(); for (CredentialEntity entity : results) { rtn.add(toModel(entity)); diff --git a/server-spi-private/src/main/java/org/keycloak/authentication/AuthenticationFlowContext.java b/server-spi-private/src/main/java/org/keycloak/authentication/AuthenticationFlowContext.java index d704f44dee..6ac49e2a3f 100755 --- a/server-spi-private/src/main/java/org/keycloak/authentication/AuthenticationFlowContext.java +++ b/server-spi-private/src/main/java/org/keycloak/authentication/AuthenticationFlowContext.java @@ -50,19 +50,6 @@ public interface AuthenticationFlowContext extends AbstractAuthenticationFlowCon */ void setUser(UserModel user); - /** - * Gets the credential currently selected in this flow - * - * @return - */ - String getSelectedCredentialId(); - - /** - * Sets a selected credential for this flow - * @param credentialModel - */ - void setSelectedCredentialId(String credentialModel); - List getAuthenticationSelections(); void setAuthenticationSelections(List credentialAuthExecMap); diff --git a/server-spi-private/src/main/java/org/keycloak/authentication/AuthenticationSelectionOption.java b/server-spi-private/src/main/java/org/keycloak/authentication/AuthenticationSelectionOption.java index 54360d26bb..64cfe9c037 100644 --- a/server-spi-private/src/main/java/org/keycloak/authentication/AuthenticationSelectionOption.java +++ b/server-spi-private/src/main/java/org/keycloak/authentication/AuthenticationSelectionOption.java @@ -1,100 +1,41 @@ package org.keycloak.authentication; -import org.keycloak.credential.CredentialModel; import org.keycloak.models.AuthenticationExecutionModel; -import org.keycloak.models.AuthenticationFlowModel; +import org.keycloak.models.KeycloakSession; public class AuthenticationSelectionOption { + + private final KeycloakSession session; private final AuthenticationExecutionModel authExec; - private final CredentialModel credential; - private final AuthenticationFlowModel authFlow; - private boolean showCredentialName = true; - private boolean showCredentialType = true; - public AuthenticationSelectionOption(AuthenticationExecutionModel authExec) { + public AuthenticationSelectionOption(KeycloakSession session, AuthenticationExecutionModel authExec) { + this.session = session; this.authExec = authExec; - this.credential = new CredentialModel(); - this.authFlow = null; } - public AuthenticationSelectionOption(AuthenticationExecutionModel authExec, CredentialModel credential) { - this.authExec = authExec; - //Allow themes to get all credential information, but not secret data - this.credential = credential.shallowClone(); - this.credential.setSecretData(""); - this.authFlow = null; - } - - public AuthenticationSelectionOption(AuthenticationExecutionModel authExec, AuthenticationFlowModel authFlow) { - this.authExec = authExec; - this.credential = new CredentialModel(); - this.authFlow = authFlow; - } - - public void setShowCredentialName(boolean showCredentialName) { - this.showCredentialName = showCredentialName; - } - public void setShowCredentialType(boolean showCredentialType) { - this.showCredentialType = showCredentialType; - } - - public boolean showCredentialName(){ - if (credential.getId() == null) { - return false; - } - return showCredentialName; - } - - public boolean showCredentialType(){ - return showCredentialType; - } public AuthenticationExecutionModel getAuthenticationExecution() { return authExec; } - public String getCredentialId(){ - return credential.getId(); - } - public String getAuthExecId(){ return authExec.getId(); } - public String getCredentialName() { - StringBuilder sb = new StringBuilder(); - if (showCredentialName()) { - if (showCredentialType()) { - sb.append(" - "); - } - if (credential.getUserLabel() == null || credential.getUserLabel().isEmpty()) { - sb.append(credential.getId()); - } else { - sb.append(credential.getUserLabel()); - } - } - return sb.toString(); - } - public String getAuthExecName() { - if (authFlow != null) { - String authFlowLabel = authFlow.getAlias(); - if (authFlowLabel == null || authFlowLabel.isEmpty()) { - authFlowLabel = authFlow.getId(); - } - return authFlowLabel; - } return authExec.getAuthenticator(); } - public String getId() { - if (getCredentialId() == null) { - return getAuthExecId() + "|"; - } - return getAuthExecId() + "|" + getCredentialId(); + public String getAuthExecDisplayName() { + // TODO: Retrieve the displayName for the authenticator from the AuthenticationFactory + // TODO: Retrieve icon CSS style + // TODO: Should be addressed as part of https://issues.redhat.com/browse/KEYCLOAK-12185 + return getAuthExecName(); } - public CredentialModel getCredential(){ - return credential; + + @Override + public String toString() { + return " authSelection - " + authExec.getAuthenticator(); } } diff --git a/server-spi-private/src/main/java/org/keycloak/forms/login/LoginFormsPages.java b/server-spi-private/src/main/java/org/keycloak/forms/login/LoginFormsPages.java index b75369209a..aad4b03d96 100755 --- a/server-spi-private/src/main/java/org/keycloak/forms/login/LoginFormsPages.java +++ b/server-spi-private/src/main/java/org/keycloak/forms/login/LoginFormsPages.java @@ -24,7 +24,7 @@ public enum LoginFormsPages { LOGIN, LOGIN_USERNAME, LOGIN_PASSWORD, LOGIN_TOTP, LOGIN_CONFIG_TOTP, LOGIN_WEBAUTHN, LOGIN_VERIFY_EMAIL, LOGIN_IDP_LINK_CONFIRM, LOGIN_IDP_LINK_EMAIL, - OAUTH_GRANT, LOGIN_RESET_PASSWORD, LOGIN_UPDATE_PASSWORD, REGISTER, INFO, ERROR, LOGIN_UPDATE_PROFILE, + OAUTH_GRANT, LOGIN_RESET_PASSWORD, LOGIN_UPDATE_PASSWORD, LOGIN_SELECT_AUTHENTICATOR, REGISTER, INFO, ERROR, LOGIN_UPDATE_PROFILE, LOGIN_PAGE_EXPIRED, CODE, X509_CONFIRM, SAML_POST_FORM; } diff --git a/server-spi-private/src/main/java/org/keycloak/forms/login/LoginFormsProvider.java b/server-spi-private/src/main/java/org/keycloak/forms/login/LoginFormsProvider.java index 7cc6294f63..324409087e 100755 --- a/server-spi-private/src/main/java/org/keycloak/forms/login/LoginFormsProvider.java +++ b/server-spi-private/src/main/java/org/keycloak/forms/login/LoginFormsProvider.java @@ -84,6 +84,8 @@ public interface LoginFormsProvider extends Provider { Response createOAuthGrant(); + Response createSelectAuthenticator(); + Response createCode(); Response createX509ConfirmPage(); diff --git a/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java b/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java index b4e623c69a..b83bcd983a 100755 --- a/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java +++ b/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java @@ -280,7 +280,6 @@ public class AuthenticationProcessor { List currentExecutions; FormMessage errorMessage; FormMessage successMessage; - String selectedCredentialId; List authenticationSelections; private Result(AuthenticationExecutionModel execution, Authenticator authenticator, List currentExecutions) { @@ -399,16 +398,6 @@ public class AuthenticationProcessor { setAutheticatedUser(user); } - @Override - public String getSelectedCredentialId() { - return selectedCredentialId; - } - - @Override - public void setSelectedCredentialId(String selectedCredentialId) { - this.selectedCredentialId = selectedCredentialId; - } - @Override public List getAuthenticationSelections() { return authenticationSelections; diff --git a/services/src/main/java/org/keycloak/authentication/AuthenticationSelectionResolver.java b/services/src/main/java/org/keycloak/authentication/AuthenticationSelectionResolver.java new file mode 100644 index 0000000000..fd96f32729 --- /dev/null +++ b/services/src/main/java/org/keycloak/authentication/AuthenticationSelectionResolver.java @@ -0,0 +1,245 @@ +/* + * Copyright 2019 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.keycloak.authentication; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +import javax.ws.rs.core.MultivaluedHashMap; +import javax.ws.rs.core.MultivaluedMap; + +import org.jboss.logging.Logger; +import org.keycloak.credential.CredentialModel; +import org.keycloak.models.AuthenticationExecutionModel; +import org.keycloak.models.AuthenticationFlowModel; +import org.keycloak.models.RealmModel; + +/** + * Resolves set of AuthenticationSelectionOptions + * + * @author Marek Posolda + */ +class AuthenticationSelectionResolver { + + private static final Logger logger = Logger.getLogger(AuthenticationSelectionResolver.class); + + /** + * This method creates the list of authenticators that is presented to the user. For a required execution, this is + * only the credentials associated to the authenticator, and for an alternative execution, this is all other alternative + * executions in the flow, including the credentials. + *

+ * In both cases, the credentials take precedence, with the order selected by the user (or his administrator). + *

+ * The implementation needs to take various subflows into account. + * + * For example during configuration of the authentication flow like this: + * - WebAuthn: ALTERNATIVE + * - Password-and-OTP subflow: ALTERNATIVE + * - Password REQUIRED + * - OTP REQUIRED + * The user can authenticate with: WebAuthn OR (Password AND OTP). In this case, the user should be able to choose between WebAuthn and Password + * even if those mechanisms are in different subflows + * + * @param model The current execution model + * @return an ordered list of the authentication selection options to present the user. + */ + static List createAuthenticationSelectionList(AuthenticationProcessor processor, AuthenticationExecutionModel model) { + List authenticationSelectionList = new ArrayList<>(); + + if (processor.getAuthenticationSession() != null) { + Map typeAuthExecMap = new HashMap<>(); + List nonCredentialExecutions = new ArrayList<>(); + + String topFlowId = getFlowIdOfTheHighestUsefulFlow(processor, model); + + if (topFlowId == null) { + addSimpleAuthenticationExecution(processor, model, typeAuthExecMap, nonCredentialExecutions); + } else { + addAllExecutionsFromSubflow(processor, topFlowId, typeAuthExecMap, nonCredentialExecutions); + } + + //add credential authenticators in order + if (processor.getAuthenticationSession().getAuthenticatedUser() != null) { + List credentials = processor.getSession().userCredentialManager() + .getStoredCredentials(processor.getRealm(), processor.getAuthenticationSession().getAuthenticatedUser()) + .stream() + .filter(credential -> typeAuthExecMap.containsKey(credential.getType())) + .collect(Collectors.toList()); + + authenticationSelectionList = credentials.stream() + .map(CredentialModel::getType) + .distinct() + .map(credentialType -> new AuthenticationSelectionOption(processor.getSession(), typeAuthExecMap.get(credentialType))) + .collect(Collectors.toList()); + } + + //add all other authenticators + for (AuthenticationExecutionModel exec : nonCredentialExecutions) { + authenticationSelectionList.add(new AuthenticationSelectionOption(processor.getSession(), exec)); + } + } + + logger.debugf("Selections when trying execution '%s' : %s", model.getAuthenticator(), authenticationSelectionList); + + return authenticationSelectionList; + } + + + /** + * Return the flowId of the "highest" subflow, which we need to take into account when creating list of authentication mechanisms + * shown to the user. + * + * For example during configuration of the authentication flow like this: + * - WebAuthn: ALTERNATIVE + * - Password-and-OTP subflow: ALTERNATIVE + * - Password REQUIRED + * - OTP REQUIRED + * + * and assuming that "execution" parameter is PasswordForm, we also need to take the higher subflow into account as user + * should be able to choose among WebAuthn and Password + * + * @param processor + * @param execution + * @return + */ + private static String getFlowIdOfTheHighestUsefulFlow(AuthenticationProcessor processor, AuthenticationExecutionModel execution) { + String flowId = null; + RealmModel realm = processor.getRealm(); + + while (true) { + if (execution.isAlternative()) { + //Consider parent flow as we need to get all alternative executions to be able to list their credentials + flowId = execution.getParentFlow(); + } else if (execution.isRequired() || execution.isConditional()) { + if (execution.isAuthenticatorFlow()) { + flowId = execution.getFlowId(); + } + + // Find the corresponding execution. If it is 1st REQUIRED execution in the particular subflow, we need to consider parent flow as well + List executions = realm.getAuthenticationExecutions(execution.getParentFlow()); + int executionIndex = executions.indexOf(execution); + if (executionIndex != 0) { + return flowId; + } else { + flowId = execution.getParentFlow(); + } + } + + AuthenticationFlowModel flow = realm.getAuthenticationFlowById(flowId); + if (flow.isTopLevel()) { + return flowId; + } + execution = realm.getAuthenticationExecutionByFlowId(flowId); + } + } + + + // Process single authenticaion execution, which does NOT point to authentication flow. + // Fill the typeAuthExecMap and nonCredentialExecutions accordingly + private static void addSimpleAuthenticationExecution(AuthenticationProcessor processor, AuthenticationExecutionModel execution, Map typeAuthExecMap, List nonCredentialExecutions) { + // Don't add already processed executions + if (DefaultAuthenticationFlow.isProcessed(processor, execution)) { + return; + } + + Authenticator localAuthenticator = processor.getSession().getProvider(Authenticator.class, execution.getAuthenticator()); + if (!(localAuthenticator instanceof CredentialValidator)) { + nonCredentialExecutions.add(execution); + } else { + CredentialValidator cv = (CredentialValidator) localAuthenticator; + typeAuthExecMap.put(cv.getType(processor.getSession()), execution); + } + } + + + /** + * Fill the typeAuthExecMap and nonCredentialExecutions collections with all available authentication mechanisms for the particular subflow with + * given flowId + * + * Return true if at least something was added to any of the list + */ + private static boolean addAllExecutionsFromSubflow(AuthenticationProcessor processor, String flowId, Map typeAuthExecMap, List nonCredentialExecutions) { + AuthenticationFlowModel flowModel = processor.getRealm().getAuthenticationFlowById(flowId); + if (flowModel == null) { + throw new AuthenticationFlowException("Flow not found", AuthenticationFlowError.INTERNAL_ERROR); + } + + DefaultAuthenticationFlow flow = new DefaultAuthenticationFlow(processor, flowModel); + + logger.debugf("Going through the flow '%s' for adding executions", flowModel.getAlias()); + + List executions = processor.getRealm().getAuthenticationExecutions(flowId); + + List requiredList = new ArrayList<>(); + List alternativeList = new ArrayList<>(); + flow.fillListsOfExecutions(executions, requiredList, alternativeList); + + // If requiredList is not empty, we're going to collect just very first execution from the flow + if (!requiredList.isEmpty()) { + AuthenticationExecutionModel requiredExecution = requiredList.stream().filter(ex -> { + + if (ex.isRequired()) return true; + + // For conditional execution, we must check if condition is true. Otherwise return false, which means trying next + // requiredExecution in the list + return !flow.isConditionalSubflowDisabled(ex); + + }).findFirst().orElse(null); + + // Not requiredExecution found. Returning false as we did not add any authenticator + if (requiredExecution == null) return false; + + // Don't add already processed executions + if (flow.isProcessed(requiredExecution)) { + return false; + } + + // Recursively add credentials from required execution + if (requiredExecution.isAuthenticatorFlow()) { + return addAllExecutionsFromSubflow(processor, requiredExecution.getFlowId(), typeAuthExecMap, nonCredentialExecutions); + } else { + addSimpleAuthenticationExecution(processor, requiredExecution, typeAuthExecMap, nonCredentialExecutions); + return true; + } + } else { + // We're going through all the alternatives + boolean anyAdded = false; + + for (AuthenticationExecutionModel execution : alternativeList) { + // Don't add already processed executions + if (flow.isProcessed(execution)) { + continue; + } + + if (!execution.isAuthenticatorFlow()) { + addSimpleAuthenticationExecution(processor, execution, typeAuthExecMap, nonCredentialExecutions); + anyAdded = true; + } else { + anyAdded |= addAllExecutionsFromSubflow(processor, execution.getFlowId(), typeAuthExecMap, nonCredentialExecutions); + } + } + + return anyAdded; + } + + } +} diff --git a/services/src/main/java/org/keycloak/authentication/DefaultAuthenticationFlow.java b/services/src/main/java/org/keycloak/authentication/DefaultAuthenticationFlow.java index 876b999d12..7356856287 100755 --- a/services/src/main/java/org/keycloak/authentication/DefaultAuthenticationFlow.java +++ b/services/src/main/java/org/keycloak/authentication/DefaultAuthenticationFlow.java @@ -20,7 +20,6 @@ package org.keycloak.authentication; import org.jboss.logging.Logger; import org.keycloak.OAuth2Constants; import org.keycloak.authentication.authenticators.conditional.ConditionalAuthenticator; -import org.keycloak.credential.CredentialModel; import org.keycloak.models.AuthenticationExecutionModel; import org.keycloak.models.AuthenticationFlowModel; import org.keycloak.models.Constants; @@ -32,15 +31,11 @@ import org.keycloak.services.util.AuthenticationFlowURLHelper; import org.keycloak.sessions.AuthenticationSessionModel; import org.keycloak.sessions.CommonClientSessionModel; -import javax.ws.rs.core.MultivaluedHashMap; import javax.ws.rs.core.MultivaluedMap; import javax.ws.rs.core.Response; import java.util.ArrayList; -import java.util.HashMap; import java.util.Iterator; import java.util.List; -import java.util.Map; -import java.util.Map.Entry; import java.util.stream.Collectors; /** @@ -62,6 +57,10 @@ public class DefaultAuthenticationFlow implements AuthenticationFlow { } protected boolean isProcessed(AuthenticationExecutionModel model) { + return isProcessed(processor, model); + } + + protected static boolean isProcessed(AuthenticationProcessor processor, AuthenticationExecutionModel model) { if (model.isDisabled()) return true; AuthenticationSessionModel.ExecutionStatus status = processor.getAuthenticationSession().getExecutionStatus().get(model.getId()); if (status == null) return false; @@ -97,12 +96,11 @@ public class DefaultAuthenticationFlow implements AuthenticationFlow { } AuthenticationExecutionModel model = processor.getRealm().getAuthenticationExecutionById(actionExecution); if (model == null) { - throw new AuthenticationFlowException("action is not in current execution", AuthenticationFlowError.INTERNAL_ERROR); + throw new AuthenticationFlowException("Execution not found", AuthenticationFlowError.INTERNAL_ERROR); } MultivaluedMap inputData = processor.getRequest().getDecodedFormParameters(); String authExecId = inputData.getFirst(Constants.AUTHENTICATION_EXECUTION); - String selectedCredentialId = inputData.getFirst(Constants.CREDENTIAL_ID); //check if the user has selected the "back" option if (inputData.containsKey("back")) { @@ -118,18 +116,29 @@ public class DefaultAuthenticationFlow implements AuthenticationFlow { recursiveClearExecutionStatusOfAllExecutionsAfterOurExecutionInclusive(lastActionExecution); - Response response = processSingleFlowExecutionModel(lastActionExecution, null, false); + Response response = processSingleFlowExecutionModel(lastActionExecution, false); if (response == null) { processor.getAuthenticationSession().removeAuthNote(AuthenticationProcessor.CURRENT_AUTHENTICATION_EXECUTION); return processFlow(); } else return response; } else { // This normally shouldn't happen as "back" button shouldn't be available on the form. If it is still triggered, we show "pageExpired" page - new AuthenticationFlowURLHelper(processor.getSession(), processor.getRealm(), processor.getUriInfo()) + return new AuthenticationFlowURLHelper(processor.getSession(), processor.getRealm(), processor.getUriInfo()) .showPageExpired(authSession); } } + // User clicked on "try another way" link + if (inputData.containsKey("tryAnotherWay")) { + logger.info("User clicked on try another way"); + + List selectionOptions = createAuthenticationSelectionList(model); + + AuthenticationProcessor.Result result = processor.createAuthenticatorContext(model, null, null); + result.setAuthenticationSelections(selectionOptions); + return result.form().createSelectAuthenticator(); + } + // check if the user has switched to a new authentication execution, and if so switch to it. if (authExecId != null && !authExecId.isEmpty()) { @@ -149,14 +158,13 @@ public class DefaultAuthenticationFlow implements AuthenticationFlow { new AuthenticationFlowHistoryHelper(processor).pushExecution(selectionOptions.get(0).getAuthExecId()); } - Response response = processSingleFlowExecutionModel(model, selectedCredentialId, false); + Response response = processSingleFlowExecutionModel(model, false); if (response == null) { - processor.getAuthenticationSession().removeAuthNote(AuthenticationProcessor.CURRENT_AUTHENTICATION_EXECUTION); - checkAndValidateParentFlow(model); - return processFlow(); + return continueAuthenticationAfterSuccessfulAction(model); } else return response; } - //handle case where execution is a flow + + //handle case where execution is a flow - This can happen during user registration for example if (model.isAuthenticatorFlow()) { logger.debug("execution is flow"); AuthenticationFlow authenticationFlow = processor.createFlowExecution(model.getFlowId(), model); @@ -169,24 +177,50 @@ public class DefaultAuthenticationFlow implements AuthenticationFlow { return flowChallenge; } } + //handle normal execution case AuthenticatorFactory factory = getAuthenticatorFactory(model); Authenticator authenticator = createAuthenticator(factory); AuthenticationProcessor.Result result = processor.createAuthenticatorContext(model, authenticator, executions); result.setAuthenticationSelections(createAuthenticationSelectionList(model)); - result.setSelectedCredentialId(selectedCredentialId); - logger.debugv("action: {0}", model.getAuthenticator()); authenticator.action(result); Response response = processResult(result, true); if (response == null) { - processor.getAuthenticationSession().removeAuthNote(AuthenticationProcessor.CURRENT_AUTHENTICATION_EXECUTION); - checkAndValidateParentFlow(model); - return processFlow(); + return continueAuthenticationAfterSuccessfulAction(model); } else return response; } + + /** + * Called after "actionExecutionModel" execution is finished (Either successful or attempted). Find the next appropriate authentication + * flow where the authentication should continue and continue with authentication process. + * + * @param actionExecutionModel + * @return Response if some more forms should be displayed during authentication. Null otherwise. + */ + private Response continueAuthenticationAfterSuccessfulAction(AuthenticationExecutionModel actionExecutionModel) { + processor.getAuthenticationSession().removeAuthNote(AuthenticationProcessor.CURRENT_AUTHENTICATION_EXECUTION); + + String firstUnfinishedParentFlowId = checkAndValidateParentFlow(actionExecutionModel); + AuthenticationExecutionModel parentFlowExecution = processor.getRealm().getAuthenticationExecutionByFlowId(firstUnfinishedParentFlowId); + + if (parentFlowExecution == null) { + // This means that 1st unfinished ancestor flow is the top flow. We can just process it from the start + return processFlow(); + } else { + Response response = processSingleFlowExecutionModel(parentFlowExecution, false); + if (response == null) { + processor.getAuthenticationSession().removeAuthNote(AuthenticationProcessor.CURRENT_AUTHENTICATION_EXECUTION); + return processFlow(); + } else { + return response; + } + } + } + + /** * Clear execution status of targetExecution and also clear execution status of all the executions, which were triggered after this execution. * This covers also "flow" executions and executions, which were set automatically @@ -261,37 +295,37 @@ public class DefaultAuthenticationFlow implements AuthenticationFlow { * This method makes sure that the parent flow's corresponding execution is considered successful if its contained * executions are successful. * The purpose is for when an execution is validated through an action, to make sure its parent flow can be successful - * when re-evaluation the flow tree. + * when re-evaluation the flow tree. If the flow is successful, we will recursively check it's parent flow as well * * @param model An execution model. + * @return flowId of the 1st ancestor flow, which is not yet successfully finished and may require some further processing */ - private void checkAndValidateParentFlow(AuthenticationExecutionModel model) { - List localExecutions = processor.getRealm().getAuthenticationExecutions(model.getParentFlow()); - AuthenticationExecutionModel parentFlowModel = processor.getRealm().getAuthenticationExecutionByFlowId(model.getParentFlow()); - if (parentFlowModel != null && - ((model.isRequired() && localExecutions.stream().allMatch(processor::isSuccessful)) || - (model.isAlternative() && localExecutions.stream().anyMatch(processor::isSuccessful)))) { - processor.getAuthenticationSession().setExecutionStatus(parentFlowModel.getId(), AuthenticationSessionModel.ExecutionStatus.SUCCESS); + private String checkAndValidateParentFlow(AuthenticationExecutionModel model) { + while (true) { + List localExecutions = processor.getRealm().getAuthenticationExecutions(model.getParentFlow()); + AuthenticationExecutionModel parentFlowExecutionModel = processor.getRealm().getAuthenticationExecutionByFlowId(model.getParentFlow()); + if (parentFlowExecutionModel != null && + ((model.isRequired() && localExecutions.stream().allMatch(processor::isSuccessful)) || + (model.isAlternative() && localExecutions.stream().anyMatch(processor::isSuccessful)))) { + processor.getAuthenticationSession().setExecutionStatus(parentFlowExecutionModel.getId(), AuthenticationSessionModel.ExecutionStatus.SUCCESS); + + // Flow is successfully finished. Recursively check whether it's parent flow is now successful as well + model = parentFlowExecutionModel; + } else { + return model.getParentFlow(); + } } } @Override public Response processFlow() { - logger.debug("processFlow"); + logger.debugf("processFlow: %s", flow.getAlias()); //separate flow elements into required and alternative elements List requiredList = new ArrayList<>(); List alternativeList = new ArrayList<>(); - for (AuthenticationExecutionModel execution : executions) { - if (isConditionalAuthenticator(execution)) { - continue; - } else if (execution.isRequired() || execution.isConditional()) { - requiredList.add(execution); - } else if (execution.isAlternative()) { - alternativeList.add(execution); - } - } + fillListsOfExecutions(executions, requiredList, alternativeList); //handle required elements : all required elements need to be executed boolean requiredElementsSuccessful = true; @@ -303,11 +337,16 @@ public class DefaultAuthenticationFlow implements AuthenticationFlow { requiredIListIterator.remove(); continue; } - Response response = processSingleFlowExecutionModel(required, null, true); + Response response = processSingleFlowExecutionModel(required, true); requiredElementsSuccessful &= processor.isSuccessful(required) || isSetupRequired(required); if (response != null) { return response; } + // Some required elements were not successful and did not return response. + // We can break as we know that the whole subflow would be considered unsuccessful as well + if (!requiredElementsSuccessful) { + break; + } } //Evaluate alternative elements only if there are no required elements. This may also occur if there was only condition elements @@ -321,7 +360,7 @@ public class DefaultAuthenticationFlow implements AuthenticationFlow { //handle alternative elements: the first alternative element to be satisfied is enough for (AuthenticationExecutionModel alternative : alternativeList) { try { - Response response = processSingleFlowExecutionModel(alternative, null, true); + Response response = processSingleFlowExecutionModel(alternative, true); if (response != null) { return response; } @@ -341,12 +380,38 @@ public class DefaultAuthenticationFlow implements AuthenticationFlow { return null; } + + /** + * Just iterates over executionsToProcess and fill "requiredList" and "alternativeList" according to it + */ + void fillListsOfExecutions(List executionsToProcess, List requiredList, List alternativeList) { + for (AuthenticationExecutionModel execution : executionsToProcess) { + if (isConditionalAuthenticator(execution)) { + continue; + } else if (execution.isRequired() || execution.isConditional()) { + requiredList.add(execution); + } else if (execution.isAlternative()) { + alternativeList.add(execution); + } + } + + if (!requiredList.isEmpty() && !alternativeList.isEmpty()) { + List alternativeIds = alternativeList.stream() + .map(AuthenticationExecutionModel::getAuthenticator) + .collect(Collectors.toList()); + + logger.warnf("REQUIRED and ALTERNATIVE elements at same level! Those alternative executions will be ignored: %s", alternativeIds); + alternativeList.clear(); + } + } + + /** * Checks if the conditional subflow passed in parameter is disabled. * @param model * @return */ - private boolean isConditionalSubflowDisabled(AuthenticationExecutionModel model) { + boolean isConditionalSubflowDisabled(AuthenticationExecutionModel model) { if (model == null || !model.isAuthenticatorFlow() || !model.isConditional()) { return false; }; @@ -395,7 +460,8 @@ public class DefaultAuthenticationFlow implements AuthenticationFlow { return AuthenticationSessionModel.ExecutionStatus.SETUP_REQUIRED.equals(processor.getAuthenticationSession().getExecutionStatus().get(model.getId())); } - private Response processSingleFlowExecutionModel(AuthenticationExecutionModel model, String selectedCredentialId, boolean calledFromFlow) { + + private Response processSingleFlowExecutionModel(AuthenticationExecutionModel model, boolean calledFromFlow) { logger.debugv("check execution: {0} requirement: {1}", model.getAuthenticator(), model.getRequirement()); if (isProcessed(model)) { @@ -429,7 +495,7 @@ public class DefaultAuthenticationFlow implements AuthenticationFlow { //If executions are alternative, get the actual execution to show based on user preference List selectionOptions = createAuthenticationSelectionList(model); if (!selectionOptions.isEmpty() && calledFromFlow) { - List finalSelectionOptions = selectionOptions.stream().filter(aso -> !aso.getAuthenticationExecution().isAuthenticatorFlow() && !isProcessed(aso.getAuthenticationExecution())).collect(Collectors.toList());; + List finalSelectionOptions = selectionOptions.stream().filter(aso -> !aso.getAuthenticationExecution().isAuthenticatorFlow() && !isProcessed(aso.getAuthenticationExecution())).collect(Collectors.toList()); if (finalSelectionOptions.isEmpty()) { //move to next return null; @@ -443,11 +509,7 @@ public class DefaultAuthenticationFlow implements AuthenticationFlow { } AuthenticationProcessor.Result context = processor.createAuthenticatorContext(model, authenticator, executions); context.setAuthenticationSelections(selectionOptions); - if (selectedCredentialId != null) { - context.setSelectedCredentialId(selectedCredentialId); - } else if (!selectionOptions.isEmpty()) { - context.setSelectedCredentialId(selectionOptions.get(0).getCredentialId()); - } + if (authenticator.requiresUser()) { if (authUser == null) { throw new AuthenticationFlowException("authenticator: " + factory.getId(), AuthenticationFlowError.UNKNOWN_USER); @@ -481,72 +543,7 @@ public class DefaultAuthenticationFlow implements AuthenticationFlow { * @return an ordered list of the authentication selection options to present the user. */ private List createAuthenticationSelectionList(AuthenticationExecutionModel model) { - List authenticationSelectionList = new ArrayList<>(); - if (processor.getAuthenticationSession() != null) { - Map typeAuthExecMap = new HashMap<>(); - List nonCredentialExecutions = new ArrayList<>(); - if (model.isAlternative()) { - //get all alternative executions to be able to list their credentials - List alternativeExecutions = processor.getRealm().getAuthenticationExecutions(model.getParentFlow()) - .stream().filter(AuthenticationExecutionModel::isAlternative).collect(Collectors.toList()); - for (AuthenticationExecutionModel execution : alternativeExecutions) { - if (!execution.isAuthenticatorFlow()) { - Authenticator localAuthenticator = processor.getSession().getProvider(Authenticator.class, execution.getAuthenticator()); - if (!(localAuthenticator instanceof CredentialValidator)) { - nonCredentialExecutions.add(execution); - continue; - } - CredentialValidator cv = (CredentialValidator) localAuthenticator; - typeAuthExecMap.put(cv.getType(processor.getSession()), execution); - } else { - nonCredentialExecutions.add(execution); - } - } - } else if (model.isRequired() && !model.isAuthenticatorFlow()) { - //only get current credentials - Authenticator authenticator = processor.getSession().getProvider(Authenticator.class, model.getAuthenticator()); - if (authenticator instanceof CredentialValidator) { - typeAuthExecMap.put(((CredentialValidator) authenticator).getType(processor.getSession()), model); - } - } - //add credential authenticators in order - if (processor.getAuthenticationSession().getAuthenticatedUser() != null) { - List credentials = processor.getSession().userCredentialManager() - .getStoredCredentials(processor.getRealm(), processor.getAuthenticationSession().getAuthenticatedUser()) - .stream() - .filter(credential -> typeAuthExecMap.containsKey(credential.getType())) - .collect(Collectors.toList()); - - MultivaluedMap countAuthSelections = new MultivaluedHashMap<>(); - - for (CredentialModel credential : credentials) { - AuthenticationSelectionOption authSel = new AuthenticationSelectionOption(typeAuthExecMap.get(credential.getType()), credential); - authenticationSelectionList.add(authSel); - countAuthSelections.add(credential.getType(), authSel); - } - for (Entry> entry : countAuthSelections.entrySet()) { - if (entry.getValue().size() == 1) { - entry.getValue().get(0).setShowCredentialName(false); - } - } - //don't show credential type if there's only a single type in the list - if (countAuthSelections.keySet().size() == 1 && nonCredentialExecutions.isEmpty()) { - for (AuthenticationSelectionOption so : authenticationSelectionList) { - so.setShowCredentialType(false); - } - } - } - //add all other authenticators (including flows) - for (AuthenticationExecutionModel exec : nonCredentialExecutions) { - if (exec.isAuthenticatorFlow()) { - authenticationSelectionList.add(new AuthenticationSelectionOption(exec, - processor.getRealm().getAuthenticationFlowById(exec.getFlowId()))); - } else { - authenticationSelectionList.add(new AuthenticationSelectionOption(exec)); - } - } - } - return authenticationSelectionList; + return AuthenticationSelectionResolver.createAuthenticationSelectionList(processor, model); } @@ -585,9 +582,6 @@ public class DefaultAuthenticationFlow implements AuthenticationFlow { return sendChallenge(result, execution); case ATTEMPTED: logger.debugv("authenticator ATTEMPTED: {0}", execution.getAuthenticator()); - if (execution.isRequired()) { - throw new AuthenticationFlowException(AuthenticationFlowError.INVALID_CREDENTIALS); - } processor.getAuthenticationSession().setExecutionStatus(execution.getId(), AuthenticationSessionModel.ExecutionStatus.ATTEMPTED); return null; case FLOW_RESET: diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/OTPFormAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/OTPFormAuthenticator.java index 9c6210a944..8426103a81 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/OTPFormAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/OTPFormAuthenticator.java @@ -26,6 +26,7 @@ import org.keycloak.authentication.RequiredActionProvider; import org.keycloak.authentication.requiredactions.UpdateTotp; import org.keycloak.credential.CredentialProvider; import org.keycloak.credential.OTPCredentialProvider; +import org.keycloak.credential.OTPCredentialProviderFactory; import org.keycloak.events.Errors; import org.keycloak.forms.login.LoginFormsProvider; import org.keycloak.models.KeycloakSession; @@ -44,6 +45,14 @@ import java.util.List; * @version $Revision: 1 $ */ public class OTPFormAuthenticator extends AbstractUsernameFormAuthenticator implements Authenticator, CredentialValidator { + + // Freemarker attribute where selected OTP credential will be stored + public static final String SELECTED_OTP_CREDENTIAL_ID = "selectedOtpCredentialId"; + + // Label to be shown in the UI for the "unnamed" OTP credential, which doesn't have userLabel + public static final String UNNAMED = "unnamed"; + + @Override public void action(AuthenticationFlowContext context) { validateOTP(context); @@ -61,14 +70,14 @@ public class OTPFormAuthenticator extends AbstractUsernameFormAuthenticator impl MultivaluedMap inputData = context.getHttpRequest().getDecodedFormParameters(); String otp = inputData.getFirst("otp"); - String credentialId = context.getSelectedCredentialId(); - //TODO this is lazy for when there is no clearly defined credentialId available (for example direct grant or console OTP), replace with getting the credential from the name + String credentialId = inputData.getFirst("selectedCredentialId"); + if (credentialId == null || credentialId.isEmpty()) { credentialId = getCredentialProvider(context.getSession()) .getDefaultCredential(context.getSession(), context.getRealm(), context.getUser()).getId(); - context.setSelectedCredentialId(credentialId); } + context.form().setAttribute(SELECTED_OTP_CREDENTIAL_ID, credentialId); UserModel userModel = context.getUser(); if (!enabledUser(context, userModel)) { @@ -131,7 +140,7 @@ public class OTPFormAuthenticator extends AbstractUsernameFormAuthenticator impl @Override public OTPCredentialProvider getCredentialProvider(KeycloakSession session) { - return (OTPCredentialProvider)session.getProvider(CredentialProvider.class, "keycloak-otp"); + return (OTPCredentialProvider)session.getProvider(CredentialProvider.class, OTPCredentialProviderFactory.PROVIDER_ID); } } 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 f8e5c2a658..adceb43052 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 @@ -18,6 +18,9 @@ package org.keycloak.authentication.authenticators.browser; import org.keycloak.authentication.AuthenticationFlowContext; +import org.keycloak.authentication.CredentialValidator; +import org.keycloak.credential.CredentialProvider; +import org.keycloak.credential.PasswordCredentialProvider; import org.keycloak.forms.login.LoginFormsProvider; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; @@ -27,7 +30,7 @@ import org.keycloak.services.messages.Messages; import javax.ws.rs.core.MultivaluedMap; import javax.ws.rs.core.Response; -public class PasswordForm extends UsernamePasswordForm { +public class PasswordForm extends UsernamePasswordForm implements CredentialValidator { protected boolean validateForm(AuthenticationFlowContext context, MultivaluedMap formData) { return validatePassword(context, context.getUser(), formData, false); @@ -56,7 +59,12 @@ public class PasswordForm extends UsernamePasswordForm { } @Override - protected String getDefaultChallengeMessage(AuthenticationFlowContext context){ + protected String getDefaultChallengeMessage(AuthenticationFlowContext context) { return Messages.INVALID_PASSWORD; } + + @Override + public PasswordCredentialProvider getCredentialProvider(KeycloakSession session) { + return (PasswordCredentialProvider)session.getProvider(CredentialProvider.class, "keycloak-password"); + } } 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 c8b4f2e425..204f504692 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 @@ -20,9 +20,6 @@ package org.keycloak.authentication.authenticators.browser; import org.jboss.resteasy.specimpl.MultivaluedMapImpl; import org.keycloak.authentication.AuthenticationFlowContext; import org.keycloak.authentication.Authenticator; -import org.keycloak.authentication.CredentialValidator; -import org.keycloak.credential.CredentialProvider; -import org.keycloak.credential.PasswordCredentialProvider; import org.keycloak.forms.login.LoginFormsProvider; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; @@ -30,7 +27,6 @@ import org.keycloak.models.UserModel; import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.services.ServicesLogger; import org.keycloak.services.managers.AuthenticationManager; -import org.keycloak.services.messages.Messages; import javax.ws.rs.core.MultivaluedMap; import javax.ws.rs.core.Response; @@ -39,7 +35,7 @@ import javax.ws.rs.core.Response; * @author Bill Burke * @version $Revision: 1 $ */ -public class UsernamePasswordForm extends AbstractUsernameFormAuthenticator implements Authenticator, CredentialValidator { +public class UsernamePasswordForm extends AbstractUsernameFormAuthenticator implements Authenticator { protected static ServicesLogger log = ServicesLogger.LOGGER; @Override @@ -108,13 +104,4 @@ public class UsernamePasswordForm extends AbstractUsernameFormAuthenticator impl } - @Override - public PasswordCredentialProvider getCredentialProvider(KeycloakSession session) { - return (PasswordCredentialProvider)session.getProvider(CredentialProvider.class, "keycloak-password"); - } - - @Override - protected String getDefaultChallengeMessage(AuthenticationFlowContext context){ - return Messages.INVALID_USER; - } } 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 e844625d73..e1842f81e3 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 @@ -28,12 +28,17 @@ import org.keycloak.WebAuthnConstants; import org.keycloak.authentication.AuthenticationFlowContext; import org.keycloak.authentication.AuthenticationFlowError; import org.keycloak.authentication.Authenticator; +import org.keycloak.authentication.CredentialValidator; import org.keycloak.authentication.RequiredActionFactory; import org.keycloak.authentication.RequiredActionProvider; import org.keycloak.authentication.requiredactions.WebAuthnRegisterFactory; import org.keycloak.common.util.Base64Url; import org.keycloak.common.util.UriUtils; +import org.keycloak.credential.CredentialProvider; +import org.keycloak.credential.OTPCredentialProvider; import org.keycloak.credential.WebAuthnCredentialModelInput; +import org.keycloak.credential.WebAuthnCredentialProvider; +import org.keycloak.credential.WebAuthnCredentialProviderFactory; import org.keycloak.events.Errors; import org.keycloak.forms.login.LoginFormsProvider; import org.keycloak.forms.login.freemarker.model.WebAuthnAuthenticatorsBean; @@ -47,7 +52,7 @@ import javax.ws.rs.core.Response; import java.util.Collections; import java.util.List; -public class WebAuthnAuthenticator implements Authenticator { +public class WebAuthnAuthenticator implements Authenticator, CredentialValidator { private static final Logger logger = Logger.getLogger(WebAuthnAuthenticator.class); private KeycloakSession session; @@ -204,6 +209,11 @@ public class WebAuthnAuthenticator implements Authenticator { // NOP } + @Override + public WebAuthnCredentialProvider getCredentialProvider(KeycloakSession session) { + return (WebAuthnCredentialProvider)session.getProvider(CredentialProvider.class, WebAuthnCredentialProviderFactory.PROVIDER_ID); + } + private static final String ERR_LABEL = "web_authn_authentication_error"; private static final String ERR_DETAIL_LABEL = "web_authn_authentication_error_detail"; private static final String ERR_NO_AUTHENTICATORS_REGISTERED = "No WebAuthn Authenticator registered."; diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/directgrant/ValidateOTP.java b/services/src/main/java/org/keycloak/authentication/authenticators/directgrant/ValidateOTP.java index 9777a124e4..3e1d949906 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/directgrant/ValidateOTP.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/directgrant/ValidateOTP.java @@ -59,12 +59,11 @@ public class ValidateOTP extends AbstractDirectGrantAuthenticator implements Cre MultivaluedMap inputData = context.getHttpRequest().getDecodedFormParameters(); String otp = inputData.getFirst("otp"); - String credentialId = context.getSelectedCredentialId(); - if (credentialId == null || credentialId.isEmpty()) { - credentialId = getCredentialProvider(context.getSession()) + + // Always use default OTP credential in case of direct grant authentication + String credentialId = getCredentialProvider(context.getSession()) .getDefaultCredential(context.getSession(), context.getRealm(), context.getUser()).getId(); - context.setSelectedCredentialId(credentialId); - } + if (otp == null) { if (context.getUser() != null) { context.getEvent().user(context.getUser()); diff --git a/services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java b/services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java index b11d54b0cd..8f03d35304 100755 --- a/services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java +++ b/services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java @@ -19,6 +19,7 @@ package org.keycloak.forms.login.freemarker; import org.jboss.logging.Logger; import org.keycloak.OAuth2Constants; import org.keycloak.authentication.AuthenticationFlowContext; +import org.keycloak.authentication.authenticators.browser.OTPFormAuthenticator; import org.keycloak.authentication.requiredactions.util.UpdateProfileContext; import org.keycloak.authentication.requiredactions.util.UserUpdateProfileContext; import org.keycloak.broker.provider.BrokeredIdentityContext; @@ -37,6 +38,7 @@ import org.keycloak.forms.login.freemarker.model.RegisterBean; import org.keycloak.forms.login.freemarker.model.RequiredActionUrlFormatterMethod; import org.keycloak.forms.login.freemarker.model.SAMLPostFormBean; import org.keycloak.forms.login.freemarker.model.TotpBean; +import org.keycloak.forms.login.freemarker.model.TotpLoginBean; import org.keycloak.forms.login.freemarker.model.UrlBean; import org.keycloak.forms.login.freemarker.model.X509ConfirmBean; import org.keycloak.models.ClientModel; @@ -214,6 +216,9 @@ public class FreeMarkerLoginFormsProvider implements LoginFormsProvider { attributes.put("brokerContext", brokerContext); attributes.put("idpAlias", idpAlias); break; + case LOGIN_TOTP: + attributes.put("otpLogin", new TotpLoginBean(session, realm, user, (String) this.attributes.get(OTPFormAuthenticator.SELECTED_OTP_CREDENTIAL_ID))); + break; case REGISTER: attributes.put("register", new RegisterBean(formData)); break; @@ -404,7 +409,7 @@ public class FreeMarkerLoginFormsProvider implements LoginFormsProvider { attributes.put("url", new UrlBean(realm, theme, baseUri, this.actionUri)); attributes.put("requiredActionUrl", new RequiredActionUrlFormatterMethod(realm, baseUri)); - attributes.put("auth", new AuthenticationContextBean(context, actionUri)); + attributes.put("auth", new AuthenticationContextBean(context, actionUri, page)); attributes.put(Constants.EXECUTION, execution); if (realm.isInternationalizationEnabled()) { @@ -547,6 +552,11 @@ public class FreeMarkerLoginFormsProvider implements LoginFormsProvider { return createResponse(LoginFormsPages.OAUTH_GRANT); } + @Override + public Response createSelectAuthenticator() { + return createResponse(LoginFormsPages.LOGIN_SELECT_AUTHENTICATOR); + } + @Override public Response createCode() { return createResponse(LoginFormsPages.CODE); diff --git a/services/src/main/java/org/keycloak/forms/login/freemarker/Templates.java b/services/src/main/java/org/keycloak/forms/login/freemarker/Templates.java index 66433aabc9..43b1b8c32c 100755 --- a/services/src/main/java/org/keycloak/forms/login/freemarker/Templates.java +++ b/services/src/main/java/org/keycloak/forms/login/freemarker/Templates.java @@ -50,6 +50,8 @@ public class Templates { return "login-reset-password.ftl"; case LOGIN_UPDATE_PASSWORD: return "login-update-password.ftl"; + case LOGIN_SELECT_AUTHENTICATOR: + return "select-authenticator.ftl"; case REGISTER: return "register.ftl"; case INFO: diff --git a/services/src/main/java/org/keycloak/forms/login/freemarker/model/AuthenticationContextBean.java b/services/src/main/java/org/keycloak/forms/login/freemarker/model/AuthenticationContextBean.java index c44e2da0f5..336f383cfd 100644 --- a/services/src/main/java/org/keycloak/forms/login/freemarker/model/AuthenticationContextBean.java +++ b/services/src/main/java/org/keycloak/forms/login/freemarker/model/AuthenticationContextBean.java @@ -24,6 +24,7 @@ import java.util.List; import org.keycloak.authentication.AuthenticationFlowContext; import org.keycloak.authentication.AuthenticationSelectionOption; +import org.keycloak.forms.login.LoginFormsPages; import org.keycloak.services.util.AuthenticationFlowHistoryHelper; /** @@ -33,10 +34,12 @@ public class AuthenticationContextBean { private final AuthenticationFlowContext context; private final URI actionUri; + private final LoginFormsPages page; - public AuthenticationContextBean(AuthenticationFlowContext context, URI actionUri) { + public AuthenticationContextBean(AuthenticationFlowContext context, URI actionUri, LoginFormsPages page) { this.context = context; this.actionUri = actionUri; + this.page = page; } @@ -44,10 +47,6 @@ public class AuthenticationContextBean { return context==null ? Collections.emptyList() : context.getAuthenticationSelections(); } - public String getSelectedCredential() { - return context==null ? null : context.getSelectedCredentialId(); - } - public boolean showBackButton() { if (context == null) { return false; @@ -55,4 +54,9 @@ public class AuthenticationContextBean { return actionUri != null && new AuthenticationFlowHistoryHelper(context.getAuthenticationSession(), context.getFlowPath()).hasAnyExecution(); } + + + public boolean showTryAnotherWayLink() { + return getAuthenticationSelections().size() > 1 && page != LoginFormsPages.LOGIN_SELECT_AUTHENTICATOR; + } } diff --git a/services/src/main/java/org/keycloak/forms/login/freemarker/model/TotpBean.java b/services/src/main/java/org/keycloak/forms/login/freemarker/model/TotpBean.java index eab4fc9fc6..534f74f84b 100755 --- a/services/src/main/java/org/keycloak/forms/login/freemarker/model/TotpBean.java +++ b/services/src/main/java/org/keycloak/forms/login/freemarker/model/TotpBean.java @@ -27,6 +27,8 @@ import org.keycloak.utils.TotpUtils; import javax.ws.rs.core.UriBuilder; /** + * Used for UpdateTotp required action + * * @author Stian Thorgersen */ public class TotpBean { diff --git a/services/src/main/java/org/keycloak/forms/login/freemarker/model/TotpLoginBean.java b/services/src/main/java/org/keycloak/forms/login/freemarker/model/TotpLoginBean.java new file mode 100644 index 0000000000..aec44e2243 --- /dev/null +++ b/services/src/main/java/org/keycloak/forms/login/freemarker/model/TotpLoginBean.java @@ -0,0 +1,93 @@ +/* + * Copyright 2019 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.keycloak.forms.login.freemarker.model; + +import java.util.List; +import java.util.stream.Collectors; + +import org.keycloak.authentication.authenticators.browser.OTPFormAuthenticator; +import org.keycloak.credential.CredentialModel; +import org.keycloak.credential.CredentialProvider; +import org.keycloak.credential.OTPCredentialProvider; +import org.keycloak.credential.OTPCredentialProviderFactory; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.RealmModel; +import org.keycloak.models.UserModel; +import org.keycloak.models.credential.OTPCredentialModel; + +/** + * Used for TOTP login + * + * @author Marek Posolda + */ +public class TotpLoginBean { + + private final String selectedCredentialId; + private final List userOtpCredentials; + + public TotpLoginBean(KeycloakSession session, RealmModel realm, UserModel user, String selectedCredentialId) { + List userOtpCredentials = session.userCredentialManager() + .getStoredCredentialsByType(realm, user, OTPCredentialModel.TYPE); + + this.userOtpCredentials = userOtpCredentials.stream() + .map(OTPCredential::new) + .collect(Collectors.toList()); + + // This means user did not yet manually selected any OTP credential through the UI. So just go with the default one with biggest priority + if (selectedCredentialId == null || selectedCredentialId.isEmpty()) { + OTPCredentialProvider otpCredentialProvider = (OTPCredentialProvider)session.getProvider(CredentialProvider.class, OTPCredentialProviderFactory.PROVIDER_ID); + OTPCredentialModel otpCredential = otpCredentialProvider + .getDefaultCredential(session, realm, user); + + selectedCredentialId = otpCredential==null ? null : otpCredential.getId(); + } + + this.selectedCredentialId = selectedCredentialId; + } + + + public List getUserOtpCredentials() { + return userOtpCredentials; + } + + public String getSelectedCredentialId() { + return selectedCredentialId; + } + + + public static class OTPCredential { + + private final String id; + private final String userLabel; + + public OTPCredential(CredentialModel credentialModel) { + this.id = credentialModel.getId(); + // TODO: "Unnamed" OTP credentials should be displayed in the UI in gray + this.userLabel = credentialModel.getUserLabel() == null || credentialModel.getUserLabel().isEmpty() ? OTPFormAuthenticator.UNNAMED : credentialModel.getUserLabel(); + } + + public String getId() { + return id; + } + + public String getUserLabel() { + return userLabel; + } + } +} diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/auth/page/login/OneTimeCode.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/auth/page/login/OneTimeCode.java index c28dd534ad..f84de459d6 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/auth/page/login/OneTimeCode.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/auth/page/login/OneTimeCode.java @@ -32,9 +32,6 @@ public class OneTimeCode extends Authenticate { @FindBy(id = "otp") private WebElement otpInputField; - @FindBy(id = "authenticators-choice") - private WebElement authenticatorSelector; - @FindBy(xpath = ".//label[@for='otp']") private WebElement otpInputLabel; @@ -62,11 +59,6 @@ public class OneTimeCode extends Authenticate { return loginErrorMessage != null ? loginErrorMessage.getText() : null; } - public void selectFactor(String name) { - Select select = new Select(authenticatorSelector); - select.selectByVisibleText(name); - } - @Override public boolean isCurrent() { return URLUtils.currentUrlStartsWith(toString() + "?") && isOtpLabelPresent(); diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/CredentialsComboboxPage.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/CredentialsComboboxPage.java deleted file mode 100644 index e3c4d8fa6b..0000000000 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/CredentialsComboboxPage.java +++ /dev/null @@ -1,62 +0,0 @@ -package org.keycloak.testsuite.pages; - -import java.util.List; -import java.util.stream.Collectors; - -import org.junit.Assert; -import org.openqa.selenium.By; -import org.openqa.selenium.NoSuchElementException; -import org.openqa.selenium.WebElement; -import org.openqa.selenium.support.FindBy; -import org.openqa.selenium.support.ui.Select; - -/** - * Login page with the list of credentials, which are available to the user (Password, OTP, WebAuthn...) - * - * @author Marek Posolda - */ -public abstract class CredentialsComboboxPage extends LanguageComboboxAwarePage { - - @FindBy(id = "authenticators-choice") - private WebElement credentialsCombobox; - - - // If false, we don't expect that credentials combobox is available. If true, we expect that it is available on the page - public void assertCredentialsComboboxAvailability(boolean expectedAvailability) { - try { - driver.findElement(By.id("authenticators-choice")); - Assert.assertTrue(expectedAvailability); - } catch (NoSuchElementException nse) { - Assert.assertFalse(expectedAvailability); - } - } - - - public List getAvailableCredentials() { - return new Select(credentialsCombobox).getOptions() - .stream() - .map(WebElement::getText) - .collect(Collectors.toList()); - } - - - public String getSelectedCredential() { - return new Select(credentialsCombobox).getOptions() - .stream() - .filter(webElement -> webElement.getAttribute("selected") != null) - .findFirst() - .orElseThrow(() -> { - - return new AssertionError("Selected credential not found"); - - }) - .getText(); - } - - - public void selectCredential(String credentialName) { - new Select(credentialsCombobox).selectByVisibleText(credentialName); - } - - -} diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LanguageComboboxAwarePage.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LanguageComboboxAwarePage.java index 1cbff4987e..63c6182f9d 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LanguageComboboxAwarePage.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LanguageComboboxAwarePage.java @@ -26,6 +26,8 @@ import org.openqa.selenium.WebElement; import org.openqa.selenium.support.FindBy; /** + * Provides some generic utils available on most of login pages (Language combobox, Link "Try another way" etc) + * * @author Marek Posolda */ public abstract class LanguageComboboxAwarePage extends AbstractPage { @@ -39,6 +41,9 @@ public abstract class LanguageComboboxAwarePage extends AbstractPage { @FindBy(id = "kc-back") private WebElement backButton; + @FindBy(id = "try-another-way") + private WebElement tryAnotherWayLink; + public String getLanguageDropdownText() { return languageText.getText(); } @@ -65,4 +70,19 @@ public abstract class LanguageComboboxAwarePage extends AbstractPage { public void clickBackButton() { backButton.click(); } + + + // If false, we don't expect form "Try another way" link available on the page. If true, we expect that it is available on the page + public void assertTryAnotherWayLinkAvailability(boolean expectedAvailability) { + try { + driver.findElement(By.id("try-another-way")); + Assert.assertTrue(expectedAvailability); + } catch (NoSuchElementException nse) { + Assert.assertFalse(expectedAvailability); + } + } + + public void clickTryAnotherWayLink() { + tryAnotherWayLink.click(); + } } diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginTotpPage.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginTotpPage.java index 647da8601a..a7db0ed9b4 100755 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginTotpPage.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginTotpPage.java @@ -16,14 +16,20 @@ */ package org.keycloak.testsuite.pages; +import java.util.List; +import java.util.stream.Collectors; + +import org.junit.Assert; import org.openqa.selenium.By; +import org.openqa.selenium.NoSuchElementException; import org.openqa.selenium.WebElement; import org.openqa.selenium.support.FindBy; +import org.openqa.selenium.support.ui.Select; /** * @author Stian Thorgersen */ -public class LoginTotpPage extends CredentialsComboboxPage { +public class LoginTotpPage extends LanguageComboboxAwarePage { @FindBy(id = "otp") private WebElement otpInput; @@ -37,6 +43,9 @@ public class LoginTotpPage extends CredentialsComboboxPage { @FindBy(className = "alert-error") private WebElement loginErrorMessage; + @FindBy(id = "selected-credential-id") + private WebElement selectedCredentialCombobox; + public void login(String totp) { otpInput.clear(); if (totp != null) otpInput.sendKeys(totp); @@ -64,4 +73,42 @@ public class LoginTotpPage extends CredentialsComboboxPage { throw new UnsupportedOperationException(); } + + // If false, we don't expect that credentials combobox is available. If true, we expect that it is available on the page + public void assertOtpCredentialSelectorAvailability(boolean expectedAvailability) { + try { + driver.findElement(By.id("selected-credential-id")); + Assert.assertTrue(expectedAvailability); + } catch (NoSuchElementException nse) { + Assert.assertFalse(expectedAvailability); + } + } + + + public List getAvailableOtpCredentials() { + return new Select(selectedCredentialCombobox).getOptions() + .stream() + .map(WebElement::getText) + .collect(Collectors.toList()); + } + + + public String getSelectedOtpCredential() { + return new Select(selectedCredentialCombobox).getOptions() + .stream() + .filter(webElement -> webElement.getAttribute("selected") != null) + .findFirst() + .orElseThrow(() -> { + + return new AssertionError("Selected OTP credential not found"); + + }) + .getText(); + } + + + public void selectOtpCredential(String credentialName) { + new Select(selectedCredentialCombobox).selectByVisibleText(credentialName); + } + } diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/PasswordPage.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/PasswordPage.java index f733b29c98..e6d6d5bd1d 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/PasswordPage.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/PasswordPage.java @@ -13,7 +13,7 @@ import org.openqa.selenium.support.FindBy; * * @author Marek Posolda */ -public class PasswordPage extends CredentialsComboboxPage { +public class PasswordPage extends LanguageComboboxAwarePage { @ArquillianResource protected OAuthClient oauth; diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/SelectAuthenticatorPage.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/SelectAuthenticatorPage.java new file mode 100644 index 0000000000..656e4af24b --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/SelectAuthenticatorPage.java @@ -0,0 +1,76 @@ +package org.keycloak.testsuite.pages; + +import java.util.List; +import java.util.stream.Collectors; + +import org.junit.Assert; +import org.keycloak.testsuite.util.DroneUtils; +import org.openqa.selenium.By; +import org.openqa.selenium.NoSuchElementException; +import org.openqa.selenium.WebElement; +import org.openqa.selenium.support.FindBy; +import org.openqa.selenium.support.ui.Select; + +/** + * Login page with the list of authentication mechanisms, which are available to the user (Password, OTP, WebAuthn...) + * + * @author Marek Posolda + */ +public class SelectAuthenticatorPage extends LanguageComboboxAwarePage { + + @FindBy(id = "authenticators-choice") + private WebElement authenticatorsSelect; + + + public List getAvailableLoginMethods() { + return new Select(authenticatorsSelect).getOptions() + .stream() + .map(WebElement::getText) + .collect(Collectors.toList()); + } + + + public String getSelectedLoginMethod() { + return new Select(authenticatorsSelect).getOptions() + .stream() + .filter(webElement -> webElement.getAttribute("selected") != null) + .findFirst() + .orElseThrow(() -> { + + return new AssertionError("Selected login method not found"); + + }) + .getText(); + } + + + public void selectLoginMethod(String loginMethod) { + new Select(authenticatorsSelect).selectByVisibleText(loginMethod); + } + + + @Override + public boolean isCurrent() { + // Check the title + if (!DroneUtils.getCurrentDriver().getTitle().startsWith("Log in to ") && !DroneUtils.getCurrentDriver().getTitle().startsWith("Anmeldung bei ")) { + return false; + } + + // Check the authenticators-choice available + try { + driver.findElement(By.id("authenticators-choice")); + } catch (NoSuchElementException nfe) { + return false; + } + + return true; + } + + + @Override + public void open() throws Exception { + throw new UnsupportedOperationException(); + } + + +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcFirstBrokerLoginNewAuthTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcFirstBrokerLoginNewAuthTest.java index a299b025f6..504c98e052 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcFirstBrokerLoginNewAuthTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcFirstBrokerLoginNewAuthTest.java @@ -10,6 +10,7 @@ import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.testsuite.Assert; import org.keycloak.testsuite.admin.ApiUtil; import org.keycloak.testsuite.pages.PasswordPage; +import org.keycloak.testsuite.pages.SelectAuthenticatorPage; import org.keycloak.testsuite.util.UserBuilder; import static org.junit.Assert.assertEquals; @@ -29,6 +30,9 @@ public class KcOidcFirstBrokerLoginNewAuthTest extends AbstractInitializedBaseBr @Page PasswordPage passwordPage; + @Page + protected SelectAuthenticatorPage selectAuthenticatorPage; + @Override protected BrokerConfiguration getBrokerConfiguration() { return KcOidcBrokerConfiguration.INSTANCE; @@ -104,7 +108,7 @@ public class KcOidcFirstBrokerLoginNewAuthTest extends AbstractInitializedBaseBr // Assert that user can't see credentials combobox. Password is the only available credentials. Assert.assertTrue(passwordPage.isCurrent("consumer")); - passwordPage.assertCredentialsComboboxAvailability(false); + passwordPage.assertTryAnotherWayLinkAvailability(false); // Login with password Assert.assertTrue(passwordPage.isCurrent("consumer")); @@ -128,11 +132,19 @@ public class KcOidcFirstBrokerLoginNewAuthTest extends AbstractInitializedBaseBr loginWithBrokerAndConfirmLinkAccount(); - // Assert that user can see credentials combobox. Password and OTP are available credentials. Password should be selected. + // Assert that user can choose between Password and OTP as available credentials. Password should be selected by default. Assert.assertTrue(passwordPage.isCurrent("consumer")); - passwordPage.assertCredentialsComboboxAvailability(true); - Assert.assertNames(passwordPage.getAvailableCredentials(), "Password", "OTP"); - Assert.assertEquals("Password", passwordPage.getSelectedCredential()); + passwordPage.assertTryAnotherWayLinkAvailability(true); + + // Just click "Try another way" to verify that both Password and OTP are available. But go back to Password then + passwordPage.clickTryAnotherWayLink(); + selectAuthenticatorPage.assertCurrent(); + Assert.assertNames(selectAuthenticatorPage.getAvailableLoginMethods(), "Password", "OTP"); + + // TODO: This is limitation of select, that it can't select the already present value. Should be improved when we change to select cart + selectAuthenticatorPage.selectLoginMethod("OTP"); + loginTotpPage.clickTryAnotherWayLink(); + selectAuthenticatorPage.selectLoginMethod("Password"); // Login with password Assert.assertTrue(passwordPage.isCurrent("consumer")); @@ -141,6 +153,7 @@ public class KcOidcFirstBrokerLoginNewAuthTest extends AbstractInitializedBaseBr assertUserAuthenticatedInConsumer(consumerRealmUserId); } + /** * Tests the firstBrokerLogin flow configured to re-authenticate with PasswordForm OR TOTP. * TOTP is configured for the user and he selects it to authenticate. Password is not used. @@ -157,12 +170,14 @@ public class KcOidcFirstBrokerLoginNewAuthTest extends AbstractInitializedBaseBr // Assert that user can see credentials combobox. Password and OTP are available credentials. Password should be selected. Assert.assertTrue(passwordPage.isCurrent("consumer")); - passwordPage.assertCredentialsComboboxAvailability(true); + passwordPage.assertTryAnotherWayLinkAvailability(true); + + // Click "Try another way", Select OTP and assert OTP form present + passwordPage.clickTryAnotherWayLink(); + selectAuthenticatorPage.assertCurrent(); + selectAuthenticatorPage.selectLoginMethod("OTP"); - // Select OTP and assert - passwordPage.selectCredential("OTP"); loginTotpPage.assertCurrent(); - Assert.assertEquals("OTP", loginTotpPage.getSelectedCredential()); // Login with OTP now loginTotpPage.login(totp.generateTOTP(totpSecret)); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BrowserFlowTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BrowserFlowTest.java index a525c6bd00..66522c37b0 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BrowserFlowTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BrowserFlowTest.java @@ -6,7 +6,9 @@ import org.jboss.arquillian.test.api.ArquillianResource; import org.junit.Assert; import org.junit.Rule; import org.junit.Test; +import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.authentication.AuthenticationFlow; +import org.keycloak.authentication.authenticators.browser.OTPFormAuthenticator; import org.keycloak.authentication.authenticators.browser.OTPFormAuthenticatorFactory; import org.keycloak.authentication.authenticators.browser.PasswordFormFactory; import org.keycloak.authentication.authenticators.browser.UsernameFormFactory; @@ -45,6 +47,7 @@ import org.keycloak.testsuite.util.OAuthClient; import org.keycloak.testsuite.authentication.ConditionalUserAttributeValueFactory; import org.keycloak.testsuite.authentication.SetUserAttributeAuthenticatorFactory; import org.keycloak.testsuite.util.URLUtils; +import org.keycloak.testsuite.util.WaitUtils; import org.openqa.selenium.By; import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebElement; @@ -137,7 +140,7 @@ public class BrowserFlowTest extends AbstractTestRealmKeycloakTest { Assert.assertFalse(loginPage.isCurrent()); Assert.assertFalse(oneTimeCodePage.isOtpLabelPresent()); Assert.assertFalse(loginTotpPage.isCurrent()); - loginTotpPage.assertCredentialsComboboxAvailability(false); + loginTotpPage.assertOtpCredentialSelectorAvailability(false); } @Test @@ -145,9 +148,10 @@ public class BrowserFlowTest extends AbstractTestRealmKeycloakTest { provideUsernamePassword("user-with-one-configured-otp"); Assert.assertTrue(oneTimeCodePage.isOtpLabelPresent()); loginTotpPage.assertCurrent(); - loginTotpPage.assertCredentialsComboboxAvailability(false); + loginTotpPage.assertOtpCredentialSelectorAvailability(false); - oneTimeCodePage.sendCode("123456"); + // Use 7 digits instead 6 to have 100% probability of failure + oneTimeCodePage.sendCode("1234567"); Assert.assertEquals(INVALID_AUTH_CODE, oneTimeCodePage.getError()); Assert.assertTrue(oneTimeCodePage.isOtpLabelPresent()); } @@ -157,7 +161,7 @@ public class BrowserFlowTest extends AbstractTestRealmKeycloakTest { provideUsernamePassword("user-with-one-configured-otp"); Assert.assertTrue(oneTimeCodePage.isOtpLabelPresent()); loginTotpPage.assertCurrent(); - loginTotpPage.assertCredentialsComboboxAvailability(false); + loginTotpPage.assertOtpCredentialSelectorAvailability(false); oneTimeCodePage.sendCode(getOtpCode("DJmQfC73VGFhw7D4QJ8A")); Assert.assertFalse(loginPage.isCurrent()); @@ -194,24 +198,24 @@ public class BrowserFlowTest extends AbstractTestRealmKeycloakTest { provideUsernamePassword("user-with-two-configured-otp"); Assert.assertTrue(oneTimeCodePage.isOtpLabelPresent()); loginTotpPage.assertCurrent(); - loginTotpPage.assertCredentialsComboboxAvailability(true); + loginTotpPage.assertOtpCredentialSelectorAvailability(true); // Check that selected credential is "first" - Assert.assertEquals("first", loginTotpPage.getSelectedCredential()); + Assert.assertEquals("first", loginTotpPage.getSelectedOtpCredential()); - // Select "second" factor but try to connect with the OTP code from the "first" one - oneTimeCodePage.selectFactor("second"); - oneTimeCodePage.sendCode(getOtpCode(firstKey)); + // Select "second" factor (which is unnamed as it doesn't have userLabel) but try to connect with the OTP code from the "first" one + loginTotpPage.selectOtpCredential(OTPFormAuthenticator.UNNAMED); + loginTotpPage.login(getOtpCode(firstKey)); Assert.assertEquals(INVALID_AUTH_CODE, oneTimeCodePage.getError()); // Select "first" factor but try to connect with the OTP code from the "second" one - oneTimeCodePage.selectFactor("first"); - oneTimeCodePage.sendCode(getOtpCode(secondKey)); + loginTotpPage.selectOtpCredential("first"); + loginTotpPage.login(getOtpCode(secondKey)); Assert.assertEquals(INVALID_AUTH_CODE, oneTimeCodePage.getError()); // Select "second" factor and try to connect with its OTP code - oneTimeCodePage.selectFactor("second"); - oneTimeCodePage.sendCode(getOtpCode(secondKey)); + loginTotpPage.selectOtpCredential(OTPFormAuthenticator.UNNAMED); + loginTotpPage.login(getOtpCode(secondKey)); Assert.assertFalse(oneTimeCodePage.isOtpLabelPresent()); } @@ -220,12 +224,12 @@ public class BrowserFlowTest extends AbstractTestRealmKeycloakTest { provideUsernamePassword(username); Assert.assertTrue(oneTimeCodePage.isOtpLabelPresent()); loginTotpPage.assertCurrent(); - loginTotpPage.assertCredentialsComboboxAvailability(true); + loginTotpPage.assertOtpCredentialSelectorAvailability(true); // Check that preferred credential is selected - Assert.assertEquals(orderedCredentials.get(0), loginTotpPage.getSelectedCredential()); + Assert.assertEquals(orderedCredentials.get(0), loginTotpPage.getSelectedOtpCredential()); // Check credentials order - List creds = loginTotpPage.getAvailableCredentials(); + List creds = loginTotpPage.getAvailableOtpCredentials(); Assert.assertEquals(2, creds.size()); Assert.assertEquals(orderedCredentials, creds); } @@ -236,7 +240,7 @@ public class BrowserFlowTest extends AbstractTestRealmKeycloakTest { int idxFirst = 0; // Credentials order is: first, password, second // Priority tells: first then second - testCredentialsOrder(username, Arrays.asList("first", "second")); + testCredentialsOrder(username, Arrays.asList("first", OTPFormAuthenticator.UNNAMED)); try { // Move first credential in last position @@ -247,66 +251,13 @@ public class BrowserFlowTest extends AbstractTestRealmKeycloakTest { }); // Priority tells: second then first - testCredentialsOrder(username, Arrays.asList("second", "first")); + testCredentialsOrder(username, Arrays.asList(OTPFormAuthenticator.UNNAMED, "first")); } finally { // Restore default testrealm.json importTestRealm(null); } } - // In a sub-flow with alternative credential executors, check which credentials are available and in which order - @Test - @AuthServerContainerExclude(REMOTE) - public void testAlternativeCredentials() { - try { - configureBrowserFlowWithAlternativeCredentials(); - - // test-user has not other credential than his password. No combobox is displayed - loginUsernameOnlyPage.open(); - loginUsernameOnlyPage.login("test-user@localhost"); - loginTotpPage.assertCredentialsComboboxAvailability(false); - - // A user with only one other credential than his password: the combobox should - // let him choose between his password and his OTP credentials - loginUsernameOnlyPage.open(); - loginUsernameOnlyPage.login("user-with-one-configured-otp"); - loginTotpPage.assertCredentialsComboboxAvailability(true); - Assert.assertEquals(Arrays.asList("Password", "OTP"), loginTotpPage.getAvailableCredentials()); - - // A user with two other credentials than his password: the combobox should - // let him choose between his 3 credentials in the order of his preferences - loginUsernameOnlyPage.open(); - loginUsernameOnlyPage.login("user-with-two-configured-otp"); - loginTotpPage.assertCredentialsComboboxAvailability(true); - Assert.assertEquals("OTP - first", loginTotpPage.getSelectedCredential()); - Assert.assertEquals(Arrays.asList("OTP - first", "Password", "OTP - second"), loginTotpPage.getAvailableCredentials()); - } finally { - revertFlows("browser - alternative"); - } - } - - private void configureBrowserFlowWithAlternativeCredentials() { - configureBrowserFlowWithAlternativeCredentials(testingClient); - } - - static void configureBrowserFlowWithAlternativeCredentials(KeycloakTestingClient testingClient) { - final String newFlowAlias = "browser - alternative"; - testingClient.server("test").run(session -> FlowUtil.inCurrentRealm(session).copyBrowserFlow(newFlowAlias)); - testingClient.server("test").run(session -> FlowUtil.inCurrentRealm(session) - .selectFlow(newFlowAlias) - .inForms(forms -> forms - .clear() - .addAuthenticatorExecution(AuthenticationExecutionModel.Requirement.REQUIRED, UsernameFormFactory.PROVIDER_ID) - .addSubFlowExecution(Requirement.CONDITIONAL, altSubFlow -> altSubFlow - // Add authenticators to this flow: 1 conditional authenticator and 2 basic authenticator executions - .addAuthenticatorExecution(Requirement.REQUIRED, ConditionalUserConfiguredAuthenticatorFactory.PROVIDER_ID) - .addAuthenticatorExecution(Requirement.ALTERNATIVE, PasswordFormFactory.PROVIDER_ID) - .addAuthenticatorExecution(Requirement.ALTERNATIVE, OTPFormAuthenticatorFactory.PROVIDER_ID) - ) - ) - .defineAsBrowserFlow() - ); - } // In a form waiting for a username only, provides a username and check if password is requested in the following execution of the flow private boolean needsPassword(String username) { @@ -442,7 +393,7 @@ public class BrowserFlowTest extends AbstractTestRealmKeycloakTest { provideUsernamePassword("user-with-two-configured-otp"); Assert.assertTrue(oneTimeCodePage.isOtpLabelPresent()); loginTotpPage.assertCurrent(); - loginTotpPage.assertCredentialsComboboxAvailability(true); + loginTotpPage.assertOtpCredentialSelectorAvailability(true); // user-with-one-configured-otp has not configured role. He should not be asked for an OTP code provideUsernamePassword("user-with-one-configured-otp"); @@ -541,49 +492,6 @@ public class BrowserFlowTest extends AbstractTestRealmKeycloakTest { } } - @Test - @AuthServerContainerExclude(REMOTE) - public void testBackButtonFromAlternativeSubflow() { - final String newFlowAlias = "browser - back button subflow"; - testingClient.server("test").run(session -> FlowUtil.inCurrentRealm(session).copyBrowserFlow(newFlowAlias)); - testingClient.server("test").run(session -> FlowUtil.inCurrentRealm(session) - .selectFlow(newFlowAlias) - .inForms(forms -> forms - .clear() - .addAuthenticatorExecution(AuthenticationExecutionModel.Requirement.REQUIRED, UsernameFormFactory.PROVIDER_ID) - .addSubFlowExecution(Requirement.REQUIRED, reqSubFlow -> reqSubFlow - // Add authenticators to this flow: 1 PASSWORD, 2 Another subflow with having only OTP as child - .addAuthenticatorExecution(Requirement.ALTERNATIVE, PasswordFormFactory.PROVIDER_ID) - .addSubFlowExecution("otp subflow", AuthenticationFlow.BASIC_FLOW, Requirement.ALTERNATIVE, altSubFlow -> altSubFlow - .addAuthenticatorExecution(Requirement.REQUIRED, OTPFormAuthenticatorFactory.PROVIDER_ID) - ) - ) - ) - .defineAsBrowserFlow() - ); - - try { - // Provide username, should be on password page - needsPassword("user-with-one-configured-otp"); - - // Select the OTP subflow. The credential selection won't be on the page due it's subflow - passwordPage.selectCredential("otp subflow"); - loginTotpPage.assertCurrent(); - loginTotpPage.assertCredentialsComboboxAvailability(false); - - // Click "back". Should be on password page - loginTotpPage.clickBackButton(); - passwordPage.assertCurrent(); - passwordPage.login("password"); - - Assert.assertFalse(passwordPage.isCurrent()); - Assert.assertFalse(loginPage.isCurrent()); - events.expectLogin().user(testRealm().users().search("user-with-one-configured-otp").get(0).getId()) - .detail(Details.USERNAME, "user-with-one-configured-otp").assertEvent(); - } finally { - revertFlows("browser - back button subflow"); - } - } // Configure a flow with a conditional sub flow with a condition where a specific role is required private void configureBrowserFlowOTPNeedsRole(String requiredRole) { @@ -731,9 +639,9 @@ public class BrowserFlowTest extends AbstractTestRealmKeycloakTest { // Assert on password page now Assert.assertTrue(oneTimeCodePage.isOtpLabelPresent()); loginTotpPage.assertCurrent(); - loginTotpPage.assertCredentialsComboboxAvailability(false); + loginTotpPage.assertOtpCredentialSelectorAvailability(false); - oneTimeCodePage.sendCode(getOtpCode("DJmQfC73VGFhw7D4QJ8A")); + loginTotpPage.login(getOtpCode("DJmQfC73VGFhw7D4QJ8A")); Assert.assertFalse(loginTotpPage.isCurrent()); events.expectLogin().user(testRealm().users().search("user-with-one-configured-otp").get(0).getId()) .detail(Details.USERNAME, "user-with-one-configured-otp").assertEvent(); @@ -797,9 +705,9 @@ public class BrowserFlowTest extends AbstractTestRealmKeycloakTest { // Assert on otp page now Assert.assertTrue(oneTimeCodePage.isOtpLabelPresent()); loginTotpPage.assertCurrent(); - loginTotpPage.assertCredentialsComboboxAvailability(true); + loginTotpPage.assertOtpCredentialSelectorAvailability(true); - oneTimeCodePage.sendCode(getOtpCode("DJmQfC73VGFhw7D4QJ8A")); + loginTotpPage.login(getOtpCode("DJmQfC73VGFhw7D4QJ8A")); Assert.assertFalse(loginTotpPage.isCurrent()); events.expectLogin().user(userId).detail(Details.USERNAME, "user-with-two-configured-otp").assertEvent(); } finally { @@ -1125,7 +1033,7 @@ public class BrowserFlowTest extends AbstractTestRealmKeycloakTest { UserRepresentation user = testRealm().users().search("test-user@localhost").get(0); Assert.assertNotNull(user); - configureBrowserFlowWithAlternativeCredentials(); + MultiFactorAuthenticationTest.configureBrowserFlowWithAlternativeCredentials(testingClient); try { RealmRepresentation realm = testRealm().toRepresentation(); realm.setLoginWithEmailAllowed(false); @@ -1249,12 +1157,16 @@ public class BrowserFlowTest extends AbstractTestRealmKeycloakTest { private void revertFlows(String flowToDeleteAlias) { - List flows = testRealm().flows().getFlows(); + revertFlows(testRealm(), flowToDeleteAlias); + } + + static void revertFlows(RealmResource realmResource, String flowToDeleteAlias) { + List flows = realmResource.flows().getFlows(); // Set default browser flow - RealmRepresentation realm = testRealm().toRepresentation(); + RealmRepresentation realm = realmResource.toRepresentation(); realm.setBrowserFlow(DefaultAuthenticationFlows.BROWSER_FLOW); - testRealm().update(realm); + realmResource.update(realm); AuthenticationFlowRepresentation flowRepresentation = AbstractAuthenticationTest.findFlowByAlias(flowToDeleteAlias, flows); @@ -1264,6 +1176,7 @@ public class BrowserFlowTest extends AbstractTestRealmKeycloakTest { throw new IllegalArgumentException("The flow with alias " + flowToDeleteAlias + " did not exists"); } - testRealm().flows().deleteFlow(flowRepresentation.getId()); + realmResource.flows().deleteFlow(flowRepresentation.getId()); } + } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/MultiFactorAuthenticationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/MultiFactorAuthenticationTest.java new file mode 100644 index 0000000000..d27fd7cb87 --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/MultiFactorAuthenticationTest.java @@ -0,0 +1,291 @@ +/* + * Copyright 2019 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.keycloak.testsuite.forms; + +import java.util.Arrays; +import java.util.List; +import java.util.function.Consumer; + +import org.jboss.arquillian.drone.api.annotation.Drone; +import org.jboss.arquillian.graphene.page.Page; +import org.jboss.arquillian.test.api.ArquillianResource; +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; +import org.keycloak.authentication.AuthenticationFlow; +import org.keycloak.authentication.authenticators.browser.OTPFormAuthenticatorFactory; +import org.keycloak.authentication.authenticators.browser.PasswordFormFactory; +import org.keycloak.authentication.authenticators.browser.UsernameFormFactory; +import org.keycloak.authentication.authenticators.browser.WebAuthnAuthenticatorFactory; +import org.keycloak.events.Details; +import org.keycloak.models.AuthenticationExecutionModel; +import org.keycloak.models.utils.TimeBasedOTP; +import org.keycloak.representations.idm.RealmRepresentation; +import org.keycloak.testsuite.AbstractTestRealmKeycloakTest; +import org.keycloak.testsuite.AssertEvents; +import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude; +import org.keycloak.testsuite.client.KeycloakTestingClient; +import org.keycloak.testsuite.pages.ErrorPage; +import org.keycloak.testsuite.pages.LoginPage; +import org.keycloak.testsuite.pages.LoginTotpPage; +import org.keycloak.testsuite.pages.LoginUsernameOnlyPage; +import org.keycloak.testsuite.pages.PasswordPage; +import org.keycloak.testsuite.pages.SelectAuthenticatorPage; +import org.keycloak.testsuite.util.FlowUtil; +import org.keycloak.testsuite.util.OAuthClient; +import org.keycloak.testsuite.util.WaitUtils; +import org.openqa.selenium.WebDriver; + +import static org.keycloak.testsuite.admin.AbstractAdminTest.loadJson; +import static org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.AuthServer.REMOTE; + +/** + * Test various scenarios for multi-factor login. Test that "Try another way" link works as expected + * and users are able to choose between various alternative authenticators for the particular factor (1st factor, 2nd factor) + * + * @author Marek Posolda + */ +@AuthServerContainerExclude(REMOTE) +public class MultiFactorAuthenticationTest extends AbstractTestRealmKeycloakTest { + + @ArquillianResource + protected OAuthClient oauth; + + @Drone + protected WebDriver driver; + + @Page + protected LoginPage loginPage; + + @Page + protected LoginUsernameOnlyPage loginUsernameOnlyPage; + + @Page + protected PasswordPage passwordPage; + + @Page + protected ErrorPage errorPage; + + @Page + protected LoginTotpPage loginTotpPage; + + @Page + protected SelectAuthenticatorPage selectAuthenticatorPage; + + @Rule + public AssertEvents events = new AssertEvents(this); + + @Override + public void configureTestRealm(RealmRepresentation testRealm) { + } + + private RealmRepresentation loadTestRealm() { + RealmRepresentation res = loadJson(getClass().getResourceAsStream("/testrealm.json"), RealmRepresentation.class); + res.setBrowserFlow("browser"); + return res; + } + + private void importTestRealm(Consumer realmUpdater) { + RealmRepresentation realm = loadTestRealm(); + if (realmUpdater != null) { + realmUpdater.accept(realm); + } + importRealm(realm); + } + + @Override + public void addTestRealms(List testRealms) { + log.debug("Adding test realm for import from testrealm.json"); + testRealms.add(loadTestRealm()); + } + + + // In a sub-flow with alternative credential executors, check which credentials are available and in which order + // This also tests "try another way" link + @Test + public void testAlternativeCredentials() { + try { + configureBrowserFlowWithAlternativeCredentials(); + + // test-user has not other credential than his password. No try-another-way link is displayed + loginUsernameOnlyPage.open(); + loginUsernameOnlyPage.login("test-user@localhost"); + passwordPage.assertCurrent(); + loginTotpPage.assertTryAnotherWayLinkAvailability(false); + + // A user with only one other credential than his password: the try-another-way link should be accessible + // and he should be able to choose between his password and his OTP credentials + loginUsernameOnlyPage.open(); + loginUsernameOnlyPage.login("user-with-one-configured-otp"); + passwordPage.assertCurrent(); + passwordPage.assertTryAnotherWayLinkAvailability(true); + passwordPage.clickTryAnotherWayLink(); + + selectAuthenticatorPage.assertCurrent(); + Assert.assertEquals(Arrays.asList("Password", "OTP"), selectAuthenticatorPage.getAvailableLoginMethods()); + + // Select OTP and see that just single OTP is available for this user + selectAuthenticatorPage.selectLoginMethod("OTP"); + loginTotpPage.assertCurrent(); + loginTotpPage.assertTryAnotherWayLinkAvailability(true); + loginTotpPage.assertOtpCredentialSelectorAvailability(false); + + // A user with two OTP credentials and password credential: He should be able to choose just between the password and OTP similarly + // like user with user-with-one-configured-otp. However OTP is preferred credential for him, so OTP mechanism will take preference + loginUsernameOnlyPage.open(); + loginUsernameOnlyPage.login("user-with-two-configured-otp"); + loginTotpPage.assertCurrent(); + loginTotpPage.assertTryAnotherWayLinkAvailability(true); + + // More OTP credentials should be available for this user + loginTotpPage.assertOtpCredentialSelectorAvailability(true); + + loginTotpPage.clickTryAnotherWayLink(); + + selectAuthenticatorPage.assertCurrent(); + Assert.assertEquals(Arrays.asList("OTP", "Password"), selectAuthenticatorPage.getAvailableLoginMethods()); + } finally { + BrowserFlowTest.revertFlows(testRealm(), "browser - alternative"); + } + } + + private void configureBrowserFlowWithAlternativeCredentials() { + configureBrowserFlowWithAlternativeCredentials(testingClient); + } + + static void configureBrowserFlowWithAlternativeCredentials(KeycloakTestingClient testingClient) { + final String newFlowAlias = "browser - alternative"; + testingClient.server("test").run(session -> FlowUtil.inCurrentRealm(session).copyBrowserFlow(newFlowAlias)); + testingClient.server("test").run(session -> FlowUtil.inCurrentRealm(session) + .selectFlow(newFlowAlias) + .inForms(forms -> forms + .clear() + .addAuthenticatorExecution(AuthenticationExecutionModel.Requirement.REQUIRED, UsernameFormFactory.PROVIDER_ID) + .addSubFlowExecution(AuthenticationExecutionModel.Requirement.REQUIRED, altSubFlow -> altSubFlow + // Add 2 basic authenticator executions + .addAuthenticatorExecution(AuthenticationExecutionModel.Requirement.ALTERNATIVE, PasswordFormFactory.PROVIDER_ID) + .addAuthenticatorExecution(AuthenticationExecutionModel.Requirement.ALTERNATIVE, OTPFormAuthenticatorFactory.PROVIDER_ID) + ) + ) + .defineAsBrowserFlow() + ); + } + + + @Test + public void testAlternativeMechanismsInDifferentSubflows() { + final String newFlowAlias = "browser - alternative mechanisms"; + testingClient.server("test").run(session -> FlowUtil.inCurrentRealm(session).copyBrowserFlow(newFlowAlias)); + testingClient.server("test").run(session -> FlowUtil.inCurrentRealm(session) + .selectFlow(newFlowAlias) + .inForms(forms -> forms + .clear() + .addAuthenticatorExecution(AuthenticationExecutionModel.Requirement.REQUIRED, UsernameFormFactory.PROVIDER_ID) + .addSubFlowExecution(AuthenticationExecutionModel.Requirement.REQUIRED, reqSubFlow -> reqSubFlow + // Add authenticators to this flow: 1 PASSWORD, 2 Another subflow with having only OTP as child + .addAuthenticatorExecution(AuthenticationExecutionModel.Requirement.ALTERNATIVE, PasswordFormFactory.PROVIDER_ID) + .addSubFlowExecution("otp subflow", AuthenticationFlow.BASIC_FLOW, AuthenticationExecutionModel.Requirement.ALTERNATIVE, altSubFlow -> altSubFlow + .addAuthenticatorExecution(AuthenticationExecutionModel.Requirement.REQUIRED, OTPFormAuthenticatorFactory.PROVIDER_ID) + ) + ) + ) + .defineAsBrowserFlow() + ); + + try { + // Provide username, should be on password page with the link "Try another way" available + loginUsernameOnlyPage.open(); + loginUsernameOnlyPage.login("user-with-one-configured-otp"); + passwordPage.assertCurrent(); + passwordPage.assertTryAnotherWayLinkAvailability(true); + + // Click "Try another way" . Ability to have both password and OTP should be possible even if OTP is in different subflow + passwordPage.clickTryAnotherWayLink(); + selectAuthenticatorPage.assertCurrent(); + Assert.assertEquals(Arrays.asList("Password", "OTP"), selectAuthenticatorPage.getAvailableLoginMethods()); + selectAuthenticatorPage.selectLoginMethod("OTP"); + + // Should be on the OTP now. Click "Try another way" again. Should see again both Password and OTP + loginTotpPage.assertCurrent(); + loginTotpPage.assertTryAnotherWayLinkAvailability(true); + + loginTotpPage.clickTryAnotherWayLink(); + selectAuthenticatorPage.assertCurrent(); + Assert.assertEquals(Arrays.asList("Password", "OTP"), selectAuthenticatorPage.getAvailableLoginMethods()); + + selectAuthenticatorPage.selectLoginMethod("Password"); + passwordPage.assertCurrent(); + passwordPage.login("password"); + + Assert.assertFalse(passwordPage.isCurrent()); + Assert.assertFalse(loginPage.isCurrent()); + events.expectLogin().user(testRealm().users().search("user-with-one-configured-otp").get(0).getId()) + .detail(Details.USERNAME, "user-with-one-configured-otp").assertEvent(); + } finally { + BrowserFlowTest.revertFlows(testRealm(),"browser - alternative mechanisms"); + } + } + + + // Test for the case when user can authenticate either with: WebAuthn OR (Password AND OTP) + // WebAuthn is not enabled for the user, so he needs to use password AND OTP + @Test + public void testAlternativeMechanismsInDifferentSubflows_firstMechanismUnavailable() { + final String newFlowAlias = "browser - alternative mechanisms"; + testingClient.server("test").run(session -> FlowUtil.inCurrentRealm(session).copyBrowserFlow(newFlowAlias)); + testingClient.server("test").run(session -> FlowUtil.inCurrentRealm(session) + .selectFlow(newFlowAlias) + .inForms(forms -> forms + .clear() + .addAuthenticatorExecution(AuthenticationExecutionModel.Requirement.REQUIRED, UsernameFormFactory.PROVIDER_ID) + .addSubFlowExecution(AuthenticationExecutionModel.Requirement.REQUIRED, reqSubFlow -> reqSubFlow + // Add authenticators to this flow: 1 WebAuthn, 2 Another subflow with having Password AND OTP as children + .addAuthenticatorExecution(AuthenticationExecutionModel.Requirement.ALTERNATIVE, WebAuthnAuthenticatorFactory.PROVIDER_ID) + .addSubFlowExecution("password and otp subflow", AuthenticationFlow.BASIC_FLOW, AuthenticationExecutionModel.Requirement.ALTERNATIVE, altSubFlow -> altSubFlow + .addAuthenticatorExecution(AuthenticationExecutionModel.Requirement.REQUIRED, PasswordFormFactory.PROVIDER_ID) + .addAuthenticatorExecution(AuthenticationExecutionModel.Requirement.REQUIRED, OTPFormAuthenticatorFactory.PROVIDER_ID) + ) + ) + ) + .defineAsBrowserFlow() + ); + + try { + // Provide username, should be on password page without the link "Try another way" available + loginUsernameOnlyPage.open(); + loginUsernameOnlyPage.login("user-with-one-configured-otp"); + passwordPage.assertCurrent(); + passwordPage.assertTryAnotherWayLinkAvailability(false); + + // Login with password. Should be on the OTP page without try-another-way link available + passwordPage.login("password"); + loginTotpPage.assertCurrent(); + loginTotpPage.assertTryAnotherWayLinkAvailability(false); + + // Successfully login with OTP + loginTotpPage.login(new TimeBasedOTP().generateTOTP("DJmQfC73VGFhw7D4QJ8A")); + Assert.assertFalse(loginTotpPage.isCurrent()); + events.expectLogin().user(testRealm().users().search("user-with-one-configured-otp").get(0).getId()) + .detail(Details.USERNAME, "user-with-one-configured-otp").assertEvent(); + } finally { + BrowserFlowTest.revertFlows(testRealm(),"browser - alternative mechanisms"); + } + } + +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ResetCredentialsAlternativeFlowsTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ResetCredentialsAlternativeFlowsTest.java index f647b633d7..6597ad2366 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ResetCredentialsAlternativeFlowsTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ResetCredentialsAlternativeFlowsTest.java @@ -127,7 +127,7 @@ public class ResetCredentialsAlternativeFlowsTest extends AbstractTestRealmKeycl @Test public void testBackButtonWhenSwitchToResetCredentialsFlowFromAlternativeBrowserFlow() { try { - BrowserFlowTest.configureBrowserFlowWithAlternativeCredentials(testingClient); + MultiFactorAuthenticationTest.configureBrowserFlowWithAlternativeCredentials(testingClient); // Provide username and then click "Forget password" provideUsernameAndClickResetPassword("login-test"); @@ -154,7 +154,7 @@ public class ResetCredentialsAlternativeFlowsTest extends AbstractTestRealmKeycl @Test public void testNotExistingUserProvidedInResetCredentialsFlow() { try { - BrowserFlowTest.configureBrowserFlowWithAlternativeCredentials(testingClient); + MultiFactorAuthenticationTest.configureBrowserFlowWithAlternativeCredentials(testingClient); // Provide username and then click "Forget password" provideUsernameAndClickResetPassword("login-test"); @@ -181,7 +181,7 @@ public class ResetCredentialsAlternativeFlowsTest extends AbstractTestRealmKeycl @Test public void testDifferentUserProvidedInResetCredentialsFlow() { try { - BrowserFlowTest.configureBrowserFlowWithAlternativeCredentials(testingClient); + MultiFactorAuthenticationTest.configureBrowserFlowWithAlternativeCredentials(testingClient); // Provide username and then click "Forget password" provideUsernameAndClickResetPassword("login-test"); @@ -207,7 +207,7 @@ public class ResetCredentialsAlternativeFlowsTest extends AbstractTestRealmKeycl @Test public void testSameUserProvidedInResetCredentialsFlow() { try { - BrowserFlowTest.configureBrowserFlowWithAlternativeCredentials(testingClient); + MultiFactorAuthenticationTest.configureBrowserFlowWithAlternativeCredentials(testingClient); // Provide username and then click "Forget password" provideUsernameAndClickResetPassword("login-test"); @@ -234,7 +234,7 @@ public class ResetCredentialsAlternativeFlowsTest extends AbstractTestRealmKeycl @Test public void testResetCredentialsFlowWithUsernameProvidedFromBrowserFlow() throws Exception { try { - BrowserFlowTest.configureBrowserFlowWithAlternativeCredentials(testingClient); + MultiFactorAuthenticationTest.configureBrowserFlowWithAlternativeCredentials(testingClient); final String newFlowAlias = "resetcred - alternative"; // Configure reset-credentials flow without ResetCredentialsChooseUser authenticator configureResetCredentialsRemoveExecutionsAndBindTheFlow( diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/testrealm.json b/testsuite/integration-arquillian/tests/base/src/test/resources/testrealm.json index a996dadc3c..d7f39158fd 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/resources/testrealm.json +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/testrealm.json @@ -146,6 +146,7 @@ "credentials" : [ { "id" : "first", + "userLabel" : "first", "type" : "otp", "secretData" : "{\"value\":\"DJmQfC73VGFhw7D4QJ8A\"}", "credentialData" : "{\"digits\":6,\"counter\":0,\"period\":30,\"algorithm\":\"HmacSHA1\",\"subType\":\"totp\"}" diff --git a/themes/src/main/resources/theme/base/login/login-otp.ftl b/themes/src/main/resources/theme/base/login/login-otp.ftl index 02d52c67a9..93f120b75d 100755 --- a/themes/src/main/resources/theme/base/login/login-otp.ftl +++ b/themes/src/main/resources/theme/base/login/login-otp.ftl @@ -1,9 +1,25 @@ -<#import "select.ftl" as layout> +<#import "template.ftl" as layout> <@layout.registrationLayout; section> <#if section = "header"> ${msg("doLogIn")} <#elseif section = "form">

+ + <#if otpLogin.userOtpCredentials?size gt 1> +
+
+ +
+
+ +
+
+ +
@@ -22,7 +38,6 @@
- value="${auth.selectedCredential}"/>
diff --git a/themes/src/main/resources/theme/base/login/login-password.ftl b/themes/src/main/resources/theme/base/login/login-password.ftl index 68b286a676..1d2bbf9801 100755 --- a/themes/src/main/resources/theme/base/login/login-password.ftl +++ b/themes/src/main/resources/theme/base/login/login-password.ftl @@ -1,4 +1,4 @@ -<#import "select.ftl" as layout> +<#import "template.ftl" as layout> <@layout.registrationLayout displayInfo=social.displayInfo displayWide=(realm.password && social.providers??); section> <#if section = "header"> ${msg("doLogIn")} @@ -23,7 +23,6 @@
- value="${auth.selectedCredential}"/>
diff --git a/themes/src/main/resources/theme/base/login/messages/messages_en.properties b/themes/src/main/resources/theme/base/login/messages/messages_en.properties index 0e1fcee83f..b2022c55f6 100755 --- a/themes/src/main/resources/theme/base/login/messages/messages_en.properties +++ b/themes/src/main/resources/theme/base/login/messages/messages_en.properties @@ -12,6 +12,7 @@ doDecline=Decline doForgotPassword=Forgot Password? doClickHere=Click here doImpersonate=Impersonate +doTryAnotherWay=Try Another Way kerberosNotConfigured=Kerberos Not Configured kerberosNotConfiguredTitle=Kerberos Not Configured bypassKerberosDetail=Either you are not logged in by Kerberos or your browser is not set up for Kerberos login. Please click continue to login in through other means diff --git a/themes/src/main/resources/theme/base/login/select-authenticator.ftl b/themes/src/main/resources/theme/base/login/select-authenticator.ftl new file mode 100644 index 0000000000..d3161dcaa0 --- /dev/null +++ b/themes/src/main/resources/theme/base/login/select-authenticator.ftl @@ -0,0 +1,35 @@ +<#import "template.ftl" as layout> +<@layout.registrationLayout displayInfo=true; section> + <#if section = "header"> + + <#elseif section = "form"> +
+
+
+ +
+
+ + +
+
+
+ + + diff --git a/themes/src/main/resources/theme/base/login/select.ftl b/themes/src/main/resources/theme/base/login/select.ftl deleted file mode 100644 index 07dcd01199..0000000000 --- a/themes/src/main/resources/theme/base/login/select.ftl +++ /dev/null @@ -1,50 +0,0 @@ -<#macro registrationLayout bodyClass="" displayInfo=false displayMessage=true displayWide=false> -<#import "template.ftl" as layout> -<@layout.registrationLayout; section> - <#if section = "header"> - - <#nested "header"> - <#elseif section = "form"> - <#if auth.authenticationSelections?size gt 1> -
-
-
- -
-
- - - value="${auth.selectedCredential}"/> -
-
-
- - <#nested "form"> - - - diff --git a/themes/src/main/resources/theme/base/login/template.ftl b/themes/src/main/resources/theme/base/login/template.ftl index d76bcac280..c45025d084 100644 --- a/themes/src/main/resources/theme/base/login/template.ftl +++ b/themes/src/main/resources/theme/base/login/template.ftl @@ -82,6 +82,17 @@ + <#if auth?has_content && auth.showTryAnotherWayLink() > +
class="${properties.kcContentWrapperClass!}"> +
class="${properties.kcFormSocialAccountContentClass!} ${properties.kcFormSocialAccountClass!}"> +
+ + ${msg("doTryAnotherWay")} +
+
+
+ + <#if displayInfo>
diff --git a/themes/src/main/resources/theme/base/login/webauthn-authenticate.ftl b/themes/src/main/resources/theme/base/login/webauthn-authenticate.ftl index f4befd60e7..c0dd2cf5d3 100644 --- a/themes/src/main/resources/theme/base/login/webauthn-authenticate.ftl +++ b/themes/src/main/resources/theme/base/login/webauthn-authenticate.ftl @@ -1,4 +1,4 @@ - <#import "select.ftl" as layout> + <#import "template.ftl" as layout> <@layout.registrationLayout; section> <#if section = "title"> title