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

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!WARNING]
> Tests have not run on the HEAD
f0cb554c5dde764044c075663433fb4ea7ce684c yet
> <hr>Wed, 12 Mar 2025 13:55:54 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [ ] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## 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.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Trisha Anand 2025-03-13 11:40:51 +05:30 committed by GitHub
parent 82294f9f85
commit ee689d7c87
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
24 changed files with 360 additions and 141 deletions

View File

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

View File

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

View File

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

View File

@ -75,7 +75,10 @@ public class AuthenticationSuccessHandlerCE implements ServerAuthenticationSucce
.map(organization -> organization.getOrganizationConfiguration().isEmailVerificationEnabled())
.cache();
Mono<User> userMono = userRepository.findByEmail(userEmail).cache();
Mono<User> userMono = organizationService
.getCurrentUserOrganizationId()
.flatMap(orgId -> userRepository.findByEmailAndOrganizationId(userEmail, orgId))
.cache();
Mono<Boolean> 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());

View File

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

View File

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

View File

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

View File

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

View File

@ -0,0 +1,5 @@
package com.appsmith.server.helpers;
import com.appsmith.server.helpers.ce.UserOrganizationHelperCE;
public interface UserOrganizationHelper extends UserOrganizationHelperCE {}

View File

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

View File

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

View File

@ -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<String> getCurrentUserOrganizationId() {
return getDefaultOrganizationId();
}
protected Mono<String> 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;
});
}
}

View File

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

View File

