dataset.xhtml - Edit button not rendered if user has only ManageDatasetPermissions

[mdmADA]

Version 4.6.1

In dataset.xhtml, the Edit button has the 'Permissions' option within its dropdown. However, this Edit button is rendered (following the method calls to the PermissionsWrapper class) only if the user has EditDataset permission.

The Edit button is not rendered if the user has only ManageDatasetPermissions so a user with only ManageDatasetPermissions will never have access to the permissions option. (The permissions option within the Edit button dropdown IS rendered if the user has been assigned ManageDatasetPermissions).

The Edit button should be rendered if the user has (EditDataset or ManageDatasetPermissions) permissions. If they only have ManageDatasetPermissions, only the Permissions option within the Edit dropdown list should be rendered/enabled. (Nothing else should be editable by the user.)

Note: this issue tracks the very bug listed above; the comments below delve into more general discussion about what a custom role with only ManageDatasetPermissions could do but no changes related to that are in scope for this issue.

[pdurbin]

@mdmADA are you making your own roles? Or changing the set of permissions for the roles that ship with Dataverse? If so, I'm a little concerned you'll introduce unexpected behavior like we saw in #3739. What I'm trying to say is that the set of permissions per role that you get out of the box should hang together and make sense. If not, that's a bug.

[mdmADA]

Hi Phil.

We are creating our own role 'Approver'. This role will be assigned to a Group. Any user assigned to this group is meant to be able to access a dataset's permissions-manage-files.xhtml page in order to Assign/Remove/Grant/Reject access to files.

So within dataverse x, I created a role 'Approver' and assigned it 'ManageDatasetPermissions'. Then I created a Group TestApproverGroup, assigned it the Approver role, and added a few test users to it.

When logged in as one of those test users, on the dataset.xhtml page, the Edit button is not rendered for that user. The minimal way to render that Edit button for the 'Approver' role is to add the 'EditDataset' permissions to that role.

This is because the Edit button is only rendered if the ManageFilePermissionsPage.canUpdateDataset() method returns true. That method in turn calls the PermissionsWrapper.canUpdateDataset() method which returns true iff the user has 'EditDataset' permission:

public boolean canUpdateDataset(DataverseRequest dr, Dataset dataset) {
return doesSessionUserHaveDataSetPermission(dr, dataset, Permission.EditDataset);
}

Of course, that seems to make sense since the button label is 'Edit' but if the user has only 'ManageDatasetPermissions', I assume they should be able to Edit just the permissions for the file through the dataset Edit button.

Note that this Approver user can access the permissions-manage-files.xhtml?id=xxxx directly if they are logged in and copy and paste that into the address bar.

In summary, we are creating a new role and not changing the set of permissions for the roles that ship with Dataverse.

I hope that is enough detail. And that it makes sense!

Thanks.

[pdurbin]

@mdmADA yes, this is helping a lot. Thanks! At https://groups.google.com/d/msg/dataverse-community/bGlCU2pbQpE/GlPo33dWBAAJ @stevenmce had written, "We are currently extending the Guestbook and Restricted Access functionality to enable this, but would love to see it embedded directly into the main codebase (and have discussed this with Gustavo and Danny)." Somewhere else, maybe in #3943 I think he said that you're the person who's working on the code. Great! I'm pleased to meet you and I have a better sense now of what you're up to. 😄

Can you please paste below the output of the following curl command, replacing $API_TOKEN with the API token of your dataverseAdmin user and 1 with the id of the "Approver" role you created in your dataverserole table ( http://phoenix.dataverse.org/schemaspy/latest/tables/dataverserole.html )?

curl -H "X-Dataverse-key: $API_TOKEN" http://localhost:8080/api/roles/1

The output should look something like this if you pretty print it:

{
  "status": "OK",
  "data": {
    "alias": "admin",
    "name": "Admin",
    "permissions": [
      "AddDataverse",
      "AddDataset",
      "ViewUnpublishedDataverse",
      "ViewUnpublishedDataset",
      "DownloadFile",
      "EditDataverse",
      "EditDataset",
      "ManageDataversePermissions",
      "ManageDatasetPermissions",
      "PublishDataverse",
      "PublishDataset",
      "DeleteDataverse",
      "DeleteDatasetDraft"
    ],
    "description": "A person who has all permissions for dataverses, datasets, and files.",
    "id": 1
  }
}

[mdmADA]

Yes, I am the developer working with @stevenmce on Dataverse modifications to meet ADA workflow requirements not currently in Dataverse. I am enjoying it.

Anyway, these are the results of the executed the curl command above (with the parameters as you requested):

{
"status": "OK",
"data": {
"alias": "approver",
"name": "Approver",
"permissions": [
"ManageDatasetPermissions"
],
"description": "Can approve/reject access",
"id": 9,
"ownerId": 1
}
}

Thanks!

[pdurbin]

@mdmADA thanks. I'm sorry that I haven't had a chance to poke around at this myself but is there any risk of privilege escalation in what you're proposing. If someone has ManageDatasetPermissions can they somehow give themselves extra power that they shouldn't have? It sounds ideally there's be a specific permission for what you want... to allow you to delegate the granting of file downloading permission without having to be an admin or curator.

[mdmADA]

Hi Phil - Thanks for your response.

I think we're discussing two issues here:

1. Whether our 'Approver' role with ManageDatasetPermissions is a good idea considering what granting ManageDatasetPermissions implies relative to what we actually want an Approver to be able to do

and

2. the fact that a user with only ManageDatasetPermissions cannot get access to the dataset Edit button to manage those permissions.

With respect to 2):
Outside of whether or not our 'Approver' role with ManageDatasetPermissions is a good idea, the user with only ManageDatasetPermissions cannot get access to the dataset Edit button to be able to manage those permissions. So I assume that is a bug.

