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

Allow for actions besides read/write in new ACL selector UI #1004

Open
LukasKalbertodt opened this issue Nov 20, 2023 · 8 comments
Open

Allow for actions besides read/write in new ACL selector UI #1004

LukasKalbertodt opened this issue Nov 20, 2023 · 8 comments
Labels
area:auth Authentication and Authorization area:backend Everything backend related area:frontend Everything frontend related kind:new-feature A new feature

Comments

@LukasKalbertodt
Copy link
Member

Currently, only read and write actions can be changed/set via the UI. But Opencast supports arbitrary actions. Most commonly, annotate and annotate-admin are used in the wild.

We currently encode the assumption that "write" implies "read" access by having a drop down with only two options ("read" and "read & write"). On the one hand, we can clearly not have an option for every combination of actions. On the other hand, these actions usually do have relationships. For example, every action I can think of implies "read". Also "annotate-admin" implies "annotate". Does "write" imply "annotate-admin"? mhh.

So yeah, we need to find a nice way to include those. Make it work very well for common cases, and well enough for all other cases.

@LukasKalbertodt LukasKalbertodt added area:backend Everything backend related area:frontend Everything frontend related kind:new-feature A new feature area:auth Authentication and Authorization labels Nov 20, 2023
@LukasKalbertodt LukasKalbertodt added this to the v2.4 milestone Nov 20, 2023
@dagraf
Copy link
Collaborator

dagraf commented Nov 21, 2023

Thank you for coming up with this feature. We are one of the adopters that wildly use annotate and annotate-admin. :) Let's discuss this in one of our upcoming meetings.

@LukasKalbertodt
Copy link
Member Author

LukasKalbertodt commented Dec 15, 2023

I thought some more about this and wanted to write down some thoughts and ideas. I would like to invite every institutions using custom actions to chime in, especially so if you feel like your use case is not being addressed here!


Actions

Actions that are used in the wild (that I have heard of so far):

  • read
  • write
  • annotate: allows users to create annotations in the annotation tool.
  • annotate-admin: annotate plus more permissions to do anything with that video in the annotation tool.
  • capture: custom one by Vienna, only useful for series, allowing users to create recordings/livestreams in a custom UI

Implications

These represent what logically "makes sense". For example, it does not make sense to give someone write but not read permissions. Of course, Opencast allows you to store an ACL with only write but not read entries, but this is all about "what makes sense logically". Tobira's ACL UI should not allow you to create a nonsensical ACL. The UI should also nicely show/teach the user what makes sense and what not. One could take the stance of "just allow creating nonsensical ACLs", but I think it improves UX and makes the whole system more robust if we don't.

Here are implications I can think of:

  • write -> everything else: if you have write permissions, you can change the ACL to give you those other permissions as well.
    • Maybe that's not always true with certain ACL inheritance rules?
  • Everything else -> read: I cannot think of any action that would not require the user to read the video or metadata, so I am pretty sure this implication is true.
  • annotate-admin -> annotate: the former just grants more permissions than the latter, so it does not make sense to only have annotate-admin permission.

Use cases

  • Simple: only two options "read" and "read & write".
  • Bern: I was told they also only want to options, "read & annotate" and "read, annotate, annotate-admin, write".
  • Annotate-use case that I assume some people use would have four or three total options:
    • "Read"
    • "Read & Annotate"
    • "Read & Annotate-admin" (optional)
    • "Write & everything else"
  • Vienna with their capture action would have "Read", "Read & Capture", "Write & everything"

Tobira should be configurable to properly cover all of those use cases (and more). This would mean configuring custom actions and their implications.

UI Options

  1. List of options where only one can be selected. Each option contains a set of actions. Example: "Read", "Read & Annotate", "Read & Annotate & Write". That's how it's currently implemented.

    • Advantages: Very simple choice for the user. Represents implications in a natural way. Seems to be sufficient for all specific organizations I talked to (there were at most like 4 total options).
    • Disadvantages: when there are many actions and in particular many independent actions, the number of options goes up expontentially.
  2. Just checkboxes, with implications being represented by greying-out and automatically-checking other checkboxes. There would probably not a checkbox for "read" as that is implied by everything. And not having "read" permission means just not having that ACL entry.

    • Advantages: allows arbitrary actions, without exponential blowup. Also maps more directly to how ACL is stored. Sometimes a checkbox is just the more natural way to model an extra action thats independent of everything else.
    • Disadvantages: Does not nicely "show" implications, they are only apparent once checking/unchecking checkboxes. When there are many implications, it will seem especially weird.
  3. Hybrid: Have a list of choices plus checkboxes for extra actions. Sounds like the best of both worlds, but I am really not sure how it would look it practice. And how to model "write implies everything" for example.