@ -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<User> {
Mono<User> findByEmail(String email, AclPermission aclPermission);
Mono<User> findByEmailAndOrganizationId(String email, String organizationId);
Mono<Boolean> isUsersEmpty();

View File

@ -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<User> implements CustomUserRepositoryCE {
@Override
public Mono<User> findByEmail(String email, AclPermission aclPermission) {
BridgeQuery<User> emailCriteria = Bridge.equal(User.Fields.email, email);
return queryBuilder().criteria(emailCriteria).permission(aclPermission).one();
}
@Override
public Mono<User> findByEmailAndOrganizationId(String email, String organizationId) {
return queryBuilder()

View File

@ -6,5 +6,5 @@ import reactor.core.publisher.Mono;
public interface PasswordResetTokenRepositoryCE extends BaseRepository<PasswordResetToken, String> {
Mono<PasswordResetToken> findByEmail(String email);
Mono<PasswordResetToken> findByEmailAndOrganizationId(String email, String organizationId);
}

View File

@ -13,9 +13,9 @@ public interface UserRepositoryCE extends BaseRepository<User, String>, CustomUs
Mono<User> findByEmail(String email);
Mono<User> findFirstByEmailIgnoreCaseOrderByCreatedAtDesc(String email);
Mono<User> findFirstByEmailIgnoreCaseAndOrganizationIdOrderByCreatedAtDesc(String email, String organizationId);
Flux<User> findAllByEmailIn(Set<String> emails);
Flux<User> findAllByEmailInAndOrganizationId(Set<String> emails, String organizationId);
/**
* This method returns the count of all users that are not deleted and are not system generated.

View File

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

View File

@ -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<OrganizationRepository, Organization, String>
implements OrganizationServiceCE {
private String defaultOrganizationId = null;
private final ConfigService configService;
private final EnvManager envManager;
@ -55,6 +54,8 @@ public class OrganizationServiceCEImpl extends BaseService<OrganizationRepositor
private final CommonConfig commonConfig;
private final ObservationRegistry observationRegistry;
private final UserOrganizationHelper userOrganizationHelper;
public OrganizationServiceCEImpl(
Validator validator,
OrganizationRepository repository,
@ -64,7 +65,8 @@ public class OrganizationServiceCEImpl extends BaseService<OrganizationRepositor
FeatureFlagMigrationHelper featureFlagMigrationHelper,
CacheableRepositoryHelper cacheableRepositoryHelper,
CommonConfig commonConfig,
ObservationRegistry observationRegistry) {
ObservationRegistry observationRegistry,
UserOrganizationHelper userOrganizationHelper) {
super(validator, repository, analyticsService);
this.configService = configService;
this.envManager = envManager;
@ -72,19 +74,12 @@ public class OrganizationServiceCEImpl extends BaseService<OrganizationRepositor
this.cacheableRepositoryHelper = cacheableRepositoryHelper;
this.commonConfig = commonConfig;
this.observationRegistry = observationRegistry;
this.userOrganizationHelper = userOrganizationHelper;
}
@Override
public Mono<String> 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

View File

@ -189,28 +189,34 @@ public class UserServiceCEImpl extends BaseService<UserRepository, User, String>
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<UserRepository, User, String>
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<UserRepository, User, String>
Mono<Organization> 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<UserRepository, User, String>
// 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<UserRepository, User, String>
// convert the user email to lowercase
user.setEmail(user.getEmail().toLowerCase());
Mono<User> 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<User> 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<UserRepository, User, String>
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<UserRepository, User, String>
}
// 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<UserRepository, User, String>
@Override
public Flux<User> getAllByEmails(Set<String> 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<UserRepository, User, String>
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<User> userMono = repository.findByEmail(email).cache();
return userMono.switchIfEmpty(repository.findFirstByEmailIgnoreCaseOrderByCreatedAtDesc(email))
.switchIfEmpty(
Mono<User> 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())) {

View File

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

View File

@ -28,6 +28,7 @@ public class UserRepositoryTest {
private UserRepository userRepository;
private final List<User> 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<User> findUserMono = userRepository.findFirstByEmailIgnoreCaseOrderByCreatedAtDesc("rafiqnayan@gmail.com");
Mono<User> 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<User> findUserByEmailMono =
userRepository.findFirstByEmailIgnoreCaseOrderByCreatedAtDesc("rafiqnayan@gmail.com");
Mono<User> 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<User> findUserByEmailMono =
userRepository.findFirstByEmailIgnoreCaseOrderByCreatedAtDesc("rafiqnayan@gmail.com");
Mono<User> 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<User> getByEmailMono = userRepository.findFirstByEmailIgnoreCaseOrderByCreatedAtDesc("nayan@gmail.com");
Mono<User> getByEmailMono = userRepository.findFirstByEmailIgnoreCaseAndOrganizationIdOrderByCreatedAtDesc(
"nayan@gmail.com", ORG_ID);
StepVerifier.create(getByEmailMono).verifyComplete();
Mono<User> getByEmailMono2 =
userRepository.findFirstByEmailIgnoreCaseOrderByCreatedAtDesc("rafiqnayan@gmail.co");
Mono<User> getByEmailMono2 = userRepository.findFirstByEmailIgnoreCaseAndOrganizationIdOrderByCreatedAtDesc(
"rafiqnayan@gmail.co", ORG_ID);
StepVerifier.create(getByEmailMono2).verifyComplete();
Mono<User> getByEmailMono3 =
userRepository.findFirstByEmailIgnoreCaseOrderByCreatedAtDesc("rafiq.nayan@gmail.com");
Mono<User> getByEmailMono3 = userRepository.findFirstByEmailIgnoreCaseAndOrganizationIdOrderByCreatedAtDesc(
"rafiq.nayan@gmail.com", ORG_ID);
StepVerifier.create(getByEmailMono3).verifyComplete();
Mono<User> getByEmailMono4 =
userRepository.findFirstByEmailIgnoreCaseOrderByCreatedAtDesc("rafiq.ayan@gmail.com");
Mono<User> 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<User> allCreatedUsers = userRepository
.findAllByEmailIn(new HashSet<>(unsortedEmails))
.findAllByEmailInAndOrganizationId(new HashSet<>(unsortedEmails), ORG_ID)
.collectList()
.block();
assertEquals(countOfUsersToBeCreated, allCreatedUsers.size());

View File

@ -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<Organization> currentUserOrganizationMono =
organizationService.getCurrentUserOrganization().cache();
// create user
User newUser = new User();
newUser.setEmail(testEmail);
Mono<User> userMono = userRepository.save(newUser);
Mono<User> userMono = currentUserOrganizationMono.flatMap(organization -> {
newUser.setOrganizationId(organization.getId());
return userRepository.save(newUser);
});
// Setting Organization Config emailVerificationEnabled to FALSE
Mono<Organization> organizationMono = organizationService
.getCurrentUserOrganization()
.flatMap(organization -> {
OrganizationConfiguration organizationConfiguration = organization.getOrganizationConfiguration();
organizationConfiguration.setEmailVerificationEnabled(Boolean.FALSE);
organization.setOrganizationConfiguration(organizationConfiguration);
return organizationService.update(organization.getId(), organization);
});
Mono<Organization> organizationMono = currentUserOrganizationMono.flatMap(organization -> {
OrganizationConfiguration organizationConfiguration = organization.getOrganizationConfiguration();
organizationConfiguration.setEmailVerificationEnabled(Boolean.FALSE);
organization.setOrganizationConfiguration(organizationConfiguration);
return organizationService.update(organization.getId(), organization);
});
Mono<Boolean> 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<User> userMono = userRepository.save(newUser);
newUser.setOrganizationId(organization.getId());
userRepository.save(newUser).block();
// Setting Organization Config emailVerificationEnabled to TRUE
Mono<Organization> 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<Boolean> emailVerificationMono =
userService.resendEmailVerification(getResendEmailVerificationDTO(testEmail), null);
Mono<Boolean> resultMono = userMono.then(organizationMono).then(emailVerificationMono);
StepVerifier.create(resultMono)
StepVerifier.create(emailVerificationMono)
.expectErrorMessage(AppsmithError.USER_ALREADY_VERIFIED.getMessage())
.verify();
}

View File

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