Protei/PromucFlow_constructor
2
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

User emails should be changed to lower case before creating/inviting user. (#1281)

Browse Source
This commit is contained in:
Trisha Anand 2020-10-19 16:39:04 +05:30 committed by GitHub
parent 920b38da90
commit 37aedcba24
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 87 additions and 122 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -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

10
app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ValidationUtils.java Normal file
View File

@ -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);
}
}

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

@ -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<String> sendMail(String to, String subject, String text, Map<String, String> params) {
return Mono
.fromSupplier(() -> {

13
app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java
View File

@ -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<UserRepository, User, String> i
return Flux.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.ORIGIN));
}
List<String> usernames = inviteUsersDTO.getUsernames();
List<String> 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<UserRepository, User, String> i
return Flux.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.ROLE));
}
List<String> usernames = new ArrayList<>();
for (String username : originalUsernames) {
usernames.add(username.toLowerCase());
}
Mono<User> currentUserMono = sessionUserService.getCurrentUser().cache();
// Check if the current user has invite permissions
@ -681,7 +687,8 @@ public class UserServiceImpl extends BaseService<UserRepository, User, String> i
private Mono<User> createNewUserAndSendInviteEmail(String email, String originHeader, Map<String, String> 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);

6
app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/UserSignup.java
View File

@ -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<User> 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()),

4
app/server/appsmith-server/src/test/java/com/appsmith/server/configurations/SeedMongoData.java
View File

@ -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))

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

@ -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<String> invalidAddresses = Arrays.asList(
"plainaddress",
"#@%^%#$@#$@#.com",
"@example.com",
"Joe Smith <email@example.com>",
"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;
}
}
}
}

30
app/server/appsmith-server/src/test/java/com/appsmith/server/services/OrganizationServiceTest.java
View File

@ -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();

33
app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceTest.java
View File

@ -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<Organization> 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<String> invalidAddresses = Arrays.asList(
"plainaddress",
"#@%^%#$@#$@#.com",
"@example.com",
"Joe Smith <email@example.com>",
"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));
}
}
}

18
app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ShareOrganizationPermissionTests.java
View File

@ -72,7 +72,7 @@ public class ShareOrganizationPermissionTests {
ArrayList<String> 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<Application> 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<String> 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<Organization> 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<String> roles = Set.of("Developer", "App Viewer");