Bug 574382 - Change Role for Server Admin

* update SA _and_ other roles
* add test case

Change-Id: I79740da4861dcff3a972de844c834b6a4a1a5d08
Signed-off-by: Johannes Faltermeier <jfaltermeier@eclipsesource.com>

bundles/org.eclipse.emf.emfstore.server/src/org/eclipse/emf/emfstore/internal/server/core/AdminEmfStoreImpl.java

@@ -568,22 +568,16 @@ public void changeRole(final SessionId sessionId, final ProjectId projectId, fin
 
 		final ProjectId resolvedProjectId = getProjectId(projectId);
 		final ACOrgUnit<?> orgUnit = getOrgUnit(orgUnitId);
-		final Role role = getRole(resolvedProjectId, orgUnit);
 
-		// remove old role first
-		if (role != null) {
-
-			if (!isServerAdmin && role.canAdministrate(resolvedProjectId)) {
-				throw new AccessControlException(
-					Messages.AdminEmfStoreImpl_RemovePA_Violation_1
-						+ Messages.AdminEmfStoreImpl_RemovePA_Violation_2);
-			}
+		// if user was server admin, remove this role
+		// there may be more roles for this project, e.g. ProjectAdmin in case the project
+		// was created by this user
+		final Role serverAdminRole = getServerAdminRole(orgUnit);
+		removeRole(isServerAdmin, resolvedProjectId, orgUnit, serverAdminRole);
 
-			role.getProjects().remove(resolvedProjectId);
-			if (role.getProjects().isEmpty()) {
-				orgUnit.getRoles().remove(role);
-			}
-		}
+		// remove old role first
+		final Role role = getProjectRole(resolvedProjectId, orgUnit);
+		removeRole(isServerAdmin, resolvedProjectId, orgUnit, role);
 
 		if (isServerAdminRole(roleClass)) {
 			orgUnit.getRoles().add(RolesFactory.eINSTANCE.createServerAdmin());
@@ -608,6 +602,23 @@ public void changeRole(final SessionId sessionId, final ProjectId projectId, fin
 		save();
 	}
 
+	protected void removeRole(final boolean isServerAdmin, final ProjectId resolvedProjectId,
+		final ACOrgUnit<?> orgUnit, final Role role) throws AccessControlException {
+		if (role != null) {
+
+			if (!isServerAdmin && role.canAdministrate(resolvedProjectId)) {
+				throw new AccessControlException(
+					Messages.AdminEmfStoreImpl_RemovePA_Violation_1
+						+ Messages.AdminEmfStoreImpl_RemovePA_Violation_2);
+			}
+
+			role.getProjects().remove(resolvedProjectId);
+			if (role.getProjects().isEmpty()) {
+				orgUnit.getRoles().remove(role);
+			}
+		}
+	}
+
 	/**
 	 * {@inheritDoc}
 	 */
@@ -963,11 +974,20 @@ private ACOrgUnit<?> getOrgUnit(ACOrgUnitId orgUnitId) throws ESException {
 		throw new ESException(Messages.AdminEmfStoreImpl_OrgUnit_Does_Not_Exist);
 	}
 
-	private Role getRole(ProjectId projectId, ACOrgUnit<?> orgUnit) {
+	private Role getServerAdminRole(ACOrgUnit<?> orgUnit) {
 		final List<Role> roles = orgUnit.getRoles();
 		for (final Role role : roles) {
-			if (isServerAdmin(role) || role.getProjects().contains(projectId)) {
-				// return (Role) ModelUtil.clone(role);
+			if (isServerAdmin(role)) {
+				return role;
+			}
+		}
+		return null;
+	}
+
+	private Role getProjectRole(ProjectId projectId, ACOrgUnit<?> orgUnit) {
+		final List<Role> roles = orgUnit.getRoles();
+		for (final Role role : roles) {
+			if (role.getProjects().contains(projectId)) {
 				return role;
 			}
 		}

tests/org.eclipse.emf.emfstore.server.test/src/org/eclipse/emf/emfstore/server/accesscontrol/test/AllAccessControlTests.java

@@ -44,7 +44,8 @@
 	AddInitialParticipantTest.class,
 	AutoCreateACUserTestTest.class,
 	DefaultESPasswordHashGeneratorTests.class,
-	DefaultESAuthorizationServiceTests.class
+	DefaultESAuthorizationServiceTests.class,
+	ChangeRoleTest.class
 
 })
 public class AllAccessControlTests {

tests/org.eclipse.emf.emfstore.server.test/src/org/eclipse/emf/emfstore/server/accesscontrol/test/ChangeRoleTest.java

@@ -0,0 +1,69 @@
+/*******************************************************************************
+ * Copyright (c) 2011-2021 EclipseSource Muenchen GmbH and others.
+ *
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * Johannes Faltermeier - initial API and implementation
+ ******************************************************************************/
+package org.eclipse.emf.emfstore.server.accesscontrol.test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import org.eclipse.core.runtime.NullProgressMonitor;
+import org.eclipse.emf.emfstore.client.test.common.util.ServerUtil;
+import org.eclipse.emf.emfstore.internal.server.model.accesscontrol.ACUser;
+import org.eclipse.emf.emfstore.internal.server.model.accesscontrol.roles.RolesPackage;
+import org.eclipse.emf.emfstore.server.auth.ESProjectAdminPrivileges;
+import org.eclipse.emf.emfstore.server.exceptions.ESException;
+import org.junit.BeforeClass;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+public class ChangeRoleTest extends ProjectAdminTest {
+
+	// BEGIN COMPLEX CODE
+	// Checkstyle complains about public modifier..
+	@Rule
+	public final ExpectedException expectedException = ExpectedException.none();
+
+	// END COMPLEX CODE
+
+	@BeforeClass
+	public static void beforeClass() {
+		startEMFStoreWithPAProperties(
+			ESProjectAdminPrivileges.ShareProject,
+			ESProjectAdminPrivileges.AssignRoleToOrgUnit // needed for share
+		);
+	}
+
+	@Test
+	public void changeSAPAtoWriter() throws ESException {
+		// setup
+		makeUserPA();
+		getLocalProject().shareProject(getUsersession(), new NullProgressMonitor());
+		makeUserSA();
+		ACUser user = ServerUtil.getUser(getSuperUsersession(), getUser());
+		assertTrue(ProjectAdminTest.hasServerAdminRole(user.getId()));
+		assertTrue(ProjectAdminTest.hasProjectAdminRole(user, getProjectSpace().getProjectId()));
+		assertEquals(2, user.getRoles().size());
+
+		// act
+		getSuperAdminBroker().changeRole(getProjectSpace().getProjectId(), user.getId(),
+			RolesPackage.eINSTANCE.getWriterRole());
+
+		// assert
+		user = ServerUtil.getUser(getSuperUsersession(), getUser());
+		assertFalse(ProjectAdminTest.hasServerAdminRole(user.getId()));
+		assertFalse(ProjectAdminTest.hasProjectAdminRole(user, getProjectSpace().getProjectId()));
+		assertTrue(ProjectAdminTest.hasWriterRole(user.getId()));
+		assertEquals(1, user.getRoles().size());
+	}
+
+}

tests/org.eclipse.emf.emfstore.server.test/src/org/eclipse/emf/emfstore/server/accesscontrol/test/ProjectAdminTest.java

@@ -179,6 +179,10 @@ public void makeUserSA() throws ESException {
 		// getUsersession().refresh();
 	}
 
+	public static boolean hasServerAdminRole(ACOrgUnitId orgUnitId) throws ESException {
+		return hasRole(orgUnitId, Roles.serverAdmin());
+	}
+
 	public static boolean hasProjectAdminRole(ACOrgUnitId orgUnitId) throws ESException {
 		return hasRole(orgUnitId, Roles.projectAdmin());
 	}