Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restrict dataset submissions to certain groups #1817

Open
laurenwalker opened this issue Jun 24, 2021 · 13 comments
Open

Restrict dataset submissions to certain groups #1817

laurenwalker opened this issue Jun 24, 2021 · 13 comments
Assignees
Labels
ADC CI-10 User management for project portals (ADC deliverable) arctic data center enhancement ESS-DIVE Issues associated with the ESS-DIVE project

Comments

@laurenwalker
Copy link
Member

laurenwalker commented Jun 24, 2021

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

See #1815

@laurenwalker laurenwalker added enhancement ESS-DIVE Issues associated with the ESS-DIVE project labels Jun 24, 2021
@gothub gothub self-assigned this Aug 10, 2021
@gothub
Copy link
Contributor

gothub commented Aug 11, 2021

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)

@mbjones
Copy link
Member

mbjones commented Aug 11, 2021

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
Copy link
Contributor

gothub commented Aug 12, 2021

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.

@mbjones
Copy link
Member

mbjones commented Aug 12, 2021

That sounds like a great approach.

@gothub
Copy link
Contributor

gothub commented Sep 10, 2021

@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

@laurenwalker
Copy link
Member Author

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
Copy link
Contributor

gothub commented Oct 12, 2021

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
Copy link
Contributor

gothub commented Oct 13, 2021

@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
Copy link
Member

mbjones commented Oct 13, 2021

@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
Copy link
Contributor

gothub commented Oct 13, 2021

@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
Copy link
Member Author

laurenwalker commented Oct 14, 2021 via email

@gothub
Copy link
Contributor

gothub commented Oct 14, 2021

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

@laurenwalker
Copy link
Member Author

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

@robyngit robyngit added the ADC CI-10 User management for project portals (ADC deliverable) label May 17, 2023
@robyngit robyngit self-assigned this Jul 13, 2023
@robyngit robyngit added this to the 2.26.0 milestone Jul 13, 2023
@robyngit robyngit modified the milestones: 2.26.0, 2.27.0 Aug 4, 2023
@robyngit robyngit modified the milestones: 2.27.0, 2.28.0 Sep 21, 2023
@robyngit robyngit modified the milestones: 2.28.0, 2.29.0 Feb 8, 2024
@robyngit robyngit modified the milestones: 2.29.0, 2.31.0 Apr 18, 2024
@robyngit robyngit removed this from the 2.31.0 milestone Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ADC CI-10 User management for project portals (ADC deliverable) arctic data center enhancement ESS-DIVE Issues associated with the ESS-DIVE project
Projects
Development

No branches or pull requests

4 participants