Sword Auth: use Dataverse 4 permission model #1070 #2495 #784 #3137
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We are switching away from the old "admin only" permission checks we were forced to used in the DVN 3.x days when the permission system was less flexible. Now the rule is that if the permission system doesn't prevent you from performing an operation (create dataset, upload file) SWORD won't prevent you either. Two SWORD operations had to be rewritten to support Dataverse 4 permissions. First, retrieval of the Service Document with permissionService.getDataversesUserHasPermissionOn has been changed to pay attention to a user's groups. It's very common for the ":authenticated-users" group to be given a role that should allow SWORD operations, for example. Second, the "list datasets in a dataverse" operation (listCollectionContents) was rewritten to allow anyone with `AddDataset` permission to be able to execute the command to they can see if a dataverse has been published or not but the only datasets that are returned are those for which the user can issue `UpdateDatasetCommand`. In the root dataverse, for example, the user should only see their datasets.
This was referenced May 24, 2016
|
I just wanted to mention that @axfelix and I chatted a bit about OJS testing at http://irclog.iq.harvard.edu/dataverse/2016-05-24#i_35578 For OSF, conversation continues at CenterForOpenScience/osf.io#5344 |
|
Got this in Apache logs from a stock OJS 2.4.8 instance hooked into dev1.dataverse.org when trying to deposit. Will drill down a little later unless you have any immediate ideas:
|
|
@axfelix uh oh, line 666 even. I'm assuming you got a 500 response? I'll past below the stack trace I'm seeing (line numbers for 94f57b0). The Let me knock on some doors around here and see what may be going on. @axfelix meanwhile if you could repeat your test for https://demo.dataverse.org (which is running v. 4.3.1 build 94-be5b26e) I'd appreciate it. I'm also sort of wondering if it's a transient error since my test suite since just completed without error though it did seem to hang for many seconds when creating datasets. So if you could run your test against https://dev1.dataverse.org a second time I'm appreciate it. If you ping me at http://chat.dataverse.org I can be tailing the logs while you're running your tests. |
|
Indeed, I couldn't duplicate! https://dev1.dataverse.org/dataset.xhtml?persistentId=doi:10.5072/FK2/UZAIBX&version=DRAFT Worked fine the second time (with an .md file rather than .py, if it matters, though I'd expect not). |
|
@axfelix thanks for re-testing. At https://ezidstatus.wordpress.com/feed/ I'm seeing the following...
... so I'm guessing that's why you got that error. That feed and https://ezidstatus.wordpress.com are linked to from http://ezid.cdlib.org/home/documentation (thank you @sekmiller for the pro tip).
|
This was referenced Jun 6, 2016
Closed
11 tasks
|
@scolapasta talked about the code as of 96d9ac4 and decided: |
pdurbin
added a commit
that referenced
this pull request
Jul 15, 2016
hasAccessToModifyDataverse was hard-coded to true since we have added other checks to be compatible with Dataverse 4 permissions. Removing this method is a no-op but it looks like a massive change, unfortunately, because of the changes in nesting.
|
@scolapasta I'm passing this back to you because I made the two changes you definitely wanted and played around with the third that we talked about. Changed in this pull request:
Played around with this in a new branch (not in this pull request):
|
|
@kcondon has been testing this pull request and noticed that groups within groups are not honored by the SWORD Service Document. This is a known issue with MyData as well (#3056) so we agreed to list this as a known issue with SWORD, which I did in af47f90. While I was on this branch I went ahead and merged the latest from develop (bd50ab7) but then the app no longer compiled! I fixed this (got the app compiling again) in aa2d7ce. This was just in a test class (UtilIT.java) so no big deal. |
|
@scolapasta @pdurbin Ready for merge. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
RFI Checklist
We are switching away from the old "admin only" permission checks we were forced to used in the DVN 3.x days when the permission system was less flexible. Now the rule is that if the permission system doesn't prevent you from performing an operation (create dataset, upload file, etc.) SWORD won't prevent you either.
Two SWORD operations had to be rewritten in this pull request to support Dataverse 4 permissions.
First, retrieval of the Service Document with permissionService.getDataversesUserHasPermissionOn has been changed to pay attention to a user's groups. It's very common for the ":authenticated-users" group to be given a role that should allow SWORD operations, for example. I'm a bit concerned about the performance impact of this change, especially with production data, which I haven't tested with. I'm putting #784 thought QA and I'd like to remind us that we the reason we lost support for groups in our performance-related fix for #2012. I basically iterated on that fix, bringing group support back in what I hope is a performant way.
Second, the "list datasets in a dataverse" operation (listCollectionContents) was rewritten to allow anyone with
AddDatasetpermission to be able to execute the command to they can see if a dataverse has been published or not but the only datasets that are returned are those for which the user can issueUpdateDatasetCommand. In the root dataverse, for example, the user should only see their datasets. This is potentially a n expensive operation in a dataverse with many datasets, such as the root.The other SWORD operations received no change to their business logic but Dataverse 4 style permission checks were added and the old "admin only" check was removed.
I updated the SWORD API Guide to explain the necessary permissions for each operation. You can see the diff at the top of 94f57b0
I also noted in the Shibboleth section of the Installation Guide that Shib groups are not supported via SWORD: 94f57b0.
1. Related Issues
2. Pull Request Checklist
3. Review Checklist
After the pull request has been submitted, fill out this section.
We are switching away from the old "admin only" permission checks we
were forced to used in the DVN 3.x days when the permission system was
less flexible. Now the rule is that if the permission system doesn't
prevent you from performing an operation (create dataset, upload file)
SWORD won't prevent you either.
Two SWORD operations had to be rewritten to support Dataverse 4
permissions.
First, retrieval of the Service Document with
permissionService.getDataversesUserHasPermissionOn has been changed to
pay attention to a user's groups. It's very common for the
":authenticated-users" group to be given a role that should allow SWORD
operations, for example.
Second, the "list datasets in a dataverse" operation
(listCollectionContents) was rewritten to allow anyone with
AddDatasetpermission to be able to execute the command to they can see if a
dataverse has been published or not but the only datasets that are
returned are those for which the user can issue
UpdateDatasetCommand.In the root dataverse, for example, the user should only see their
datasets.