From 1f7630218f5aee31401640767d58812c85ce4f18 Mon Sep 17 00:00:00 2001 From: Areeb Jamal Date: Mon, 3 Sep 2018 12:19:06 +0530 Subject: [PATCH] fix: Handle specific exceptions while email verification (#191) --- .../verification/DuplicateEmailException.java | 8 ++++ .../EmailVerificationController.java | 12 +++-- .../EmailVerificationService.java | 44 ++++++++++++------- .../RecentVerificationException.java | 29 ++++++++++++ .../TokenVerificationException.java | 9 ++++ .../DuplicateEmailExceptionTest.java | 29 ++++++++++++ .../RecentVerificationExceptionTest.java | 35 +++++++++++++++ 7 files changed, 145 insertions(+), 21 deletions(-) create mode 100644 src/main/java/amu/zhcet/auth/verification/RecentVerificationException.java create mode 100644 src/main/java/amu/zhcet/auth/verification/TokenVerificationException.java create mode 100644 src/test/java/amu/zhcet/auth/verification/DuplicateEmailExceptionTest.java create mode 100644 src/test/java/amu/zhcet/auth/verification/RecentVerificationExceptionTest.java diff --git a/src/main/java/amu/zhcet/auth/verification/DuplicateEmailException.java b/src/main/java/amu/zhcet/auth/verification/DuplicateEmailException.java index 149282ff..60fc5d66 100644 --- a/src/main/java/amu/zhcet/auth/verification/DuplicateEmailException.java +++ b/src/main/java/amu/zhcet/auth/verification/DuplicateEmailException.java @@ -1,9 +1,17 @@ package amu.zhcet.auth.verification; +import lombok.Data; +import lombok.EqualsAndHashCode; + +@Data +@EqualsAndHashCode(callSuper = false) public class DuplicateEmailException extends RuntimeException { + private String email; + public DuplicateEmailException(String email) { super("'" + email + "' is already registered by another user"); + this.email = email; } } diff --git a/src/main/java/amu/zhcet/auth/verification/EmailVerificationController.java b/src/main/java/amu/zhcet/auth/verification/EmailVerificationController.java index f7984bbf..e53444cf 100644 --- a/src/main/java/amu/zhcet/auth/verification/EmailVerificationController.java +++ b/src/main/java/amu/zhcet/auth/verification/EmailVerificationController.java @@ -33,9 +33,13 @@ private void sendVerificationLink(String email, RedirectAttributes redirectAttri } catch (DuplicateEmailException de) { log.warn("Duplicate Email", de); redirectAttributes.addFlashAttribute("email_error", de.getMessage()); + } catch (RecentVerificationException re) { + log.warn("Recently Sent Email", re); + redirectAttributes.addFlashAttribute("email_error", RecentVerificationException.MESSAGE); } catch (RuntimeException re) { log.warn("Error sending verification link", re); - redirectAttributes.addFlashAttribute("email_error", re.getMessage()); + redirectAttributes.addFlashAttribute("email_error", + "There was some error sending the email"); } } @@ -77,9 +81,9 @@ public String verifyEmail(Model model, @RequestParam("auth") String token) { try { emailVerificationService.verifyEmail(token); model.addAttribute("success", "Your email was successfully verified!"); - } catch (IllegalStateException | DuplicateEmailException ie) { - log.warn("Email Verification Error {}", ie.getMessage()); - model.addAttribute("error", ie.getMessage()); + } catch (DuplicateEmailException | TokenVerificationException de) { + log.warn("Email Verification Error {}", de.getMessage()); + model.addAttribute("error", de.getMessage()); } return "user/verify_email"; diff --git a/src/main/java/amu/zhcet/auth/verification/EmailVerificationService.java b/src/main/java/amu/zhcet/auth/verification/EmailVerificationService.java index 008ce64c..b035a27c 100644 --- a/src/main/java/amu/zhcet/auth/verification/EmailVerificationService.java +++ b/src/main/java/amu/zhcet/auth/verification/EmailVerificationService.java @@ -9,6 +9,7 @@ import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.ApplicationEventPublisher; +import org.springframework.mail.MailException; import org.springframework.stereotype.Service; import javax.transaction.Transactional; @@ -45,8 +46,9 @@ public EmailVerificationService(UserService userService, LinkMailService linkMai private VerificationToken createVerificationToken(String email) { User user = userService.getLoggedInUser().orElseThrow(() -> new IllegalStateException("No user logged in")); // Check if link was already sent - if (emailCache.getIfPresent(user.getUserId()) != null) - throw new RuntimeException("Verification link was recently sent. Please wait for some time"); + LocalDateTime sentTime = emailCache.getIfPresent(user.getUserId()); + if (sentTime != null) + throw new RecentVerificationException(sentTime); emailCache.put(user.getUserId(), LocalDateTime.now()); @@ -101,7 +103,7 @@ public void generate(String email) { /** * Retrieves the verification token from database and validates it by performing checks * that it exists, is not already used and not expired. If any of the validation fails, - * throws an {@link IllegalStateException} with the corresponding message + * throws an {@link TokenVerificationException} with the corresponding message * @param token String token ID * @return {@link VerificationToken} */ @@ -109,14 +111,14 @@ private VerificationToken getAndValidateOrThrow(String token) { VerificationToken verificationToken = verificationTokenRepository.findByToken(token); if (verificationToken == null) - throw new IllegalStateException("Token: " + token + " is invalid"); + throw new TokenVerificationException("Token: " + token + " is invalid"); if (verificationToken.isUsed()) - throw new IllegalStateException("Token: " + token + " is already used! Please request another link!"); + throw new TokenVerificationException("Token: " + token + " is already used! Please request another link!"); Calendar cal = Calendar.getInstance(); if ((verificationToken.getExpiry().getTime() - cal.getTime().getTime()) <= 0) - throw new IllegalStateException("Token: " + token + " has expired"); + throw new TokenVerificationException("Token: " + token + " has expired"); return verificationToken; } @@ -186,17 +188,25 @@ public void sendMail(VerificationToken token) { String relativeUrl = "/login/email/verify?auth=" + token.getToken(); log.debug("Verification link generated : {}", relativeUrl); - linkMailService.sendEmail(getPayLoad(token.getEmail(), token.getUser(), relativeUrl), false); - - // Now we set the email to the user and disable email verified - User user = token.getUser(); - log.debug("Saving user email {} -> {}", user.getUserId(), token.getEmail()); - user.setEmail(token.getEmail()); - user.setEmailVerified(false); - - userService.save(user); - log.debug("Saved user email"); - eventPublisher.publishEvent(new EmailVerifiedEvent(user)); + try { + linkMailService.sendEmail(getPayLoad(token.getEmail(), token.getUser(), relativeUrl), false); + // Now we set the email to the user and disable email verified + User user = token.getUser(); + log.debug("Saving user email {} -> {}", user.getUserId(), token.getEmail()); + user.setEmail(token.getEmail()); + user.setEmailVerified(false); + + userService.save(user); + log.debug("Saved user email"); + eventPublisher.publishEvent(new EmailVerifiedEvent(user)); + } catch (MailException mailException) { + // There was an error sending the mail, so remove the token and sent time + verificationTokenRepository.delete(token); + emailCache.invalidate(token.getUser().getUserId()); + log.warn("Email sending for {} '{}' failed, so we removed the verification token", + token.getUser(), token.getEmail(), mailException); + throw mailException; + } } } diff --git a/src/main/java/amu/zhcet/auth/verification/RecentVerificationException.java b/src/main/java/amu/zhcet/auth/verification/RecentVerificationException.java new file mode 100644 index 00000000..72412f72 --- /dev/null +++ b/src/main/java/amu/zhcet/auth/verification/RecentVerificationException.java @@ -0,0 +1,29 @@ +package amu.zhcet.auth.verification; + +import lombok.Data; +import lombok.EqualsAndHashCode; +import org.springframework.lang.Nullable; + +import java.time.LocalDateTime; + +@Data +@EqualsAndHashCode(callSuper = false) +public class RecentVerificationException extends RuntimeException { + + public static final String MESSAGE = "Verification link was recently sent. Please wait for some time"; + + @Nullable + private LocalDateTime sentTime; + + public RecentVerificationException() { + super(MESSAGE); + } + + public RecentVerificationException(LocalDateTime sentTime) { + super(String.format( + "Verification link was recently sent at %s. Please wait for some time", + sentTime.toString())); + this.sentTime = sentTime; + } + +} diff --git a/src/main/java/amu/zhcet/auth/verification/TokenVerificationException.java b/src/main/java/amu/zhcet/auth/verification/TokenVerificationException.java new file mode 100644 index 00000000..402a2bc2 --- /dev/null +++ b/src/main/java/amu/zhcet/auth/verification/TokenVerificationException.java @@ -0,0 +1,9 @@ +package amu.zhcet.auth.verification; + +public class TokenVerificationException extends IllegalStateException { + + public TokenVerificationException(String message) { + super(message); + } + +} diff --git a/src/test/java/amu/zhcet/auth/verification/DuplicateEmailExceptionTest.java b/src/test/java/amu/zhcet/auth/verification/DuplicateEmailExceptionTest.java new file mode 100644 index 00000000..644309f9 --- /dev/null +++ b/src/test/java/amu/zhcet/auth/verification/DuplicateEmailExceptionTest.java @@ -0,0 +1,29 @@ +package amu.zhcet.auth.verification; + +import org.junit.Before; +import org.junit.Test; + +import static org.junit.Assert.*; + +public class DuplicateEmailExceptionTest { + + private DuplicateEmailException duplicateEmailException; + + @Before + public void setUp() { + duplicateEmailException = new DuplicateEmailException("apop@gmail.com"); + } + + @Test + public void testConstructorMessage() { + assertEquals( + "'apop@gmail.com' is already registered by another user", + duplicateEmailException.getMessage()); + } + + @Test + public void testGettingEmail() { + assertEquals("apop@gmail.com", duplicateEmailException.getEmail()); + } + +} \ No newline at end of file diff --git a/src/test/java/amu/zhcet/auth/verification/RecentVerificationExceptionTest.java b/src/test/java/amu/zhcet/auth/verification/RecentVerificationExceptionTest.java new file mode 100644 index 00000000..55780be7 --- /dev/null +++ b/src/test/java/amu/zhcet/auth/verification/RecentVerificationExceptionTest.java @@ -0,0 +1,35 @@ +package amu.zhcet.auth.verification; + +import org.junit.Test; + +import java.time.LocalDateTime; + +import static org.junit.Assert.*; + +public class RecentVerificationExceptionTest { + + private LocalDateTime localDateTime = LocalDateTime.of(2018, 9, 1, 12, 23); + + @Test + public void testNoConstructorMessage() { + RecentVerificationException recentVerificationException = new RecentVerificationException(); + assertEquals( + "Verification link was recently sent. Please wait for some time", + recentVerificationException.getMessage()); + } + + @Test + public void testConstructorMessage() { + RecentVerificationException recentVerificationException = new RecentVerificationException(localDateTime); + assertEquals( + "Verification link was recently sent at 2018-09-01T12:23. Please wait for some time", + recentVerificationException.getMessage()); + } + + @Test + public void testGettingSentTime() { + RecentVerificationException recentVerificationException = new RecentVerificationException(localDateTime); + assertEquals(localDateTime, recentVerificationException.getSentTime()); + } + +} \ No newline at end of file