Hyrax: User can remove (repo admin or any user) permission in Sharing tab on work

[julesies]

Descriptive summary

Hyrax 2.1. We are testing permissions for our go live of hyrax and came across this bug which I confirmed on nurax today.

A user/depositor can remove any person/role via the Sharing tab in the UI including the repo admin. We tested adding a work and having the depositor remove access to the repo admin, which was allowed. The repo admin could still access the work and edit it, but on visiting the sharing tab, the role was not visible.

Expected behavior

depositor can only see, remove roles/permissions that they added. permissions that are removed, are actually removed.

Steps to reproduce the behavior

1. Create a work as a regular user
2. Go to sharing tab
3. Remove admin access, hit save
4. log in as admin, find and edit work, hit save

Done looks like

• [x] Admins cannot have their edit access revoked by any user for any work
• [x] Users cannot override edit access permissions created by the work's admin set or collection
• [x] User view: the "currently shared with" table hides rows for admin and any groups that exist as participants in the work's admin set or collection
• [x] Admin/ Manager/ Editor view: the "currently shared with" table shows admin and all users and groups that have edit access to the work (for example, if User A is listed as a Manager of a collection, User A can go to any work in that collect, click Edit, view the Sharing tab, and see a table that lists all users and groups that have edit access to that work and act according to the settings of the admin set or collection containing that work).

[julesies]

@vantuyls @chrisdaaz @no-reply ping!

[julesies]

Also, I just tested adding an editor to the work, then revoking the editor and that worked as expected.

[chrisdaaz]

@julesies would removing this row from the "Currently Shared With" table satisfy the issue?

i believe admins should always have edit access to all works in the repository. it doesn't seem useful to show the user that admins have edit access to the work they deposited, let alone showing them options to edit or revoke that access.

[julesies]

yeah agree it seems odd to display admin access on the work view but it does go beyond admins. We have 3 groups from using role mapper, for example and the user can "remove" any user or group who has access granted to them another way. Since currently the only way to remove an individual users listed on the admin set is by visiting each work, I'm thinking you would have to show that to managers, admins, (or what ever group has the right to edit permissions on works).

here is an example: (tab bug reported here https://github.com/samvera/hyrax/issues/3175)
In this case, the work was granted this access to three groups and one individual from the admin set participants settings (not added to the individual work)

[chrisdaaz]

could we add this to the issue? @julesies

Done looks like

• [ ] Users cannot override edit access permissions created by the work's admin set
• [ ] The "currently shared with" table hides admin and any groups that exist as participants in the work's admin set

[julesies]

yes, but add something like _only show those groups to admins, managers, editors_? and change to groups and individuals from admin sets _or collections_. (this seems right, what do you think)

[chrisdaaz]

@julesies okay, i added that stuff to the issue to make it more actionable, but i hope it's not too complicated. take a look and add any clarifications if necessary.

[laritakr]

After talking with @julesies my understanding is
1) show permissions inherited from a collection or admin set only to participants of the collection or admin set
2) don't show admin at all, since it can't be removed anyway (and we also don't show public or registered access at the work level.)

[laritakr]

While working on this issue, it became apparent that this change as requested could be very confusing and misleading to a work's editor.

A work can be in more than one collection, and if so, we wouldn't know which collection affected the permissions, because we don't track that. And permission changes on a collection don't carry through to a work, at least not at this point. So the tie between the collection and the work's permissions is tenuous at best.

Admin set permissions always carry through to the work when it is created. Collection permissions only do if you create a new work from the button off of the collection's show view. Creating a work and adding it to a collection while you create it does not change permissions, nor does adding an existing work to a collection.

So a work could have been created, and the depositor/editor could have given it permissions that they can now not change because it just happens to be the same permissions as the collection had. They also could add or attempt to add a permission which would appear to not be working because that permission is also on the collection or admin set.

I would recommend showing all permissions but limiting the edit and delete access to them, and optionally showing something to identify the collection which is limiting the ability to change those permissions.

So the changes I intend to implement to the work's permissions view are:

• No one can view admin permissions on the work, because they should always exist and we don't want to allow them to be edited
• no one can edit a work's permissions which inherit from the manager role of a collection/admin set unless they are also a manager of that collection/admin set. these permissions will show on the work but not be editable, and the collection which causes them to be frozen from change will be identified
• other permissions may be edited on the work by anyone with edit rights

(note: An editor could still change these rights by removing the work from the collection, removing the rights, and adding it back to the collection, because the inheritance of permissions only happens during the work's creation.)

@julesies Does this sound like what we discussed as the best option?
@vantuyls @chrisdaaz Do you agree with this solution?

[laritakr]

Just a note: the code in my (now closed because of lack of progress) PR was working as I believed it should, and was waiting for feature approval before I completed it. It only needed the associated specs to be complete. Obviously by now, there will probably be a number of conflicts but starting from what I did should simplify this quite a bit.

[jeremyf]

@jlhardes now that you're product owner, I'm wondering about this issue. What are your thoughts? There is now aging work that @laritakr did, and I can help carry this to completion OR close this out.

[jlhardes]

@jeremyf It looks like this is still happening and it seems like good changes to make to clarify that Sharing view and prevent that action of removing permission when you can’t actually remove that permission.

How aged is @laritakr's code changes? Is this something that is feasible to include in Hyrax 3.0 release? I don't know that it is worth adding to the list of things for Hyrax 3.0 if it extends the timeline for the release. Would it work better as a Hyrax 3.1 change to incorporate?

[jeremyf]

@jlhardes I've rebased the code to the current Hyrax state. It's something that I'm prepared to move forward. I believe that it wouldn't take too much to get the code into 3.0 release.

@laritakr spent considerable time working through the possible scenarios and ensuring proper behavior.

[jlhardes]

Tested locally and regular user can no longer remove (or see) admin access and repo-admin user can still see the regular user's work and make edits.