diff --git a/bundles/org.eclipse.emf.emfstore.server/src/org/eclipse/emf/emfstore/internal/server/core/AdminEmfStoreImpl.java b/bundles/org.eclipse.emf.emfstore.server/src/org/eclipse/emf/emfstore/internal/server/core/AdminEmfStoreImpl.java index 8db8a51ec..33986da55 100644 --- a/bundles/org.eclipse.emf.emfstore.server/src/org/eclipse/emf/emfstore/internal/server/core/AdminEmfStoreImpl.java +++ b/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 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 roles = orgUnit.getRoles(); + for (final Role role : roles) { + if (role.getProjects().contains(projectId)) { return role; } } diff --git a/tests/org.eclipse.emf.emfstore.server.test/src/org/eclipse/emf/emfstore/server/accesscontrol/test/AllAccessControlTests.java b/tests/org.eclipse.emf.emfstore.server.test/src/org/eclipse/emf/emfstore/server/accesscontrol/test/AllAccessControlTests.java index 94e55ac1b..6cd4308a1 100644 --- a/tests/org.eclipse.emf.emfstore.server.test/src/org/eclipse/emf/emfstore/server/accesscontrol/test/AllAccessControlTests.java +++ b/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 { diff --git a/tests/org.eclipse.emf.emfstore.server.test/src/org/eclipse/emf/emfstore/server/accesscontrol/test/ChangeRoleTest.java b/tests/org.eclipse.emf.emfstore.server.test/src/org/eclipse/emf/emfstore/server/accesscontrol/test/ChangeRoleTest.java new file mode 100644 index 000000000..dd2582e11 --- /dev/null +++ b/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()); + } + +} diff --git a/tests/org.eclipse.emf.emfstore.server.test/src/org/eclipse/emf/emfstore/server/accesscontrol/test/ProjectAdminTest.java b/tests/org.eclipse.emf.emfstore.server.test/src/org/eclipse/emf/emfstore/server/accesscontrol/test/ProjectAdminTest.java index a5ecf8805..ac7726f2d 100644 --- a/tests/org.eclipse.emf.emfstore.server.test/src/org/eclipse/emf/emfstore/server/accesscontrol/test/ProjectAdminTest.java +++ b/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()); }