diff --git a/forms/src/main/resources/org/keycloak/forms/messages.properties b/forms/src/main/resources/org/keycloak/forms/messages.properties index aa13cf8499..5353df4293 100644 --- a/forms/src/main/resources/org/keycloak/forms/messages.properties +++ b/forms/src/main/resources/org/keycloak/forms/messages.properties @@ -59,6 +59,7 @@ errorHeader=Error! emailForgotHeader=Forgot Your Password? emailUpdateHeader=Update password emailSent=You should receive an email shortly with further instructions. +emailSendError=Failed to send email, please try again later emailError=Invalid username or email. emailErrorInfo=Please, fill in the fields again. emailInstruction=Enter your username and email address and we will send you instructions on how to create a new password. diff --git a/services/src/main/java/org/keycloak/services/email/EmailException.java b/services/src/main/java/org/keycloak/services/email/EmailException.java new file mode 100644 index 0000000000..d15c022b9b --- /dev/null +++ b/services/src/main/java/org/keycloak/services/email/EmailException.java @@ -0,0 +1,12 @@ +package org.keycloak.services.email; + +/** + * @author Stian Thorgersen + */ +public class EmailException extends Exception { + + public EmailException(Throwable cause) { + super(cause); + } + +} 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 185ca837ae..b2f72ce32e 100755 --- a/services/src/main/java/org/keycloak/services/email/EmailSender.java +++ b/services/src/main/java/org/keycloak/services/email/EmailSender.java @@ -54,7 +54,7 @@ public class EmailSender { this.config = config; } - public void send(String address, String subject, String body) throws MessagingException { + public void send(String address, String subject, String body) throws EmailException { Properties props = new Properties(); props.setProperty("mail.smtp.host", config.get("host")); @@ -81,24 +81,28 @@ public class EmailSender { String from = config.get("from"); - Session session = Session.getInstance(props); + try { + Session session = Session.getInstance(props); - Message msg = new MimeMessage(session); - msg.setFrom(new InternetAddress(from)); - msg.setSubject(subject); - msg.setText(body); - msg.saveChanges(); + Message msg = new MimeMessage(session); + msg.setFrom(new InternetAddress(from)); + msg.setSubject(subject); + msg.setText(body); + msg.saveChanges(); - Transport transport = session.getTransport("smtp"); - if (auth) { - transport.connect(config.get("user"), config.get("password")); - } else { - transport.connect(); + Transport transport = session.getTransport("smtp"); + if (auth) { + transport.connect(config.get("user"), config.get("password")); + } else { + transport.connect(); + } + transport.sendMessage(msg, new InternetAddress[]{new InternetAddress(address)}); + } catch (Exception e) { + throw new EmailException(e); } - transport.sendMessage(msg, new InternetAddress[] { new InternetAddress(address) }); } - public void sendEmailVerification(UserModel user, RealmModel realm, AccessCodeEntry accessCode, UriInfo uriInfo) { + public void sendEmailVerification(UserModel user, RealmModel realm, AccessCodeEntry accessCode, UriInfo uriInfo) throws EmailException { UriBuilder builder = Urls.loginActionEmailVerificationBuilder(uriInfo.getBaseUri()); builder.queryParam("key", accessCode.getId()); @@ -115,14 +119,10 @@ public class EmailSender { sb.append("Thanks,\n"); sb.append("The Keycloak Team"); - try { - send(user.getEmail(), "Verify email", sb.toString()); - } catch (Exception e1) { - log.warn("Failed to send email verification"); - } + send(user.getEmail(), "Verify email", sb.toString()); } - public void sendPasswordReset(UserModel user, RealmModel realm, AccessCodeEntry accessCode, UriInfo uriInfo) { + public void sendPasswordReset(UserModel user, RealmModel realm, AccessCodeEntry accessCode, UriInfo uriInfo) throws EmailException { UriBuilder builder = Urls.loginPasswordResetBuilder(uriInfo.getBaseUri()); builder.queryParam("key", accessCode.getId()); @@ -140,11 +140,7 @@ public class EmailSender { sb.append("Thanks,\n"); sb.append("The Keycloak Team"); - try { - send(user.getEmail(), "Reset password link", sb.toString()); - } catch (Exception e) { - log.warn("Failed to send reset password link", e); - } + 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 a62a0647cc..a9fb6de4f2 100755 --- a/services/src/main/java/org/keycloak/services/resources/AccountService.java +++ b/services/src/main/java/org/keycloak/services/resources/AccountService.java @@ -46,7 +46,6 @@ import javax.ws.rs.*; import javax.ws.rs.core.*; import javax.ws.rs.ext.Providers; import java.net.URI; -import java.net.URISyntaxException; import java.util.List; /** @@ -175,8 +174,7 @@ public class AccountService { user.setLastName(formData.getFirst("lastName")); user.setEmail(formData.getFirst("email")); - return Flows.forms(realm, request, uriInfo).setUser(user).setError("accountUpdated") - .setErrorType(FormFlows.MessageType.SUCCESS).forwardToAccount(); + return Flows.forms(realm, request, uriInfo).setUser(user).setSuccess("accountUpdated").forwardToAccount(); } @Path("totp-remove") @@ -190,8 +188,7 @@ public class AccountService { UserModel user = auth.getUser(); user.setTotp(false); - return Flows.forms(realm, request, uriInfo).setError("successTotpRemoved").setErrorType(FormFlows.MessageType.SUCCESS) - .setUser(user).forwardToTotp(); + return Flows.forms(realm, request, uriInfo).setSuccess("successTotpRemoved").setUser(user).forwardToTotp(); } @Path("totp") @@ -222,8 +219,7 @@ public class AccountService { user.setTotp(true); - return Flows.forms(realm, request, uriInfo).setError("successTotp").setErrorType(FormFlows.MessageType.SUCCESS) - .setUser(user).forwardToTotp(); + return Flows.forms(realm, request, uriInfo).setSuccess("successTotp").setUser(user).forwardToTotp(); } @Path("password") @@ -266,8 +262,7 @@ public class AccountService { realm.updateCredential(user, credentials); - return Flows.forms(realm, request, uriInfo).setUser(user).setError("accountPasswordUpdated") - .setErrorType(FormFlows.MessageType.SUCCESS).forwardToPassword(); + return Flows.forms(realm, request, uriInfo).setUser(user).setSuccess("accountPasswordUpdated").forwardToPassword(); } @Path("login-redirect") diff --git a/services/src/main/java/org/keycloak/services/resources/RequiredActionsService.java b/services/src/main/java/org/keycloak/services/resources/RequiredActionsService.java index 3199c01847..8faeb01982 100755 --- a/services/src/main/java/org/keycloak/services/resources/RequiredActionsService.java +++ b/services/src/main/java/org/keycloak/services/resources/RequiredActionsService.java @@ -31,6 +31,7 @@ import org.keycloak.models.UserModel; import org.keycloak.models.UserModel.RequiredAction; import org.keycloak.models.utils.TimeBasedOTP; import org.keycloak.representations.idm.CredentialRepresentation; +import org.keycloak.services.email.EmailException; import org.keycloak.services.email.EmailSender; import org.keycloak.services.managers.AccessCodeEntry; import org.keycloak.services.managers.AuthenticationManager; @@ -265,10 +266,14 @@ public class RequiredActionsService { accessCode.setRequiredActions(requiredActions); accessCode.setExpiration(System.currentTimeMillis() / 1000 + realm.getAccessCodeLifespanUserAction()); - new EmailSender(realm.getSmtpConfig()).sendPasswordReset(user, realm, accessCode, uriInfo); + try { + new EmailSender(realm.getSmtpConfig()).sendPasswordReset(user, realm, accessCode, uriInfo); + } catch (EmailException e) { + logger.error("Failed to send password reset email", e); + return Flows.forms(realm, request, uriInfo).setError("emailSendError").forwardToErrorPage(); + } - return Flows.forms(realm, request, uriInfo).setError("emailSent").setErrorType(FormFlows.MessageType.SUCCESS) - .forwardToPasswordReset(); + return Flows.forms(realm, request, uriInfo).setSuccess("emailSent").forwardToPasswordReset(); } private AccessCodeEntry getAccessCodeEntry(RequiredAction requiredAction) { diff --git a/services/src/main/java/org/keycloak/services/resources/SocialResource.java b/services/src/main/java/org/keycloak/services/resources/SocialResource.java index 908844c2a3..9877beefa1 100755 --- a/services/src/main/java/org/keycloak/services/resources/SocialResource.java +++ b/services/src/main/java/org/keycloak/services/resources/SocialResource.java @@ -27,7 +27,6 @@ import org.jboss.resteasy.spi.HttpRequest; import org.jboss.resteasy.spi.HttpResponse; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; -import org.keycloak.models.RoleModel; import org.keycloak.models.SocialLinkModel; import org.keycloak.models.UserModel; import org.keycloak.services.managers.AuthenticationManager; 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 e95c3aa094..fea4f982b6 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 @@ -28,6 +28,7 @@ import org.keycloak.models.RoleModel; import org.keycloak.models.UserModel; import org.keycloak.models.UserModel.RequiredAction; import org.keycloak.services.FormService; +import org.keycloak.services.email.EmailException; import org.keycloak.services.email.EmailSender; import org.keycloak.services.managers.AccessCodeEntry; import org.keycloak.services.messages.Messages; @@ -51,8 +52,7 @@ public class FormFlows { public static final String CODE = "code"; - // TODO refactor/rename "error" to "message" everywhere where it makes sense - private String error; + private String message; public static enum MessageType {SUCCESS, WARNING, ERROR}; private MessageType messageType = MessageType.ERROR; @@ -86,7 +86,11 @@ public class FormFlows { case UPDATE_PASSWORD: return forwardToActionForm(Pages.LOGIN_UPDATE_PASSWORD, Messages.ACTION_WARN_PASSWD); case VERIFY_EMAIL: - new EmailSender(realm.getSmtpConfig()).sendEmailVerification(userModel, realm, accessCode, uriInfo); + try { + new EmailSender(realm.getSmtpConfig()).sendEmailVerification(userModel, realm, accessCode, uriInfo); + } catch (EmailException e) { + return setError("emailSendError").forwardToErrorPage(); + } return forwardToActionForm(Pages.LOGIN_VERIFY_EMAIL, Messages.ACTION_WARN_EMAIL); default: return Response.serverError().build(); @@ -146,7 +150,7 @@ public class FormFlows { public Response forwardToForm(String template) { - FormService.FormServiceDataBean formDataBean = new FormService.FormServiceDataBean(realm, userModel, formData, queryParams, error); + FormService.FormServiceDataBean formDataBean = new FormService.FormServiceDataBean(realm, userModel, formData, queryParams, message); formDataBean.setMessageType(messageType); return forwardToForm(template, formDataBean); @@ -156,9 +160,9 @@ public class FormFlows { // If no other message is set, notify user about required action in the warning window // so it's clear that this is a req. action form not a login form - if (error == null){ + if (message == null){ messageType = MessageType.WARNING; - error = warningSummary; + message = warningSummary; } return forwardToForm(template); @@ -198,7 +202,7 @@ public class FormFlows { public Response forwardToOAuthGrant(){ - FormService.FormServiceDataBean formDataBean = new FormService.FormServiceDataBean(realm, userModel, formData, queryParams, error); + FormService.FormServiceDataBean formDataBean = new FormService.FormServiceDataBean(realm, userModel, formData, queryParams, message); formDataBean.setOAuthRealmRolesRequested((List) request.getAttribute("realmRolesRequested")); formDataBean.setOAuthResourceRolesRequested((MultivaluedMap) request.getAttribute("resourceRolesRequested")); @@ -222,13 +226,21 @@ public class FormFlows { return this; } - public FormFlows setError(String error) { - this.error = error; + public FormFlows setError(String message) { + this.message = message; + this.messageType = MessageType.ERROR; return this; } - public FormFlows setErrorType(MessageType errorType) { - this.messageType = errorType; + public FormFlows setSuccess(String message) { + this.message = message; + this.messageType = MessageType.SUCCESS; + return this; + } + + public FormFlows setWarning(String message) { + this.message = message; + this.messageType = MessageType.WARNING; return this; } diff --git a/services/src/test/java/org/keycloak/services/email/EmailSenderTest.java b/services/src/test/java/org/keycloak/services/email/EmailSenderTest.java index e749f16116..fa76623f90 100755 --- a/services/src/test/java/org/keycloak/services/email/EmailSenderTest.java +++ b/services/src/test/java/org/keycloak/services/email/EmailSenderTest.java @@ -55,7 +55,7 @@ public class EmailSenderTest { } @Test - public void sendMail() throws MessagingException, IOException { + public void sendMail() throws Exception { emailSender.send("test@test.com", "Test subject", "Test body"); MimeMessage[] receivedMessages = greenMail.getReceivedMessages();