Skip to content

Commit

Permalink
Merge pull request #10342 from vera/assign-roles-without-privilege-es…
Browse files Browse the repository at this point in the history
…calation

assign roles without privilege escalation
  • Loading branch information
sekmiller authored Mar 4, 2024
2 parents 891e421 + c0c247e commit c6568ca
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -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.
2 changes: 1 addition & 1 deletion doc/sphinx-guides/source/user/dataset-management.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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).

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down
30 changes: 29 additions & 1 deletion src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand All @@ -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());

Expand All @@ -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());

}

Expand Down

0 comments on commit c6568ca

Please sign in to comment.