From 6f173d4554de7f9cb768500a3d8e8182789aacd8 Mon Sep 17 00:00:00 2001 From: mposolda Date: Thu, 11 Sep 2014 20:54:48 +0200 Subject: [PATCH] KEYCLOAK-678 Preserve form values in account mgmt after error --- .../org/keycloak/account/AccountProvider.java | 3 +++ .../freemarker/FreeMarkerAccountProvider.java | 10 +++++++++- .../account/freemarker/model/AccountBean.java | 14 +++++++++----- .../services/resources/AccountService.java | 4 ++-- .../keycloak/testsuite/account/AccountTest.java | 16 ++++++++++++---- .../pages/AccountUpdateProfilePage.java | 10 +++++++++- 6 files changed, 44 insertions(+), 13 deletions(-) diff --git a/forms/account-api/src/main/java/org/keycloak/account/AccountProvider.java b/forms/account-api/src/main/java/org/keycloak/account/AccountProvider.java index ddc7b95ede..7677a2091e 100755 --- a/forms/account-api/src/main/java/org/keycloak/account/AccountProvider.java +++ b/forms/account-api/src/main/java/org/keycloak/account/AccountProvider.java @@ -6,6 +6,7 @@ import org.keycloak.models.UserModel; import org.keycloak.models.UserSessionModel; import org.keycloak.provider.Provider; +import javax.ws.rs.core.MultivaluedMap; import javax.ws.rs.core.Response; import javax.ws.rs.core.UriInfo; import java.util.List; @@ -27,6 +28,8 @@ public interface AccountProvider extends Provider { AccountProvider setUser(UserModel user); + AccountProvider setProfileFormData(MultivaluedMap formData); + AccountProvider setStatus(Response.Status status); AccountProvider setRealm(RealmModel realm); diff --git a/forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/FreeMarkerAccountProvider.java b/forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/FreeMarkerAccountProvider.java index 7738b929b6..b42090f3e9 100755 --- a/forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/FreeMarkerAccountProvider.java +++ b/forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/FreeMarkerAccountProvider.java @@ -25,6 +25,7 @@ import org.keycloak.models.UserModel; import org.keycloak.models.UserSessionModel; import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.MultivaluedMap; import javax.ws.rs.core.Response; import javax.ws.rs.core.UriBuilder; import javax.ws.rs.core.UriInfo; @@ -43,6 +44,7 @@ public class FreeMarkerAccountProvider implements AccountProvider { private static final Logger logger = Logger.getLogger(FreeMarkerAccountProvider.class); private UserModel user; + private MultivaluedMap profileFormData; private Response.Status status = Response.Status.OK; private RealmModel realm; private String[] referrer; @@ -126,7 +128,7 @@ public class FreeMarkerAccountProvider implements AccountProvider { switch (page) { case ACCOUNT: - attributes.put("account", new AccountBean(user)); + attributes.put("account", new AccountBean(user, profileFormData)); break; case TOTP: attributes.put("totp", new TotpBean(user, baseUri)); @@ -187,6 +189,12 @@ public class FreeMarkerAccountProvider implements AccountProvider { return this; } + @Override + public AccountProvider setProfileFormData(MultivaluedMap formData) { + this.profileFormData = formData; + return this; + } + @Override public AccountProvider setRealm(RealmModel realm) { this.realm = realm; diff --git a/forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/model/AccountBean.java b/forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/model/AccountBean.java index 06ef6c2ba8..e9528adb76 100644 --- a/forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/model/AccountBean.java +++ b/forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/model/AccountBean.java @@ -1,5 +1,7 @@ package org.keycloak.account.freemarker.model; +import javax.ws.rs.core.MultivaluedMap; + import org.keycloak.models.UserModel; /** @@ -7,18 +9,20 @@ import org.keycloak.models.UserModel; */ public class AccountBean { - private UserModel user; + private final UserModel user; + private final MultivaluedMap profileFormData; - public AccountBean(UserModel user) { + public AccountBean(UserModel user, MultivaluedMap profileFormData) { this.user = user; + this.profileFormData = profileFormData; } public String getFirstName() { - return user.getFirstName(); + return profileFormData != null ? profileFormData.getFirst("firstName") : user.getFirstName(); } public String getLastName() { - return user.getLastName(); + return profileFormData != null ? profileFormData.getFirst("lastName") :user.getLastName(); } public String getUsername() { @@ -26,7 +30,7 @@ public class AccountBean { } public String getEmail() { - return user.getEmail(); + return profileFormData != null ? profileFormData.getFirst("email") :user.getEmail(); } } diff --git a/services/src/main/java/org/keycloak/services/resources/AccountService.java b/services/src/main/java/org/keycloak/services/resources/AccountService.java index 19e4e4ce11..3c918cfec9 100755 --- a/services/src/main/java/org/keycloak/services/resources/AccountService.java +++ b/services/src/main/java/org/keycloak/services/resources/AccountService.java @@ -407,7 +407,7 @@ public class AccountService { String error = Validation.validateUpdateProfileForm(formData); if (error != null) { setReferrerOnPage(); - return account.setError(error).createResponse(AccountPages.ACCOUNT); + return account.setError(error).setProfileFormData(formData).createResponse(AccountPages.ACCOUNT); } try { @@ -430,7 +430,7 @@ public class AccountService { return account.setSuccess("accountUpdated").createResponse(AccountPages.ACCOUNT); } catch (ModelReadOnlyException roe) { setReferrerOnPage(); - return account.setError(Messages.READ_ONLY_USER).createResponse(AccountPages.ACCOUNT); + return account.setError(Messages.READ_ONLY_USER).setProfileFormData(formData).createResponse(AccountPages.ACCOUNT); } } diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/account/AccountTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/account/AccountTest.java index ab61236d61..e22a6308eb 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/account/AccountTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/account/AccountTest.java @@ -282,23 +282,31 @@ public class AccountTest { Assert.assertEquals("Please specify first name", profilePage.getError()); Assert.assertEquals("", profilePage.getFirstName()); - Assert.assertEquals("", profilePage.getLastName()); - Assert.assertEquals("test-user@localhost", profilePage.getEmail()); + Assert.assertEquals("New last", profilePage.getLastName()); + Assert.assertEquals("new@email.com", profilePage.getEmail()); events.assertEmpty(); profilePage.updateProfile("New first", "", "new@email.com"); Assert.assertEquals("Please specify last name", profilePage.getError()); - Assert.assertEquals("", profilePage.getFirstName()); + Assert.assertEquals("New first", profilePage.getFirstName()); Assert.assertEquals("", profilePage.getLastName()); - Assert.assertEquals("test-user@localhost", profilePage.getEmail()); + Assert.assertEquals("new@email.com", profilePage.getEmail()); events.assertEmpty(); profilePage.updateProfile("New first", "New last", ""); Assert.assertEquals("Please specify email", profilePage.getError()); + Assert.assertEquals("New first", profilePage.getFirstName()); + Assert.assertEquals("New last", profilePage.getLastName()); + Assert.assertEquals("", profilePage.getEmail()); + + events.assertEmpty(); + + profilePage.clickCancel(); + Assert.assertEquals("", profilePage.getFirstName()); Assert.assertEquals("", profilePage.getLastName()); Assert.assertEquals("test-user@localhost", profilePage.getEmail()); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/AccountUpdateProfilePage.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/AccountUpdateProfilePage.java index a786037cff..3b11fda6b2 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/AccountUpdateProfilePage.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/AccountUpdateProfilePage.java @@ -48,9 +48,12 @@ public class AccountUpdateProfilePage extends AbstractAccountPage { @FindBy(id = "referrer") private WebElement backToApplicationLink; - @FindBy(css = "button[type=\"submit\"]") + @FindBy(css = "button[type=\"submit\"][value=\"Save\"]") private WebElement submitButton; + @FindBy(css = "button[type=\"submit\"][value=\"Cancel\"]") + private WebElement cancelButton; + @FindBy(className = "alert-success") private WebElement successMessage; @@ -68,6 +71,11 @@ public class AccountUpdateProfilePage extends AbstractAccountPage { submitButton.click(); } + public void clickCancel() { + cancelButton.click(); + } + + public String getFirstName() { return firstNameInput.getAttribute("value"); }