NCEAS / metacatui

MetacatUI: A client-side web interface for DataONE data repositories
https://nceas.github.io/metacatui
Apache License 2.0
42 stars 27 forks source link

Restrict dataset submissions to certain groups #1817

Open laurenwalker opened 3 years ago

laurenwalker commented 3 years ago

Add config option to disable submissions unless a person is in one or more of a list of groups.

See #1815

gothub commented 3 years ago

Possible changes for this issue:

mbjones commented 3 years ago

While this would be a useful feature, as would the request in #1815, I would stress that access control responsibility lies with Metacat, not MetacatUI. Any restrictions on who can upload or use services should continue to be enforced by Metacat. Information about those restrictions on the Metacat side could be passed to MetacatUI in order to modify the UI to prevent unauthorized actions from occurring, but the information on which services are available to which users should be configured in Metacat, not in MetacatUI, lest people get around those restrictions through API calls. ANd lest the information in MetacatUI is not updated when updates are made to the Metacat config.

We've discussed this in the past, and Metacat does already support access rules on our API services via the DataONE API, which we have only lightly used. Here's the documentation of the field: https://dataone-architecture-documentation.readthedocs.io/en/latest/apis/Types.html#Types.ServiceMethodRestriction

gothub commented 3 years ago

From the docs, Metacat can be configured with

My understanding (after discussing with @taojing2002 ) is that setting these Metacat parameters will cause the MNCore.getCapabilities() call to return an updated MN node capabilities document that includes the service restrictions mentioned above, in this case for MNStorage.

A possible approach would be to have MetacatUI call MNCore.getCapabilities() for a node, search for the <service name="MNStorage" version="v1" available="true"/> entry and get the list of allowed and denied submitters for create and update, then update the UI accordingly, based on the currently logged in user.

mbjones commented 3 years ago

That sounds like a great approach.

gothub commented 3 years ago

@laurenwalker as requested, here are my notes from the MetacatUI meeting on 20210909, which I have reworked and edited a bit. Please review when you have the time:

Proposed changes:

Questions:

Other points

laurenwalker commented 3 years ago

Thanks @gothub for writing this all up, I think this all looks great. Just a few points below:

create new function checkAllowedSubmitter() or similiar name

not sure what model js this should be in
    it is similar to DataONEobject.checkAuthority, but that function checks access for current subject against accessPolicy of current object.
    the new function is not specific to a DataONE pid, but instead to a subject

I think the UserModel would be a good spot for this, since the check is specific to a user/subject.

UserView.js

either disable the 'submit data' button or perform some repo defined action 

display a configurable message telling user they are not authorized to submit data maybe provide a template that could be used for info links, email, etc.

We'll want to show the message, via a customizable template, in the EditorView, not the UserView.

I suggest we don't disable the Submit Data buttons, at least for now. When someone clicks on that button and they are not allowed to submit, they will go to the EditorView and see the message.

Your Questions:

why would a non-submitting subject be allowed into the metadata and portal editor views?

is it so that they could view/change access rules?

I would think that if a person is not allowed to update objects at the repository level, they should not be able to change access rules, either. I believe the changePermission permission in the DataONE model implies view and write permissions as well. So if someone is not in the allowed updaters (i.e. write) list at the repo level, they shouldn't be able to changePermission either.

And on that note, we may want to restrict people from giving Can edit and Is owner permissions to people that are not allowed updaters at the repo level. (This is probably a separate task and separate GH issue to complete later).

would it be better to grey out 'submit data' and 'edit' if user is not allowed to submit?

could an informative/configuraable message be displayed when rolling over greyed out button?

As mentioned above, I think let's skip disabling the Submit Data button for now and just let those people see the message in the EditorView. Disabling the button would save those people a click, but I think this gets the job done for now.

We will need to make sure the Edit button is hidden in the MetadataView by adding a call to UserModel.checkAllowedSubmitter() in the SolrResult.checkAuthority() function. The MetadataView still uses the SolrResult model and not the new DataONEObject model. (We need to refactor that, but for now that's how it is.)

Non-DataONE repos

We need to make sure this functionality works for member nodes that are not registered in DataONE. Since we are grabbing the node info from the CN node info doc, unregistered nodes will not be there. We'll have to grab the node info from the MN API for those repos.

Editor error message

During testing, it would be good to force create and update calls from the editor to make sure Metacat fails to create the object, and that MetacatUI displays a coherent error message. This will account for (hopefully rare) cases where the node allowed lists are changed while they are editing.

gothub commented 3 years ago

Here is a screenshot of the error message displayed in MetacatUI when a user is not in the allowedSubmitters list:

Screen Shot 2021-10-12 at 6 27 43 AM

Note that the blue info box appears after See technical details has been pressed - nicely done!

gothub commented 3 years ago

@laurenwalker what do you recommend for handling MNs that are not in the DataONE network - i.e. those that don't report their capabilties to the CN?

mbjones commented 3 years ago

@gothub For those that are not registered with DataONE, can we still access the local node service on metacat to get the node description with allowed submitters?

gothub commented 3 years ago

@mbjones yes, if NodeModel.js can't obtain MN capabilities from the CN then it can certainly get them directly from the MN.

@laurenwalker does that sound good to you? Should this change (to NodeModel.js) go into a PR for this ticket, or into a new ticket?

laurenwalker commented 3 years ago

Yep, sounds good to me!

Do we catch that people cannot submit to the node before the editor is loaded so that they do not get this far? So that way this error message is just a fail-safe?

It would also be nice if we could parse the error message and display something more user friendly - that may be a new ticket since there are a few error responses that I would love to reword.

On Wed, Oct 13, 2021 at 2:21 PM Peter Slaughter @.***> wrote:

@mbjones https://github.com/mbjones yes, if NodeModel.js can't obtain MN capabilities from the CN then it can certainly get them directly from the MN.

@laurenwalker https://github.com/laurenwalker does that sound good to you? Should this change go into a PR for this ticket, or in a new one?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NCEAS/metacatui/issues/1817#issuecomment-942592703, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSV4FREPQQZT64Q3CDIXTLUGXE35ANCNFSM47IKKWFQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- National Center for Ecological Analysis and Synthesis (NCEAS) University of California Santa Barbara (UCSB)

gothub commented 3 years ago

@laurenwalker the logic of the code checked into the feature-restrict-submission-#1817, if the allowedSubmitters list is set and the user is not in the list:

The screen grab shown above was generated by running on dev.nceas directly when metacat auth.allowedSubmitter was set, so my local code changes were not in effect for this test, so this was more of a test of MetacatUI for the case you mentioned above:

During testing, it would be good to force create and update calls from the editor to make sure Metacat
fails to create the  object, and that MetacatUI displays a coherent error message. This will account
for (hopefully rare) cases where the node allowed lists are changed while they are editing.

Yes, updating the error messages should probably be in a new ticket.

laurenwalker commented 3 years ago

Perfect, sounds great! I'll create that new ticket for error messages.