diff --git a/server-application-server/source/java/ch/systemsx/cisd/openbis/generic/server/task/UserManager.java b/server-application-server/source/java/ch/systemsx/cisd/openbis/generic/server/task/UserManager.java index 6da46d503ac584123beffa0d479bd2bc19199a82..76cf029ae4c4f1aa6d5f59e8488311de219d237e 100644 --- a/server-application-server/source/java/ch/systemsx/cisd/openbis/generic/server/task/UserManager.java +++ b/server-application-server/source/java/ch/systemsx/cisd/openbis/generic/server/task/UserManager.java @@ -273,6 +273,7 @@ public class UserManager manageGroup(sessionToken, groupCode, users, currentState, report); } updateHomeSpaces(sessionToken, currentState, report); + removeUsersFromGlobalGroup(sessionToken, currentState, report); } catch (Throwable e) { report.addErrorMessage("Error: " + e.toString()); @@ -426,6 +427,21 @@ public class UserManager return personUpdate; } + private void removeUsersFromGlobalGroup(String sessionToken, CurrentState currentState, UserManagerReport report) + { + AuthorizationGroup globalGroup = currentState.getGlobalGroup(); + if (globalGroup != null) + { + Context context = new Context(sessionToken, service, currentState, report); + + for (String userId : currentState.getUsersToBeRemovedFromGlobalGroup()) + { + removePersonFromAuthorizationGroup(context, globalGroup.getCode(), userId); + } + context.executeOperations(); + } + } + private void manageInstanceAdmins(String sessionToken, CurrentState currentState, UserManagerReport report) { if (instanceAdmins != null) @@ -814,9 +830,9 @@ public class UserManager private void manageUsers(Context context, String groupCode, Map<String, Principal> groupUsers) { UserGroup group = groupsByCode.get(groupCode); - Map<String, Person> currentUsersOfGroup = context.currentState.getCurrentUsersOfGroup(groupCode); + Map<String, Person> currentUsersOfGroup = context.getCurrentState().getCurrentUsersOfGroup(groupCode); Set<String> usersToBeRemoved = new TreeSet<>(currentUsersOfGroup.keySet()); - AuthorizationGroup globalGroup = context.currentState.getGlobalGroup(); + AuthorizationGroup globalGroup = context.getCurrentState().getGlobalGroup(); String adminGroupCode = createAdminGroupCode(groupCode); boolean createUserSpace = group == null || group.isCreateUserSpace(); boolean useEmailAsUserId = group != null && group.isUseEmailAsUserId(); @@ -833,6 +849,7 @@ public class UserManager addPersonToAuthorizationGroup(context, groupCode, userId); if (globalGroup != null) { + context.getCurrentState().addPersonToGlobalGroup(userId); addPersonToAuthorizationGroup(context, globalGroup.getCode(), userId); } if (isAdmin(userId, groupCode)) @@ -850,10 +867,10 @@ public class UserManager private void handleRoleAssignmentForUserSpaces(Context context, String groupCode) { UserGroup group = groupsByCode.get(groupCode); - Map<String, Person> currentUsersOfGroup = context.currentState.getCurrentUsersOfGroup(groupCode); + Map<String, Person> currentUsersOfGroup = context.getCurrentState().getCurrentUsersOfGroup(groupCode); Set<String> allUserSpaces = getAllUserSpaces(context, groupCode, currentUsersOfGroup); Role userSpaceRole = group.getUserSpaceRole(); - AuthorizationGroup authorizationGroup = context.currentState.groupsByCode.get(groupCode); + AuthorizationGroup authorizationGroup = context.getCurrentState().groupsByCode.get(groupCode); AuthorizationGroupPermId groupId = new AuthorizationGroupPermId(groupCode); for (String spaceCode : allUserSpaces) @@ -880,7 +897,7 @@ public class UserManager } if (roleAssignments.isEmpty() && userSpaceRole != null) { - Space space = context.currentState.getSpace(spaceCode); + Space space = context.getCurrentState().getSpace(spaceCode); createRoleAssignment(context, groupId, userSpaceRole, space.getPermId()); } } @@ -975,12 +992,12 @@ public class UserManager { removePersonFromAuthorizationGroup(context, groupCode, userId); removePersonFromAuthorizationGroup(context, adminGroupCode, userId); - AuthorizationGroup globalGroup = context.currentState.getGlobalGroup(); + AuthorizationGroup globalGroup = context.getCurrentState().getGlobalGroup(); if (globalGroup != null) { - removePersonFromAuthorizationGroup(context, globalGroup.getCode(), userId); + context.getCurrentState().removeUserFromGlobalGroup(userId); } - Person user = context.currentState.getUser(userId); + Person user = context.getCurrentState().getUser(userId); for (RoleAssignment roleAssignment : user.getRoleAssignments()) { Space space = roleAssignment.getSpace(); @@ -1038,7 +1055,8 @@ public class UserManager private void addPersonToAuthorizationGroup(Context context, String groupCode, String userId) { - if (context.currentState.getCurrentUsersOfGroup(groupCode).keySet().contains(userId) == false) + System.err.println(context.getCurrentState().getCurrentUsersOfGroup(groupCode)); + if (context.getCurrentState().getCurrentUsersOfGroup(groupCode).keySet().contains(userId) == false) { AuthorizationGroupUpdate groupUpdate = new AuthorizationGroupUpdate(); groupUpdate.setAuthorizationGroupId(new AuthorizationGroupPermId(groupCode)); @@ -1050,7 +1068,7 @@ public class UserManager private void removePersonFromAuthorizationGroup(Context context, String groupCode, String userId) { - if (context.currentState.getCurrentUsersOfGroup(groupCode).keySet().contains(userId)) + if (context.getCurrentState().getCurrentUsersOfGroup(groupCode).keySet().contains(userId)) { AuthorizationGroupUpdate groupUpdate = new AuthorizationGroupUpdate(); groupUpdate.setAuthorizationGroupId(new AuthorizationGroupPermId(groupCode)); @@ -1121,6 +1139,10 @@ public class UserManager private Set<String> newUsers = new TreeSet<>(); + private Set<String> usersAddedToGlobalGroup = new TreeSet<>(); + + private Set<String> usersRemovedFromGlobalGroup = new TreeSet<>(); + private AuthorizationGroup globalGroup; CurrentState(List<AuthorizationGroup> authorizationGroups, AuthorizationGroup globalGroup, List<Space> spaces, List<Person> users) @@ -1132,6 +1154,22 @@ public class UserManager users.forEach(user -> usersById.put(user.getUserId(), user)); } + public void removeUserFromGlobalGroup(String userId) + { + usersRemovedFromGlobalGroup.add(userId); + } + + public void addPersonToGlobalGroup(String userId) + { + usersAddedToGlobalGroup.add(userId); + } + + public Set<String> getUsersToBeRemovedFromGlobalGroup() + { + usersRemovedFromGlobalGroup.removeAll(usersAddedToGlobalGroup); + return usersRemovedFromGlobalGroup; + } + public Map<String, Person> getCurrentUsersOfGroup(String groupCode) { Map<String, Person> result = new TreeMap<>(); diff --git a/server-application-server/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/task/UserManagerTest.java b/server-application-server/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/task/UserManagerTest.java index d0e7aeefc1d434b23881704ab21ae04eab71ce95..3f7f94a15f8dbe7c3226856ffc24824d39653ed3 100644 --- a/server-application-server/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/task/UserManagerTest.java +++ b/server-application-server/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/task/UserManagerTest.java @@ -214,8 +214,8 @@ public class UserManagerTest extends AbstractTest // Then assertEquals(report.getErrorReport(), ""); assertEquals(report.getAuditLog(), "1970-01-01 01:00:00 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: G1, user: u2\n" - + "1970-01-01 01:00:01 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: ALL_GROUPS, user: u2\n" - + "1970-01-01 01:00:02 [UNASSIGN-ROLE-FORM-USER] user: u2, role: SPACE_ADMIN for G1_U2\n"); + + "1970-01-01 01:00:01 [UNASSIGN-ROLE-FORM-USER] user: u2, role: SPACE_ADMIN for G1_U2\n" + + "1970-01-01 01:00:02 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: ALL_GROUPS, user: u2\n"); UserManagerExpectationsBuilder builder = createBuilder(); builder.groups("G1").commonSpaces(commonSpaces).users(U1, U2); builder.space("A").observer(U1).non(U2); @@ -751,8 +751,8 @@ public class UserManagerTest extends AbstractTest // Then assertEquals(report.getErrorReport(), ""); assertEquals(report.getAuditLog(), "1970-01-01 01:00:00 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: G2, user: u2\n" - + "1970-01-01 01:00:01 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: ALL_GROUPS, user: u2\n" - + "1970-01-01 01:00:02 [UNASSIGN-ROLE-FORM-USER] user: u2, role: SPACE_ADMIN for G2_U2\n"); + + "1970-01-01 01:00:01 [UNASSIGN-ROLE-FORM-USER] user: u2, role: SPACE_ADMIN for G2_U2\n" + + "1970-01-01 01:00:02 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: ALL_GROUPS, user: u2\n"); UserManagerExpectationsBuilder builder = createBuilder(); builder.groups("G2").commonSpaces(commonSpaces).users(U1, U3); builder.usersWithoutAuthentication(U2); @@ -770,6 +770,110 @@ public class UserManagerTest extends AbstractTest builder.assertExpectations(); } + @Test + public void testMoveUserToAnotherGroup() + { + // Given + // 1. create group G1 with users U1 (admin), U2 and group G2 with user U3 (admin) + MockLogger logger = new MockLogger(); + Map<Role, List<String>> commonSpaces = commonSpaces(); + UserManager userManager = new UserManagerBuilder(v3api, logger, report()).commonSpaces(commonSpaces).get(); + List<String> globalSpaces = Arrays.asList("A", "B"); + userManager.setGlobalSpaces(globalSpaces); + userManager.addGroup(new UserGroupAsBuilder("G1").admins(U1), users(U1, U2)); + userManager.addGroup(new UserGroupAsBuilder("G2").admins(U3), users(U3)); + assertEquals(manage(userManager).getErrorReport(), ""); + // 2. move user U2 from group G1 to group G2 + userManager = new UserManagerBuilder(v3api, logger, report()).commonSpaces(commonSpaces).get(); + userManager.setGlobalSpaces(globalSpaces); + userManager.addGroup(new UserGroupAsBuilder("G1").admins(U1), users(U1)); + userManager.addGroup(new UserGroupAsBuilder("G2").admins(U3), users(U2, U3)); + + // When + UserManagerReport report = manage(userManager); + + // Then + assertEquals(report.getErrorReport(), ""); + assertEquals(report.getAuditLog(), "1970-01-01 01:00:00 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: G1, user: u2\n" + + "1970-01-01 01:00:01 [UNASSIGN-ROLE-FORM-USER] user: u2, role: SPACE_ADMIN for G1_U2\n" + + "1970-01-01 01:00:02 [ADD-SPACE] G2_U2\n" + + "1970-01-01 01:00:03 [ASSIGN-ROLE-TO-USER] user: u2, role: SPACE_ADMIN for G2_U2\n" + + "1970-01-01 01:00:04 [ASSIGN-ROLE-TO-AUTHORIZATION-GROUP] group: G2_ADMIN, role: SPACE_ADMIN for G2_U2\n" + + "1970-01-01 01:00:05 [ADD-USER-TO-AUTHORIZATION-GROUP] group: G2, user: u2\n" + + "1970-01-01 01:00:06 [ASSIGN-HOME-SPACE-FOR-USER] user: u2, home space: G2_U2\n"); + UserManagerExpectationsBuilder builder = createBuilder(); + builder.groups("G1").commonSpaces(commonSpaces).users(U1); + builder.groups("G2").commonSpaces(commonSpaces).users(U2, U3); + builder.usersWithoutAuthentication(); + builder.space("A").observer(U1, U2, U3); + builder.space("B").observer(U1, U2, U3); + builder.space("G1_ALPHA").admin(U1).non(U2, U3); + builder.space("G1_BETA").admin(U1).non(U2, U3); + builder.space("G1_GAMMA").admin(U1).non(U2, U3); + builder.space("G2_ALPHA").admin(U3).user(U2).non(U1); + builder.space("G2_BETA").admin(U3).user(U2).non(U1); + builder.space("G2_GAMMA").admin(U3).observer(U2).non(U1); + builder.homeSpace(U1, "G1_U1"); + builder.homeSpace(U2, "G2_U2"); + builder.homeSpace(U3, "G2_U3"); + builder.assertExpectations(); + } + + + @Test + public void testMoveUserToAnotherGroupAndRemovePreviousGroup() + { + // Given + // 1. create group G1 with users U1 (admin), U2 and group G2 with user U3 (admin) + MockLogger logger = new MockLogger(); + Map<Role, List<String>> commonSpaces = commonSpaces(); + UserManager userManager = new UserManagerBuilder(v3api, logger, report()).commonSpaces(commonSpaces).get(); + List<String> globalSpaces = Arrays.asList("A", "B"); + userManager.setGlobalSpaces(globalSpaces); + userManager.addGroup(new UserGroupAsBuilder("G1").admins(U1), users(U1, U2)); + userManager.addGroup(new UserGroupAsBuilder("G2").admins(U3), users(U3)); + assertEquals(manage(userManager).getErrorReport(), ""); + // 2. move user U2 from group G1 to group G2 and remove group G1 + userManager = new UserManagerBuilder(v3api, logger, report()).commonSpaces(commonSpaces).get(); + userManager.setGlobalSpaces(globalSpaces); + userManager.addGroup(new UserGroupAsBuilder("G2").admins(U3), users(U2, U3)); + + // When + UserManagerReport report = manage(userManager); + + // Then + assertEquals(report.getErrorReport(), ""); + assertEquals(report.getAuditLog(), "1970-01-01 01:00:00 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: G1, user: u1\n" + + "1970-01-01 01:00:01 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: G1_ADMIN, user: u1\n" + + "1970-01-01 01:00:02 [UNASSIGN-ROLE-FORM-USER] user: u1, role: SPACE_ADMIN for G1_U1\n" + + "1970-01-01 01:00:03 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: G1, user: u2\n" + + "1970-01-01 01:00:04 [UNASSIGN-ROLE-FORM-USER] user: u2, role: SPACE_ADMIN for G1_U2\n" + + "1970-01-01 01:00:05 [REMOVE-AUTHORIZATION-GROUP] G1\n" + + "1970-01-01 01:00:06 [REMOVE-AUTHORIZATION-GROUP] G1_ADMIN\n" + + "1970-01-01 01:00:07 [ADD-SPACE] G2_U2\n" + + "1970-01-01 01:00:08 [ASSIGN-ROLE-TO-USER] user: u2, role: SPACE_ADMIN for G2_U2\n" + + "1970-01-01 01:00:09 [ASSIGN-ROLE-TO-AUTHORIZATION-GROUP] group: G2_ADMIN, role: SPACE_ADMIN for G2_U2\n" + + "1970-01-01 01:00:10 [ADD-USER-TO-AUTHORIZATION-GROUP] group: G2, user: u2\n" + + "1970-01-01 01:00:11 [ASSIGN-HOME-SPACE-FOR-USER] user: u2, home space: G2_U2\n" + + "1970-01-01 01:00:12 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: ALL_GROUPS, user: u1\n"); + UserManagerExpectationsBuilder builder = createBuilder(); + builder.groups("G1").commonSpaces(commonSpaces).users(); + builder.groups("G2").commonSpaces(commonSpaces).users(U2, U3); + builder.usersWithoutAuthentication(U1); + builder.space("A").observer(U2, U3); + builder.space("B").observer(U2, U3); + builder.space("G1_ALPHA").non(U2, U3); + builder.space("G1_BETA").non(U2, U3); + builder.space("G1_GAMMA").non(U2, U3); + builder.space("G2_ALPHA").admin(U3).user(U2); + builder.space("G2_BETA").admin(U3).user(U2); + builder.space("G2_GAMMA").admin(U3).observer(U2); + builder.homeSpace(U1, "G1_U1"); + builder.homeSpace(U2, "G2_U2"); + builder.homeSpace(U3, "G2_U3"); + builder.assertExpectations(); + } + @Test public void testDisableAGroup() { @@ -794,14 +898,14 @@ public class UserManagerTest extends AbstractTest assertEquals(report.getErrorReport(), ""); assertEquals(report.getAuditLog(), "1970-01-01 01:00:00 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: G2, user: u1\n" + "1970-01-01 01:00:01 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: G2_ADMIN, user: u1\n" - + "1970-01-01 01:00:02 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: ALL_GROUPS, user: u1\n" - + "1970-01-01 01:00:03 [UNASSIGN-ROLE-FORM-USER] user: u1, role: SPACE_ADMIN for G2_U1\n" - + "1970-01-01 01:00:04 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: G2, user: u2\n" - + "1970-01-01 01:00:05 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: ALL_GROUPS, user: u2\n" - + "1970-01-01 01:00:06 [UNASSIGN-ROLE-FORM-USER] user: u2, role: SPACE_ADMIN for G2_U2\n" - + "1970-01-01 01:00:07 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: G2, user: u3\n" - + "1970-01-01 01:00:08 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: ALL_GROUPS, user: u3\n" - + "1970-01-01 01:00:09 [UNASSIGN-ROLE-FORM-USER] user: u3, role: SPACE_ADMIN for G2_U3\n"); + + "1970-01-01 01:00:02 [UNASSIGN-ROLE-FORM-USER] user: u1, role: SPACE_ADMIN for G2_U1\n" + + "1970-01-01 01:00:03 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: G2, user: u2\n" + + "1970-01-01 01:00:04 [UNASSIGN-ROLE-FORM-USER] user: u2, role: SPACE_ADMIN for G2_U2\n" + + "1970-01-01 01:00:05 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: G2, user: u3\n" + + "1970-01-01 01:00:06 [UNASSIGN-ROLE-FORM-USER] user: u3, role: SPACE_ADMIN for G2_U3\n" + + "1970-01-01 01:00:07 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: ALL_GROUPS, user: u1\n" + + "1970-01-01 01:00:08 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: ALL_GROUPS, user: u2\n" + + "1970-01-01 01:00:09 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: ALL_GROUPS, user: u3\n"); UserManagerExpectationsBuilder builder = createBuilder(); builder.groups("G2").users(); builder.usersWithoutAuthentication(U1, U2, U3); @@ -902,13 +1006,13 @@ public class UserManagerTest extends AbstractTest assertEquals(report.getErrorReport(), ""); assertEquals(report.getAuditLog(), "1970-01-01 01:00:00 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: G2, user: u2\n" + "1970-01-01 01:00:01 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: G2_ADMIN, user: u2\n" - + "1970-01-01 01:00:02 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: ALL_GROUPS, user: u2\n" - + "1970-01-01 01:00:03 [UNASSIGN-ROLE-FORM-USER] user: u2, role: SPACE_ADMIN for G2_U2\n" - + "1970-01-01 01:00:04 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: G2, user: u3\n" - + "1970-01-01 01:00:05 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: ALL_GROUPS, user: u3\n" - + "1970-01-01 01:00:06 [UNASSIGN-ROLE-FORM-USER] user: u3, role: SPACE_ADMIN for G2_U3\n" - + "1970-01-01 01:00:07 [REMOVE-AUTHORIZATION-GROUP] G2\n" - + "1970-01-01 01:00:08 [REMOVE-AUTHORIZATION-GROUP] G2_ADMIN\n"); + + "1970-01-01 01:00:02 [UNASSIGN-ROLE-FORM-USER] user: u2, role: SPACE_ADMIN for G2_U2\n" + + "1970-01-01 01:00:03 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: G2, user: u3\n" + + "1970-01-01 01:00:04 [UNASSIGN-ROLE-FORM-USER] user: u3, role: SPACE_ADMIN for G2_U3\n" + + "1970-01-01 01:00:05 [REMOVE-AUTHORIZATION-GROUP] G2\n" + + "1970-01-01 01:00:06 [REMOVE-AUTHORIZATION-GROUP] G2_ADMIN\n" + + "1970-01-01 01:00:07 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: ALL_GROUPS, user: u2\n" + + "1970-01-01 01:00:08 [REMOVE-USER-FROM-AUTHORIZATION-GROUP] group: ALL_GROUPS, user: u3\n"); UserManagerExpectationsBuilder builder = createBuilder(); builder.groups("G1").commonSpaces(commonSpaces).users(U1); builder.usersWithoutAuthentication(U2, U3);