I would think that to fix this, render only the Edit->Permissions option in the dataset.xhtml page if the user has only ManageDatasetPermissions.

With respect to 1) and whether or not the Approver role is a good idea:

You are correct with your statement that we want the Approver role "to allow [users] to delegate the granting of file downloading permission [without the 'delegatee'] having to be an admin or curator".

I assume your concern is that our Approver role will have full ManageDatasetPermissions when all we want them to do is have enough access to hit the Grant/Reject button and nothing else (so essentially a subset of MDP). And that your concern over possible privilege escalation is the fact that with the ManageDatasetPermissions, an Approver will have access to Edit->Permissions->Dataset and Edit->Permissions->Files, and all the permissions related to Users/Groups (including the Approvers themselves) that go along with those. (Is there anything else, besides the permissions-manage-files.xhtml page, that a ManageDatasetPermissions user will have and that we are not aware of?)

As this Approver role is a business case for which ADA needs to find a solution, I see 3 possible approaches:

1. Create the Approver role for applicable datasets, assign the ManageDatasetPermissions to it, add the relevant users and hope for the best in terms of Approvers' behaviour:

Since Approvers (in our Dataverse installation) will belong to the organization that owns the dataset, it is their dataset to give to whomever they want, so it would be upon them to ensure they are using it properly and not 'abusing' privileges. And, of course, Admin users have to be careful of what roles they create and who they assign to them so it would incumbent upon the ADA team, in conjunction with the dataset owners, to make sure they know what they are doing and who they are assigning to various roles, including the Approver role.

2. Assign the ManagaDatasetPermissions to the Approver role and modify dataset.xhtml and permissions-manage-files.xhtml based on the 'approver' role:

If Approvers having access to the functionality within Edit->Permissions->Dataset and Edit->Permissions->Files is a concern for the ADA team, make the following modifications to the code:

a) dataset.xhtml

If the user has 'EditDataset' or 'ManageDatasetPermissions', render the Edit button.
If the user has 'ManageDatasetPermissions' only, render only the 'Permissions' option within the Edit button.
If the user has 'ManageDatasetPermissions' AND their role is 'approver', render only the 'Permissions'->Files sub-option.

b) permissions-manage-files.xhtml

To ensure Approvers can only "Grant" or "Reject" access requests and to prevent Approvers from seeing the "Assign Access" or "Grant Access to Users/Groups", add a logic check for role=='approver'. If role=='approver', render only requests that have been referred and render only the Grant/Reject/Refer buttons for each (referred) request. Do not render any of the other sections/buttons.

3. Create a new 'ManageDatafilePermissions' or 'ManageAccessRequestsPermissions' although I am not sure how fine-grained permissions can be.

ManageDatafilePermissions would give users access to only the Edit->Permissions->Files.
ManageAccessRequestsPermissions would give users access to only the Grant/Reject/Refer buttons in the latter.

Thanks for your feedback!

[pdurbin]

Is there anything else, besides the permissions-manage-files.xhtml page, that a ManageDatasetPermissions user will have and that we are not aware of?

Yes, potentially. Please look for "assignments" at http://guides.dataverse.org/en/4.7/api/native-api.html for how API users can assign roles to others or themselves, if they're allowed to. I'm not trying to make you paranoid. I'm just trying to explain possible ways some privilege escalation could take place. I haven't tested any of this related to ManageDatasetPermissions myself but I do use some of these "assignments" API endpoints when constructing API tests to exercise Dataverse functionality.

