diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/UserAndAccessManagementServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/UserAndAccessManagementServiceCEImpl.java index d8180e4c95..f2f4a6251f 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/UserAndAccessManagementServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/UserAndAccessManagementServiceCEImpl.java @@ -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 workspaceMono = permissionGroupMono.flatMap(permissionGroup -> workspaceService.getById(permissionGroup.getDefaultWorkspaceId())).cache(); + // Get all the default permision groups of the workspace + Mono> 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> 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 defaultPermissionGroups = tuple.getT5(); - return userRepository.findByEmail(username) + Mono 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 throwErrorIfUserAlreadyExistsInWorkspace(User user, + List 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)); + } + } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/WorkspaceServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/WorkspaceServiceTest.java index af62eaa3f9..96bcb81aa4 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/WorkspaceServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/WorkspaceServiceTest.java @@ -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 workspaceMono = workspaceService + .create(workspace) + .cache(); + + Flux permissionGroupFlux = workspaceMono + .flatMapMany(workspace1 -> permissionGroupRepository.findAllById(workspace1.getDefaultPermissionGroups())); + + Mono adminPermissionGroupMono = permissionGroupFlux + .filter(permissionGroup -> permissionGroup.getName().startsWith(ADMINISTRATOR)) + .single(); + + Mono developerPermissionGroupMono = permissionGroupFlux + .filter(permissionGroup -> permissionGroup.getName().startsWith(DEVELOPER)) + .single(); + + InviteUsersDTO inviteUsersDTO = new InviteUsersDTO(); + ArrayList users = new ArrayList<>(); + users.add("usertest@usertest.com"); + inviteUsersDTO.setUsernames(users); + + + Mono> 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() { diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ShareWorkspacePermissionTests.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ShareWorkspacePermissionTests.java index 22cd009a64..ede5d42ec7 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ShareWorkspacePermissionTests.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ShareWorkspacePermissionTests.java @@ -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 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