From 144f5f9cfd1558b83c713873224ae144b244548f Mon Sep 17 00:00:00 2001 From: vrockai Date: Mon, 30 Sep 2013 16:51:41 +0200 Subject: [PATCH] KEYCLOAK-74 Adapting the Forget password forms to original design --- .../java/org/keycloak/forms/ErrorBean.java | 22 ++++++++++++ .../org/keycloak/service/FormServiceImpl.java | 5 +++ .../forms/theme/default/css/forms.css | 8 ++--- .../css/img/feedback-success-arrow-down.png | Bin 0 -> 678 bytes .../default/css/img/feedback-success-sign.png | Bin 0 -> 410 bytes .../theme/default/css/login-register.css | 6 ++++ .../theme/default/login-reset-password.ftl | 24 ++++++++----- .../theme/default/login-update-password.ftl | 12 +++---- .../org/keycloak/forms/messages.properties | 12 +++++++ .../org/keycloak/services/FormService.java | 12 +++++++ .../keycloak/services/email/EmailSender.java | 11 ++++-- .../services/resources/AccountService.java | 6 ++-- .../services/resources/flows/FormFlows.java | 11 ++++++ .../testsuite/forms/ResetPasswordTest.java | 34 +++++++++++++++++- .../pages/LoginPasswordResetPage.java | 9 ++++- 15 files changed, 145 insertions(+), 27 deletions(-) create mode 100644 forms/src/main/resources/META-INF/resources/forms/theme/default/css/img/feedback-success-arrow-down.png create mode 100644 forms/src/main/resources/META-INF/resources/forms/theme/default/css/img/feedback-success-sign.png diff --git a/forms/src/main/java/org/keycloak/forms/ErrorBean.java b/forms/src/main/java/org/keycloak/forms/ErrorBean.java index ff140bdcf3..d6c18b6a61 100644 --- a/forms/src/main/java/org/keycloak/forms/ErrorBean.java +++ b/forms/src/main/java/org/keycloak/forms/ErrorBean.java @@ -21,6 +21,8 @@ */ package org.keycloak.forms; +import org.keycloak.services.resources.flows.FormFlows; + /** * @author Stian Thorgersen */ @@ -28,12 +30,32 @@ public class ErrorBean { private String summary; + private FormFlows.ErrorType type; + + // Message is considered ERROR by default public ErrorBean(String summary) { + this(summary, FormFlows.ErrorType.ERROR); + } + + public ErrorBean(String summary, FormFlows.ErrorType type) { this.summary = summary; + this.type = type; } public String getSummary() { return summary; } + public boolean isSuccess(){ + return FormFlows.ErrorType.SUCCESS.equals(this.type); + } + + public boolean isWarning(){ + return FormFlows.ErrorType.WARNING.equals(this.type); + } + + public boolean isError(){ + return FormFlows.ErrorType.ERROR.equals(this.type); + } + } \ No newline at end of file diff --git a/forms/src/main/java/org/keycloak/service/FormServiceImpl.java b/forms/src/main/java/org/keycloak/service/FormServiceImpl.java index 1df2436972..7efa87dab3 100644 --- a/forms/src/main/java/org/keycloak/service/FormServiceImpl.java +++ b/forms/src/main/java/org/keycloak/service/FormServiceImpl.java @@ -42,6 +42,7 @@ import org.keycloak.forms.TotpBean; import org.keycloak.forms.UrlBean; import org.keycloak.forms.UserBean; import org.keycloak.services.FormService; +import org.keycloak.services.resources.flows.FormFlows; import org.keycloak.services.resources.flows.Pages; /** @@ -149,6 +150,10 @@ public class FormServiceImpl implements FormService { private class CommandPassword implements Command { public void exec(Map attributes, FormServiceDataBean dataBean) { + if (dataBean.getError() != null){ + attributes.put("message", new ErrorBean(dataBean.getError(), dataBean.getErrorType())); + } + RealmBean realm = new RealmBean(dataBean.getRealm()); attributes.put("realm", realm); diff --git a/forms/src/main/resources/META-INF/resources/forms/theme/default/css/forms.css b/forms/src/main/resources/META-INF/resources/forms/theme/default/css/forms.css index 062012cc48..78a16e4122 100644 --- a/forms/src/main/resources/META-INF/resources/forms/theme/default/css/forms.css +++ b/forms/src/main/resources/META-INF/resources/forms/theme/default/css/forms.css @@ -250,19 +250,19 @@ button.primary:enabled:active { background-position: 1.27272727272727em 1.63636363636364em; } .feedback.error { - background-image: url(img/feedback-error-arrow-down.svg); + background-image: url(img/feedback-error-arrow-down.png); } .feedback.error p { border-color: #b91415; - background-image: url(img/feedback-error-sign.svg); + background-image: url(img/feedback-error-sign.png); background-color: #f8e7e7; } .feedback.success { - background-image: url(img/feedback-success-arrow-down.svg); + background-image: url(img/feedback-success-arrow-down.png); } .feedback.success p { border-color: #4b9e39; - background-image: url(img/feedback-success-sign.svg); + background-image: url(img/feedback-success-sign.png); background-color: #e4f1e1; } .feedback.warning p { diff --git a/forms/src/main/resources/META-INF/resources/forms/theme/default/css/img/feedback-success-arrow-down.png b/forms/src/main/resources/META-INF/resources/forms/theme/default/css/img/feedback-success-arrow-down.png new file mode 100644 index 0000000000000000000000000000000000000000..03cc0c45dcc73e42f6b3718c8fe4a42aab71d424 GIT binary patch literal 678 zcmeAS@N?(olHy`uVBq!ia0y~yV15B)b91l($v3l?hXN^euD81vt(&PyxuVl0?xy+oT=qfZ( zU<;qzo;L~X{8Dl*OuQ;g6C?jcvTY4MH)UgY@3N+I%Y&B-aM#Gmz4*K9-@h9B>+j#Q z@1I{-{^;BJ=zrF!cK!}Pr!g=bh|HcT>Z=m}wDHmbyC0nDaV#gVm&uycGotW zpv;r|b9e9Fzj)!!_FHF``Wx|qb*+1{*5LE=)%xG0rlsBtdT?-dwR{mlor@ArQ!T)1>sQ|BRGRy6pUXk5Eq%Jj zt#hDSUl#1`1Hp4rtdp{v4HrVu6{1-oD!M}vd literal 0 HcmV?d00001 diff --git a/forms/src/main/resources/META-INF/resources/forms/theme/default/css/img/feedback-success-sign.png b/forms/src/main/resources/META-INF/resources/forms/theme/default/css/img/feedback-success-sign.png new file mode 100644 index 0000000000000000000000000000000000000000..640bd71cab7bdfc7a8adcf28ffaf6db736a1c008 GIT binary patch literal 410 zcmeAS@N?(olHy`uVBq!ia0vp^f*{Pn1|+R>-G2comSQK*5Dp-y;YjHK@;M7UB8!3Q zuY)k7lg8`{prB-lYeY$Kep*R+Vo@qXd3m{BW?pu2a$-TMUVc&f>~}U&3=E9oo-U3d z7QI&|@AYC16lmSQ(|nzi6W2vA0hQiX{shsj9%4*QVx6m7=T11}S0g4pTOrBJ(L2zj zqqUbmAu&;Tnwis)`FE;yFXbH4TM_-V#F%}bS9C7N)nccy^_%s?ydQOE$UA=bQASGdP zUS5*^?Y}eFBd=Y%8?p303zx;a-hVOO%tz8T)jmnDmV3_E#n4nO@i}MX#=5_=rbOm! z*!s=OFZw-8%CQyiuZC^?Jc-4l;#4*gQu&X%Q~loCIB2_ BsrLW? literal 0 HcmV?d00001 diff --git a/forms/src/main/resources/META-INF/resources/forms/theme/default/css/login-register.css b/forms/src/main/resources/META-INF/resources/forms/theme/default/css/login-register.css index 073ff3957a..8908e5277b 100644 --- a/forms/src/main/resources/META-INF/resources/forms/theme/default/css/login-register.css +++ b/forms/src/main/resources/META-INF/resources/forms/theme/default/css/login-register.css @@ -324,6 +324,12 @@ a.zocial:before { line-height: 1.3em; } +.rcue-login-register.reset p.subtitle { + margin-bottom: 10px; + position: inherit; + text-align: right; +} + .rcue-login-register .background-area p.instruction.instruction.second { color: #999999; } diff --git a/forms/src/main/resources/META-INF/resources/forms/theme/default/login-reset-password.ftl b/forms/src/main/resources/META-INF/resources/forms/theme/default/login-reset-password.ftl index 6aaddd7b8c..60ae59a4cf 100755 --- a/forms/src/main/resources/META-INF/resources/forms/theme/default/login-reset-password.ftl +++ b/forms/src/main/resources/META-INF/resources/forms/theme/default/login-reset-password.ftl @@ -1,30 +1,36 @@ <#import "template-login-action.ftl" as layout> -<@layout.registrationLayout bodyClass=""; section> +<@layout.registrationLayout bodyClass="reset"; section> <#if section = "title"> - Reset password + ${rb.getString('emailForgotHeader')} <#elseif section = "header"> - Reset password + ${rb.getString('emailForgotHeader')} <#elseif section = "form">
+ <#if message?has_content> + <#if message.success> + + + <#if message.error> + + + + +

${rb.getString('emailInstruction')}

- - +
- - +
-
- <#elseif section = "info" >
diff --git a/forms/src/main/resources/META-INF/resources/forms/theme/default/login-update-password.ftl b/forms/src/main/resources/META-INF/resources/forms/theme/default/login-update-password.ftl index 7a13ecc978..a9b1ed24fd 100755 --- a/forms/src/main/resources/META-INF/resources/forms/theme/default/login-update-password.ftl +++ b/forms/src/main/resources/META-INF/resources/forms/theme/default/login-update-password.ftl @@ -1,24 +1,22 @@ <#import "template-login-action.ftl" as layout> -<@layout.registrationLayout bodyClass=""; section> +<@layout.registrationLayout bodyClass="reset"; section> <#if section = "title"> - Update password + ${rb.getString('emailUpdateHeader')} <#elseif section = "header"> - Update password + ${rb.getString('emailUpdateHeader')} <#elseif section = "form">
- - +
- - +
diff --git a/forms/src/main/resources/org/keycloak/forms/messages.properties b/forms/src/main/resources/org/keycloak/forms/messages.properties index ce6667ae39..92722f94e7 100644 --- a/forms/src/main/resources/org/keycloak/forms/messages.properties +++ b/forms/src/main/resources/org/keycloak/forms/messages.properties @@ -17,6 +17,7 @@ email=Email password=Password passwordConfirm=Confirm Password passwordNew=New Password +passwordNewConfirm=New Password confirmation authenticatorCode=One-time-password clientCertificate=Client Certificate @@ -39,3 +40,14 @@ invalidTotp=Invalid authenticator code usernameExists=Username already exists error=A system error has occured, contact admin + +successHeader=Success! +errorHeader=Error! + +# Forgot password part + +emailForgotHeader=Forgot Your Password? +emailUpdateHeader=Update password +emailSent=You should receive an email shortly with further instructions. +emailError=Invalid username or email. +emailInstruction=Enter your username and email address and we will send you instructions on how to create a new password. \ No newline at end of file diff --git a/services/src/main/java/org/keycloak/services/FormService.java b/services/src/main/java/org/keycloak/services/FormService.java index 8ddf517101..50a095924f 100644 --- a/services/src/main/java/org/keycloak/services/FormService.java +++ b/services/src/main/java/org/keycloak/services/FormService.java @@ -27,6 +27,7 @@ import javax.ws.rs.core.MultivaluedMap; import org.keycloak.services.models.RealmModel; import org.keycloak.services.models.UserModel; +import org.keycloak.services.resources.flows.FormFlows; /** * @author Viliam Rockai @@ -42,6 +43,9 @@ public interface FormService { private RealmModel realm; private UserModel userModel; private String error; + + private FormFlows.ErrorType errorType; + private MultivaluedMap formData; private URI baseURI; @@ -121,5 +125,13 @@ public interface FormService { public void setUserModel(UserModel userModel) { this.userModel = userModel; } + + public FormFlows.ErrorType getErrorType() { + return errorType; + } + + public void setErrorType(FormFlows.ErrorType errorType) { + this.errorType = errorType; + } } } diff --git a/services/src/main/java/org/keycloak/services/email/EmailSender.java b/services/src/main/java/org/keycloak/services/email/EmailSender.java index cdf52d84d6..8bd181cdd6 100644 --- a/services/src/main/java/org/keycloak/services/email/EmailSender.java +++ b/services/src/main/java/org/keycloak/services/email/EmailSender.java @@ -103,9 +103,16 @@ public class EmailSender { URI uri = builder.build(realm.getId()); StringBuilder sb = new StringBuilder(); + + sb.append("Hi ").append(user.getFirstName()).append(",\n\n"); + sb.append("Someone just requested to change your Keycloak account's password. "); + sb.append("If this was you, click on the link below to set a new password:\n"); sb.append(uri.toString()); - sb.append("\n"); - sb.append("Expires in " + TimeUnit.SECONDS.toMinutes(realm.getAccessCodeLifespanUserAction())); + sb.append("\n\nThis link will expire within ").append(TimeUnit.SECONDS.toMinutes(realm.getAccessCodeLifespanUserAction())); + sb.append(" minutes.\n\n"); + sb.append("If you don't want to reset your password, just ignore this message and nothing will be changed.\n\n"); + sb.append("Thanks,\n"); + sb.append("The Keycloak Team"); try { send(user.getEmail(), "Reset password link", sb.toString()); 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 d4b643c1a1..94e46820c1 100755 --- a/services/src/main/java/org/keycloak/services/resources/AccountService.java +++ b/services/src/main/java/org/keycloak/services/resources/AccountService.java @@ -240,8 +240,7 @@ public class AccountService { UserModel user = realm.getUser(username); if (user == null || !email.equals(user.getEmail())) { - return Flows.forms(realm, request, uriInfo).setError("Invalid username or email") - .forwardToAction(RequiredAction.UPDATE_PASSWORD); + return Flows.forms(realm, request, uriInfo).setError("emailError").forwardToPasswordReset(); } Set requiredActions = new HashSet(user.getRequiredActions()); @@ -253,7 +252,8 @@ public class AccountService { new EmailSender().sendPasswordReset(user, realm, accessCode, uriInfo); - return Flows.forms(realm, request, uriInfo).forwardToPasswordReset(); + return Flows.forms(realm, request, uriInfo).setError("emailSent").setErrorType(FormFlows.ErrorType.SUCCESS) + .forwardToPasswordReset(); } @Path("email-verification") diff --git a/services/src/main/java/org/keycloak/services/resources/flows/FormFlows.java b/services/src/main/java/org/keycloak/services/resources/flows/FormFlows.java index 0c106d03f2..aac82602b5 100755 --- a/services/src/main/java/org/keycloak/services/resources/flows/FormFlows.java +++ b/services/src/main/java/org/keycloak/services/resources/flows/FormFlows.java @@ -53,7 +53,12 @@ public class FormFlows { public static final String SOCIAL_REGISTRATION = "socialRegistration"; public static final String CODE = "code"; + // TODO refactor/rename "error" to "message" everywhere where it makes sense private String error; + + public static enum ErrorType {SUCCESS, WARNING, ERROR}; + private ErrorType errorType; + private MultivaluedMap formData; private RealmModel realm; @@ -98,6 +103,7 @@ public class FormFlows { private Response forwardToForm(String template) { FormService.FormServiceDataBean formDataBean = new FormService.FormServiceDataBean(realm, userModel, formData, error); + formDataBean.setErrorType(errorType == null ? ErrorType.ERROR : errorType); // Getting URI needed by form processing service ResteasyUriInfo uriInfo = request.getUri(); @@ -172,6 +178,11 @@ public class FormFlows { return this; } + public FormFlows setErrorType(ErrorType errorType) { + this.errorType = errorType; + return this; + } + public FormFlows setUser(UserModel userModel) { this.userModel = userModel; return this; diff --git a/testsuite/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java b/testsuite/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java index 8dd56e34a1..9b5ca0f1b9 100644 --- a/testsuite/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java +++ b/testsuite/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java @@ -85,12 +85,14 @@ public class ResetPasswordTest { resetPasswordPage.assertCurrent(); + Assert.assertEquals("Success!", resetPasswordPage.getMessage()); + Assert.assertEquals(1, greenMail.getReceivedMessages().length); MimeMessage message = greenMail.getReceivedMessages()[0]; String body = (String) message.getContent(); - String changePasswordUrl = body.split("\n")[0]; + String changePasswordUrl = body.split("\n")[3]; driver.navigate().to(changePasswordUrl.trim()); @@ -109,4 +111,34 @@ public class ResetPasswordTest { Assert.assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); } + @Test + public void resetPasswordWrongUsername() throws IOException, MessagingException { + loginPage.open(); + loginPage.resetPassword(); + + resetPasswordPage.assertCurrent(); + + resetPasswordPage.changePassword("invalid", "test-user@localhost"); + + resetPasswordPage.assertCurrent(); + + Assert.assertNotEquals("Success!", resetPasswordPage.getMessage()); + Assert.assertEquals("Error!", resetPasswordPage.getMessage()); + } + + @Test + public void resetPasswordWrongEmail() throws IOException, MessagingException { + loginPage.open(); + loginPage.resetPassword(); + + resetPasswordPage.assertCurrent(); + + resetPasswordPage.changePassword("test-user@localhost", "invalid"); + + resetPasswordPage.assertCurrent(); + + Assert.assertNotEquals("Success!", resetPasswordPage.getMessage()); + Assert.assertEquals("Error!", resetPasswordPage.getMessage()); + } + } diff --git a/testsuite/src/test/java/org/keycloak/testsuite/pages/LoginPasswordResetPage.java b/testsuite/src/test/java/org/keycloak/testsuite/pages/LoginPasswordResetPage.java index 94edee87cc..45fc30a512 100644 --- a/testsuite/src/test/java/org/keycloak/testsuite/pages/LoginPasswordResetPage.java +++ b/testsuite/src/test/java/org/keycloak/testsuite/pages/LoginPasswordResetPage.java @@ -38,6 +38,9 @@ public class LoginPasswordResetPage extends Page { @FindBy(css = "input[type=\"submit\"]") private WebElement submitButton; + @FindBy(css = ".feedback > p > strong") + private WebElement emailErrorMessage; + public void changePassword(String username, String email) { usernameInput.sendKeys(username); emailInput.sendKeys(email); @@ -46,11 +49,15 @@ public class LoginPasswordResetPage extends Page { } public boolean isCurrent() { - return driver.getTitle().equals("Reset password"); + return driver.getTitle().equals("Forgot Your Password?"); } public void open() { throw new UnsupportedOperationException(); } + public String getMessage() { + return emailErrorMessage != null ? emailErrorMessage.getText() : null; + } + }