diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/acl/AclPermission.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/acl/AclPermission.java index d35fc6078b..802d1d7513 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/acl/AclPermission.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/acl/AclPermission.java @@ -16,8 +16,13 @@ public enum AclPermission { UPDATE("update", null), DELETE("delete", null), - USER_MANAGE_ORGANIZATIONS("manage:userOrganiation", User.class), - USER_READ_ORGANIZATIONS("read:userOrganiation", User.class), + USER_MANAGE_ORGANIZATIONS("manage:userOrganization", User.class), + USER_READ_ORGANIZATIONS("read:userOrganization", User.class), + + // TODO: Add these permissions to PolicyGenerator to assign them to the user when they sign up + READ_USERS("read:users", User.class), + MANAGE_USERS("manage:users", User.class), + RESET_PASSWORD_USERS("resetPassword:users", User.class), MANAGE_ORGANIZATIONS("manage:organizations", Organization.class), READ_ORGANIZATIONS("read:organizations", Organization.class), diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomUserRepository.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomUserRepository.java new file mode 100644 index 0000000000..caf9d83523 --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomUserRepository.java @@ -0,0 +1,10 @@ +package com.appsmith.server.repositories; + +import com.appsmith.server.acl.AclPermission; +import com.appsmith.server.domains.User; +import reactor.core.publisher.Mono; + +public interface CustomUserRepository extends AppsmithRepository { + + Mono findByEmail(String email, AclPermission aclPermission); +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomUserRepositoryImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomUserRepositoryImpl.java new file mode 100644 index 0000000000..f336dc69f2 --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomUserRepositoryImpl.java @@ -0,0 +1,33 @@ +package com.appsmith.server.repositories; + +import com.appsmith.server.acl.AclPermission; +import com.appsmith.server.domains.QOrganization; +import com.appsmith.server.domains.QUser; +import com.appsmith.server.domains.User; +import lombok.extern.slf4j.Slf4j; +import org.springframework.data.mongodb.core.ReactiveMongoOperations; +import org.springframework.data.mongodb.core.convert.MongoConverter; +import org.springframework.data.mongodb.core.query.Criteria; +import org.springframework.stereotype.Component; +import reactor.core.publisher.Mono; + +import java.util.List; + +import static org.springframework.data.mongodb.core.query.Criteria.where; + +@Component +@Slf4j +public class CustomUserRepositoryImpl extends BaseAppsmithRepositoryImpl implements CustomUserRepository { + + public CustomUserRepositoryImpl(ReactiveMongoOperations mongoOperations, MongoConverter mongoConverter) { + super(mongoOperations, mongoConverter); + } + + @Override + public Mono findByEmail(String email, AclPermission aclPermission) { + log.debug("Going to find user by email: {}", email); + Criteria emailCriterita = where(fieldName(QUser.user.email)).is(email); + + return queryOne(List.of(emailCriterita), aclPermission); + } +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/UserRepository.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/UserRepository.java index 9938004d31..77a09ea8d9 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/UserRepository.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/UserRepository.java @@ -7,7 +7,6 @@ import reactor.core.publisher.Mono; @Repository @AclEntity("users") -public interface UserRepository extends BaseRepository { - +public interface UserRepository extends BaseRepository, CustomUserRepository { Mono findByEmail(String email); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserService.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserService.java index a7decf3f51..11cc0b4481 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserService.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserService.java @@ -27,8 +27,6 @@ public interface UserService extends CrudService { Mono confirmInviteUser(InviteUser inviteUser, String originHeader); - Mono> getAnonymousAuthorities(); - Mono getUserProfile(); Mono createUser(User user, String originHeader); 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 953d021167..146753e769 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 @@ -23,7 +23,6 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.domain.Example; import org.springframework.data.mongodb.core.ReactiveMongoTemplate; import org.springframework.data.mongodb.core.convert.MongoConverter; -import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.userdetails.ReactiveUserDetailsService; import org.springframework.security.core.userdetails.UserDetails; import org.springframework.security.core.userdetails.UsernameNotFoundException; @@ -37,13 +36,15 @@ import javax.validation.Validator; import java.io.IOException; import java.net.URLEncoder; import java.nio.charset.StandardCharsets; -import java.util.Collection; import java.util.HashMap; import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.UUID; +import static com.appsmith.server.acl.AclPermission.READ_USERS; +import static com.appsmith.server.acl.AclPermission.RESET_PASSWORD_USERS; + @Slf4j @Service public class UserServiceImpl extends BaseService implements UserService, ReactiveUserDetailsService { @@ -165,7 +166,7 @@ public class UserServiceImpl extends BaseService i log.debug("Password reset Token: {} for email: {}", token, email); // Check if the user exists in our DB. If not, we will not send a password reset link to the user - Mono userMono = repository.findByEmail(email) + Mono userMono = repository.findByEmail(email, RESET_PASSWORD_USERS) .switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, FieldName.USER, email))); // Generate the password reset link for the user @@ -225,7 +226,7 @@ public class UserServiceImpl extends BaseService i } return repository - .findByEmail(email) + .findByEmail(email, RESET_PASSWORD_USERS) .switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, "user", email))) .map(user -> { user.setPasswordResetInitiated(true); @@ -249,7 +250,7 @@ public class UserServiceImpl extends BaseService i public Mono resetPasswordAfterForgotPassword(String token, User user) { return repository - .findByEmail(user.getEmail()) + .findByEmail(user.getEmail(), RESET_PASSWORD_USERS) .switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, "user", user.getEmail()))) .flatMap(userFromDb -> { if (!userFromDb.getPasswordResetInitiated()) { @@ -382,7 +383,7 @@ public class UserServiceImpl extends BaseService i // If the email Id is not found in the users collection, it means this is a new user. We still want the mono to emit // so that the flow can continue. Hence, returning empty user object. - Mono userMono = repository.findByEmail(inviteUser.getEmail()) + Mono userMono = repository.findByEmail(inviteUser.getEmail(), RESET_PASSWORD_USERS) .switchIfEmpty(Mono.just(new User())); return Mono.zip(inviteUserMono, userMono, (newUser, user) -> { @@ -408,12 +409,6 @@ public class UserServiceImpl extends BaseService i }).flatMap(result -> result); } - @Override - public Mono> getAnonymousAuthorities() { - return repository.findByEmail("anonymousUser") - .map(user -> user.getAuthorities()); - } - @Override public Mono create(User user) { return createUser(user, null); @@ -485,7 +480,8 @@ public class UserServiceImpl extends BaseService i @Override public Mono update(String id, User userUpdate) { - Mono userFromRepository = repository.findById(id); + Mono userFromRepository = repository.findById(id, READ_USERS) + .switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, FieldName.USER, id))); if (userUpdate.getPassword() != null) { // The password is being updated. Hash it first and then store it @@ -496,7 +492,6 @@ public class UserServiceImpl extends BaseService i .flatMap(this::validateUpdate) //Once the new update has been validated, update the user with the new fields. .then(userFromRepository) - .switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, id))) .map(existingUser -> { BeanCopyUtils.copyNewFieldValuesIntoOldObject(userUpdate, existingUser); return existingUser; 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 4d37d54d2d..51ac6676be 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 @@ -32,6 +32,7 @@ import static com.appsmith.server.acl.AclPermission.MANAGE_PAGES; import static com.appsmith.server.acl.AclPermission.ORGANIZATION_MANAGE_APPLICATIONS; import static com.appsmith.server.acl.AclPermission.READ_APPLICATIONS; import static com.appsmith.server.acl.AclPermission.READ_ORGANIZATIONS; +import static com.appsmith.server.acl.AclPermission.READ_USERS; import static com.appsmith.server.acl.AclPermission.USER_MANAGE_ORGANIZATIONS; import static org.springframework.data.mongodb.core.query.Criteria.where; @@ -48,29 +49,40 @@ public class SeedMongoData { ReactiveMongoTemplate mongoTemplate) { log.info("Seeding the data"); + final String API_USER_EMAIL = "api_user"; + final String TEST_USER_EMAIL = "usertest@usertest.com"; + Policy readAppPolicy = Policy.builder().permission(READ_APPLICATIONS.getValue()) - .users(Set.of("api_user")) + .users(Set.of(API_USER_EMAIL)) .build(); Policy manageAppPolicy = Policy.builder().permission(ORGANIZATION_MANAGE_APPLICATIONS.getValue()) - .users(Set.of("api_user")) + .users(Set.of(API_USER_EMAIL)) .build(); Policy userManageOrgPolicy = Policy.builder().permission(USER_MANAGE_ORGANIZATIONS.getValue()) - .users(Set.of("api_user")) + .users(Set.of(API_USER_EMAIL)) .build(); Policy managePagePolicy = Policy.builder().permission(MANAGE_PAGES.getValue()) - .users(Set.of("api_user")) + .users(Set.of(API_USER_EMAIL)) .build(); Policy readOrgPolicy = Policy.builder().permission(READ_ORGANIZATIONS.getValue()) - .users(Set.of("api_user")) + .users(Set.of(API_USER_EMAIL)) + .build(); + + Policy readApiUserPolicy = Policy.builder().permission(READ_USERS.getValue()) + .users(Set.of(API_USER_EMAIL)) + .build(); + + Policy readTestUserPolicy = Policy.builder().permission(READ_USERS.getValue()) + .users(Set.of(TEST_USER_EMAIL)) .build(); Object[][] userData = { - {"user test", "usertest@usertest.com", UserState.ACTIVATED, new HashSet<>()}, - {"api_user", "api_user", UserState.ACTIVATED, Set.of(userManageOrgPolicy)}, + {"user test", TEST_USER_EMAIL, UserState.ACTIVATED, Set.of(readTestUserPolicy)}, + {"api_user", API_USER_EMAIL, UserState.ACTIVATED, Set.of(userManageOrgPolicy, readApiUserPolicy)}, }; Object[][] orgData = { {"Spring Test Organization", "appsmith-spring-test.com", "appsmith.com", "spring-test-organization", Set.of(manageAppPolicy)} 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 593cc28d1f..11b93d7cf2 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 @@ -1,9 +1,11 @@ package com.appsmith.server.services; +import com.appsmith.server.constants.FieldName; import com.appsmith.server.domains.Organization; import com.appsmith.server.domains.User; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; +import lombok.extern.slf4j.Slf4j; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -14,9 +16,11 @@ import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.junit4.SpringRunner; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; +import reactor.util.function.Tuple2; import static org.assertj.core.api.Assertions.assertThat; +@Slf4j @RunWith(SpringRunner.class) @SpringBootTest @DirtiesContext @@ -51,25 +55,32 @@ public class UserServiceTest { StepVerifier.create(userMono1) .expectErrorMatches(throwable -> throwable instanceof AppsmithException && - throwable.getMessage().equals(AppsmithError.INVALID_PARAMETER.getMessage("Random-UserId-%Not-In_The-System_For_SUre"))) + throwable.getMessage().equals(AppsmithError.NO_RESOURCE_FOUND.getMessage(FieldName.USER, "Random-UserId-%Not-In_The-System_For_SUre"))) .verify(); } @Test @WithUserDetails(value = "api_user") public void updateUserWithValidOrganization() { - User updateUser = new User(); - //Add valid organization id to the updateUser object. - organizationMono - .map(organization -> { - updateUser.setCurrentOrganizationId(organization.getId()); - return updateUser; - }).block(); + // Create a new organization + Organization updateOrg = new Organization(); + updateOrg.setName("UserServiceTest Update Org"); - Mono userMono1 = userMono.flatMap(user -> userService.update(user.getId(), updateUser)); - StepVerifier.create(userMono1) - .assertNext(updatedUserInRepository -> { - assertThat(updatedUserInRepository.getCurrentOrganizationId()).isEqualTo(updateUser.getCurrentOrganizationId()); + User updateUser = new User(); + + Mono userMono1 = userService.findByEmail("api_user") + .switchIfEmpty(Mono.error(new Exception("Unable to find user"))); + + //Add valid organization id to the updateUser object. + Mono userMono = organizationService.create(updateOrg) + .flatMap(org -> { + updateUser.setCurrentOrganizationId(org.getId()); + return userMono1.flatMap(user -> userService.update(user.getId(), updateUser)); + }); + + StepVerifier.create(userMono) + .assertNext(user -> { + assertThat(user.getCurrentOrganizationId()).isEqualTo(updateUser.getCurrentOrganizationId()); }) .verifyComplete(); }