Screenshot for option (1):

image

@LukasKalbertodt
Copy link
Member Author

I suppose I would suggest going with option (1) for now. It works for every specific use case that I have heard of and should be fairly simple to implement. Only problem is that we might need to add checkboxes (i.e. hybrid) in the future for other use cases. Not sure how bad this is.

But again: let me know what you think.

@lkiesow
Copy link
Contributor

lkiesow commented Dec 15, 2023

Speaking for Osnabrück University, we want this to be as simple as possible. We don't want to generate a new admin interface. That's what the admin interface is for ;-)

I like the dropdown best. It's true that when there are many actions and in particular many independent actions, the number of options goes up exponentially. But in that case, e.g. having a list of checkboxes would still be complex and likely not very easy to understand for non-technical end users.

@KatrinIhler
Copy link
Member

KatrinIhler commented Dec 15, 2023

Wild idea I don't have a lot of time to consider bc my food is about to be ready:

Config simply allows a list of <label>, <description>, <list of actions>. This allows both action combinations as well as a label for each action separately. And then a second config that says something like allowMultiple, which would change the UI presentation to allow selecting multiple options (can be off by default or something).

Stray thought: Of course the disadvantage of a custom label/description is that it won't be translated. Maybe allow to configure multiple strings for different languages? But imo this problem always presents itself as soon as stuff is configurable.

@KatrinIhler
Copy link
Member

But if I had to choose between option 1 and 2, I'd prefer 1 as well, bc it requires less knowledge of the user in regards to permissions and which combinations make sense.

@LukasKalbertodt
Copy link
Member Author

@lkiesow Thanks for the feedback!

Wild idea [...]

That's indeed an interesting idea! So basically that allowMultiple would toggle the behavior from radio-buttons to check-boxes. Ohhh and one would even be able to have implications in this. For example if you configure one option as having the actions ["read", "annotate"] and another having ["read", "annotate", "write"], then Tobira would know that the second contains strictly more actions and could thus, if that second one is checked, grey out and check the first one to convey to the user that it is implied. Interesting!

And this is just a nice extension of (1). So we could go with (1) now and add allowMultiple some time in the future, without having to change lots of other things.
I have to think about this more, but I think I really like this!

Maybe allow to configure multiple strings for different languages?

We already have a system in place for that and many strings in the config can already be configured in multiple languages. So that's not a problem.

@LukasKalbertodt LukasKalbertodt modified the milestones: v2.4, v2.5 Dec 15, 2023
@KatrinIhler
Copy link
Member

KatrinIhler commented Dec 15, 2023

And this is just a nice extension of (1). So we could go with (1) now and add allowMultiple some time in the future, without having to change lots of other things.

Yeah, exactly!

I have to think about this more, but I think I really like this!

Yaay, I was helpful. :3

We already have a system in place for that and many strings in the config can already be configured in multiple languages. So that's not a problem.

Oh, that's cool! :D

KatrinIhler added a commit to opencast/opencast that referenced this issue Jan 16, 2024
Previously, only read and write roles were transmitted.

This can go into 14 as it is not a breaking change. Well, one thing
changes: `ROLE_ADMIN` is not explicitly included in the event ACL
anymore. But since this is not a public free-for-all API, but only used
by Tobira, and since Tobira has implicit `ROLE_ADMIN` checks everywhere,
this is no problem. In fact, Tobira even removes `ROLE_ADMIN` in the
incoming ACL before storing it in the DB.

Related issue: elan-ev/tobira#1004

### Your pull request should…

* [ ] have a concise title
* [ ] [close an accompanying
issue](https://help.github.com/en/articles/closing-issues-using-keywords)
if one exists
* [ ] [be against the correct
branch](https://docs.opencast.org/develop/developer/development-process#acceptance-criteria-for-patches-in-different-versions)
* [ ] include migration scripts and documentation, if appropriate
* [ ] pass automated tests
* [ ] have a clean commit history
* [ ] [have proper commit messages (title and body) for all
commits](https://medium.com/@steveamaza/e028865e5791)
@LukasKalbertodt LukasKalbertodt modified the milestones: v2.6, v2.7 Feb 14, 2024
@LukasKalbertodt LukasKalbertodt removed this from the v2.7 milestone Mar 12, 2024
LukasKalbertodt added a commit that referenced this issue May 16, 2024
Custom actions for events are now stored in DB with a new column. This
is only the backend side of things, so they won't show up in the ACL UI
yet.

Part of #1004
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:auth Authentication and Authorization area:backend Everything backend related area:frontend Everything frontend related kind:new-feature A new feature
Projects
Status: Todo
Development

No branches or pull requests

4 participants