From b0ffea699e662f868bddfd076aa8cb8ad841aaa4 Mon Sep 17 00:00:00 2001 From: Peter Zaoral Date: Wed, 5 Feb 2020 10:06:17 +0100 Subject: [PATCH] KEYCLOAK-12186 Improve the OTP login form -created and implemented login form design, where OTP device can be selected -implemented selectable-card-view logic in jQuery -edited related css and ftl theme resources -fixed affected BrowserFlow tests Signed-off-by: Peter Zaoral --- .../java/org/keycloak/events/Details.java | 1 + .../browser/OTPFormAuthenticator.java | 3 + .../testsuite/pages/LoginTotpPage.java | 58 ++++++---- .../resources/theme/base/login/login-otp.ftl | 107 +++++++++++------- .../keycloak/login/resources/css/login.css | 4 + .../theme/keycloak/login/theme.properties | 6 + 6 files changed, 118 insertions(+), 61 deletions(-) diff --git a/server-spi-private/src/main/java/org/keycloak/events/Details.java b/server-spi-private/src/main/java/org/keycloak/events/Details.java index c8f9701abc..4e681cbbe1 100755 --- a/server-spi-private/src/main/java/org/keycloak/events/Details.java +++ b/server-spi-private/src/main/java/org/keycloak/events/Details.java @@ -73,4 +73,5 @@ public interface Details { String X509_CERTIFICATE_ISSUER_DISTINGUISHED_NAME = "x509_cert_issuer_distinguished_name"; String CREDENTIAL_TYPE = "credential_type"; + String SELECTED_CREDENTIAL_ID = "selected_credential_id"; } 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 f64817a5b1..74145c2173 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 @@ -27,6 +27,7 @@ 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.Details; import org.keycloak.events.Errors; import org.keycloak.forms.login.LoginFormsProvider; import org.keycloak.models.KeycloakSession; @@ -79,6 +80,8 @@ public class OTPFormAuthenticator extends AbstractUsernameFormAuthenticator impl .getDefaultCredential(context.getSession(), context.getRealm(), context.getUser()); credentialId = defaultOtpCredential==null ? "" : defaultOtpCredential.getId(); } + context.getEvent().detail(Details.SELECTED_CREDENTIAL_ID, credentialId); + context.form().setAttribute(SELECTED_OTP_CREDENTIAL_ID, credentialId); UserModel userModel = context.getUser(); 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 65f531ccc5..752cb51d84 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 @@ -20,11 +20,13 @@ import java.util.List; import java.util.stream.Collectors; import org.junit.Assert; +import org.keycloak.common.util.Retry; +import org.keycloak.testsuite.util.UIUtils; +import org.keycloak.testsuite.util.WaitUtils; 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 @@ -43,9 +45,6 @@ public class LoginTotpPage extends LanguageComboboxAwarePage { @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); @@ -75,7 +74,7 @@ public class LoginTotpPage extends LanguageComboboxAwarePage { // 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")); + driver.findElement(By.className("card-pf-view-single-select")); Assert.assertTrue(expectedAvailability); } catch (NoSuchElementException nse) { Assert.assertFalse(expectedAvailability); @@ -84,29 +83,50 @@ public class LoginTotpPage extends LanguageComboboxAwarePage { public List getAvailableOtpCredentials() { - return new Select(selectedCredentialCombobox).getOptions() - .stream() - .map(WebElement::getText) - .collect(Collectors.toList()); + return driver.findElements(getXPathForLookupAllCards()) + .stream().map(WebElement::getText).collect(Collectors.toList()); } public String getSelectedOtpCredential() { - return new Select(selectedCredentialCombobox).getOptions() - .stream() - .filter(webElement -> webElement.getAttribute("selected") != null) - .findFirst() - .orElseThrow(() -> { + try { + WebElement selected = driver.findElement(getXPathForLookupActiveCard()); + return selected.getText(); + } catch (NoSuchElementException nse) { + // No selected element found + return null; + } + } - return new AssertionError("Selected OTP credential not found"); + private By getXPathForLookupAllCards() { + return By.xpath("//div[contains(@class, 'card-pf-view-single-select')]//h2"); + } - }) - .getText(); + private By getXPathForLookupActiveCard() { + return By.xpath("//div[contains(@class, 'card-pf-view-single-select active')]//h2"); + } + + private By getXPathForLookupCardWithName(String credentialName) { + return By.xpath("//div[contains(@class, 'card-pf-view-single-select')]//h2[normalize-space() = '"+ credentialName +"']"); } public void selectOtpCredential(String credentialName) { - new Select(selectedCredentialCombobox).selectByVisibleText(credentialName); + waitForElement(getXPathForLookupActiveCard()); + + WebElement webElement = driver.findElement( + getXPathForLookupCardWithName(credentialName)); + UIUtils.clickLink(webElement); } -} + + // Workaround, but works with HtmlUnit (WaitUtils.waitForElement doesn't). Find better solution for the future... + private void waitForElement(By by) { + Retry.executeWithBackoff((currentCount) -> { + + driver.findElement(by); + + }, 10, 10); + } + +} \ No newline at end of file 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 93f120b75d..9acc6ac19d 100755 --- a/themes/src/main/resources/theme/base/login/login-otp.ftl +++ b/themes/src/main/resources/theme/base/login/login-otp.ftl @@ -1,47 +1,70 @@ <#import "template.ftl" as layout> -<@layout.registrationLayout; section> - <#if section = "header"> - ${msg("doLogIn")} - <#elseif section = "form"> -
+ <@layout.registrationLayout; section> + <#if section="header"> + ${msg("doLogIn")} + <#elseif section="form"> + + <#if otpLogin.userOtpCredentials?size gt 1> +
+
+ <#list otpLogin.userOtpCredentials as otpCredential> +
+ +
+ +

+ ${otpCredential.userLabel} +

+
+
+ +
+
+ - <#if otpLogin.userOtpCredentials?size gt 1> -
-
- +
+
+ +
+ +
+ +
-
- + +
+
+
+
+
+ +
+ +
-
- + + + + + \ No newline at end of file diff --git a/themes/src/main/resources/theme/keycloak/login/resources/css/login.css b/themes/src/main/resources/theme/keycloak/login/resources/css/login.css index fd1668bf6a..f838cf15a0 100644 --- a/themes/src/main/resources/theme/keycloak/login/resources/css/login.css +++ b/themes/src/main/resources/theme/keycloak/login/resources/css/login.css @@ -511,6 +511,10 @@ a.zocial { width: 100%; } +.login-pf-page .card-pf{ + margin-bottom: 10px; +} + #kc-form-login div.form-group:last-of-type, #kc-register-form div.form-group:last-of-type, #kc-update-profile-form div.form-group:last-of-type { diff --git a/themes/src/main/resources/theme/keycloak/login/theme.properties b/themes/src/main/resources/theme/keycloak/login/theme.properties index 03f0505d40..d266b1e107 100644 --- a/themes/src/main/resources/theme/keycloak/login/theme.properties +++ b/themes/src/main/resources/theme/keycloak/login/theme.properties @@ -86,3 +86,9 @@ kcAuthenticatorPasswordClass=fa fa-unlock list-view-pf-icon-lg kcAuthenticatorOTPClass=fa fa-mobile list-view-pf-icon-lg kcAuthenticatorWebAuthnClass=fa fa-key list-view-pf-icon-lg kcAuthenticatorWebAuthnPasswordlessClass=fa fa-key list-view-pf-icon-lg + +##### css classes for the OTP Login Form +kcSelectOTPListClass=card-pf card-pf-view card-pf-view-select card-pf-view-single-select +kcSelectOTPListItemClass=card-pf-body card-pf-top-element +kcAuthenticatorOtpCircleClass=fa fa-mobile card-pf-icon-circle +kcSelectOTPItemHeadingClass=card-pf-title text-center \ No newline at end of file