fix: invite user should not update existing member's role (#19254)
Fixes #19244 Fixes #17606 If an existing member is invited again from the Share modal with a different role, the user gets assigned the other role as well. This breaks the framework that a member of the workspace can only have a SINGLE role. This breaks other flows like update role/delete member etc because they all run with the assumption that the member can not have more than one role in the workspace. This PR fixes this by throwing an error if a user is being invited again to the workspace.
This commit is contained in:
Trisha Anand
GitHub
parent
4446e33a76
commit
788cfe995b
|
|
@ -29,6 +29,7 @@ import java.util.Map;
|
|||
import java.util.stream.Collectors;
|
||||
|
||||
import static com.appsmith.server.services.ce.UserServiceCEImpl.INVITE_USER_EMAIL_TEMPLATE;
|
||||
import static java.lang.Boolean.TRUE;
|
||||
|
||||
@Slf4j
|
||||
public class UserAndAccessManagementServiceCEImpl implements UserAndAccessManagementServiceCE {
|
||||
|
|
@ -108,18 +109,33 @@ public class UserAndAccessManagementServiceCEImpl implements UserAndAccessManage
|
|||
// Get workspace from the default group.
|
||||
Mono<Workspace> workspaceMono = permissionGroupMono.flatMap(permissionGroup -> workspaceService.getById(permissionGroup.getDefaultWorkspaceId())).cache();
|
||||
|
||||
// Get all the default permision groups of the workspace
|
||||
Mono<List<PermissionGroup>> defaultPermissionGroupsMono =
|
||||
workspaceMono.flatMap(workspace ->
|
||||
permissionGroupService.findAllByIds(workspace.getDefaultPermissionGroups())
|
||||
.collectList()
|
||||
).cache();
|
||||
|
||||
// Check if the invited user exists. If yes, return the user, else create a new user by triggering
|
||||
// createNewUserAndSendInviteEmail. In both the cases, send the appropriate emails
|
||||
Mono<List<User>> inviteUsersMono = Flux.fromIterable(usernames)
|
||||
.flatMap(username -> Mono.zip(Mono.just(username), workspaceMono, currentUserMono, permissionGroupMono))
|
||||
.flatMap(username -> Mono.zip(Mono.just(username), workspaceMono, currentUserMono, permissionGroupMono, defaultPermissionGroupsMono))
|
||||
.flatMap(tuple -> {
|
||||
String username = tuple.getT1();
|
||||
Workspace workspace = tuple.getT2();
|
||||
eventData.put(FieldName.WORKSPACE, workspace);
|
||||
User currentUser = tuple.getT3();
|
||||
PermissionGroup permissionGroup = tuple.getT4();
|
||||
List<PermissionGroup> defaultPermissionGroups = tuple.getT5();
|
||||
|
||||
return userRepository.findByEmail(username)
|
||||
Mono<User> getUserFromDbAndCheckIfUserExists = userRepository.findByEmail(username)
|
||||
.flatMap(user -> {
|
||||
return throwErrorIfUserAlreadyExistsInWorkspace(user, defaultPermissionGroups)
|
||||
// If no errors, proceed forward
|
||||
.thenReturn(user);
|
||||
});
|
||||
|
||||
return getUserFromDbAndCheckIfUserExists
|
||||
.flatMap(existingUser -> {
|
||||
// The user already existed, just send an email informing that the user has been added
|
||||
// to a new workspace
|
||||
|
|
@ -170,4 +186,17 @@ public class UserAndAccessManagementServiceCEImpl implements UserAndAccessManage
|
|||
return bulkAddUserResultMono.then(sendAnalyticsEventMono).then(inviteUsersMono);
|
||||
}
|
||||
|
||||
private Mono<Boolean> throwErrorIfUserAlreadyExistsInWorkspace(User user,
|
||||
List<PermissionGroup> defaultPermissionGroups) {
|
||||
|
||||
return Flux.fromIterable(defaultPermissionGroups)
|
||||
.map(permissionGroup -> {
|
||||
if (permissionGroup.getAssignedToUserIds().contains(user.getId())) {
|
||||
throw new AppsmithException(AppsmithError.USER_ALREADY_EXISTS_IN_WORKSPACE, user.getUsername(), permissionGroup.getName());
|
||||
}
|
||||
return TRUE;
|
||||
})
|
||||
.then(Mono.just(TRUE));
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1253,6 +1253,62 @@ public class WorkspaceServiceTest {
|
|||
.verifyComplete();
|
||||
}
|
||||
|
||||
@Test
|
||||
@WithUserDetails(value = "api_user")
|
||||
public void addUserToWorkspaceIfUserAlreadyMember_throwsError() {
|
||||
Workspace workspace = new Workspace();
|
||||
workspace.setName("addUserToWorkspaceIfUserAlreadyMember_throwsError");
|
||||
workspace.setDomain("example.com");
|
||||
workspace.setWebsite("https://example.com");
|
||||
|
||||
Mono<Workspace> workspaceMono = workspaceService
|
||||
.create(workspace)
|
||||
.cache();
|
||||
|
||||
Flux<PermissionGroup> permissionGroupFlux = workspaceMono
|
||||
.flatMapMany(workspace1 -> permissionGroupRepository.findAllById(workspace1.getDefaultPermissionGroups()));
|
||||
|
||||
Mono<PermissionGroup> adminPermissionGroupMono = permissionGroupFlux
|
||||
.filter(permissionGroup -> permissionGroup.getName().startsWith(ADMINISTRATOR))
|
||||
.single();
|
||||
|
||||
Mono<PermissionGroup> developerPermissionGroupMono = permissionGroupFlux
|
||||
.filter(permissionGroup -> permissionGroup.getName().startsWith(DEVELOPER))
|
||||
.single();
|
||||
|
||||
InviteUsersDTO inviteUsersDTO = new InviteUsersDTO();
|
||||
ArrayList<String> users = new ArrayList<>();
|
||||
users.add("usertest@usertest.com");
|
||||
inviteUsersDTO.setUsernames(users);
|
||||
|
||||
|
||||
Mono<List<User>> userAddedToWorkspaceTwiceMono = adminPermissionGroupMono
|
||||
.flatMap(adminPermissionGroup -> {
|
||||
|
||||
// Add user to workspace first as admin
|
||||
inviteUsersDTO.setPermissionGroupId(adminPermissionGroup.getId());
|
||||
|
||||
return userAndAccessManagementService.inviteUsers(inviteUsersDTO, origin);
|
||||
})
|
||||
.then(developerPermissionGroupMono)
|
||||
.flatMap(developerPermissionGroup -> {
|
||||
|
||||
// Now try to add the user to the workspace as developer
|
||||
inviteUsersDTO.setPermissionGroupId(developerPermissionGroup.getId());
|
||||
|
||||
return userAndAccessManagementService.inviteUsers(inviteUsersDTO, origin);
|
||||
});
|
||||
|
||||
|
||||
StepVerifier
|
||||
.create(userAddedToWorkspaceTwiceMono)
|
||||
.expectErrorMatches(throwable -> throwable instanceof AppsmithException
|
||||
&& throwable.getMessage()
|
||||
.equals(AppsmithError.USER_ALREADY_EXISTS_IN_WORKSPACE
|
||||
.getMessage("usertest@usertest.com", "Administrator - addUserToWorkspaceIfUserAlreadyMember_throwsError")))
|
||||
.verify();
|
||||
}
|
||||
|
||||
@Test
|
||||
@WithUserDetails(value = "api_user")
|
||||
public void inviteRolesGivenAdministrator() {
|
||||
|
|
|
|||
|
|
@ -7,7 +7,6 @@ import com.appsmith.server.domains.Application;
|
|||
import com.appsmith.server.domains.PermissionGroup;
|
||||
import com.appsmith.server.domains.User;
|
||||
import com.appsmith.server.domains.Workspace;
|
||||
import com.appsmith.server.dtos.InviteUsersDTO;
|
||||
import com.appsmith.server.dtos.PermissionGroupInfoDTO;
|
||||
import com.appsmith.server.helpers.PluginExecutorHelper;
|
||||
import com.appsmith.server.repositories.ApplicationRepository;
|
||||
|
|
@ -20,6 +19,7 @@ import com.appsmith.server.services.NewActionService;
|
|||
import com.appsmith.server.services.NewPageService;
|
||||
import com.appsmith.server.services.PermissionGroupService;
|
||||
import com.appsmith.server.services.PluginService;
|
||||
import com.appsmith.server.services.SessionUserService;
|
||||
import com.appsmith.server.services.UserService;
|
||||
import com.appsmith.server.services.UserWorkspaceService;
|
||||
import com.appsmith.server.services.WorkspaceService;
|
||||
|
|
@ -36,7 +36,6 @@ import org.springframework.test.context.junit.jupiter.SpringExtension;
|
|||
import reactor.core.publisher.Mono;
|
||||
import reactor.test.StepVerifier;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import java.util.Set;
|
||||
|
||||
|
|
@ -98,6 +97,9 @@ public class ShareWorkspacePermissionTests {
|
|||
@Autowired
|
||||
UserAndAccessManagementService userAndAccessManagementService;
|
||||
|
||||
@Autowired
|
||||
SessionUserService sessionUserService;
|
||||
|
||||
Application savedApplication;
|
||||
|
||||
Workspace savedWorkspace;
|
||||
|
|
@ -105,7 +107,6 @@ public class ShareWorkspacePermissionTests {
|
|||
String workspaceId;
|
||||
|
||||
@BeforeEach
|
||||
@WithUserDetails(value = "api_user")
|
||||
public void setup() {
|
||||
User apiUser = userService.findByEmail("api_user").block();
|
||||
|
||||
|
|
@ -119,9 +120,6 @@ public class ShareWorkspacePermissionTests {
|
|||
application.setWorkspaceId(workspaceId);
|
||||
savedApplication = applicationPageService.createApplication(application, workspaceId).block();
|
||||
|
||||
InviteUsersDTO inviteUsersDTO = new InviteUsersDTO();
|
||||
ArrayList<String> emails = new ArrayList<>();
|
||||
|
||||
PermissionGroup adminPermissionGroup = permissionGroupService.getByDefaultWorkspace(savedWorkspace, AclPermission.READ_PERMISSION_GROUP_MEMBERS)
|
||||
.collectList().block()
|
||||
.stream()
|
||||
|
|
@ -134,28 +132,17 @@ public class ShareWorkspacePermissionTests {
|
|||
.filter(permissionGroupElem -> permissionGroupElem.getName().startsWith(FieldName.DEVELOPER))
|
||||
.findFirst().get();
|
||||
|
||||
// Invite Admin
|
||||
emails.add("admin@solutiontest.com");
|
||||
inviteUsersDTO.setUsernames(emails);
|
||||
inviteUsersDTO.setPermissionGroupId(adminPermissionGroup.getId());
|
||||
userAndAccessManagementService.inviteUsers(inviteUsersDTO, "http://localhost:8080").block();
|
||||
|
||||
emails.clear();
|
||||
|
||||
// Invite Developer
|
||||
emails.add("developer@solutiontest.com");
|
||||
inviteUsersDTO.setUsernames(emails);
|
||||
inviteUsersDTO.setPermissionGroupId(developerPermissionGroup.getId());
|
||||
userAndAccessManagementService.inviteUsers(inviteUsersDTO, "http://localhost:8080").block();
|
||||
|
||||
User userAdmin = userService.findByEmail("admin@solutiontest.com").block();
|
||||
User userDeveloper = userService.findByEmail("developer@solutiontest.com").block();
|
||||
|
||||
// Set the correct ownerships and permissions
|
||||
// Manually set the admin and developer access for the workspace (instead of going via the service functions)
|
||||
adminPermissionGroup.setAssignedToUserIds(Set.of(apiUser.getId(), userAdmin.getId()));
|
||||
permissionGroupRepository.save(adminPermissionGroup).block();
|
||||
developerPermissionGroup.setAssignedToUserIds(Set.of(userDeveloper.getId()));
|
||||
permissionGroupRepository.save(developerPermissionGroup).block();
|
||||
|
||||
permissionGroupService.cleanPermissionGroupCacheForUsers(List.of(userAdmin.getEmail(), userDeveloper.getEmail())).block();
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user