From ee689d7c87f924b3e41bf5a1dae53971ea452aca Mon Sep 17 00:00:00 2001 From: Trisha Anand Date: Thu, 13 Mar 2025 11:40:51 +0530 Subject: [PATCH] chore: Remove fetching users without organizationId (#39656) ## Description > [!TIP] > _Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team)._ > > _Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR._ Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /test all ### :mag: Cypress test results > [!WARNING] > Tests have not run on the HEAD f0cb554c5dde764044c075663433fb4ea7ce684c yet >
Wed, 12 Mar 2025 13:55:54 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit - **New Features** - Enhanced user authentication: Login, social sign-in, password reset, and email verification processes now use organization-specific context to improve security and accuracy. - Added organization ID to password reset tokens for better context integrity. - **Refactor** - Internal workflows have been streamlined to ensure that all user-related operations reliably respect the organization context, reducing potential cross-organization issues. - Introduction of `UserOrganizationHelper` for managing organization-specific logic across user services. - **Bug Fixes** - Updated user retrieval methods to ensure they are scoped to the correct organization, enhancing accuracy in user-related operations. --- .../handlers/CustomFormLoginServiceImpl.java | 5 +- .../handlers/CustomOAuth2UserServiceImpl.java | 6 +- .../handlers/CustomOidcUserServiceImpl.java | 7 +- .../ce/AuthenticationSuccessHandlerCE.java | 10 +- .../ce/CustomFormLoginServiceCEImpl.java | 15 ++- .../ce/CustomOAuth2UserServiceCEImpl.java | 16 ++- .../ce/CustomOidcUserServiceCEImpl.java | 15 ++- .../server/domains/PasswordResetToken.java | 1 + .../helpers/UserOrganizationHelper.java | 5 + .../helpers/UserOrganizationHelperImpl.java | 14 ++ .../helpers/ce/UserOrganizationHelperCE.java | 11 ++ .../ce/UserOrganizationHelperCEImpl.java | 44 +++++++ ...AddOrganizationIdToPasswordResetToken.java | 56 ++++++++ .../ce/CustomUserRepositoryCE.java | 3 - .../ce/CustomUserRepositoryCEImpl.java | 8 -- .../ce/PasswordResetTokenRepositoryCE.java | 2 +- .../repositories/ce/UserRepositoryCE.java | 4 +- .../services/OrganizationServiceImpl.java | 7 +- .../ce/OrganizationServiceCEImpl.java | 19 +-- .../server/services/ce/UserServiceCEImpl.java | 123 +++++++++++------- .../CustomFormLoginServiceImplUnitTest.java | 18 ++- .../repositories/UserRepositoryTest.java | 35 +++-- .../server/services/UserServiceTest.java | 69 +++++----- .../UserServiceWithDisabledSignupTest.java | 8 ++ 24 files changed, 360 insertions(+), 141 deletions(-) create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/UserOrganizationHelper.java create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/UserOrganizationHelperImpl.java create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/UserOrganizationHelperCE.java create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/UserOrganizationHelperCEImpl.java create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration068_AddOrganizationIdToPasswordResetToken.java diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/CustomFormLoginServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/CustomFormLoginServiceImpl.java index a107aad98d..a257c932a0 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/CustomFormLoginServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/CustomFormLoginServiceImpl.java @@ -1,6 +1,7 @@ package com.appsmith.server.authentication.handlers; import com.appsmith.server.authentication.handlers.ce.CustomFormLoginServiceCEImpl; +import com.appsmith.server.helpers.UserOrganizationHelper; import com.appsmith.server.repositories.UserRepository; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; @@ -11,7 +12,7 @@ import org.springframework.stereotype.Service; public class CustomFormLoginServiceImpl extends CustomFormLoginServiceCEImpl { @Autowired - public CustomFormLoginServiceImpl(UserRepository repository) { - super(repository); + public CustomFormLoginServiceImpl(UserRepository repository, UserOrganizationHelper userOrganizationHelper) { + super(repository, userOrganizationHelper); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/CustomOAuth2UserServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/CustomOAuth2UserServiceImpl.java index 4f19268aca..e27176c001 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/CustomOAuth2UserServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/CustomOAuth2UserServiceImpl.java @@ -1,6 +1,7 @@ package com.appsmith.server.authentication.handlers; import com.appsmith.server.authentication.handlers.ce.CustomOAuth2UserServiceCEImpl; +import com.appsmith.server.helpers.UserOrganizationHelper; import com.appsmith.server.repositories.UserRepository; import com.appsmith.server.services.UserService; import lombok.extern.slf4j.Slf4j; @@ -19,8 +20,9 @@ public class CustomOAuth2UserServiceImpl extends CustomOAuth2UserServiceCEImpl private UserService userService; @Autowired - public CustomOAuth2UserServiceImpl(UserRepository repository, UserService userService) { - super(repository, userService); + public CustomOAuth2UserServiceImpl( + UserRepository repository, UserService userService, UserOrganizationHelper userOrganizationHelper) { + super(repository, userService, userOrganizationHelper); this.repository = repository; this.userService = userService; } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/CustomOidcUserServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/CustomOidcUserServiceImpl.java index ea036f40ce..f09457dd73 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/CustomOidcUserServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/CustomOidcUserServiceImpl.java @@ -1,6 +1,7 @@ package com.appsmith.server.authentication.handlers; import com.appsmith.server.authentication.handlers.ce.CustomOidcUserServiceCEImpl; +import com.appsmith.server.helpers.UserOrganizationHelper; import com.appsmith.server.repositories.UserRepository; import com.appsmith.server.services.UserService; import lombok.extern.slf4j.Slf4j; @@ -17,10 +18,12 @@ public class CustomOidcUserServiceImpl extends CustomOidcUserServiceCEImpl private UserRepository repository; private UserService userService; + private UserOrganizationHelper userOrganizationHelper; @Autowired - public CustomOidcUserServiceImpl(UserRepository repository, UserService userService) { - super(repository, userService); + public CustomOidcUserServiceImpl( + UserRepository repository, UserService userService, UserOrganizationHelper userOrganizationHelper) { + super(repository, userService, userOrganizationHelper); this.repository = repository; this.userService = userService; } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/AuthenticationSuccessHandlerCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/AuthenticationSuccessHandlerCE.java index c9a02b286a..5e89d71530 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/AuthenticationSuccessHandlerCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/AuthenticationSuccessHandlerCE.java @@ -75,7 +75,10 @@ public class AuthenticationSuccessHandlerCE implements ServerAuthenticationSucce .map(organization -> organization.getOrganizationConfiguration().isEmailVerificationEnabled()) .cache(); - Mono userMono = userRepository.findByEmail(userEmail).cache(); + Mono userMono = organizationService + .getCurrentUserOrganizationId() + .flatMap(orgId -> userRepository.findByEmailAndOrganizationId(userEmail, orgId)) + .cache(); Mono verificationRequiredMono = null; if ("signup".equals(method)) { @@ -367,8 +370,9 @@ public class AuthenticationSuccessHandlerCE implements ServerAuthenticationSucce // In case no workspaces are found for the user, create a new default workspace String email = ((User) authentication.getPrincipal()).getEmail(); - return userRepository - .findByEmail(email) + return organizationService + .getCurrentUserOrganizationId() + .flatMap(orgId -> userRepository.findByEmailAndOrganizationId(email, orgId)) .flatMap(user -> workspaceService.createDefault(new Workspace(), user)) .map(workspace -> { application.setWorkspaceId(workspace.getId()); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/CustomFormLoginServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/CustomFormLoginServiceCEImpl.java index ba4f31edc8..1e762e4f7f 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/CustomFormLoginServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/CustomFormLoginServiceCEImpl.java @@ -2,6 +2,7 @@ package com.appsmith.server.authentication.handlers.ce; import com.appsmith.server.domains.LoginSource; import com.appsmith.server.exceptions.AppsmithError; +import com.appsmith.server.helpers.UserOrganizationHelper; import com.appsmith.server.repositories.UserRepository; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang.WordUtils; @@ -16,10 +17,12 @@ import reactor.core.publisher.Mono; public class CustomFormLoginServiceCEImpl implements ReactiveUserDetailsService { private UserRepository repository; + private UserOrganizationHelper userOrganizationHelper; @Autowired - public CustomFormLoginServiceCEImpl(UserRepository repository) { + public CustomFormLoginServiceCEImpl(UserRepository repository, UserOrganizationHelper userOrganizationHelper) { this.repository = repository; + this.userOrganizationHelper = userOrganizationHelper; } /** @@ -31,9 +34,13 @@ public class CustomFormLoginServiceCEImpl implements ReactiveUserDetailsService */ @Override public Mono findByUsername(String username) { - return repository - .findByEmail(username) - .switchIfEmpty(repository.findFirstByEmailIgnoreCaseOrderByCreatedAtDesc(username)) + + return userOrganizationHelper + .getCurrentUserOrganizationId() + .flatMap(orgId -> repository + .findByEmailAndOrganizationId(username, orgId) + .switchIfEmpty(repository.findFirstByEmailIgnoreCaseAndOrganizationIdOrderByCreatedAtDesc( + username, orgId))) .switchIfEmpty(Mono.error(new UsernameNotFoundException("Unable to find username: " + username))) .onErrorMap(error -> { log.error("Can't find user {}", username); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/CustomOAuth2UserServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/CustomOAuth2UserServiceCEImpl.java index fc8ce6974f..3a6e98b83d 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/CustomOAuth2UserServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/CustomOAuth2UserServiceCEImpl.java @@ -5,6 +5,7 @@ import com.appsmith.server.domains.User; import com.appsmith.server.domains.UserState; import com.appsmith.server.exceptions.AppsmithException; import com.appsmith.server.exceptions.AppsmithOAuth2AuthenticationException; +import com.appsmith.server.helpers.UserOrganizationHelper; import com.appsmith.server.repositories.UserRepository; import com.appsmith.server.services.UserService; import lombok.extern.slf4j.Slf4j; @@ -26,11 +27,14 @@ public class CustomOAuth2UserServiceCEImpl extends DefaultReactiveOAuth2UserServ private UserRepository repository; private UserService userService; + private UserOrganizationHelper userOrganizationHelper; @Autowired - public CustomOAuth2UserServiceCEImpl(UserRepository repository, UserService userService) { + public CustomOAuth2UserServiceCEImpl( + UserRepository repository, UserService userService, UserOrganizationHelper userOrganizationHelper) { this.repository = repository; this.userService = userService; + this.userOrganizationHelper = userOrganizationHelper; } @Override @@ -76,8 +80,12 @@ public class CustomOAuth2UserServiceCEImpl extends DefaultReactiveOAuth2UserServ } protected Mono findByUsername(String email) { - return repository - .findByEmail(email) - .switchIfEmpty(repository.findFirstByEmailIgnoreCaseOrderByCreatedAtDesc(email)); + + return userOrganizationHelper.getCurrentUserOrganizationId().flatMap(organizationId -> { + return repository + .findByEmailAndOrganizationId(email, organizationId) + .switchIfEmpty(repository.findFirstByEmailIgnoreCaseAndOrganizationIdOrderByCreatedAtDesc( + email, organizationId)); + }); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/CustomOidcUserServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/CustomOidcUserServiceCEImpl.java index 29ef72f2a5..856ee0d9b0 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/CustomOidcUserServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/CustomOidcUserServiceCEImpl.java @@ -5,6 +5,7 @@ import com.appsmith.server.domains.User; import com.appsmith.server.domains.UserState; import com.appsmith.server.exceptions.AppsmithException; import com.appsmith.server.exceptions.AppsmithOAuth2AuthenticationException; +import com.appsmith.server.helpers.UserOrganizationHelper; import com.appsmith.server.repositories.UserRepository; import com.appsmith.server.services.UserService; import lombok.extern.slf4j.Slf4j; @@ -28,11 +29,14 @@ public class CustomOidcUserServiceCEImpl extends OidcReactiveOAuth2UserService { private UserRepository repository; private UserService userService; + private UserOrganizationHelper userOrganizationHelper; @Autowired - public CustomOidcUserServiceCEImpl(UserRepository repository, UserService userService) { + public CustomOidcUserServiceCEImpl( + UserRepository repository, UserService userService, UserOrganizationHelper userOrganizationHelper) { this.repository = repository; this.userService = userService; + this.userOrganizationHelper = userOrganizationHelper; } @Override @@ -82,8 +86,11 @@ public class CustomOidcUserServiceCEImpl extends OidcReactiveOAuth2UserService { } protected Mono findByUsername(String email) { - return repository - .findByEmail(email) - .switchIfEmpty(repository.findFirstByEmailIgnoreCaseOrderByCreatedAtDesc(email)); + return userOrganizationHelper.getCurrentUserOrganizationId().flatMap(organizationId -> { + return repository + .findByEmailAndOrganizationId(email, organizationId) + .switchIfEmpty(repository.findFirstByEmailIgnoreCaseAndOrganizationIdOrderByCreatedAtDesc( + email, organizationId)); + }); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/PasswordResetToken.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/PasswordResetToken.java index f364159239..67c0b1f16a 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/PasswordResetToken.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/PasswordResetToken.java @@ -17,6 +17,7 @@ import java.time.Instant; public class PasswordResetToken extends BaseDomain { String tokenHash; String email; + String organizationId; int requestCount; // number of requests in last 24 hours Instant firstRequestTime; // when a request was first generated in last 24 hours } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/UserOrganizationHelper.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/UserOrganizationHelper.java new file mode 100644 index 0000000000..d39023ea5c --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/UserOrganizationHelper.java @@ -0,0 +1,5 @@ +package com.appsmith.server.helpers; + +import com.appsmith.server.helpers.ce.UserOrganizationHelperCE; + +public interface UserOrganizationHelper extends UserOrganizationHelperCE {} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/UserOrganizationHelperImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/UserOrganizationHelperImpl.java new file mode 100644 index 0000000000..4fff69b7b2 --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/UserOrganizationHelperImpl.java @@ -0,0 +1,14 @@ +package com.appsmith.server.helpers; + +import com.appsmith.server.helpers.ce.UserOrganizationHelperCEImpl; +import com.appsmith.server.repositories.OrganizationRepository; +import org.springframework.stereotype.Component; + +@Component +public class UserOrganizationHelperImpl extends UserOrganizationHelperCEImpl implements UserOrganizationHelper { + public UserOrganizationHelperImpl( + OrganizationRepository organizationRepository, + InMemoryCacheableRepositoryHelper inMemoryCacheableRepositoryHelper) { + super(organizationRepository, inMemoryCacheableRepositoryHelper); + } +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/UserOrganizationHelperCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/UserOrganizationHelperCE.java new file mode 100644 index 0000000000..d128bbed41 --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/UserOrganizationHelperCE.java @@ -0,0 +1,11 @@ +package com.appsmith.server.helpers.ce; + +import reactor.core.publisher.Mono; + +public interface UserOrganizationHelperCE { + /** + * Gets the organization ID for the current user. If not found in user context, falls back to default organization. + * @return Mono containing the organization ID + */ + Mono getCurrentUserOrganizationId(); +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/UserOrganizationHelperCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/UserOrganizationHelperCEImpl.java new file mode 100644 index 0000000000..ac738e89ee --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/UserOrganizationHelperCEImpl.java @@ -0,0 +1,44 @@ +package com.appsmith.server.helpers.ce; + +import com.appsmith.server.domains.Organization; +import com.appsmith.server.helpers.InMemoryCacheableRepositoryHelper; +import com.appsmith.server.repositories.OrganizationRepository; +import lombok.extern.slf4j.Slf4j; +import org.springframework.util.StringUtils; +import reactor.core.publisher.Mono; + +import static com.appsmith.server.constants.FieldName.DEFAULT; + +@Slf4j +public class UserOrganizationHelperCEImpl implements UserOrganizationHelperCE { + + private final OrganizationRepository organizationRepository; + private final InMemoryCacheableRepositoryHelper inMemoryCacheableRepositoryHelper; + + public UserOrganizationHelperCEImpl( + OrganizationRepository organizationRepository, + InMemoryCacheableRepositoryHelper inMemoryCacheableRepositoryHelper) { + this.organizationRepository = organizationRepository; + this.inMemoryCacheableRepositoryHelper = inMemoryCacheableRepositoryHelper; + } + + @Override + public Mono getCurrentUserOrganizationId() { + return getDefaultOrganizationId(); + } + + protected Mono getDefaultOrganizationId() { + // If the value exists in cache, return it as is + if (StringUtils.hasLength(inMemoryCacheableRepositoryHelper.getDefaultOrganizationId())) { + return Mono.just(inMemoryCacheableRepositoryHelper.getDefaultOrganizationId()); + } + return organizationRepository + .findBySlug(DEFAULT) + .map(Organization::getId) + .map(organizationId -> { + // Set the cache value before returning. + inMemoryCacheableRepositoryHelper.setDefaultOrganizationId(organizationId); + return organizationId; + }); + } +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration068_AddOrganizationIdToPasswordResetToken.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration068_AddOrganizationIdToPasswordResetToken.java new file mode 100644 index 0000000000..d13f91833e --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration068_AddOrganizationIdToPasswordResetToken.java @@ -0,0 +1,56 @@ +package com.appsmith.server.migrations.db.ce; + +import io.mongock.api.annotations.ChangeUnit; +import io.mongock.api.annotations.Execution; +import io.mongock.api.annotations.RollbackExecution; +import lombok.extern.slf4j.Slf4j; +import org.bson.Document; +import org.springframework.data.mongodb.core.MongoTemplate; +import org.springframework.data.mongodb.core.query.Query; + +import java.util.Arrays; +import java.util.List; + +import static org.springframework.data.mongodb.core.query.Criteria.where; + +@Slf4j +@ChangeUnit(id = "add-organization-id-to-password-reset-token", order = "068") +public class Migration068_AddOrganizationIdToPasswordResetToken { + + private final MongoTemplate mongoTemplate; + + public Migration068_AddOrganizationIdToPasswordResetToken(MongoTemplate mongoTemplate) { + this.mongoTemplate = mongoTemplate; + } + + @RollbackExecution + public void rollbackExecution() {} + + @Execution + public void execute() { + try { + // Get the organization ID from the organization collection + Document organization = mongoTemplate.findOne(new Query(), Document.class, "organization"); + if (organization == null) { + log.info("No organization found to migrate password reset tokens"); + return; + } + String organizationId = organization.getObjectId("_id").toString(); + + // Update all password reset tokens that don't have an organizationId + Query query = new Query(where("organizationId").exists(false)); + List pipeline = + Arrays.asList(new Document("$set", new Document("organizationId", organizationId))); + + long updatedCount = mongoTemplate + .getCollection("passwordResetToken") + .updateMany(query.getQueryObject(), pipeline) + .getModifiedCount(); + + log.info("Successfully set organizationId for {} password reset token documents", updatedCount); + } catch (Exception e) { + log.error("Error while setting organizationId for password reset tokens: ", e); + throw e; + } + } +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomUserRepositoryCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomUserRepositoryCE.java index 856f1c54e1..8498bd8457 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomUserRepositoryCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomUserRepositoryCE.java @@ -1,6 +1,5 @@ package com.appsmith.server.repositories.ce; -import com.appsmith.server.acl.AclPermission; import com.appsmith.server.domains.User; import com.appsmith.server.repositories.AppsmithRepository; import org.springframework.data.mongodb.core.query.UpdateDefinition; @@ -10,8 +9,6 @@ import java.util.Set; public interface CustomUserRepositoryCE extends AppsmithRepository { - Mono findByEmail(String email, AclPermission aclPermission); - Mono findByEmailAndOrganizationId(String email, String organizationId); Mono isUsersEmpty(); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomUserRepositoryCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomUserRepositoryCEImpl.java index 96e6650ee5..411c78606b 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomUserRepositoryCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomUserRepositoryCEImpl.java @@ -1,12 +1,10 @@ package com.appsmith.server.repositories.ce; -import com.appsmith.server.acl.AclPermission; import com.appsmith.server.constants.FieldName; import com.appsmith.server.domains.User; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; import com.appsmith.server.helpers.ce.bridge.Bridge; -import com.appsmith.server.helpers.ce.bridge.BridgeQuery; import com.appsmith.server.projections.IdOnly; import com.appsmith.server.repositories.BaseAppsmithRepositoryImpl; import lombok.extern.slf4j.Slf4j; @@ -19,12 +17,6 @@ import java.util.Set; @Slf4j public class CustomUserRepositoryCEImpl extends BaseAppsmithRepositoryImpl implements CustomUserRepositoryCE { - @Override - public Mono findByEmail(String email, AclPermission aclPermission) { - BridgeQuery emailCriteria = Bridge.equal(User.Fields.email, email); - return queryBuilder().criteria(emailCriteria).permission(aclPermission).one(); - } - @Override public Mono findByEmailAndOrganizationId(String email, String organizationId) { return queryBuilder() diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/PasswordResetTokenRepositoryCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/PasswordResetTokenRepositoryCE.java index 6cff2bc214..eee4a57a3f 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/PasswordResetTokenRepositoryCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/PasswordResetTokenRepositoryCE.java @@ -6,5 +6,5 @@ import reactor.core.publisher.Mono; public interface PasswordResetTokenRepositoryCE extends BaseRepository { - Mono findByEmail(String email); + Mono findByEmailAndOrganizationId(String email, String organizationId); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/UserRepositoryCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/UserRepositoryCE.java index 17895ce93f..54fa6ec92c 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/UserRepositoryCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/UserRepositoryCE.java @@ -13,9 +13,9 @@ public interface UserRepositoryCE extends BaseRepository, CustomUs Mono findByEmail(String email); - Mono findFirstByEmailIgnoreCaseOrderByCreatedAtDesc(String email); + Mono findFirstByEmailIgnoreCaseAndOrganizationIdOrderByCreatedAtDesc(String email, String organizationId); - Flux findAllByEmailIn(Set emails); + Flux findAllByEmailInAndOrganizationId(Set emails, String organizationId); /** * This method returns the count of all users that are not deleted and are not system generated. diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/OrganizationServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/OrganizationServiceImpl.java index f8c1b46cf2..98156bebe6 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/OrganizationServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/OrganizationServiceImpl.java @@ -2,6 +2,7 @@ package com.appsmith.server.services; import com.appsmith.server.configurations.CommonConfig; import com.appsmith.server.helpers.FeatureFlagMigrationHelper; +import com.appsmith.server.helpers.UserOrganizationHelper; import com.appsmith.server.repositories.CacheableRepositoryHelper; import com.appsmith.server.repositories.OrganizationRepository; import com.appsmith.server.services.ce.OrganizationServiceCEImpl; @@ -23,7 +24,8 @@ public class OrganizationServiceImpl extends OrganizationServiceCEImpl implement FeatureFlagMigrationHelper featureFlagMigrationHelper, CacheableRepositoryHelper cacheableRepositoryHelper, CommonConfig commonConfig, - ObservationRegistry observationRegistry) { + ObservationRegistry observationRegistry, + UserOrganizationHelper userOrganizationHelper) { super( validator, repository, @@ -33,6 +35,7 @@ public class OrganizationServiceImpl extends OrganizationServiceCEImpl implement featureFlagMigrationHelper, cacheableRepositoryHelper, commonConfig, - observationRegistry); + observationRegistry, + userOrganizationHelper); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/OrganizationServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/OrganizationServiceCEImpl.java index e71b235538..b7797c309f 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/OrganizationServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/OrganizationServiceCEImpl.java @@ -14,6 +14,7 @@ import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; import com.appsmith.server.helpers.CollectionUtils; import com.appsmith.server.helpers.FeatureFlagMigrationHelper; +import com.appsmith.server.helpers.UserOrganizationHelper; import com.appsmith.server.repositories.CacheableRepositoryHelper; import com.appsmith.server.repositories.OrganizationRepository; import com.appsmith.server.services.AnalyticsService; @@ -42,8 +43,6 @@ import static java.lang.Boolean.TRUE; public class OrganizationServiceCEImpl extends BaseService implements OrganizationServiceCE { - private String defaultOrganizationId = null; - private final ConfigService configService; private final EnvManager envManager; @@ -55,6 +54,8 @@ public class OrganizationServiceCEImpl extends BaseService getCurrentUserOrganizationId() { - // If the value exists in cache, return it as is - if (StringUtils.hasLength(defaultOrganizationId)) { - return Mono.just(defaultOrganizationId); - } - return repository.findBySlug(FieldName.DEFAULT).map(Organization::getId).map(organizationId -> { - // Set the cache value before returning. - this.defaultOrganizationId = organizationId; - return organizationId; - }); + return userOrganizationHelper.getCurrentUserOrganizationId(); } @Override diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java index 502bcf59a4..ca72f1285f 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java @@ -189,28 +189,34 @@ public class UserServiceCEImpl extends BaseService final String token = UUID.randomUUID().toString(); // Check if the user exists in our DB. If not, we will not send a password reset link to the user - return repository - .findByEmail(email) - .switchIfEmpty(repository.findFirstByEmailIgnoreCaseOrderByCreatedAtDesc(email)) - .switchIfEmpty( - Mono.error(new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, FieldName.USER, email))) - .flatMap(user -> { - // an user found with the provided email address - // Generate the password reset link for the user - return passwordResetTokenRepository - .findByEmail(user.getEmail()) - .switchIfEmpty(Mono.defer(() -> { - PasswordResetToken passwordResetToken = new PasswordResetToken(); - passwordResetToken.setEmail(user.getEmail()); - passwordResetToken.setRequestCount(0); - passwordResetToken.setFirstRequestTime(Instant.now()); - return Mono.just(passwordResetToken); - })) - .map(resetToken -> { - // check the validity of the token - validateResetLimit(resetToken); - resetToken.setTokenHash(passwordEncoder.encode(token)); - return resetToken; + return organizationService + .getCurrentUserOrganizationId() + .flatMap(organizationId -> { + return repository + .findByEmailAndOrganizationId(email, organizationId) + .switchIfEmpty(repository.findFirstByEmailIgnoreCaseAndOrganizationIdOrderByCreatedAtDesc( + email, organizationId)) + .switchIfEmpty(Mono.error( + new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, FieldName.USER, email))) + .flatMap(user -> { + // an user found with the provided email address + // Generate the password reset link for the user + return passwordResetTokenRepository + .findByEmailAndOrganizationId(user.getEmail(), user.getOrganizationId()) + .switchIfEmpty(Mono.defer(() -> { + PasswordResetToken passwordResetToken = new PasswordResetToken(); + passwordResetToken.setEmail(user.getEmail()); + passwordResetToken.setRequestCount(0); + passwordResetToken.setOrganizationId(organizationId); + passwordResetToken.setFirstRequestTime(Instant.now()); + return Mono.just(passwordResetToken); + })) + .map(resetToken -> { + // check the validity of the token + validateResetLimit(resetToken); + resetToken.setTokenHash(passwordEncoder.encode(token)); + return resetToken; + }); }); }) .flatMap(passwordResetTokenRepository::save) @@ -284,8 +290,10 @@ public class UserServiceCEImpl extends BaseService return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.TOKEN)); } - return passwordResetTokenRepository - .findByEmail(emailTokenDTO.getEmail()) + return organizationService + .getCurrentUserOrganizationId() + .flatMap(organizationId -> passwordResetTokenRepository.findByEmailAndOrganizationId( + emailTokenDTO.getEmail(), organizationId)) .switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.INVALID_PASSWORD_RESET))) .map(obj -> this.passwordEncoder.matches(emailTokenDTO.getToken(), obj.getTokenHash())); } @@ -309,10 +317,12 @@ public class UserServiceCEImpl extends BaseService Mono organizationMono = organizationService .getCurrentUserOrganization() - .switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, USER, ORGANIZATION))); + .switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, USER, ORGANIZATION))) + .cache(); - return passwordResetTokenRepository - .findByEmail(emailTokenDTO.getEmail()) + return organizationMono + .flatMap(organization -> passwordResetTokenRepository.findByEmailAndOrganizationId( + emailTokenDTO.getEmail(), organization.getId())) .switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.INVALID_PASSWORD_RESET))) .map(passwordResetToken -> { boolean matches = @@ -358,8 +368,11 @@ public class UserServiceCEImpl extends BaseService // password flow to set up their password, enable the user's account as well userFromDb.setIsEnabled(true); - return passwordResetTokenRepository - .findByEmail(userFromDb.getEmail()) + return organizationService + .getCurrentUserOrganizationId() + .flatMap( + organizationId -> passwordResetTokenRepository.findByEmailAndOrganizationId( + userFromDb.getEmail(), organizationId)) .switchIfEmpty(Mono.error(new AppsmithException( AppsmithError.NO_RESOURCE_FOUND, FieldName.TOKEN, @@ -398,17 +411,20 @@ public class UserServiceCEImpl extends BaseService // convert the user email to lowercase user.setEmail(user.getEmail().toLowerCase()); - Mono userWithOrgMono = Mono.just(user).flatMap(userBeforeSave -> { - if (userBeforeSave.getOrganizationId() == null) { - return organizationService.getCurrentUserOrganizationId().map(organizationId -> { - userBeforeSave.setOrganizationId(organizationId); - return userBeforeSave; - }); - } - // The org has been set already. No need to set the default org id. - return Mono.just(userBeforeSave); - }); - + Mono userWithOrgMono = Mono.just(user) + .flatMap(userBeforeSave -> { + if (userBeforeSave.getOrganizationId() == null) { + return organizationService + .getCurrentUserOrganizationId() + .map(organizationId -> { + userBeforeSave.setOrganizationId(organizationId); + return userBeforeSave; + }); + } + // The org has been set already. No need to set the default org id. + return Mono.just(userBeforeSave); + }) + .cache(); // Save the new user return userWithOrgMono .flatMap(this::validateObject) @@ -423,7 +439,8 @@ public class UserServiceCEImpl extends BaseService return Mono.just(crudUser); }) .then(Mono.zip( - repository.findByEmail(user.getUsername()), + userWithOrgMono.flatMap(userWithOrg -> repository.findByEmailAndOrganizationId( + user.getUsername(), userWithOrg.getOrganizationId())), userDataService.getForUserEmail(user.getUsername()))) .flatMap(tuple -> analyticsService.identifyUser(tuple.getT1(), tuple.getT2())); } @@ -447,8 +464,12 @@ public class UserServiceCEImpl extends BaseService } // If the user doesn't exist, create the user. If the user exists, return a duplicate key exception - return repository - .findFirstByEmailIgnoreCaseOrderByCreatedAtDesc(user.getUsername()) + return organizationService + .getCurrentUserOrganizationId() + .flatMap(organizationId -> { + return repository.findFirstByEmailIgnoreCaseAndOrganizationIdOrderByCreatedAtDesc( + user.getUsername(), organizationId); + }) .flatMap(savedUser -> { if (!savedUser.isEnabled()) { return isSignupAllowed(user).flatMap(isSignupAllowed -> { @@ -729,7 +750,9 @@ public class UserServiceCEImpl extends BaseService @Override public Flux getAllByEmails(Set emails, AclPermission permission) { - return repository.findAllByEmailIn(emails); + return organizationService + .getCurrentUserOrganizationId() + .flatMapMany(organizationId -> repository.findAllByEmailInAndOrganizationId(emails, organizationId)); } @Override @@ -752,9 +775,15 @@ public class UserServiceCEImpl extends BaseService final String token = UUID.randomUUID().toString(); // Check if the user exists in our DB. If not, we will not send the email verification link to the user - Mono userMono = repository.findByEmail(email).cache(); - return userMono.switchIfEmpty(repository.findFirstByEmailIgnoreCaseOrderByCreatedAtDesc(email)) - .switchIfEmpty( + Mono userMono = organizationService + .getCurrentUserOrganizationId() + .flatMap(organizationId -> repository + .findByEmailAndOrganizationId(email, organizationId) + .switchIfEmpty(repository.findFirstByEmailIgnoreCaseAndOrganizationIdOrderByCreatedAtDesc( + email, organizationId))) + .cache(); + + return userMono.switchIfEmpty( Mono.error(new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, FieldName.USER, email))) .flatMap(user -> { if (TRUE.equals(user.getEmailVerified())) { diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/authentication/handlers/CustomFormLoginServiceImplUnitTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/authentication/handlers/CustomFormLoginServiceImplUnitTest.java index 2d0dbbdbd5..fde5f14a88 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/authentication/handlers/CustomFormLoginServiceImplUnitTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/authentication/handlers/CustomFormLoginServiceImplUnitTest.java @@ -1,6 +1,7 @@ package com.appsmith.server.authentication.handlers; import com.appsmith.server.domains.User; +import com.appsmith.server.helpers.UserOrganizationHelper; import com.appsmith.server.repositories.UserRepository; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -22,16 +23,23 @@ public class CustomFormLoginServiceImplUnitTest { private ReactiveUserDetailsService reactiveUserDetailsService; + @MockBean + UserOrganizationHelper userOrganizationHelper; + @BeforeEach public void setUp() { - reactiveUserDetailsService = new CustomFormLoginServiceImpl(repository); + reactiveUserDetailsService = new CustomFormLoginServiceImpl(repository, userOrganizationHelper); } @Test public void findByUsername_WhenUserNameNotFound_ThrowsException() { String sampleEmail = "sample-email@example.com"; + Mockito.when(userOrganizationHelper.getCurrentUserOrganizationId()).thenReturn(Mono.just("default-org-id")); Mockito.when(repository.findByEmail(sampleEmail)).thenReturn(Mono.empty()); - Mockito.when(repository.findFirstByEmailIgnoreCaseOrderByCreatedAtDesc(sampleEmail)) + Mockito.when(repository.findByEmailAndOrganizationId(Mockito.eq(sampleEmail), Mockito.eq("default-org-id"))) + .thenReturn(Mono.empty()); + Mockito.when(repository.findFirstByEmailIgnoreCaseAndOrganizationIdOrderByCreatedAtDesc( + Mockito.eq(sampleEmail), Mockito.any())) .thenReturn(Mono.empty()); StepVerifier.create(reactiveUserDetailsService.findByUsername(sampleEmail)) @@ -46,8 +54,12 @@ public class CustomFormLoginServiceImplUnitTest { user.setPassword("1234"); user.setEmail(sampleEmail2.toLowerCase()); + Mockito.when(userOrganizationHelper.getCurrentUserOrganizationId()).thenReturn(Mono.just("default-org-id")); Mockito.when(repository.findByEmail(sampleEmail2)).thenReturn(Mono.empty()); - Mockito.when(repository.findFirstByEmailIgnoreCaseOrderByCreatedAtDesc(sampleEmail2)) + Mockito.when(repository.findByEmailAndOrganizationId(Mockito.eq(sampleEmail2), Mockito.eq("default-org-id"))) + .thenReturn(Mono.empty()); + Mockito.when(repository.findFirstByEmailIgnoreCaseAndOrganizationIdOrderByCreatedAtDesc( + Mockito.eq(sampleEmail2), Mockito.any())) .thenReturn(Mono.just(user)); StepVerifier.create(reactiveUserDetailsService.findByUsername(sampleEmail2)) diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/repositories/UserRepositoryTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/repositories/UserRepositoryTest.java index ce068ed1bf..681a13262a 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/repositories/UserRepositoryTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/repositories/UserRepositoryTest.java @@ -28,6 +28,7 @@ public class UserRepositoryTest { private UserRepository userRepository; private final List savedUsers = new ArrayList<>(); + private static final String ORG_ID = UUID.randomUUID().toString(); @BeforeEach public void setUp() { @@ -45,10 +46,12 @@ public class UserRepositoryTest { public void findByCaseInsensitiveEmail_WhenCaseIsSame_ReturnsResult() { User user = new User(); user.setEmail("rafiqnayan@gmail.com"); + user.setOrganizationId(ORG_ID); User savedUser = userRepository.save(user).block(); savedUsers.add(savedUser); - Mono findUserMono = userRepository.findFirstByEmailIgnoreCaseOrderByCreatedAtDesc("rafiqnayan@gmail.com"); + Mono findUserMono = userRepository.findFirstByEmailIgnoreCaseAndOrganizationIdOrderByCreatedAtDesc( + "rafiqnayan@gmail.com", ORG_ID); StepVerifier.create(findUserMono) .assertNext(u -> { @@ -61,11 +64,12 @@ public class UserRepositoryTest { public void findByCaseInsensitiveEmail_WhenCaseIsDifferent_ReturnsResult() { User user = new User(); user.setEmail("rafiqNAYAN@gmail.com"); + user.setOrganizationId(ORG_ID); User savedUser = userRepository.save(user).block(); savedUsers.add(savedUser); - Mono findUserByEmailMono = - userRepository.findFirstByEmailIgnoreCaseOrderByCreatedAtDesc("rafiqnayan@gmail.com"); + Mono findUserByEmailMono = userRepository.findFirstByEmailIgnoreCaseAndOrganizationIdOrderByCreatedAtDesc( + "rafiqnayan@gmail.com", ORG_ID); StepVerifier.create(findUserByEmailMono) .assertNext(u -> { @@ -78,16 +82,18 @@ public class UserRepositoryTest { public void findByCaseInsensitiveEmail_WhenMultipleMatches_ReturnsResult() { User user1 = new User(); user1.setEmail("rafiqNAYAN@gmail.com"); + user1.setOrganizationId(ORG_ID); User savedUser1 = userRepository.save(user1).block(); savedUsers.add(savedUser1); User user2 = new User(); user2.setEmail("RAFIQNAYAN@gmail.com"); + user2.setOrganizationId(ORG_ID); User savedUser2 = userRepository.save(user2).block(); savedUsers.add(savedUser2); - Mono findUserByEmailMono = - userRepository.findFirstByEmailIgnoreCaseOrderByCreatedAtDesc("rafiqnayan@gmail.com"); + Mono findUserByEmailMono = userRepository.findFirstByEmailIgnoreCaseAndOrganizationIdOrderByCreatedAtDesc( + "rafiqnayan@gmail.com", ORG_ID); StepVerifier.create(findUserByEmailMono) .assertNext(u -> { @@ -100,22 +106,24 @@ public class UserRepositoryTest { public void findByCaseInsensitiveEmail_WhenNoMatch_ReturnsNone() { User user = new User(); user.setEmail("rafiqnayan@gmail.com"); + user.setOrganizationId(ORG_ID); User savedUser = userRepository.save(user).block(); savedUsers.add(savedUser); - Mono getByEmailMono = userRepository.findFirstByEmailIgnoreCaseOrderByCreatedAtDesc("nayan@gmail.com"); + Mono getByEmailMono = userRepository.findFirstByEmailIgnoreCaseAndOrganizationIdOrderByCreatedAtDesc( + "nayan@gmail.com", ORG_ID); StepVerifier.create(getByEmailMono).verifyComplete(); - Mono getByEmailMono2 = - userRepository.findFirstByEmailIgnoreCaseOrderByCreatedAtDesc("rafiqnayan@gmail.co"); + Mono getByEmailMono2 = userRepository.findFirstByEmailIgnoreCaseAndOrganizationIdOrderByCreatedAtDesc( + "rafiqnayan@gmail.co", ORG_ID); StepVerifier.create(getByEmailMono2).verifyComplete(); - Mono getByEmailMono3 = - userRepository.findFirstByEmailIgnoreCaseOrderByCreatedAtDesc("rafiq.nayan@gmail.com"); + Mono getByEmailMono3 = userRepository.findFirstByEmailIgnoreCaseAndOrganizationIdOrderByCreatedAtDesc( + "rafiq.nayan@gmail.com", ORG_ID); StepVerifier.create(getByEmailMono3).verifyComplete(); - Mono getByEmailMono4 = - userRepository.findFirstByEmailIgnoreCaseOrderByCreatedAtDesc("rafiq.ayan@gmail.com"); + Mono getByEmailMono4 = userRepository.findFirstByEmailIgnoreCaseAndOrganizationIdOrderByCreatedAtDesc( + "rafiq.ayan@gmail.com", ORG_ID); StepVerifier.create(getByEmailMono4).verifyComplete(); } @@ -132,11 +140,12 @@ public class UserRepositoryTest { unsortedEmails.forEach(email -> { User user = new User(); user.setEmail(email); + user.setOrganizationId(ORG_ID); userRepository.save(user).block(); }); List allCreatedUsers = userRepository - .findAllByEmailIn(new HashSet<>(unsortedEmails)) + .findAllByEmailInAndOrganizationId(new HashSet<>(unsortedEmails), ORG_ID) .collectList() .block(); assertEquals(countOfUsersToBeCreated, allCreatedUsers.size()); 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 5cc2d8e327..15534dbfc1 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 @@ -66,6 +66,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; @Slf4j @SpringBootTest @@ -282,7 +283,7 @@ public class UserServiceTest { StepVerifier.create(userMono) .assertNext(user -> { - assertEquals(newUser.getEmail(), user.getEmail()); + assertEquals(newUser.getEmail().toLowerCase(), user.getEmail()); assertEquals(LoginSource.FORM, user.getSource()); assertThat(user.getIsEnabled()).isTrue(); }) @@ -307,7 +308,7 @@ public class UserServiceTest { StepVerifier.create(userMono) .assertNext(user -> { - assertEquals(newUser.getEmail(), user.getEmail()); + assertEquals(newUser.getEmail().toLowerCase(), user.getEmail()); assertEquals(LoginSource.GOOGLE, user.getSource()); assertTrue(user.getIsEnabled()); }) @@ -511,7 +512,8 @@ public class UserServiceTest { passwordResetToken.setFirstRequestTime(Instant.now()); // mock the passwordResetTokenRepository to return request count 3 in 24 hours - Mockito.when(passwordResetTokenRepository.findByEmail(testEmail)).thenReturn(Mono.just(passwordResetToken)); + Mockito.when(passwordResetTokenRepository.findByEmailAndOrganizationId(any(), any())) + .thenReturn(Mono.just(passwordResetToken)); ResetUserPasswordDTO resetUserPasswordDTO = new ResetUserPasswordDTO(); resetUserPasswordDTO.setEmail("test-email-for-password-reset"); @@ -540,7 +542,8 @@ public class UserServiceTest { public void verifyPasswordResetToken_WhenTokenDoesNotExist_ThrowsException() { String testEmail = "abc@example.org"; // mock the passwordResetTokenRepository to return empty - Mockito.when(passwordResetTokenRepository.findByEmail(testEmail)).thenReturn(Mono.empty()); + Mockito.when(passwordResetTokenRepository.findByEmailAndOrganizationId(any(), any())) + .thenReturn(Mono.empty()); StepVerifier.create(userService.verifyPasswordResetToken(getEncodedToken(testEmail, "123456789"))) .expectErrorMessage(AppsmithError.INVALID_PASSWORD_RESET.getMessage()) @@ -553,7 +556,8 @@ public class UserServiceTest { passwordResetToken.setTokenHash(passwordEncoder.encode(token1)); // mock the passwordResetTokenRepository to return empty - Mockito.when(passwordResetTokenRepository.findByEmail(testEmail)).thenReturn(Mono.just(passwordResetToken)); + Mockito.when(passwordResetTokenRepository.findByEmailAndOrganizationId(any(), any())) + .thenReturn(Mono.just(passwordResetToken)); StepVerifier.create(userService.verifyPasswordResetToken(getEncodedToken(testEmail, token2))) .expectNext(expectedResult) @@ -584,7 +588,8 @@ public class UserServiceTest { // ** check if token is not present in DB ** // // mock the passwordResetTokenRepository to return empty - Mockito.when(passwordResetTokenRepository.findByEmail(testEmail)).thenReturn(Mono.empty()); + Mockito.when(passwordResetTokenRepository.findByEmailAndOrganizationId(any(), any())) + .thenReturn(Mono.empty()); StepVerifier.create(userService.resetPasswordAfterForgotPassword(token, null)) .expectErrorMessage(AppsmithError.INVALID_PASSWORD_RESET.getMessage()) @@ -595,7 +600,8 @@ public class UserServiceTest { passwordResetToken.setTokenHash(passwordEncoder.encode("abcdef")); // mock the passwordResetTokenRepository to return empty - Mockito.when(passwordResetTokenRepository.findByEmail(testEmail)).thenReturn(Mono.just(passwordResetToken)); + Mockito.when(passwordResetTokenRepository.findByEmailAndOrganizationId(any(), any())) + .thenReturn(Mono.just(passwordResetToken)); StepVerifier.create(userService.resetPasswordAfterForgotPassword(token, null)) .expectErrorMessage(AppsmithError.GENERIC_BAD_REQUEST.getMessage(FieldName.TOKEN)) @@ -659,23 +665,27 @@ public class UserServiceTest { } @Test + @WithUserDetails("api_user") public void emailVerificationTokenGenerate_WhenInstanceEmailVerificationIsNotEnabled_ThrowsException() { String testEmail = "test-email-for-verification"; + Mono currentUserOrganizationMono = + organizationService.getCurrentUserOrganization().cache(); // create user User newUser = new User(); newUser.setEmail(testEmail); - Mono userMono = userRepository.save(newUser); + Mono userMono = currentUserOrganizationMono.flatMap(organization -> { + newUser.setOrganizationId(organization.getId()); + return userRepository.save(newUser); + }); // Setting Organization Config emailVerificationEnabled to FALSE - Mono organizationMono = organizationService - .getCurrentUserOrganization() - .flatMap(organization -> { - OrganizationConfiguration organizationConfiguration = organization.getOrganizationConfiguration(); - organizationConfiguration.setEmailVerificationEnabled(Boolean.FALSE); - organization.setOrganizationConfiguration(organizationConfiguration); - return organizationService.update(organization.getId(), organization); - }); + Mono organizationMono = currentUserOrganizationMono.flatMap(organization -> { + OrganizationConfiguration organizationConfiguration = organization.getOrganizationConfiguration(); + organizationConfiguration.setEmailVerificationEnabled(Boolean.FALSE); + organization.setOrganizationConfiguration(organizationConfiguration); + return organizationService.update(organization.getId(), organization); + }); Mono emailVerificationMono = userService.resendEmailVerification(getResendEmailVerificationDTO(testEmail), null); @@ -688,31 +698,32 @@ public class UserServiceTest { } @Test + @WithUserDetails("api_user") public void emailVerificationTokenGenerate_WhenUserEmailAlreadyVerified_ThrowsException() { String testEmail = "test-email-for-verification-user-already-verified"; - // create user + // Get current organization first + Organization organization = + organizationService.getCurrentUserOrganization().block(); + + // create user with organization ID User newUser = new User(); newUser.setEmail(testEmail); newUser.setEmailVerified(Boolean.TRUE); - Mono userMono = userRepository.save(newUser); + newUser.setOrganizationId(organization.getId()); + userRepository.save(newUser).block(); // Setting Organization Config emailVerificationEnabled to TRUE - Mono organizationMono = organizationService - .getCurrentUserOrganization() - .flatMap(organization -> { - OrganizationConfiguration organizationConfiguration = organization.getOrganizationConfiguration(); - organizationConfiguration.setEmailVerificationEnabled(Boolean.TRUE); - organization.setOrganizationConfiguration(organizationConfiguration); - return organizationService.update(organization.getId(), organization); - }); + + OrganizationConfiguration organizationConfiguration = organization.getOrganizationConfiguration(); + organizationConfiguration.setEmailVerificationEnabled(Boolean.TRUE); + organization.setOrganizationConfiguration(organizationConfiguration); + organizationService.update(organization.getId(), organization).block(); Mono emailVerificationMono = userService.resendEmailVerification(getResendEmailVerificationDTO(testEmail), null); - Mono resultMono = userMono.then(organizationMono).then(emailVerificationMono); - - StepVerifier.create(resultMono) + StepVerifier.create(emailVerificationMono) .expectErrorMessage(AppsmithError.USER_ALREADY_VERIFIED.getMessage()) .verify(); } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceWithDisabledSignupTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceWithDisabledSignupTest.java index bb28aca333..1953520cd1 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceWithDisabledSignupTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceWithDisabledSignupTest.java @@ -7,6 +7,7 @@ import com.appsmith.server.domains.LoginSource; import com.appsmith.server.domains.User; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; +import com.appsmith.server.helpers.UserOrganizationHelper; import com.appsmith.server.repositories.PermissionGroupRepository; import com.appsmith.server.repositories.UserRepository; import com.appsmith.server.repositories.WorkspaceRepository; @@ -47,6 +48,9 @@ public class UserServiceWithDisabledSignupTest { @Autowired PermissionGroupRepository permissionGroupRepository; + @Autowired + UserOrganizationHelper userOrganizationHelper; + @SpyBean CommonConfig commonConfig; @@ -146,9 +150,13 @@ public class UserServiceWithDisabledSignupTest { @Test @WithMockAppsmithUser public void signUpViaFormLoginIfAlreadyInvited() { + String organizationId = + userOrganizationHelper.getCurrentUserOrganizationId().block(); + User newUser = new User(); newUser.setEmail("alreadyInvited@alreadyInvited.com"); newUser.setIsEnabled(false); + newUser.setOrganizationId(organizationId); userRepository.save(newUser).block();