metacatui icon indicating copy to clipboard operation
metacatui copied to clipboard

Restrict dataset submissions to certain groups

Open laurenwalker opened this issue 4 years ago • 13 comments

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

See #1815

laurenwalker avatar Jun 24 '21 17:06 laurenwalker

Possible changes for this issue:

  • add allowUploadDatasetsForSubjects config parameter
  • if the parameter is defined and the current subject is not in the list, hide Upload Data menu item from user drop down menu
  • if the parameter defined and the current subject is not in the list, hide the Edit button on dataset landing pages (MetadataView)

gothub avatar Aug 11 '21 17:08 gothub

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

mbjones avatar Aug 11 '21 18:08 mbjones

From the docs, Metacat can be configured with

  • auth.allowedSubmitters
  • auth.deniedSubmitters

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.

gothub avatar Aug 12 '21 18:08 gothub

That sounds like a great approach.

mbjones avatar Aug 12 '21 18:08 mbjones

@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:

  • DataONE service restriction from MNCore.getCapabilities() will be checked by MetacatUI:
    • Service=MNStorage, method=create()
      • a list of DataONE subjects that are allowed to call this API (from MetacatUI)
    • Service=MNStorage, method=update()
      • a list of DataONE subjects that are allowed to call this API (from MetacatUI)
    • note that in metacat, setting "auth.allowedSubmitters" affects both "create()" and "update()" service restrictions, as metacat considers creating an object and modifying it with update() to both be submissions, as a new pid is created for both of these operations.
      • Therefor, it's not possible to use this mechanism to support the use case of 'allowed' subjects (who are included in the list) to be able to create a dataset or portal, and then have less priviledged subjects (who are not in the list may have been granted access via an accessPolicy rule) to be allowed to only edit these datasets or portals.
      • Note also that the 'auth.allowedSubmitters' does not affect MNStorage.updateSystemMetadata.

Proposed changes:

  • NodeModel.js obtains node documents from CN i.e. MNCore.getCapabilities()

    • update NodeModel.js to retain service names/restrictions, if it doesn't already
  • 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
    • this new fuction will
      • check if the NodeModel has been initialized yet, and the node report ifno (getCapabilties) for the current node is available
      • avoid race condition as the NodeModel may not have obtained info from the CN yet, before the edit page loads
      • if "allowedSubmitters" list exists and the current subject is in the service restriction MNStorage create() list, check passes
      • if "allowedSubmitters" list exists and the current subject is not in the service restriction MNStorage create() list, check fails
      • if no list, check passes
      • call MetacatUI.appUserModel.hasIdentityOverlap to check if the current user's id is in the list of allowed submitters, including equivalent identities and group membership
  • EML211EditorView.js

    • near line 188, add call to checkAllowedSubmitter()
  • UserView.js

    • either disable the 'submit data' button or perform some repo defined action
      • the repo defined action (set via config option) may be
        • 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.
        • this action would be performed by a new function in editorView - possibly called 'showNotalloMmessage()'? - this would be similar to 'showsignin()'
  • in PortalEditorView.js

    • implemeent same checks as in EML211EditorView

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?
  • 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?

Other points

  • note that the operation of these MetacatUI config parameteters should not be affected by the MN service restrictions:
    • allowAccessPolicyChanges, allowAccessPolicyChangesDatasets, allowAccessPolicyChangesDatasetsForSubjects, allowAccessPolicyChangesPortals, allowAccessPolicyChangesPortalsForSubjects, allowChangeRightsHolder

gothub avatar Sep 10 '21 23:09 gothub

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.

laurenwalker avatar Sep 13 '21 17:09 laurenwalker

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 avatar Oct 12 '21 16:10 gothub

@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?

gothub avatar Oct 13 '21 17:10 gothub

@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?

mbjones avatar Oct 13 '21 17:10 mbjones

@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?

gothub avatar Oct 13 '21 18:10 gothub

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)

laurenwalker avatar Oct 14 '21 16:10 laurenwalker

@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:

  • from the main menu Submit button, they are allowed into the EditorView (if logged in) but an alert is displayed with a configurable msg.
  • in the MetadataView, the 'Edit' button is not displayed

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.

gothub avatar Oct 14 '21 16:10 gothub

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

laurenwalker avatar Oct 14 '21 17:10 laurenwalker