diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/User.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/User.java index d1536f3ffd..8a9ca549a4 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/User.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/User.java @@ -15,7 +15,6 @@ import org.springframework.security.oauth2.core.oidc.OidcUserInfo; 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; @@ -31,7 +30,6 @@ public class User extends BaseDomain implements UserDetails, OidcUser { private String name; - @Email private String email; //TODO: This is deprecated in favour of groups diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ValidationUtils.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ValidationUtils.java new file mode 100644 index 0000000000..dc4b650878 --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ValidationUtils.java @@ -0,0 +1,10 @@ +package com.appsmith.server.helpers; + +import org.apache.commons.validator.routines.EmailValidator; + +public final class ValidationUtils { + + public static boolean validateEmail(String emailStr) { + return EmailValidator.getInstance().isValid(emailStr); + } +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/notifications/EmailSender.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/notifications/EmailSender.java index 553cafd05f..6c17e6bf2b 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/notifications/EmailSender.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/notifications/EmailSender.java @@ -5,7 +5,6 @@ import com.github.mustachejava.DefaultMustacheFactory; 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; @@ -22,6 +21,8 @@ import java.io.StringWriter; import java.io.UnsupportedEncodingException; import java.util.Map; +import static com.appsmith.server.helpers.ValidationUtils.validateEmail; + @Component @Slf4j public class EmailSender { @@ -42,10 +43,6 @@ public class EmailSender { REPLY_TO = makeReplyTo(); } - private static boolean validateEmail(String emailStr) { - return EmailValidator.getInstance().isValid(emailStr); - } - public Mono sendMail(String to, String subject, String text, Map params) { return Mono .fromSupplier(() -> { diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java index 515cdc3ead..54d23655fa 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java @@ -39,6 +39,7 @@ import reactor.core.scheduler.Scheduler; import javax.validation.Validator; import java.net.URLEncoder; import java.nio.charset.StandardCharsets; +import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -575,9 +576,9 @@ public class UserServiceImpl extends BaseService i return Flux.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.ORIGIN)); } - List usernames = inviteUsersDTO.getUsernames(); + List originalUsernames = inviteUsersDTO.getUsernames(); - if (usernames == null || usernames.isEmpty()) { + if (originalUsernames == null || originalUsernames.isEmpty()) { return Flux.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.USERNAMES)); } @@ -585,6 +586,11 @@ public class UserServiceImpl extends BaseService i return Flux.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.ROLE)); } + List usernames = new ArrayList<>(); + for (String username : originalUsernames) { + usernames.add(username.toLowerCase()); + } + Mono currentUserMono = sessionUserService.getCurrentUser().cache(); // Check if the current user has invite permissions @@ -681,7 +687,8 @@ public class UserServiceImpl extends BaseService i private Mono createNewUserAndSendInviteEmail(String email, String originHeader, Map params) { User newUser = new User(); - newUser.setEmail(email); + newUser.setEmail(email.toLowerCase()); + // This is a new user. Till the user signs up, this user would be disabled. newUser.setIsEnabled(false); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/UserSignup.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/UserSignup.java index 80fefc9614..b53bdf212d 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/UserSignup.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/UserSignup.java @@ -27,6 +27,7 @@ import reactor.core.publisher.Mono; import java.net.URI; import java.net.URISyntaxException; +import static com.appsmith.server.helpers.ValidationUtils.validateEmail; import static org.springframework.security.web.server.context.WebSessionServerSecurityContextRepository.DEFAULT_SPRING_SECURITY_CONTEXT_ATTR_NAME; @Component @@ -51,6 +52,11 @@ public class UserSignup { * @return Mono of User, published the saved user object with a non-null value for its `getId()`. */ public Mono signupAndLogin(User user, ServerWebExchange exchange) { + + if (!validateEmail(user.getUsername())) { + return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.EMAIL)); + } + return Mono .zip( userService.createUserAndSendEmail(user, exchange.getRequest().getHeaders().getOrigin()), diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/configurations/SeedMongoData.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/configurations/SeedMongoData.java index 3962f4ce53..d2955a8175 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/configurations/SeedMongoData.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/configurations/SeedMongoData.java @@ -59,8 +59,8 @@ public class SeedMongoData { log.info("Seeding the data"); final String API_USER_EMAIL = "api_user"; final String TEST_USER_EMAIL = "usertest@usertest.com"; - final String ADMIN_USER_EMAIL = "admin@solutionTest.com"; - final String DEV_USER_EMAIL = "developer@solutionTest.com"; + final String ADMIN_USER_EMAIL = "admin@solutiontest.com"; + final String DEV_USER_EMAIL = "developer@solutiontest.com"; Policy manageAppPolicy = Policy.builder().permission(MANAGE_APPLICATIONS.getValue()) .users(Set.of(API_USER_EMAIL)) diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/controllers/UserControllerTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/controllers/UserControllerTest.java deleted file mode 100644 index 795fe93390..0000000000 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/controllers/UserControllerTest.java +++ /dev/null @@ -1,86 +0,0 @@ -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 invalidAddresses = Arrays.asList( - "plainaddress", - "#@%^%#$@#$@#.com", - "@example.com", - "Joe Smith ", - "email.example.com", - "email@example@example.com", - ".email@example.com", - "email.@example.com", - "email..email@example.com", - "email@example.com (Joe Smith)", - "email@-example.com", - "email@example..com", - "Abc..123@example.com" - ); - 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; - } - } - } -} diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/OrganizationServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/OrganizationServiceTest.java index 3bee52a4f5..24891b514b 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/OrganizationServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/OrganizationServiceTest.java @@ -413,20 +413,20 @@ public class OrganizationServiceTest { assertThat(org.getName()).isEqualTo("Another Test Organization"); assertThat(org.getUserRoles().stream() .map(role -> role.getUsername()) - .filter(username -> username.equals("newEmailWhichShouldntExist@usertest.com")) + .filter(username -> username.equals("newemailwhichshouldntexist@usertest.com")) .collect(Collectors.toSet()) ).hasSize(1); Policy manageOrgAppPolicy = Policy.builder().permission(ORGANIZATION_MANAGE_APPLICATIONS.getValue()) - .users(Set.of("api_user", "newEmailWhichShouldntExist@usertest.com")) + .users(Set.of("api_user", "newemailwhichshouldntexist@usertest.com")) .build(); Policy manageOrgPolicy = Policy.builder().permission(MANAGE_ORGANIZATIONS.getValue()) - .users(Set.of("api_user", "newEmailWhichShouldntExist@usertest.com")) + .users(Set.of("api_user", "newemailwhichshouldntexist@usertest.com")) .build(); Policy readOrgPolicy = Policy.builder().permission(READ_ORGANIZATIONS.getValue()) - .users(Set.of("api_user", "newEmailWhichShouldntExist@usertest.com")) + .users(Set.of("api_user", "newemailwhichshouldntexist@usertest.com")) .build(); assertThat(org.getPolicies()).isNotEmpty(); @@ -486,16 +486,16 @@ public class OrganizationServiceTest { assertThat(org).isNotNull(); assertThat(org.getName()).isEqualTo("Add Viewer to Test Organization"); assertThat(org.getUserRoles().stream() - .filter(role -> role.getUsername().equals("newEmailWhichShouldntExistAsViewer@usertest.com")) + .filter(role -> role.getUsername().equals("newemailwhichshouldntexistasviewer@usertest.com")) .collect(Collectors.toSet()) ).hasSize(1); Policy readOrgAppsPolicy = Policy.builder().permission(ORGANIZATION_READ_APPLICATIONS.getValue()) - .users(Set.of("api_user", "newEmailWhichShouldntExistAsViewer@usertest.com")) + .users(Set.of("api_user", "newemailwhichshouldntexistasviewer@usertest.com")) .build(); Policy readOrgPolicy = Policy.builder().permission(READ_ORGANIZATIONS.getValue()) - .users(Set.of("api_user", "newEmailWhichShouldntExistAsViewer@usertest.com")) + .users(Set.of("api_user", "newemailwhichshouldntexistasviewer@usertest.com")) .build(); assertThat(org.getPolicies()).isNotEmpty(); @@ -878,19 +878,19 @@ public class OrganizationServiceTest { .map(userRole -> userRole.getUsername()) .filter(username -> !username.equals("api_user")) .collect(Collectors.toSet()) - ).containsAll(Set.of("newEmailWhichShouldntExistAsViewer1@usertest.com", "newEmailWhichShouldntExistAsViewer2@usertest.com", - "newEmailWhichShouldntExistAsViewer3@usertest.com")); + ).containsAll(Set.of("newemailwhichshouldntexistasviewer1@usertest.com", "newemailwhichshouldntexistasviewer2@usertest.com", + "newemailwhichshouldntexistasviewer3@usertest.com")); Policy readOrgAppsPolicy = Policy.builder().permission(ORGANIZATION_READ_APPLICATIONS.getValue()) - .users(Set.of("api_user", "newEmailWhichShouldntExistAsViewer1@usertest.com", - "newEmailWhichShouldntExistAsViewer2@usertest.com", - "newEmailWhichShouldntExistAsViewer3@usertest.com")) + .users(Set.of("api_user", "newemailwhichshouldntexistasviewer1@usertest.com", + "newemailwhichshouldntexistasviewer2@usertest.com", + "newemailwhichshouldntexistasviewer3@usertest.com")) .build(); Policy readOrgPolicy = Policy.builder().permission(READ_ORGANIZATIONS.getValue()) - .users(Set.of("api_user", "newEmailWhichShouldntExistAsViewer1@usertest.com", - "newEmailWhichShouldntExistAsViewer2@usertest.com", - "newEmailWhichShouldntExistAsViewer3@usertest.com")) + .users(Set.of("api_user", "newemailwhichshouldntexistasviewer1@usertest.com", + "newemailwhichshouldntexistasviewer2@usertest.com", + "newemailwhichshouldntexistasviewer3@usertest.com")) .build(); assertThat(org.getPolicies()).isNotEmpty(); diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceTest.java index f3c607c5b8..322616f863 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceTest.java @@ -13,6 +13,7 @@ import com.appsmith.server.dtos.InviteUsersDTO; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; import com.appsmith.server.repositories.UserRepository; +import com.appsmith.server.solutions.UserSignup; import lombok.extern.slf4j.Slf4j; import org.junit.Before; import org.junit.Test; @@ -30,7 +31,9 @@ import reactor.core.publisher.Mono; import reactor.test.StepVerifier; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashSet; +import java.util.List; import java.util.Set; import static com.appsmith.server.acl.AclPermission.MANAGE_APPLICATIONS; @@ -66,6 +69,9 @@ public class UserServiceTest { Mono organizationMono; + @Autowired + UserSignup userSignup; + @Before public void setup() { userMono = userService.findByEmail("usertest@usertest.com"); @@ -387,5 +393,32 @@ public class UserServiceTest { .verify(); } + + @Test + public void createUserWithInvalidEmailAddress() { + List invalidAddresses = Arrays.asList( + "plainaddress", + "#@%^%#$@#$@#.com", + "@example.com", + "Joe Smith ", + "email.example.com", + "email@example@example.com", + ".email@example.com", + "email.@example.com", + "email..email@example.com", + "email@example.com (Joe Smith)", + "email@-example.com", + "email@example..com", + "Abc..123@example.com" + ); + for (String invalidAddress : invalidAddresses) { + User user = new User(); + user.setEmail(invalidAddress); + user.setPassword("test-password"); + user.setName("test-name"); + StepVerifier.create(userSignup.signupAndLogin(user, null)) + .expectErrorMessage(AppsmithError.INVALID_PARAMETER.getMessage(FieldName.EMAIL)); + } + } } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ShareOrganizationPermissionTests.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ShareOrganizationPermissionTests.java index 2bd7366fcd..31731ce2da 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ShareOrganizationPermissionTests.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ShareOrganizationPermissionTests.java @@ -72,7 +72,7 @@ public class ShareOrganizationPermissionTests { ArrayList emails = new ArrayList<>(); // Invite Admin - emails.add("admin@solutionTest.com"); + emails.add("admin@solutiontest.com"); inviteUsersDTO.setUsernames(emails); inviteUsersDTO.setRoleName(AppsmithRole.ORGANIZATION_ADMIN.getName()); userService.inviteUser(inviteUsersDTO, "http://localhost:8080").blockLast(); @@ -80,21 +80,21 @@ public class ShareOrganizationPermissionTests { emails.clear(); // Invite Developer - emails.add("developer@solutionTest.com"); + emails.add("developer@solutiontest.com"); inviteUsersDTO.setUsernames(emails); inviteUsersDTO.setRoleName(AppsmithRole.ORGANIZATION_DEVELOPER.getName()); userService.inviteUser(inviteUsersDTO, "http://localhost:8080").blockLast(); } @Test - @WithUserDetails(value = "admin@solutionTest.com") + @WithUserDetails(value = "admin@solutiontest.com") public void testAdminPermissionsForInviteAndMakePublic() { Policy inviteUserPolicy = Policy.builder().permission(ORGANIZATION_INVITE_USERS.getValue()) - .users(Set.of("admin@solutionTest.com", "developer@solutionTest.com")) + .users(Set.of("admin@solutiontest.com", "developer@solutiontest.com")) .build(); Policy makePublicApp = Policy.builder().permission(MAKE_PUBLIC_APPLICATIONS.getValue()) - .users(Set.of("admin@solutionTest.com")) + .users(Set.of("admin@solutiontest.com")) .build(); Mono applicationMono = applicationService.findById(savedApplication.getId()); @@ -113,7 +113,7 @@ public class ShareOrganizationPermissionTests { } @Test - @WithUserDetails(value = "admin@solutionTest.com") + @WithUserDetails(value = "admin@solutiontest.com") public void testAdminInviteRoles() { Set roles = Set.of("Administrator", "Developer", "App Viewer"); @@ -128,10 +128,10 @@ public class ShareOrganizationPermissionTests { } @Test - @WithUserDetails(value = "developer@solutionTest.com") + @WithUserDetails(value = "developer@solutiontest.com") public void testDevPermissionsForInvite() { Policy inviteUserPolicy = Policy.builder().permission(ORGANIZATION_INVITE_USERS.getValue()) - .users(Set.of("admin@solutionTest.com", "developer@solutionTest.com")) + .users(Set.of("admin@solutiontest.com", "developer@solutiontest.com")) .build(); Mono organizationMono = organizationService.findById(organizationId, READ_ORGANIZATIONS); @@ -145,7 +145,7 @@ public class ShareOrganizationPermissionTests { } @Test - @WithUserDetails(value = "developer@solutionTest.com") + @WithUserDetails(value = "developer@solutiontest.com") public void testDeveloperInviteRoles() { Set roles = Set.of("Developer", "App Viewer");