Skip to content

Add support for granular permissions in operation#963

Open
salmart-dev wants to merge 2 commits intomainfrom
feat/extended-acl-support
Open

Add support for granular permissions in operation#963
salmart-dev wants to merge 2 commits intomainfrom
feat/extended-acl-support

Conversation

@salmart-dev
Copy link
Copy Markdown
Contributor

@salmart-dev salmart-dev commented Apr 29, 2026

Description

Adds partial* support for granular permissions in Operation keeping the current behaviour intact.

* refers to the fact that additional work is required to support this in the UI. The current use-case is for WFE Runtime Operations, which are not stored in the DB and not shown in the UI.

Implementation details

Currently the operation field of the operation is always set to deny. This PR adds a json object of the shape { "permissions": int } to allow the definition of a custom set of permissions for the operation.

The StorageWrapper then checks against those permissions to see if the action is to be denied or not and returns the masked permissions when appropriate.

Note: AI has been used to review the code and concept and to fix some issues with the original code.

TODO

  • Add integration tests

@salmart-dev salmart-dev self-assigned this Apr 29, 2026
@salmart-dev salmart-dev force-pushed the feat/extended-acl-support branch from e7fc3ef to 41774f9 Compare April 29, 2026 11:06
@salmart-dev salmart-dev marked this pull request as ready for review April 29, 2026 11:17
@salmart-dev salmart-dev requested a review from hweihwang as a code owner April 29, 2026 11:17
Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com>

# Conflicts:
#	lib/StorageWrapper.php
#	tests/Unit/StorageWrapperTest.php
@salmart-dev salmart-dev force-pushed the feat/extended-acl-support branch from 41774f9 to 74d6fa5 Compare April 29, 2026 11:21
@nickvergessen nickvergessen self-requested a review April 29, 2026 15:47
Copy link
Copy Markdown
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This app has a quite good set of behat integration tests.
Would be good if they are extended to cover this.


Is this only going to be used in a scripted way, or is it planned to add a frontend (preferable not 🙈)?

Comment thread lib/CacheWrapper.php
Comment thread lib/StorageWrapper.php
@salmart-dev
Copy link
Copy Markdown
Contributor Author

This app has a quite good set of behat integration tests. Would be good if they are extended to cover this.

Is this only going to be used in a scripted way, or is it planned to add a frontend (preferable not 🙈)?

So far no plans to add a frontend, but how comes would you prefer not to?

@nickvergessen
Copy link
Copy Markdown
Member

So far no plans to add a frontend, but how comes would you prefer not to?

Because it adds javascript/npm and the full trail as a dependency 🙈

@salmart-dev
Copy link
Copy Markdown
Contributor Author

This app has a quite good set of behat integration tests.
Would be good if they are extended to cover this.

Good point, I'll look into this!

@nickvergessen
Copy link
Copy Markdown
Member

Otherwise looks good

Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com>
@salmart-dev salmart-dev force-pushed the feat/extended-acl-support branch from d612907 to a6d8c4d Compare April 30, 2026 17:00
@salmart-dev
Copy link
Copy Markdown
Contributor Author

salmart-dev commented Apr 30, 2026

@nickvergessen added some tests and documented (with tests) the current response for permissions as I find it very confusing 😆

The way files are visible although blocked is so funky 😓 they are visible because we don't filter using the READ permission when listing directories and then the permission appears as R for those files because in server's DavUtil we have a condition that checks if the SHARE permission is there then add R to the permissions string, which is how we end up with R permission but blocked file 🤯

I will push the cs fixes when squashing, to avoid triggering CI one more time 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants