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

Possible permission issue with folder in Group Folder for user who is member of multiple groups #2280

Closed
vdietmar opened this issue Feb 26, 2023 · 5 comments
Labels
0. Needs triage Issues that need to be triaged bug

Comments

@vdietmar
Copy link

Steps to reproduce

  1. Create user groups "User Group A" and "User Group B"
  2. Create user "User AB" and as member of "User Group A" and "User Group B"
  3. Create Group Folder and give "User Group A" and "User Group B" access with "Write/Share/Delete "rights
  4. Create folder "See me" in this Group Folder
  5. Set the advanced permissions of this folder to deny "Read" for "User Group A"
  6. Login in as "User AB" and view Group Folder

Expected behaviour

As "User AB" is a member of both groups "User Group A" and "User Group B", and "User Group B" still has inherited "Read" access to the folder "See me", it should be visible to "User AB".

Actual behaviour

"User AB" does not see the folder "See me".

Server configuration

**Operating system: Linux 5.4.0-137-generic x86_64

**Web server: unknown

**Database: mysql

**PHP version: 8.0.25

**Nextcloud version: 24.0.7 Enterprise

**Group folders version: 12.0.3

**Updated from an older Nextcloud/ownCloud or fresh install: fresh

**Where did you install Nextcloud from: managed Nextcloud at IONOS

**Are you using external storage, if yes which one: no

**Are you using encryption: no

**Are you using an external user-backend, if yes which one: no

Client configuration

**Browser: Chrome

**Operating system: Mac OS

Logs

Web server error log

Web server error log Not available b/c managed Nextcloud

Nextcloud log (data/nextcloud.log)

Nextcloud log Not available b/c managed Nextcloud

Browser log

Browser log Not relevant b/c no error is actually thrown
@vdietmar vdietmar added 0. Needs triage Issues that need to be triaged bug labels Feb 26, 2023
@koelle25
Copy link

I think this would be desired behaviour. I don't remember if there is anything in the docs about permission precedence, but in e.g. NTFS it is like this:

The hierarchy of precedence for the permissions can be summarized as follows, with the higher precedence permissions listed at the top of the list:

  • Explicit Deny
  • Explicit Allow
  • Inherited Deny
  • Inherited Allow

(see https://www.ntfs.com/ntfs-permissions-precedence.htm)

@vbier
Copy link

vbier commented Nov 17, 2023

If you look at the readme here, it says:
If multiple configured advanced permissions for a single file or folder apply for a single user (such as when a user belongs to multiple groups), the "allow" permission will overwrite any "deny" permission.

So this is no expected behaviour, but a bug. IMHO, any other behaviour makes a logical project and group setup impossible.

@vbier
Copy link

vbier commented Nov 20, 2023

The problem seems to lie in the code that combines the rules (ACLManager.php). It combines all rules for a given user and path level, and then applies the resulting rules level after level. So in the example above it combines "write/share/delete" as permissions for the group folder level and deny as permissions for the "See me" folder level and then applies the deny permission on top of the write/share/delete of the group folder level.

Obviously, this does not work if different groups are used on different levels (as in the example in the issue report).

A correct impementation should combine permissions for every used group/user from the top level down to the respective node in a first step and then in a second step combine all those permissions (with allow overriding deny).

@icewind1991, as you seem to be the only one actively working on the groupfolders app, do you have time to look into this? The bug pretty much makes groupfolders unusable in projects with a little bit more complex permission setup. If you agree with my analysis, I could try to come up with a fix, even though I am not familiar with php.

@vbier
Copy link

vbier commented Nov 20, 2023

I just saw that this has been worked on in #1654. Unfortunately progress has stopped. At least my analysis seems to be correct. :-)

@vbier
Copy link

vbier commented Nov 21, 2023

This issue is a duplicate of #1212 and can be closed.

@Rello Rello closed this as completed Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. Needs triage Issues that need to be triaged bug
Projects
None yet
Development

No branches or pull requests

4 participants