[KEYCLOAK-12184] Remove BACK button from login forms (#6657)

This commit is contained in:
Martin Bartoš 2020-01-15 12:25:37 +01:00 committed by Marek Posolda
parent 789e8c70ce
commit 5aab03d915
10 changed files with 2 additions and 395 deletions

View file

@ -26,10 +26,7 @@ import org.keycloak.models.Constants;
import org.keycloak.models.RealmModel;
import org.keycloak.models.UserModel;
import org.keycloak.services.ServicesLogger;
import org.keycloak.services.util.AuthenticationFlowHistoryHelper;
import org.keycloak.services.util.AuthenticationFlowURLHelper;
import org.keycloak.sessions.AuthenticationSessionModel;
import org.keycloak.sessions.CommonClientSessionModel;
import javax.ws.rs.core.MultivaluedMap;
import javax.ws.rs.core.Response;
@ -102,32 +99,6 @@ public class DefaultAuthenticationFlow implements AuthenticationFlow {
MultivaluedMap<String, String> inputData = processor.getRequest().getDecodedFormParameters();
String authExecId = inputData.getFirst(Constants.AUTHENTICATION_EXECUTION);
//check if the user has selected the "back" option
if (inputData.containsKey("back")) {
AuthenticationSessionModel authSession = processor.getAuthenticationSession();
AuthenticationFlowHistoryHelper history = new AuthenticationFlowHistoryHelper(processor);
if (history.hasAnyExecution()) {
String executionId = history.pullExecution();
AuthenticationExecutionModel lastActionExecution = processor.getRealm().getAuthenticationExecutionById(executionId);
logger.debugf("Moving back to authentication execution '%s'", lastActionExecution.getAuthenticator());
recursiveClearExecutionStatusOfAllExecutionsAfterOurExecutionInclusive(lastActionExecution);
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
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");
@ -153,11 +124,6 @@ public class DefaultAuthenticationFlow implements AuthenticationFlow {
model = processor.getRealm().getAuthenticationExecutionById(authExecId);
// In case that new execution is a flow, we will add the 1st item from the selection (preferred credential) to the history, so when later click "back", we will return to it.
if (model.isAuthenticatorFlow()) {
new AuthenticationFlowHistoryHelper(processor).pushExecution(selectionOptions.get(0).getAuthExecId());
}
Response response = processSingleFlowExecutionModel(model, false);
if (response == null) {
return continueAuthenticationAfterSuccessfulAction(model);
@ -553,10 +519,6 @@ public class DefaultAuthenticationFlow implements AuthenticationFlow {
switch (status) {
case SUCCESS:
logger.debugv("authenticator SUCCESS: {0}", execution.getAuthenticator());
if (isAction) {
new AuthenticationFlowHistoryHelper(processor).pushExecution(execution.getId());
}
processor.getAuthenticationSession().setExecutionStatus(execution.getId(), AuthenticationSessionModel.ExecutionStatus.SUCCESS);
return null;
case FAILED:

View file

@ -409,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, page));
attributes.put("auth", new AuthenticationContextBean(context, page));
attributes.put(Constants.EXECUTION, execution);
if (realm.isInternationalizationEnabled()) {

View file

@ -18,14 +18,12 @@
package org.keycloak.forms.login.freemarker.model;
import java.net.URI;
import java.util.Collections;
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;
/**
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
@ -33,29 +31,17 @@ import org.keycloak.services.util.AuthenticationFlowHistoryHelper;
public class AuthenticationContextBean {
private final AuthenticationFlowContext context;
private final URI actionUri;
private final LoginFormsPages page;
public AuthenticationContextBean(AuthenticationFlowContext context, URI actionUri, LoginFormsPages page) {
public AuthenticationContextBean(AuthenticationFlowContext context, LoginFormsPages page) {
this.context = context;
this.actionUri = actionUri;
this.page = page;
}
public List<AuthenticationSelectionOption> getAuthenticationSelections() {
return context==null ? Collections.emptyList() : context.getAuthenticationSelections();
}
public boolean showBackButton() {
if (context == null) {
return false;
}
return actionUri != null && new AuthenticationFlowHistoryHelper(context.getAuthenticationSession(), context.getFlowPath()).hasAnyExecution();
}
public boolean showTryAnotherWayLink() {
return getAuthenticationSelections().size() > 1 && page != LoginFormsPages.LOGIN_SELECT_AUTHENTICATOR;
}

View file

@ -1,138 +0,0 @@
/*
* 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.services.util;
import java.util.Arrays;
import java.util.regex.Pattern;
import org.jboss.logging.Logger;
import org.keycloak.authentication.AuthenticationProcessor;
import org.keycloak.sessions.AuthenticationSessionModel;
/**
* Used to track executions visited by user during authentication. Useful for form "back" button
*
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
*/
public class AuthenticationFlowHistoryHelper {
private static final Logger log = Logger.getLogger(AuthenticationFlowHistoryHelper.class);
/**
* Prefix of the authentication session note with the list of IDs of successful authentication action executions.
* Those corresponds with the authenticator executions, which were shown to the user.
*
* The suffix of the note is the actual path of the authentication flow, so we can track history of more flow types during same authentication session
*
* IDs are divided by {@link #DELIMITER}
*/
private static final String SUCCESSFUL_ACTION_EXECUTIONS = "successful.action.executions";
private static final String DELIMITER = "::";
// Just perf optimization
private static final Pattern PATTERN = Pattern.compile(DELIMITER);
private final AuthenticationSessionModel authenticationSession;
private final String flowPath;
private final String authNoteName;
public AuthenticationFlowHistoryHelper(AuthenticationProcessor processor) {
this(processor.getAuthenticationSession(), processor.getFlowPath());
}
public AuthenticationFlowHistoryHelper(AuthenticationSessionModel authenticationSession, String flowPath) {
this.authenticationSession = authenticationSession;
this.flowPath = flowPath;
this.authNoteName = SUCCESSFUL_ACTION_EXECUTIONS + DELIMITER + flowPath;
}
/**
* Push executionId to the history if it's not already there
*
* @param executionId
*/
public void pushExecution(String executionId) {
if (containsExecution(executionId)) {
log.tracef("Not adding execution %s to authentication session. Execution is already there", executionId);
return;
}
log.tracef("Adding execution %s to authentication session. Flow path: %s", executionId, flowPath);
String history = authenticationSession.getAuthNote(authNoteName);
history = (history == null) ? executionId : history + DELIMITER + executionId;
authenticationSession.setAuthNote(authNoteName, history);
}
/**
* Check if there is any executionId in the history
*
* @return
*/
public boolean hasAnyExecution() {
return authenticationSession.getAuthNote(authNoteName) != null;
}
/**
* Return the last executionId from the history and remove it from the history.
*
* @return
*/
public String pullExecution() {
String history = authenticationSession.getAuthNote(authNoteName);
if (history == null) {
return null;
}
String[] splits = PATTERN.split(history);
String lastActionExecutionId = splits[splits.length - 1];
if (splits.length == 1) {
authenticationSession.removeAuthNote(authNoteName);
} else {
String newHistory = history.substring(0, history.length() - DELIMITER.length() - lastActionExecutionId.length());
authenticationSession.setAuthNote(authNoteName, newHistory);
}
log.tracef("Returning to execution %s in the authentication session. Flow path: %s", lastActionExecutionId, flowPath);
return lastActionExecutionId;
}
private boolean containsExecution(String executionId) {
String history = authenticationSession.getAuthNote(authNoteName);
if (history == null) {
return false;
}
String[] splits = PATTERN.split(history);
return Arrays.asList(splits).contains(executionId);
}
}

View file

@ -38,9 +38,6 @@ public abstract class LanguageComboboxAwarePage extends AbstractPage {
@FindBy(id = "kc-locale-dropdown")
private WebElement localeDropdown;
@FindBy(id = "kc-back")
private WebElement backButton;
@FindBy(id = "try-another-way")
private WebElement tryAnotherWayLink;
@ -55,23 +52,6 @@ public abstract class LanguageComboboxAwarePage extends AbstractPage {
WaitUtils.waitForPageToLoad();
}
// If false, we don't expect form "Back" button available on the page. If true, we expect that it is available on the page
public void assertBackButtonAvailability(boolean expectedAvailability) {
try {
driver.findElement(By.id("kc-back"));
Assert.assertTrue(expectedAvailability);
} catch (NoSuchElementException nse) {
Assert.assertFalse(expectedAvailability);
}
}
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 {

View file

@ -932,78 +932,4 @@ public abstract class AbstractFirstBrokerLoginTest extends AbstractInitializedBa
assertNumFederatedIdentities(realm.users().search(bc.getUserLogin()).get(0).getId(), 1);
}
@Test
public void testFormBackButton() {
updateExecutions(AbstractBrokerTest::setUpMissingUpdateProfileOnFirstLogin);
String existingUser = createUser("consumer");
driver.navigate().to(getAccountUrl(bc.consumerRealmName()));
log.debug("Clicking social " + bc.getIDPAlias());
loginPage.clickSocial(bc.getIDPAlias());
waitForPage(driver, "log in to", true);
Assert.assertTrue("Driver should be on the provider realm page right now",
driver.getCurrentUrl().contains("/auth/realms/" + bc.providerRealmName() + "/"));
log.debug("Logging in");
loginPage.login(bc.getUserLogin(), bc.getUserPassword());
waitForPage(driver, "update account information", false);
updateAccountInformationPage.assertCurrent();
// Assert "back" button not available
updateAccountInformationPage.assertBackButtonAvailability(false);
updateAccountInformationPage.updateAccountInformation("FirstName", "LastName");
waitForPage(driver, "account already exists", false);
assertTrue(idpConfirmLinkPage.isCurrent());
assertEquals("User with email user@localhost.com already exists. How do you want to continue?", idpConfirmLinkPage.getMessage());
// Assert "back" button available. Click it.
idpConfirmLinkPage.assertBackButtonAvailability(true);
idpConfirmLinkPage.clickBackButton();
// Assert on "update profile" page. "Back" button shouldn't still be available
waitForPage(driver, "update account information", false);
updateAccountInformationPage.assertCurrent();
updateAccountInformationPage.assertBackButtonAvailability(false);
// Update profile and click "Confirm link"
updateAccountInformationPage.updateAccountInformation("FirstName", "LastName");
waitForPage(driver, "account already exists", false);
assertTrue(idpConfirmLinkPage.isCurrent());
idpConfirmLinkPage.clickLinkAccount();
assertEquals("Authenticate as consumer to link your account with " + bc.getIDPAlias(), loginPage.getInfoMessage());
loginPage.assertBackButtonAvailability(true);
// Click "Back" button two times. Should be on "Review profile" page
loginPage.clickBackButton();
waitForPage(driver, "account already exists", false);
assertTrue(idpConfirmLinkPage.isCurrent());
idpConfirmLinkPage.assertBackButtonAvailability(true);
idpConfirmLinkPage.clickBackButton();
// Assert on "update profile" page. "Back" button shouldn't still be available
waitForPage(driver, "update account information", false);
updateAccountInformationPage.assertCurrent();
updateAccountInformationPage.assertBackButtonAvailability(false);
// Finally authenticate
updateAccountInformationPage.updateAccountInformation("FirstName", "LastName");
waitForPage(driver, "account already exists", false);
assertTrue(idpConfirmLinkPage.isCurrent());
idpConfirmLinkPage.clickLinkAccount();
loginPage.login("password");
waitForPage(driver, "keycloak account management", true);
accountUpdateProfilePage.assertCurrent();
assertNumFederatedIdentities(existingUser, 1);
}
}

View file

@ -186,52 +186,6 @@ public class KcOidcFirstBrokerLoginNewAuthTest extends AbstractInitializedBaseBr
}
/**
* Tests the firstBrokerLogin flow configured to re-authenticate with PasswordForm authenticator.
* Do some testing with back button
*/
@Test
public void testBackButtonWithOTPEnabled() {
configureBrokerFlowToReAuthenticationWithPasswordForm(bc.getIDPAlias(), "first broker login with password form");
// Create user and link him with TOTP
String consumerRealmUserId = createUser("consumer");
String totpSecret = addTOTPToUser("consumer");
loginWithBrokerAndConfirmLinkAccount();
// Login with password
Assert.assertTrue(passwordPage.isCurrent("consumer"));
passwordPage.login("password");
// Assert on TOTP page. Assert "Back" button available
loginTotpPage.assertCurrent();
loginTotpPage.assertBackButtonAvailability(true);
// Click "Back" 2 times. Should be on "Confirm account" page
loginTotpPage.clickBackButton();
Assert.assertTrue(passwordPage.isCurrent("consumer"));
passwordPage.assertBackButtonAvailability(true);
passwordPage.clickBackButton();
// Back button won't be available on "Confirm Link" page. It was the first authenticator
idpConfirmLinkPage.assertCurrent();
idpConfirmLinkPage.assertBackButtonAvailability(false);
// Authenticate
idpConfirmLinkPage.clickLinkAccount();
Assert.assertTrue(passwordPage.isCurrent("consumer"));
passwordPage.login("password");
loginTotpPage.assertCurrent();
loginTotpPage.login(totp.generateTOTP(totpSecret));
assertUserAuthenticatedInConsumer(consumerRealmUserId);
}
// Add OTP to the user. Return TOTP secret
private String addTOTPToUser(String username) {

View file

@ -7,7 +7,6 @@ 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;
@ -36,7 +35,6 @@ import org.keycloak.testsuite.admin.authentication.AbstractAuthenticationTest;
import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude;
import org.keycloak.testsuite.auth.page.login.OneTimeCode;
import org.keycloak.testsuite.broker.SocialLoginTest;
import org.keycloak.testsuite.client.KeycloakTestingClient;
import org.keycloak.testsuite.pages.ErrorPage;
import org.keycloak.testsuite.pages.LoginPage;
import org.keycloak.testsuite.pages.LoginTotpPage;
@ -47,7 +45,6 @@ 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;
@ -168,27 +165,6 @@ public class BrowserFlowTest extends AbstractTestRealmKeycloakTest {
Assert.assertFalse(oneTimeCodePage.isOtpLabelPresent());
}
@Test
public void testBackButton() {
provideUsernamePassword("user-with-one-configured-otp");
Assert.assertTrue(oneTimeCodePage.isOtpLabelPresent());
// Assert "Back" button available on the TOTP page
loginTotpPage.assertBackButtonAvailability(true);
loginTotpPage.clickBackButton();
// Assert "Back" button not available on the Browser page
loginPage.assertCurrent();
loginPage.assertBackButtonAvailability(false);
// Login
loginPage.login("user-with-one-configured-otp", "password");
oneTimeCodePage.sendCode(getOtpCode("DJmQfC73VGFhw7D4QJ8A"));
Assert.assertFalse(loginPage.isCurrent());
Assert.assertFalse(oneTimeCodePage.isOtpLabelPresent());
}
@Test
public void testUserWithTwoAdditionalFactors() {
final String firstKey = "DJmQfC73VGFhw7D4QJ8A";

View file

@ -120,33 +120,6 @@ public class ResetCredentialsAlternativeFlowsTest extends AbstractTestRealmKeycl
}
// Test with default reset-credentials flow and alternative browser flow with separate username and password screen.
//
// Click "Forget password" on browser flow passwordPAge and assert that button "Back" is not available as we switched to
// different flow (reset-credentials" flow).
@Test
public void testBackButtonWhenSwitchToResetCredentialsFlowFromAlternativeBrowserFlow() {
try {
MultiFactorAuthenticationTest.configureBrowserFlowWithAlternativeCredentials(testingClient);
// Provide username and then click "Forget password"
provideUsernameAndClickResetPassword("login-test");
// Click "back to login" link. Should be on password page of the browser flow (under URL "authenticate")
resetPasswordPage.backToLogin();
passwordPage.assertCurrent();
Assert.assertTrue(URLUtils.currentUrlMatches("/login-actions/authenticate"));
passwordPage.assertBackButtonAvailability(true);
// Click "back". Should be on usernameForm
passwordPage.clickBackButton();
loginUsernameOnlyPage.assertCurrent();
} finally {
revertFlows();
}
}
// Test with default reset-credentials flow and alternative browser flow with separate username and password screen.
//
// Provide username and click "Forget password" on browser flow. Then provide non-existing username in reset-credentials 1st screen.
@ -290,7 +263,6 @@ public class ResetCredentialsAlternativeFlowsTest extends AbstractTestRealmKeycl
// Assert switched to the "reset-credentials" flow, but button "back" not available
resetPasswordPage.assertCurrent();
Assert.assertTrue(URLUtils.currentUrlMatches("/login-actions/reset-credentials"));
resetPasswordPage.assertBackButtonAvailability(false);
}

View file

@ -71,17 +71,6 @@
<#nested "form">
<#if auth?has_content && auth.showBackButton() >
<form id="kc-select-back-form" action="${url.loginAction}" method="post" <#if displayWide>class="${properties.kcContentWrapperClass!}"</#if>>
<div <#if displayWide>class="${properties.kcFormSocialAccountContentClass!} ${properties.kcFormSocialAccountClass!}"</#if>>
<div class="${properties.kcFormGroupClass!}">
<input class="${properties.kcButtonClass!} ${properties.kcButtonDefaultClass!} ${properties.kcButtonBlockClass!} ${properties.kcButtonLargeClass!}"
name="back" id="kc-back" type="submit" value="${msg("doBack")}"/>
</div>
</div>
</form>
</#if>
<#if auth?has_content && auth.showTryAnotherWayLink() >
<form id="kc-select-try-another-way-form" action="${url.loginAction}" method="post" <#if displayWide>class="${properties.kcContentWrapperClass!}"</#if>>
<div <#if displayWide>class="${properties.kcFormSocialAccountContentClass!} ${properties.kcFormSocialAccountClass!}"</#if>>