From 7d44f70b1db22832e5d22acaa9b216e127b6d3da Mon Sep 17 00:00:00 2001
From: felmer <franz-josef.elmer@id.ethz.ch>
Date: Wed, 9 May 2018 10:56:26 +0200
Subject: [PATCH] SSDM-6061: Bug fixed: Removed users are not removed from
authorization group ALL_GROUPS. New test added for this case.
---
.../generic/server/task/UserManager.java | 5 ++
.../systemtest/task/UserManagerTest.java | 50 ++++++++++++++++++-
2 files changed, 53 insertions(+), 2 deletions(-)
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/task/UserManager.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/task/UserManager.java
index 7415906d9e0..a163ced6782 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/task/UserManager.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/task/UserManager.java
@@ -599,6 +599,11 @@ public class UserManager
{
removePersonFromAuthorizationGroup(context, groupCode, userId);
removePersonFromAuthorizationGroup(context, adminGroupCode, userId);
+ AuthorizationGroup globalGroup = context.currentState.getGlobalGroup();
+ if (globalGroup != null)
+ {
+ removePersonFromAuthorizationGroup(context, globalGroup.getCode(), userId);
+ }
Person user = context.currentState.getUser(userId);
Space homeSpace = user.getSpace();
for (RoleAssignment roleAssignment : user.getRoleAssignments())
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/task/UserManagerTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/task/UserManagerTest.java
index 04f9518ea9e..8c244ea3d40 100644
--- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/task/UserManagerTest.java
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/task/UserManagerTest.java
@@ -130,6 +130,46 @@ public class UserManagerTest extends AbstractTest
builder.assertExpectations();
}
+ @Test
+ public void testAddAndRemoveAUserWhichAlreadyHasAHomeSpace()
+ {
+ // Given
+ // 1. create user U2 with home space TEST-SPACE and SPACE_ADMIN on this space
+ createUserForTestSpace(U2);
+ // 2. create groip G1 with users U1 (admin) and U2
+ MockLogger logger = new MockLogger();
+ Map<Role, List<String>> commonSpaces = commonSpaces();
+ UserManager userManager = new UserManagerBuilder(v3api, logger).commonSpaces(commonSpaces).get();
+ userManager.setGlobalSpaces(Arrays.asList("A"));
+ userManager.addGroup(group("G1", U1.getUserId(), "blabla"), users(U1, U2));
+ assertEquals(manage(userManager).getErrorReport(), "");
+ createBuilder().groups("G1").space("A").observer(U1, U2).assertExpectations();
+ // 3. remove U2 from G1
+ userManager = new UserManagerBuilder(v3api, logger).commonSpaces(commonSpaces).get();
+ userManager.setGlobalSpaces(Arrays.asList("A"));
+ userManager.addGroup(group("G1", U1.getUserId()), users(U1));
+
+ // 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 [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");
+ UserManagerExpectationsBuilder builder = createBuilder();
+ builder.groups("G1").commonSpaces(commonSpaces).users(U1, U2);
+ builder.space("A").observer(U1).non(U2);
+ builder.space("G1_ALPHA").admin(U1).non(U2);
+ builder.space("G1_BETA").admin(U1).non(U2);
+ builder.space("G1_GAMMA").admin(U1).non(U2);
+ builder.space("G1_U1").admin(U1).non(U2);
+ builder.space("G1_U2").admin(U1).non(U2);
+ builder.homeSpace(U1, "G1_U1");
+ builder.homeSpace(U2, "TEST-SPACE");
+ builder.assertExpectations();
+ }
+
@Test
public void testAddGlobalSpacesAndCommonSpacesSamplesAndExperiments()
{
@@ -486,10 +526,13 @@ public class UserManagerTest extends AbstractTest
MockLogger logger = new MockLogger();
Map<Role, List<String>> commonSpaces = commonSpaces();
UserManager userManager = new UserManagerBuilder(v3api, logger).commonSpaces(commonSpaces).get();
+ List<String> globalSpaces = Arrays.asList("A", "B");
+ userManager.setGlobalSpaces(globalSpaces);
userManager.addGroup(group("G2", U1.getUserId()), users(U1, U2, U3));
assertEquals(manage(userManager).getErrorReport(), "");
// 2. remove U2 from group G2
userManager = new UserManagerBuilder(v3api, logger).commonSpaces(commonSpaces).get();
+ userManager.setGlobalSpaces(globalSpaces);
userManager.addGroup(group("G2", U1.getUserId()), users(U1, U3));
// When
@@ -498,11 +541,14 @@ 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 [UNASSIGN-ROLE-FORM-USER] user: u2, role: SPACE_ADMIN for G2_U2\n"
- + "1970-01-01 01:00:02 [REMOVE-HOME-SPACE-FROM-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:03 [REMOVE-HOME-SPACE-FROM-USER] u2\n");
UserManagerExpectationsBuilder builder = createBuilder();
builder.groups("G2").commonSpaces(commonSpaces).users(U1, U3);
builder.usersWithoutAuthentication(U2);
+ builder.space("A").observer(U1).observer(U3);
+ builder.space("B").observer(U1).observer(U3);
builder.space("G2_ALPHA").admin(U1).user(U3);
builder.space("G2_BETA").admin(U1).user(U3);
builder.space("G2_GAMMA").admin(U1).observer(U3);
--
GitLab