Thanks for the long reply but it's too late here to give you more of a response. If you want to send a pull request or two with various ideas of how to move forward, please go ahead. I recently wrote this: http://guides.dataverse.org/en/4.7/developers/version-control.html#how-to-make-a-pull-request

[scolapasta]

Hi Marina! ( @mdmADA ). I've been meaning to respond to this and give some more background.

So first, just to confirm this is a bug, and yes this should be fixed, independent of anything else. We didn't notice it because we don't have any such roles like your Approver, who can manage permissions and not edit. I would suggest doing this a small, specific pull request and not as part of the main functionality you're developing and pinging our design team to see if there's an other aspects of this we're not considering (i.e. if you only have manage, should the button itself still say edit? and other UI aspects like that).

I'll probably just repeat (with different words) what you said, but:

The reason we don't is because we wanted to built in roles that we provide to be small in number, but to cover as many use cases as possible. In particular the reason we didn't think of creating a role that can manage permissions but not edit, is because in reality, that user can edit the dataset, if they want. Since they can manage permissions, they can give themselves the ability to edit. So while the code doesn't enforce it, we considered (some of) the permissions to build upon other permissions, e.g.

• having edit makes no sense without having view unpublished
• having manage without having edit

But I can see the reasons for your use case in that, even though they can change their role to edit, they won't because it's not their job, and it simplifies their UI to only see manage permissions and edit. Is that correct?

So if you're OK with 1 above (that ADA knows that these users technically have the power to do much more than just manage file requests), I'd go with that.

If you do want to restrict then only for that, then we would really suggest the new permissions approach. It's significantly more involved, but is the right way to consider this. (we try to avoid as much as possible code that expects a specific role, though we don'y fully succeed on that). In addition, you'd have to remember to consider this every time new functionality was added related to the manage permissions permission.

In summary, if you're ok with "it would be upon them to ensure they are using it properly and not 'abusing' privileges.", I would 100% go that way. Just know, that using this Approver role would really just be about providing a different user experience for them, and not about not allowing them the same abilities of the dataset Curators (i.e owners).

[mdmADA]

Hi Gustavo - Thanks for your feedback to my essay-like response to Phil. I wrote so much (sorry, Phil!) more to clarify implications in my own mind rather than for any other reason once I realized what Phil was pointing out in terms of permissions. And his points are, of course, valid.

So I will try, but may not succeed, to write a shorter essay in response to your comments....

I can see why permissions and the built-in roles are designed/implemented the way they are. BUT anyone running their own Dataverse install can create a custom role, for whatever reason, with only ManageDatasetPermissions. And if a user shouldn't have these rights outside of Admin/Curator permissions, then it shouldn't be an "assignable" permission for roles. In my opinion, anyway. Of course there are always reasons for any design/implementation... and somebody will always ask 'Why is it like that?" "Well, because..."

For now, subject to @stevenmce's approval, I will leave our solution at giving the dataset owners the Approver role and trust they won't abuse it. I am 99.99% positive that the people in this role would have no idea how to issue API commands (not a criticism, just a fact) so they wouldn't touch that. Somebody with access to an Approver's API key and with the knowledge of how to issue API commands would be able to do it but that is a risk across the board of Dataverse permissions and API keys. And an Approver might grant permissions to themselves or others through the permissions-xxxxxx.xhtml pages but again, that's on them.

Finally, I will do what you suggest wrt the design team and the pull request to address the ManageDatasetPermissions/Edit issue. I will do that most likely in the next couple of days.

Thanks to you and Phil for your assistance!

M.

[pdurbin]

I am 99.99% positive that the people in this role would have no idea how to issue API commands (not a criticism, just a fact) so they wouldn't touch that.

It's good that you have trustworthy users. If I were you I would double-check that the people with your custom "Approver" role can't escalate their own privileges via the GUI. Again, I'm probably paranoid but it's something that I'd recommend that you test just so you understand the risks. I'm not sure what the worst case scenario is... that they could delete unpublished data or publish data they shouldn't? I'm not sure. I'd have to go play with your custom role and put my hacker hat on! 😄

[scolapasta]

Thanks @mdmADA let us know if for some reason that approach does not work. This issue can remain, of course, for the very specific bug mentioned in this description. (I'm going to add a line their, as well to make it easier to "ignore" all these comments.

@pdurbin to be clear, the worse case scenario is they could do anything with the dataset that a Curator could do, since they can give themselves that role.

[mdmADA]

I certainly appreciate the feedback and discussion but isn't this a risk across ALL custom roles then? So why allow custom roles instead of just restricting to the out-of-the-box roles to ensure that nobody creates potential permissions risks? I am probably opening up a can of worms with that question 😄 so I will say that I will consult with @stevenmce to discuss all of these potential issues, and decide on a plan of action.

Thanks again!