Seperate user and non-user roles#1006
Conversation
Changes the access policy tab of various details modals. User roles get displayed in a seperate table from the other roles, with user name and email. This should make it more intuitive to "give someone access to a video/ series". Functionality should not change, the resulting ACLs will look the same as before. Caveat 1: If roles are sanitized, we cannot reliably derive the user from their user role. Therefore, this feature will be disabled if user role sanitization is enabled in Opencast. Caveat 2: I did not find a performant way to query for user information for each and every user. Therefore, if there are upwards of multiple thousands of users, the access policy tab will take several seconds to load.
|
Use Run test server using develop.opencast.org as backend: Specify a different backend like stable.opencast.org: It may take a few seconds for the interface to spin up. |
|
This pull request is deployed at test.admin-interface.opencast.org/1006/2025-05-16_12-56-22/ . |
|
This pull request has conflicts ☹ |
|
I have tested this PR together with opencast/opencast#6382 and have a few remarks.
|
|
Thanks for testing, will look into these issues! |
The options in the dropdown for selecting a user in the access policy tab would show the user role (aka the thing actually being selected) instead of the user information like in the input field. This fixes that. Also adds some sensible fallbacks in case someone decides to have users without names.
Add support for the access roles ROLE_UI_EVENTS_DETAILS_ACL_USER_ROLES_VIEW ROLE_UI_EVENTS_DETAILS_ACL_NONUSER_ROLES_VIEW ROLE_UI_SERIES_DETAILS_ACL_USER_ROLES_VIEW ROLE_UI_SERIES_DETAILS_ACL_NONUSER_ROLES_VIEW
|
Both issues should now be fixed. |
|
This pull request has conflicts ☹ |
|
Should an event with ROLE_USER_x be visible to user x? I would, perhaps naively, assume that if I give a user read and/or write that they'd be able to see it in the admin UI... Otherwise works correctly. |
|
Could you specify what exactly is different from how you expect it? Which combination of role and actions does not do what you would think it do? |
|
Thanks @Arnei for addressing my comments.
|
To make sure I understand you correctly: If the user does not have any of the roles mentioned above, the "Access Policy" tab should not be displayed at all. For example in your screenshot, the user would go from "Processing" straight to "Summary". Is that right? What roles should the event have in that case? I would be tempted to leave fixing that to a different PR, since for me this raises questions about what the role |
Yes, the user would go from "Processing" straight to "Summary". In our case we have the ACL merge mode "actions" active, this means the roles for the series are valid for such an event (and the series is mandatory for each event).
Personally, I see the purpose of the different roles as follows:
For us, the split view for ACLs only makes sense if we can hide the non-user roles part in both the create and edit dialog. So I'd prefer to have the ACL view roles issue fixed in this PR. But if it is much easier for you to address this in another PR, then go ahead. |
This should make the search for searchable dropdowns work as expected, meaning the search string is now matched correctly against the existing labels. The custom filtering this patch removes only serves to break the search, and what it tries to achieve the 'react-select' search already does out of the box.
In the create event modal, do not display the step "Access policy" if the user is missing the role "ROLE_UI_EVENTS_DETAILS_ACL_VIEW". The step gets completely skipped. Per default, this results in an ACL with read/write access for the user.
|
Thanks for the very detailed explanation on the roles, that helped me a lot. I made a change so that the user requires the role "ROLE_UI_EVENTS_DETAILS_ACL_VIEW" to see the "Access Policy" step in the "Create Event" modal. This should work the default merge mode as well, and result in the other roles working as intended. Please test again.
Searching should now work as intended. |
|
This pull request has conflicts ☹ |
gregorydlogan
left a comment
gregorydlogan
left a comment
There was a problem hiding this comment.
Urgh. So fresh build of 17.x+relevant PR and this PR + a delete of package-lock.json:
- I can't see the test user I created when trying to upload an event
- I also don't see any REST query which looks like a 'fetch-all-users' request. Nor does Chrome find my test user when I search for it in the requests log.
- The user's role does appear in the bottom set of roles, regardless of
sanitizesetting - so the user definitely exists at least :)
- The bottom set of role definitions needs a better title than 'Details' :)
- The ACL templates just straight up don't work at all, 404ing looking for
/admin-ng/users/usersforroles.json
It's unclear to me which of these might be affected by the current bugbash work.
|
#works-on-my-machine. In my local deployment users and templates are working. Current bugbash work should have had not effect on this (so far).
Would you help me out with a suggestion? I agree that "Details" is not a very useful descriptor here, but I'm struggling to come up with something better. |
|
This pull request has conflicts ☹ |
|
Now it has been affected by the bugbash. Not in a way that would explain your problems though. |
In the old admin UI, this section was called "Non-user Roles." To be consistent with the naming of the user section, we could name it "Roles and Groups Authorized for the Event"? Though technically, we have user roles in the user section... However, end users will not be aware of that. |
|
My test user's |
|
This pull request has conflicts ☹ |
Replaces "Details" with "Roles and Groups for [...]".
|
You'll have to show me your setup sometime Greg. It kinda sounds like your admin interface is not pointing at the correct Opencast, but also like there is more going on. |
|
For me it works as well: I did use a local OC build based on r/17.x incl. opencast/opencast#6382 (but I also had to recreate package-lock.json for the admin UI). |
|
This pull request has conflicts ☹ |
|
I... don't get what I was doing wrong, but after git clean of literally everything it works fine. |
4 participants
Fixes #905, #561.
Changes the access policy tab of various details modals. User roles get displayed in a seperate table from the other roles, with user name and email. This should make it more intuitive to "give someone access to a video/ series". Functionality should not change, the resulting ACLs will look the same as before.
Caveat 1: If roles are sanitized, we cannot reliably derive the user from their user role. Therefore, this feature will be disabled if user role sanitization is enabled in Opencast. (The old admin ui allowed for sanitized roles and did some best effort guesswork to derive users from roles. Even though that approach worked for many, I'd rather not reintroduce such brittle code here.)
Caveat 2: I did not find a performant way to query for user information for each and every user. Therefore, if there are upwards of multiple thousands of users, the access policy tab will take several seconds to load.
Requires backend changes. Will break the UI otherwise. opencast/opencast#6382
How to test this patch
As this requires backend changes, the automatically generated stuff from github-actions below will not work.
Install the backend PR in your Opencast. Run this PR against your Opencast, like so
PROXY_TARGET=http://localhost:8080 npm start. Thesanitizeflag can be found in your Opencast configuration files inetc/org.opencastproject.userdirectory.UserIdRoleProvider.cfg.Screenshots:
