From 3756cf629b9af312976997280e85fab8f7e91796 Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Mon, 19 Nov 2018 14:32:28 +0100 Subject: [PATCH] KEYCLOAK-7081 Fixes for manual/qr mode switches on login config otp page (#5717) --- .../requiredactions/UpdateTotp.java | 11 ++-- .../login/freemarker/model/TotpBean.java | 4 +- .../testsuite/pages/LoginConfigTotpPage.java | 11 ++++ .../actions/RequiredActionTotpSetupTest.java | 58 +++++++++++++++++++ .../theme/base/login/login-config-totp.ftl | 1 + 5 files changed, 76 insertions(+), 9 deletions(-) diff --git a/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateTotp.java b/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateTotp.java index 188ba2a2d5..fdcf5ffb33 100644 --- a/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateTotp.java +++ b/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateTotp.java @@ -45,15 +45,11 @@ public class UpdateTotp implements RequiredActionProvider, RequiredActionFactory @Override public void requiredActionChallenge(RequiredActionContext context) { Response challenge = context.form() - .setAttribute("mode", getMode(context)) + .setAttribute("mode", context.getUriInfo().getQueryParameters().getFirst("mode")) .createResponse(UserModel.RequiredAction.CONFIGURE_TOTP); context.challenge(challenge); } - private String getMode(RequiredActionContext context) { - return context.getUriInfo().getQueryParameters().getFirst("mode"); - } - @Override public void processAction(RequiredActionContext context) { EventBuilder event = context.getEvent(); @@ -61,17 +57,18 @@ public class UpdateTotp implements RequiredActionProvider, RequiredActionFactory MultivaluedMap formData = context.getHttpRequest().getDecodedFormParameters(); String totp = formData.getFirst("totp"); String totpSecret = formData.getFirst("totpSecret"); + String mode = formData.getFirst("mode"); if (Validation.isBlank(totp)) { Response challenge = context.form() - .setAttribute("mode", getMode(context)) + .setAttribute("mode", mode) .setError(Messages.MISSING_TOTP) .createResponse(UserModel.RequiredAction.CONFIGURE_TOTP); context.challenge(challenge); return; } else if (!CredentialValidation.validOTP(context.getRealm(), totp, totpSecret)) { Response challenge = context.form() - .setAttribute("mode", getMode(context)) + .setAttribute("mode", mode) .setError(Messages.INVALID_TOTP) .createResponse(UserModel.RequiredAction.CONFIGURE_TOTP); context.challenge(challenge); 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 76594592ad..84bbfecfa0 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 @@ -64,11 +64,11 @@ public class TotpBean { } public String getManualUrl() { - return uriBuilder.replaceQueryParam("mode", "manual").build().toString(); + return uriBuilder.replaceQueryParam("session_code").replaceQueryParam("mode", "manual").build().toString(); } public String getQrUrl() { - return uriBuilder.replaceQueryParam("mode", "qr").build().toString(); + return uriBuilder.replaceQueryParam("session_code").replaceQueryParam("mode", "qr").build().toString(); } public OTPPolicy getPolicy() { diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginConfigTotpPage.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginConfigTotpPage.java index 2ee83509ed..2479b08673 100755 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginConfigTotpPage.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginConfigTotpPage.java @@ -39,11 +39,18 @@ public class LoginConfigTotpPage extends AbstractPage { @FindBy(id = "mode-manual") private WebElement manualLink; + @FindBy(className = "alert-error") + private WebElement loginErrorMessage; + public void configure(String totp) { totpInput.sendKeys(totp); submitButton.click(); } + public void submit() { + submitButton.click(); + } + public String getTotpSecret() { return totpSecret.getAttribute("value"); } @@ -64,4 +71,8 @@ public class LoginConfigTotpPage extends AbstractPage { barcodeLink.click(); } + public String getError() { + return loginErrorMessage.getText(); + } + } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionTotpSetupTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionTotpSetupTest.java index c2ba191674..22215764d2 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionTotpSetupTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionTotpSetupTest.java @@ -196,6 +196,64 @@ public class RequiredActionTotpSetupTest extends AbstractTestRealmKeycloakTest { assertFalse(pageSource.contains("Scan barcode?")); } + // KEYCLOAK-7081 + @Test + public void setupTotpRegisterManualModeSwitchesOnBadSubmit() { + loginPage.open(); + loginPage.clickRegister(); + registerPage.register("firstName", "lastName", "setupTotpRegisterManualModeSwitchesOnBadSubmit@mail.com", "setupTotpRegisterManualModeSwitchesOnBadSubmit", "password", "password"); + + String pageSource = driver.getPageSource(); + + assertTrue(pageSource.contains("Unable to scan?")); + assertFalse(pageSource.contains("Scan barcode?")); + + totpPage.clickManual(); + + pageSource = driver.getPageSource(); + + assertFalse(pageSource.contains("Unable to scan?")); + assertTrue(pageSource.contains("Scan barcode?")); + + totpPage.submit(); + + pageSource = driver.getPageSource(); + + assertFalse(pageSource.contains("Unable to scan?")); + assertTrue(pageSource.contains("Scan barcode?")); + + assertEquals("Please specify authenticator code.", totpPage.getError()); + } + + // KEYCLOAK-7081 + @Test + public void setupTotpRegisterBarcodeModeSwitchesOnBadSubmit() { + loginPage.open(); + loginPage.clickRegister(); + registerPage.register("firstName", "lastName", "setupTotpRegisterBarcodeModeSwitchesOnBadSubmit@mail.com", "setupTotpRegisterBarcodeModeSwitchesOnBadSubmit", "password", "password"); + + String pageSource = driver.getPageSource(); + + assertTrue(pageSource.contains("Unable to scan?")); + assertFalse(pageSource.contains("Scan barcode?")); + + totpPage.submit(); + + pageSource = driver.getPageSource(); + + assertTrue(pageSource.contains("Unable to scan?")); + assertFalse(pageSource.contains("Scan barcode?")); + + assertEquals("Please specify authenticator code.", totpPage.getError()); + + totpPage.clickManual(); + + pageSource = driver.getPageSource(); + + assertFalse(pageSource.contains("Unable to scan?")); + assertTrue(pageSource.contains("Scan barcode?")); + } + @Test public void setupTotpModifiedPolicy() { RealmResource realm = testRealm(); diff --git a/themes/src/main/resources/theme/base/login/login-config-totp.ftl b/themes/src/main/resources/theme/base/login/login-config-totp.ftl index f4dc921d03..f3604e78e9 100755 --- a/themes/src/main/resources/theme/base/login/login-config-totp.ftl +++ b/themes/src/main/resources/theme/base/login/login-config-totp.ftl @@ -55,6 +55,7 @@ + <#if mode??>