Added email verification while creating user and sending email (#1208)

app/server/appsmith-server/pom.xml

@@ -124,6 +124,12 @@
  <artifactId>commons-io</artifactId>
  <version>2.6</version>
  </dependency>
+ <dependency>
+ <groupId>commons-validator</groupId>
+ <artifactId>commons-validator</artifactId>
+ <version>1.7</version>
+ </dependency>
+
 
  <!-- Plugin dependencies -->
  <!-- This has to be declared BEFORE the com.appsmith:interfaces dependency. -->

app/server/appsmith-server/src/main/java/com/appsmith/server/domains/User.java

@@ -15,6 +15,7 @@
 import org.springframework.security.oauth2.core.oidc.user.OidcUser;
 import org.springframework.util.StringUtils;
 
+import javax.validation.constraints.Email;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -30,6 +31,7 @@ public class User extends BaseDomain implements UserDetails, OidcUser {
 
  private String name;
 
+ @Email
  private String email;
 
  //TODO: This is deprecated in favour of groups

app/server/appsmith-server/src/main/java/com/appsmith/server/notifications/EmailSender.java

@@ -5,6 +5,7 @@
 import com.github.mustachejava.Mustache;
 import com.github.mustachejava.MustacheFactory;
 import lombok.extern.slf4j.Slf4j;
+import org.apache.commons.validator.routines.EmailValidator;
 import org.springframework.mail.MailException;
 import org.springframework.mail.javamail.JavaMailSender;
 import org.springframework.mail.javamail.MimeMessageHelper;
@@ -20,24 +21,19 @@
 import java.io.StringWriter;
 import java.io.UnsupportedEncodingException;
 import java.util.Map;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 
 @Component
 @Slf4j
 public class EmailSender {
 
- final JavaMailSender javaMailSender;
+ private final JavaMailSender javaMailSender;
 
- final EmailConfig emailConfig;
+ private final EmailConfig emailConfig;
 
  private final InternetAddress MAIL_FROM;
 
  private final InternetAddress REPLY_TO;
 
- public static final Pattern VALID_EMAIL_ADDRESS_REGEX =
- Pattern.compile("^[A-Z0-9._%+-]+@[A-Z0-9.-]+\\.[A-Z]{2,6}$", Pattern.CASE_INSENSITIVE);
-
  public EmailSender(JavaMailSender javaMailSender, EmailConfig emailConfig) {
  this.javaMailSender = javaMailSender;
  this.emailConfig = emailConfig;
@@ -47,8 +43,7 @@ public EmailSender(JavaMailSender javaMailSender, EmailConfig emailConfig) {
  }
 
  private static boolean validateEmail(String emailStr) {
- Matcher matcher = VALID_EMAIL_ADDRESS_REGEX.matcher(emailStr);
- return matcher.find();
+ return EmailValidator.getInstance().isValid(emailStr);
  }
 
  public Mono<String> sendMail(String to, String subject, String text, Map<String, String> params) {
@@ -64,9 +59,7 @@ public Mono<String> sendMail(String to, String subject, String text, Map<String,
  // to implement a fire-and-forget strategy.
  // CAUTION : We may run into scenarios where too many tasks have been created and queued and the master tasks have already exited with success.
  .doOnNext(emailBody ->
- Mono.fromRunnable(() -> {
- sendMailSync(to, subject, emailBody);
- })
+ Mono.fromRunnable(() -> sendMailSync(to, subject, emailBody))
  // Scheduling using boundedElastic because the number of active tasks are capped
  // and hence not allowing the background threads to grow indefinitely
  .subscribeOn(Schedulers.boundedElastic())

app/server/appsmith-server/src/test/java/com/appsmith/server/controllers/UserControllerTest.java

@@ -0,0 +1,86 @@
+package com.appsmith.server.controllers;
+
+import com.appsmith.server.configurations.SecurityTestConfig;
+import com.appsmith.server.services.SessionUserService;
+import com.appsmith.server.services.UserOrganizationService;
+import com.appsmith.server.services.UserService;
+import com.appsmith.server.solutions.UserSignup;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.autoconfigure.web.reactive.WebFluxTest;
+import org.springframework.boot.test.mock.mockito.MockBean;
+import org.springframework.context.annotation.Import;
+import org.springframework.http.MediaType;
+import org.springframework.security.test.context.support.WithMockUser;
+import org.springframework.test.context.junit4.SpringRunner;
+import org.springframework.test.web.reactive.server.WebTestClient;
+import org.springframework.web.reactive.function.BodyInserters;
+
+import java.util.Arrays;
+import java.util.List;
+
+@RunWith(SpringRunner.class)
+@WebFluxTest(UserController.class)
+@Import(SecurityTestConfig.class)
+public class UserControllerTest {
+ @Autowired
+ private WebTestClient webTestClient;
+
+ @MockBean
+ private UserService userService;
+
+ @MockBean
+ private SessionUserService sessionUserService;
+
+ @MockBean
+ private UserOrganizationService userOrganizationService;
+
+ @MockBean
+ private UserSignup userSignup;
+
+ @Test
+ @WithMockUser
+ public void createUserWithInvalidEmailAddress() {
+ List<String> invalidAddresses = Arrays.asList(
+ "plainaddress",
+ "#@%^%#$@#$@#.com",
+ "@example.com",
+ "Joe Smith <[email protected]>",
+ "email.example.com",
+ "email@[email protected]",
+ "[email protected]",
+ "[email protected]",
+ "[email protected]",
+ "[email protected] (Joe Smith)",
+ "[email protected]",
+ "[email protected]",
+ "[email protected]"
+ );
+ for (String invalidAddress : invalidAddresses) {
+ try {
+ webTestClient.post().uri("/api/v1/users").
+ contentType(MediaType.APPLICATION_JSON).
+ body(BodyInserters.fromValue(String.format("{\"name\":\"test-name\"," +
+ "\"email\":\"%s\",\"password\":\"test-password\"}", invalidAddress))).
+ exchange().
+ expectStatus().isEqualTo(400).
+ expectBody().json("{\n" +
+ " \"responseMeta\": {\n" +
+ " \"status\": 400,\n" +
+ " \"success\": false,\n" +
+ " \"error\": {\n" +
+ " \"code\": 4028,\n" +
+ " \"message\": \"Validation Failure(s): {email=must be a well-formed email address}\"\n" +
+ " }\n" +
+ " }\n" +
+ "}");
+ } catch (Throwable exc) {
+ System.out.println("******************************");
+ System.out.println(String.format("Failed for >>> %s", invalidAddress));
+ System.out.println("******************************");
+ throw exc;
+ }
+ }
+ }
+}

app/server/appsmith-server/src/test/java/com/appsmith/server/notifications/EmailSenderTest.java

@@ -0,0 +1,68 @@
+package com.appsmith.server.notifications;
+
+import com.appsmith.server.configurations.EmailConfig;
+import lombok.extern.slf4j.Slf4j;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.boot.test.mock.mockito.MockBean;
+import org.springframework.mail.javamail.JavaMailSender;
+import org.springframework.test.annotation.DirtiesContext;
+import org.springframework.test.context.junit4.SpringRunner;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.verifyNoInteractions;
+
+@Slf4j
+@RunWith(SpringRunner.class)
+@SpringBootTest
+@DirtiesContext
+public class EmailSenderTest {
+ @MockBean
+ private JavaMailSender javaMailSender;
+
+ @MockBean
+ private EmailConfig emailConfig;
+
+ @Autowired
+ private EmailSender emailSender;
+
+ @Test
+ public void itShouldNotSendMailsWithInvalidAddresses() {
+ //noinspection ResultOfMethodCallIgnored
+ doReturn(true).when(emailConfig).isEmailEnabled();
+ List<String> invalidAddresses = Arrays.asList(
+ "plainaddress",
+ "#@%^%#$@#$@#.com",
+ "@example.com",
+ "Joe Smith <[email protected]>",
+ "email.example.com",
+ "email@[email protected]",
+ "[email protected]",
+ "[email protected]",
+ "[email protected]",
+ "[email protected] (Joe Smith)",
+ "[email protected]",
+ "[email protected]",
+ "[email protected]"
+ );
+
+ for (String invalidAddress : invalidAddresses) {
+ try {
+ emailSender.sendMail(invalidAddress, "test-subject", "email/welcomeUserTemplate.html", Collections.emptyMap()).block();
+
+ verifyNoInteractions(javaMailSender);
+ } catch (Throwable exc) {
+ System.out.println("******************************");
+ System.out.println(String.format("Failed for >>> %s", invalidAddress));
+ System.out.println("******************************");
+ throw exc;
+ }
+ }
+ }
+}