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

Confusing UI when setting ACL's on groupfolder causing folders to be locked out for all. #3377

Closed
toontoet opened this issue Oct 21, 2024 · 4 comments
Labels
0. Needs triage Issues that need to be triaged bug

Comments

@toontoet
Copy link

toontoet commented Oct 21, 2024

How to use GitHub

  • Please use the 👍 reaction to show that you are affected by the same issue.
  • Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
  • Subscribe to receive notifications on status change and new comments.

Steps to reproduce

  1. Create two groups: Everyone & NotEveryone and join both groups
  2. Create a shared folder with all permissions for both Everyone and NotEveryone, Allow NotEveryone (group) to manage permissions
  3. Go to the sharing sidebar of the group folders and give Everyone (group) and NotEveryone (group) all permission: (allow)

Image

  1. Create a new folder in the shared folder
  2. For this specific subfolder I want to limit access for users in the Everyone folder so only NotEveryone users can access it. To do so, open the sharing sidebar and set the permissions for the Everyone group to deny.

Image

  1. Refresh the page

Expected behaviour

While I'm both in the Everyone and NotEveryone I expect to be able to see the created subfolder. The UI shows allow signs for the group NotEveryone:

Image

Actual behaviour

But after the refresh I cannot see the subfolder anymore.

Image

Workaround

After struggling with this for a while I found a work-around:

If I first set the permissions for the Sub-folder to Allow for NotEveryone and then I set the permissions to deny for Everyone the acl's work as expected:

Image

If you watch carefully you see there is a X at the end of the permissions line. This is the only sign that these permissions are set on the subfolder itself and the permissions are not inherited from the parent folder.

So this works:
Image

And this not:
Image

I expect that Allow is permitted before Deny is applied. That is the case when you set allow straight on subfolder. If the allow is inherited from the parent folder it's not.

AND, it would be nice to have a UI where you can see what you get. While the UI show's me all allow signs for the NotEveryone group, it's not allowing me in. A notice or alert when you lock yourself out, as also suggested in #1339 would also be appreciated.

Server configuration

Operating system:
Linux 5.4.0-150-generic x86_64
Web server:

Database:
mysql 10.2.27
PHP version:
8.1.28
Nextcloud version: (see Nextcloud admin page)
27.1.10

Group folders version:
15.3.10

Updated from an older Nextcloud/ownCloud or fresh install:

Where did you install Nextcloud from:

Are you using external storage, if yes which one: local/s3/smb/sftp/...

Are you using encryption: yes/no

Are you using an external user-backend, if yes which one: LDAP/ActiveDirectory/Webdav/Saml/...

Client configuration

Browser:
All

Operating system:
All

@toontoet toontoet added 0. Needs triage Issues that need to be triaged bug labels Oct 21, 2024
@provokateurin
Copy link
Member

Duplicate of #598

@provokateurin provokateurin marked this as a duplicate of #598 Oct 21, 2024
@provokateurin provokateurin closed this as not planned Won't fix, can't repro, duplicate, stale Oct 21, 2024
@provokateurin
Copy link
Member

To be clear: The UI is not the problem, the permission logic is wrong and the UI is right.

@toontoet
Copy link
Author

well, that's just a matter of perspective... from the end-users point of view the UI does not represent the way the permission logic is working at the moment. It could be made a lot less confusing using a small UI improvement, by separating the folder ACL's from the parent ACL's:

Folder permissions (overrule Inherited permissions):
Everyone X X X X X

Inherited permissions:
NotEveryone v v v v v
Everyone v v v v v

It's also UI/UX to add a confirmation dialog when you're about to set a permission that will lock the current user out.

Anyway... still hope #598 will be fixed some day.

@provokateurin
Copy link
Member

Yes, but it depends on what the intention is. The UI implements the intention of the feature correctly, while the logic is wrong. So yeah the UI could be changed until it is fixed, but it is much better to fix the underlying logic that causes the problems.
I also just mentioned this issue to my manager, since so many people have this problem and it should really get fixed.

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

2 participants