assign roles without privilege escalation

doc/release-notes/10342-assign-roles-without-privilege-escalation.md

@@ -0,0 +1 @@
+The permissions required to assign a role have been fixed. It is no longer possible to assign a role that includes permissions that the assigning user doesn't have.

doc/sphinx-guides/source/user/dataset-management.rst

@@ -566,7 +566,7 @@ This is where you will enable a particular Guestbook for your dataset, which is
 Roles & Permissions
 ===================
 
-Dataverse installation user accounts can be granted roles that define which actions they are allowed to take on specific Dataverse collections, datasets, and/or files. Each role comes with a set of permissions, which define the specific actions that users may take.
+Dataverse installation user accounts can be granted roles that define which actions they are allowed to take on specific Dataverse collections, datasets, and/or files. Each role comes with a set of permissions, which define the specific actions that users may take. It is not possible to grant a role that comes with a permission that the granting user themselves does not have.
 
 Roles and permissions may also be granted to groups. Groups can be defined as a set of Dataverse user accounts, a collection of IP addresses (e.g. all users of a library's computers), or a collection of all users who log in using a particular institutional login (e.g. everyone who logs in with a particular university's account credentials).
 

src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AssignRoleCommand.java

@@ -19,6 +19,7 @@
 import edu.harvard.iq.dataverse.engine.command.exception.CommandException;
 import edu.harvard.iq.dataverse.engine.command.exception.IllegalCommandException;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 
@@ -75,9 +76,19 @@ public RoleAssignment execute(CommandContext ctxt) throws CommandException {
     @Override
     public Map<String, Set<Permission>> getRequiredPermissions() {
         // for data file check permission on owning dataset
-        return Collections.singletonMap("",
-                defPoint instanceof Dataverse ? Collections.singleton(Permission.ManageDataversePermissions)
-                : defPoint instanceof Dataset ? Collections.singleton(Permission.ManageDatasetPermissions) : Collections.singleton(Permission.ManageFilePermissions));
+        Set<Permission> requiredPermissions = new HashSet<Permission>();
+
+        if (defPoint instanceof Dataverse) {
+            requiredPermissions.add(Permission.ManageDataversePermissions);
+        } else if (defPoint instanceof Dataset) {
+            requiredPermissions.add(Permission.ManageDatasetPermissions);
+        } else {
+            requiredPermissions.add(Permission.ManageFilePermissions);
+        }
+
+        requiredPermissions.addAll(role.permissions());
+
+        return Collections.singletonMap("", requiredPermissions);
     }
 
     @Override

src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java

@@ -1670,6 +1670,26 @@ public void testAddRoles(){
         giveRandoPermission = UtilIT.grantRoleOnDataset(datasetPersistentId, "fileDownloader", "@" + randomUsername, apiToken);
                 giveRandoPermission.prettyPrint();
         assertEquals(200, giveRandoPermission.getStatusCode());
+
+        // Create another random user to become curator:
+
+        Response createCuratorUser = UtilIT.createRandomUser();
+        createCuratorUser.prettyPrint();
+        String curatorUsername = UtilIT.getUsernameFromResponse(createCuratorUser);
+        String curatorUserApiToken = UtilIT.getApiTokenFromResponse(createCuratorUser);
+
+        Response giveCuratorPermission = UtilIT.grantRoleOnDataset(datasetPersistentId, "curator", "@" + curatorUsername, apiToken);
+        giveCuratorPermission.prettyPrint();
+        assertEquals(200, giveCuratorPermission.getStatusCode());
+
+        // Test if privilege escalation is possible: curator should not be able to assign admin rights
+        Response giveTooMuchPermission = UtilIT.grantRoleOnDataset(datasetPersistentId, "admin", "@" + curatorUsername, curatorUserApiToken);
+        giveTooMuchPermission.prettyPrint();
+        assertEquals(401, giveTooMuchPermission.getStatusCode());
+
+        giveTooMuchPermission = UtilIT.grantRoleOnDataset(datasetPersistentId, "admin", "@" + randomUsername, curatorUserApiToken);
+        giveTooMuchPermission.prettyPrint();
+        assertEquals(401, giveTooMuchPermission.getStatusCode());
 
         String idToDelete = JsonPath.from(giveRandoPermission.getBody().asString()).getString("data.id");                
 
@@ -1692,7 +1712,7 @@ public void testAddRoles(){
         deleteGrantedAccess.prettyPrint();
         assertEquals(200, deleteGrantedAccess.getStatusCode());
 
-       Response deleteDatasetResponse = UtilIT.deleteDatasetViaNativeApi(datasetId, apiToken);
+        Response deleteDatasetResponse = UtilIT.deleteDatasetViaNativeApi(datasetId, apiToken);
         deleteDatasetResponse.prettyPrint();
         assertEquals(200, deleteDatasetResponse.getStatusCode());
 
@@ -1703,6 +1723,14 @@ public void testAddRoles(){
         Response deleteUserResponse = UtilIT.deleteUser(username);
         deleteUserResponse.prettyPrint();
         assertEquals(200, deleteUserResponse.getStatusCode());
+
+        deleteUserResponse = UtilIT.deleteUser(randomUsername);
+        deleteUserResponse.prettyPrint();
+        assertEquals(200, deleteUserResponse.getStatusCode());
+
+        deleteUserResponse = UtilIT.deleteUser(curatorUsername);
+        deleteUserResponse.prettyPrint();
+        assertEquals(200, deleteUserResponse.getStatusCode());
 
     }