chore: Removing user repo fetches only by email (#40165)
## 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 sanity ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/14330369135> > Commit: ea2dba000d77e0b8cd7a46ba2259a076bfb4086e > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=14330369135&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Sanity` > Spec: > <hr>Tue, 08 Apr 2025 10:21: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 - **Refactor** - Streamlined user account and session management workflows by integrating organization-specific checks into processes like password resets, invitations, and general user retrieval. - Phased out older lookup methods in favor of a more robust, organization-aware approach that enhances both accuracy and security. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
parent
507bb90307
commit
933e968d34
|
|
@ -11,6 +11,7 @@ import java.util.Set;
|
||||||
|
|
||||||
public interface UserRepositoryCE extends BaseRepository<User, String>, CustomUserRepository {
|
public interface UserRepositoryCE extends BaseRepository<User, String>, CustomUserRepository {
|
||||||
|
|
||||||
|
@Deprecated
|
||||||
Mono<User> findByEmail(String email);
|
Mono<User> findByEmail(String email);
|
||||||
|
|
||||||
Mono<User> findFirstByEmailIgnoreCaseAndOrganizationIdOrderByCreatedAtDesc(String email, String organizationId);
|
Mono<User> findFirstByEmailIgnoreCaseAndOrganizationIdOrderByCreatedAtDesc(String email, String organizationId);
|
||||||
|
|
|
||||||
|
|
@ -44,7 +44,9 @@ public class SessionUserServiceCEImpl implements SessionUserServiceCE {
|
||||||
@Override
|
@Override
|
||||||
public Mono<User> refreshCurrentUser(ServerWebExchange exchange) {
|
public Mono<User> refreshCurrentUser(ServerWebExchange exchange) {
|
||||||
return Mono.zip(
|
return Mono.zip(
|
||||||
getCurrentUser().map(User::getEmail).flatMap(userRepository::findByEmail),
|
getCurrentUser()
|
||||||
|
.flatMap(sessionUser -> userRepository.findByEmailAndOrganizationId(
|
||||||
|
sessionUser.getEmail(), sessionUser.getOrganizationId())),
|
||||||
ReactiveSecurityContextHolder.getContext(),
|
ReactiveSecurityContextHolder.getContext(),
|
||||||
exchange.getSession())
|
exchange.getSession())
|
||||||
.flatMap(tuple -> {
|
.flatMap(tuple -> {
|
||||||
|
|
|
||||||
|
|
@ -324,6 +324,8 @@ public class UserServiceCEImpl extends BaseService<UserRepository, User, String>
|
||||||
.switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, USER, ORGANIZATION)))
|
.switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, USER, ORGANIZATION)))
|
||||||
.cache();
|
.cache();
|
||||||
|
|
||||||
|
Mono<String> orgIdMono = organizationMono.map(Organization::getId);
|
||||||
|
|
||||||
return organizationMono
|
return organizationMono
|
||||||
.flatMap(organization -> passwordResetTokenRepository.findByEmailAndOrganizationId(
|
.flatMap(organization -> passwordResetTokenRepository.findByEmailAndOrganizationId(
|
||||||
emailTokenDTO.getEmail(), organization.getId()))
|
emailTokenDTO.getEmail(), organization.getId()))
|
||||||
|
|
@ -337,69 +339,69 @@ public class UserServiceCEImpl extends BaseService<UserRepository, User, String>
|
||||||
return emailTokenDTO.getEmail();
|
return emailTokenDTO.getEmail();
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
.flatMap(emailAddress -> repository
|
.zipWith(orgIdMono)
|
||||||
.findByEmail(emailAddress)
|
.flatMap(tuple -> {
|
||||||
.switchIfEmpty(Mono.error(
|
String emailAddress = tuple.getT1();
|
||||||
new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, FieldName.USER, emailAddress)))
|
String orgId = tuple.getT2();
|
||||||
.zipWith(organizationMono)
|
return repository
|
||||||
.flatMap(tuple -> {
|
.findByEmailAndOrganizationId(emailAddress, orgId)
|
||||||
User userFromDb = tuple.getT1();
|
.switchIfEmpty(Mono.error(new AppsmithException(
|
||||||
OrganizationConfiguration organizationConfiguration =
|
AppsmithError.NO_RESOURCE_FOUND, FieldName.USER, emailAddress)));
|
||||||
tuple.getT2().getOrganizationConfiguration();
|
})
|
||||||
boolean isStrongPasswordPolicyEnabled = organizationConfiguration != null
|
.zipWith(organizationMono)
|
||||||
&& Boolean.TRUE.equals(
|
.flatMap(tuple -> {
|
||||||
organizationConfiguration.getIsStrongPasswordPolicyEnabled());
|
User userFromDb = tuple.getT1();
|
||||||
|
OrganizationConfiguration organizationConfiguration =
|
||||||
|
tuple.getT2().getOrganizationConfiguration();
|
||||||
|
boolean isStrongPasswordPolicyEnabled = organizationConfiguration != null
|
||||||
|
&& Boolean.TRUE.equals(organizationConfiguration.getIsStrongPasswordPolicyEnabled());
|
||||||
|
|
||||||
if (!validateUserPassword(user.getPassword(), isStrongPasswordPolicyEnabled)) {
|
if (!validateUserPassword(user.getPassword(), isStrongPasswordPolicyEnabled)) {
|
||||||
return isStrongPasswordPolicyEnabled
|
return isStrongPasswordPolicyEnabled
|
||||||
? Mono.error(new AppsmithException(
|
? Mono.error(new AppsmithException(
|
||||||
AppsmithError.INSUFFICIENT_PASSWORD_STRENGTH,
|
AppsmithError.INSUFFICIENT_PASSWORD_STRENGTH,
|
||||||
LOGIN_PASSWORD_MIN_LENGTH,
|
LOGIN_PASSWORD_MIN_LENGTH,
|
||||||
LOGIN_PASSWORD_MAX_LENGTH))
|
LOGIN_PASSWORD_MAX_LENGTH))
|
||||||
: Mono.error(new AppsmithException(
|
: Mono.error(new AppsmithException(
|
||||||
AppsmithError.INVALID_PASSWORD_LENGTH,
|
AppsmithError.INVALID_PASSWORD_LENGTH,
|
||||||
LOGIN_PASSWORD_MIN_LENGTH,
|
LOGIN_PASSWORD_MIN_LENGTH,
|
||||||
LOGIN_PASSWORD_MAX_LENGTH));
|
LOGIN_PASSWORD_MAX_LENGTH));
|
||||||
}
|
}
|
||||||
|
|
||||||
// User has verified via the forgot password token verfication route. Allow the user to set
|
// User has verified via the forgot password token verfication route. Allow the user to set
|
||||||
// new password.
|
// new password.
|
||||||
userFromDb.setPasswordResetInitiated(false);
|
userFromDb.setPasswordResetInitiated(false);
|
||||||
userFromDb.setPassword(passwordEncoder.encode(user.getPassword()));
|
userFromDb.setPassword(passwordEncoder.encode(user.getPassword()));
|
||||||
|
|
||||||
// If the user has been invited but has not signed up yet, and is following the route of
|
// If the user has been invited but has not signed up yet, and is following the route of
|
||||||
// reset
|
// reset
|
||||||
// password flow to set up their password, enable the user's account as well
|
// password flow to set up their password, enable the user's account as well
|
||||||
userFromDb.setIsEnabled(true);
|
userFromDb.setIsEnabled(true);
|
||||||
|
|
||||||
return organizationService
|
return organizationService
|
||||||
.getCurrentUserOrganizationId()
|
.getCurrentUserOrganizationId()
|
||||||
.flatMap(
|
.flatMap(organizationId -> passwordResetTokenRepository.findByEmailAndOrganizationId(
|
||||||
organizationId -> passwordResetTokenRepository.findByEmailAndOrganizationId(
|
userFromDb.getEmail(), organizationId))
|
||||||
userFromDb.getEmail(), organizationId))
|
.switchIfEmpty(Mono.error(new AppsmithException(
|
||||||
.switchIfEmpty(Mono.error(new AppsmithException(
|
AppsmithError.NO_RESOURCE_FOUND, FieldName.TOKEN, emailTokenDTO.getToken())))
|
||||||
AppsmithError.NO_RESOURCE_FOUND,
|
.flatMap(passwordResetTokenRepository::delete)
|
||||||
FieldName.TOKEN,
|
.then(repository.save(userFromDb))
|
||||||
emailTokenDTO.getToken())))
|
.doOnSuccess(result -> {
|
||||||
.flatMap(passwordResetTokenRepository::delete)
|
// In a separate thread, we delete all other sessions of this user.
|
||||||
.then(repository.save(userFromDb))
|
sessionUserService
|
||||||
.doOnSuccess(result -> {
|
.logoutAllSessions(userFromDb.getEmail())
|
||||||
// In a separate thread, we delete all other sessions of this user.
|
.subscribeOn(Schedulers.boundedElastic())
|
||||||
sessionUserService
|
.subscribe();
|
||||||
.logoutAllSessions(userFromDb.getEmail())
|
|
||||||
.subscribeOn(Schedulers.boundedElastic())
|
|
||||||
.subscribe();
|
|
||||||
|
|
||||||
// we reset the counter for user's login attempts once password is reset
|
// we reset the counter for user's login attempts once password is reset
|
||||||
rateLimitService
|
rateLimitService
|
||||||
.resetCounter(
|
.resetCounter(
|
||||||
RateLimitConstants.BUCKET_KEY_FOR_LOGIN_API,
|
RateLimitConstants.BUCKET_KEY_FOR_LOGIN_API, userFromDb.getEmail())
|
||||||
userFromDb.getEmail())
|
.subscribeOn(Schedulers.boundedElastic())
|
||||||
.subscribeOn(Schedulers.boundedElastic())
|
.subscribe();
|
||||||
.subscribe();
|
})
|
||||||
})
|
.thenReturn(true);
|
||||||
.thenReturn(true);
|
});
|
||||||
}));
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
|
|
@ -696,7 +698,7 @@ public class UserServiceCEImpl extends BaseService<UserRepository, User, String>
|
||||||
.flatMap(user -> updateWithoutPermission(user.getId(), updates)
|
.flatMap(user -> updateWithoutPermission(user.getId(), updates)
|
||||||
.then(
|
.then(
|
||||||
exchange == null
|
exchange == null
|
||||||
? repository.findByEmail(user.getEmail())
|
? findByEmail(user.getEmail())
|
||||||
: sessionUserService.refreshCurrentUser(exchange)))
|
: sessionUserService.refreshCurrentUser(exchange)))
|
||||||
.cache();
|
.cache();
|
||||||
monos.add(updatedUserMono.then());
|
monos.add(updatedUserMono.then());
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,7 @@
|
||||||
package com.appsmith.server.solutions;
|
package com.appsmith.server.solutions;
|
||||||
|
|
||||||
import com.appsmith.server.configurations.CommonConfig;
|
import com.appsmith.server.configurations.CommonConfig;
|
||||||
|
import com.appsmith.server.helpers.UserOrganizationHelper;
|
||||||
import com.appsmith.server.repositories.UserRepository;
|
import com.appsmith.server.repositories.UserRepository;
|
||||||
import com.appsmith.server.services.AnalyticsService;
|
import com.appsmith.server.services.AnalyticsService;
|
||||||
import com.appsmith.server.services.CaptchaService;
|
import com.appsmith.server.services.CaptchaService;
|
||||||
|
|
@ -28,8 +29,8 @@ public class UserAndAccessManagementServiceImpl extends UserAndAccessManagementS
|
||||||
PermissionGroupPermission permissionGroupPermission,
|
PermissionGroupPermission permissionGroupPermission,
|
||||||
EmailService emailService,
|
EmailService emailService,
|
||||||
CommonConfig commonConfig,
|
CommonConfig commonConfig,
|
||||||
|
UserOrganizationHelper userOrganizationHelper,
|
||||||
CaptchaService captchaService) {
|
CaptchaService captchaService) {
|
||||||
|
|
||||||
super(
|
super(
|
||||||
sessionUserService,
|
sessionUserService,
|
||||||
permissionGroupService,
|
permissionGroupService,
|
||||||
|
|
@ -40,6 +41,7 @@ public class UserAndAccessManagementServiceImpl extends UserAndAccessManagementS
|
||||||
permissionGroupPermission,
|
permissionGroupPermission,
|
||||||
emailService,
|
emailService,
|
||||||
commonConfig,
|
commonConfig,
|
||||||
|
userOrganizationHelper,
|
||||||
captchaService);
|
captchaService);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -9,6 +9,7 @@ import com.appsmith.server.domains.Workspace;
|
||||||
import com.appsmith.server.dtos.InviteUsersDTO;
|
import com.appsmith.server.dtos.InviteUsersDTO;
|
||||||
import com.appsmith.server.exceptions.AppsmithError;
|
import com.appsmith.server.exceptions.AppsmithError;
|
||||||
import com.appsmith.server.exceptions.AppsmithException;
|
import com.appsmith.server.exceptions.AppsmithException;
|
||||||
|
import com.appsmith.server.helpers.UserOrganizationHelper;
|
||||||
import com.appsmith.server.helpers.ValidationUtils;
|
import com.appsmith.server.helpers.ValidationUtils;
|
||||||
import com.appsmith.server.repositories.UserRepository;
|
import com.appsmith.server.repositories.UserRepository;
|
||||||
import com.appsmith.server.services.AnalyticsService;
|
import com.appsmith.server.services.AnalyticsService;
|
||||||
|
|
@ -47,7 +48,7 @@ public class UserAndAccessManagementServiceCEImpl implements UserAndAccessManage
|
||||||
private final UserService userService;
|
private final UserService userService;
|
||||||
private final PermissionGroupPermission permissionGroupPermission;
|
private final PermissionGroupPermission permissionGroupPermission;
|
||||||
private final EmailService emailService;
|
private final EmailService emailService;
|
||||||
private final CommonConfig commonConfig;
|
private final UserOrganizationHelper userOrganizationHelper;
|
||||||
|
|
||||||
private final CaptchaService captchaService;
|
private final CaptchaService captchaService;
|
||||||
|
|
||||||
|
|
@ -61,6 +62,7 @@ public class UserAndAccessManagementServiceCEImpl implements UserAndAccessManage
|
||||||
PermissionGroupPermission permissionGroupPermission,
|
PermissionGroupPermission permissionGroupPermission,
|
||||||
EmailService emailService,
|
EmailService emailService,
|
||||||
CommonConfig commonConfig,
|
CommonConfig commonConfig,
|
||||||
|
UserOrganizationHelper userOrganizationHelper,
|
||||||
CaptchaService captchaService) {
|
CaptchaService captchaService) {
|
||||||
|
|
||||||
this.sessionUserService = sessionUserService;
|
this.sessionUserService = sessionUserService;
|
||||||
|
|
@ -71,7 +73,7 @@ public class UserAndAccessManagementServiceCEImpl implements UserAndAccessManage
|
||||||
this.userService = userService;
|
this.userService = userService;
|
||||||
this.emailService = emailService;
|
this.emailService = emailService;
|
||||||
this.permissionGroupPermission = permissionGroupPermission;
|
this.permissionGroupPermission = permissionGroupPermission;
|
||||||
this.commonConfig = commonConfig;
|
this.userOrganizationHelper = userOrganizationHelper;
|
||||||
this.captchaService = captchaService;
|
this.captchaService = captchaService;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -160,8 +162,10 @@ public class UserAndAccessManagementServiceCEImpl implements UserAndAccessManage
|
||||||
eventData.put(FieldName.WORKSPACE, workspace);
|
eventData.put(FieldName.WORKSPACE, workspace);
|
||||||
List<PermissionGroup> defaultPermissionGroups = tuple.getT3();
|
List<PermissionGroup> defaultPermissionGroups = tuple.getT3();
|
||||||
|
|
||||||
Mono<User> getUserFromDbAndCheckIfUserExists = userRepository
|
Mono<User> getUserFromDbAndCheckIfUserExists = userOrganizationHelper
|
||||||
.findByEmail(username)
|
.getCurrentUserOrganizationId()
|
||||||
|
.flatMap(organizationId ->
|
||||||
|
userRepository.findByEmailAndOrganizationId(username, organizationId))
|
||||||
.flatMap(user -> throwErrorIfUserAlreadyExistsInWorkspace(user, defaultPermissionGroups)
|
.flatMap(user -> throwErrorIfUserAlreadyExistsInWorkspace(user, defaultPermissionGroups)
|
||||||
.thenReturn(user));
|
.thenReturn(user));
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,7 @@
|
||||||
package com.appsmith.server.solutions.ce_compatible;
|
package com.appsmith.server.solutions.ce_compatible;
|
||||||
|
|
||||||
import com.appsmith.server.configurations.CommonConfig;
|
import com.appsmith.server.configurations.CommonConfig;
|
||||||
|
import com.appsmith.server.helpers.UserOrganizationHelper;
|
||||||
import com.appsmith.server.repositories.UserRepository;
|
import com.appsmith.server.repositories.UserRepository;
|
||||||
import com.appsmith.server.services.AnalyticsService;
|
import com.appsmith.server.services.AnalyticsService;
|
||||||
import com.appsmith.server.services.CaptchaService;
|
import com.appsmith.server.services.CaptchaService;
|
||||||
|
|
@ -16,6 +17,7 @@ import org.springframework.stereotype.Component;
|
||||||
@Component
|
@Component
|
||||||
public class UserAndAccessManagementServiceCECompatibleImpl extends UserAndAccessManagementServiceCEImpl
|
public class UserAndAccessManagementServiceCECompatibleImpl extends UserAndAccessManagementServiceCEImpl
|
||||||
implements UserAndAccessManagementServiceCECompatible {
|
implements UserAndAccessManagementServiceCECompatible {
|
||||||
|
|
||||||
public UserAndAccessManagementServiceCECompatibleImpl(
|
public UserAndAccessManagementServiceCECompatibleImpl(
|
||||||
SessionUserService sessionUserService,
|
SessionUserService sessionUserService,
|
||||||
PermissionGroupService permissionGroupService,
|
PermissionGroupService permissionGroupService,
|
||||||
|
|
@ -26,6 +28,7 @@ public class UserAndAccessManagementServiceCECompatibleImpl extends UserAndAcces
|
||||||
PermissionGroupPermission permissionGroupPermission,
|
PermissionGroupPermission permissionGroupPermission,
|
||||||
EmailService emailService,
|
EmailService emailService,
|
||||||
CommonConfig commonConfig,
|
CommonConfig commonConfig,
|
||||||
|
UserOrganizationHelper userOrganizationHelper,
|
||||||
CaptchaService captchaService) {
|
CaptchaService captchaService) {
|
||||||
super(
|
super(
|
||||||
sessionUserService,
|
sessionUserService,
|
||||||
|
|
@ -37,6 +40,7 @@ public class UserAndAccessManagementServiceCECompatibleImpl extends UserAndAcces
|
||||||
permissionGroupPermission,
|
permissionGroupPermission,
|
||||||
emailService,
|
emailService,
|
||||||
commonConfig,
|
commonConfig,
|
||||||
|
userOrganizationHelper,
|
||||||
captchaService);
|
captchaService);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user