Skip to content

Conversation

@provokateurin
Copy link
Member

If a user has no access to a Team folder, no rules are returned and it is assumed that this user has all permissions.
Instead of applying the folder permissions as a mask, it would also be possible to just check if the folder permissions are 0 and then use 0 as the path permissions directly, but in the end this results in the same output.

@provokateurin provokateurin added this to the Nextcloud 32 milestone Jun 24, 2025
@provokateurin provokateurin added bug 3. to review Items that need to be reviewed labels Jun 24, 2025
@provokateurin
Copy link
Member Author

/backport to stable31

@provokateurin
Copy link
Member Author

/backport to stable30

@icewind1991
Copy link
Member

This change also impacts users that do have access to the groupfolder, but the group they have access trough doesn't have full (non-acl) permissions.

For example, if a user has access trough a group that doesn't have the "delete" base permissions, and a -share acl on a folder.

With master the acl test returns

+read, +write, +create, +delete, -share

With this PR it returns

+read, +write, +create, -share

I'm not sure if either of those are "correct" output.

+read, +write, +create, -delete, -share

Is probably most correct (though it might mislead admins into thinking the -delete comes from an ACL rule, having an "explain" option that show how the permissions are calculated has been on my whish list for a while).

I would suggest either changing this PR to show that output, or scope it down to only handle the case where a user doesn't have access at all.

…the folder in the first place

Signed-off-by: provokateurin <[email protected]>
@provokateurin provokateurin force-pushed the fix/acl/command-test-permissions branch from 67cc662 to e749891 Compare June 30, 2025 13:03
@provokateurin provokateurin changed the title fix(ACL): Apply folder permissions mask when testing path permission fix(ACL): Don't check ACL permissions if user doesn't have access to the folder in the first place Jun 30, 2025
@provokateurin
Copy link
Member Author

@icewind1991 please review again.

@provokateurin provokateurin merged commit e62ad60 into master Jun 30, 2025
53 of 55 checks passed
@provokateurin provokateurin deleted the fix/acl/command-test-permissions branch June 30, 2025 14:05
@skjnldsv skjnldsv mentioned this pull request Aug 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Items that need to be reviewed bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants