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

Permission issue #2934

Closed
Mululu opened this issue Apr 25, 2024 · 23 comments
Closed

Permission issue #2934

Mululu opened this issue Apr 25, 2024 · 23 comments
Labels
0. Needs triage Issues that need to be triaged bug discussion feature: acl Items related to the groupfolders ACL or "Advanced Permissions"

Comments

@Mululu
Copy link

Mululu commented Apr 25, 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 a group folder and assign 2 groups.

  2. Group 1 read, write

  3. Group 2 read, write, delete

  4. Try deleting folder with user in group 2

Expected behaviour

Users in group 2 should be able to delete

Actual behaviour

User from group 2 cannot delete because he is also in group 1 and is not allowed to delete group 1. The higher permission in this case, allowing deletion, should override the deletion note

Nextcloud Hub 8 (29.0.0)

@Mululu Mululu added 0. Needs triage Issues that need to be triaged bug labels Apr 25, 2024
@XueSheng-GIT
Copy link

Just tried with my NC29 test installation and followed your step. But actually user in group2 is able to delete.
Please double check whether the steps to reproduce are complete. Thanks.

@Mululu
Copy link
Author

Mululu commented May 6, 2024

Hello,

I tried a few things back and forth, but unfortunately to no avail. This error is causing major problems in our organization. Steps I have now taken

  • Deleted all permissions using the database and reset them >> no improvement

To narrow down the error further, I have done the following. 1 & 2 work as expected, but the error already occurs in 3., which is why I started the post.

1.

  • The "Admin" group gets all rights for the main "Images" folder
  • The "All" group does not get read rights

Result Users in the "Admin" group can see the "Images" folder // Users in the "All" group cannot // Users who are in the "Admin" group and in the "All" group at the same time can see the "Images" folder. >> Works

2.

  • The "Admin" group gets all rights for the main folder "Pictures"
  • The "All" group gets only read rights

Result Users in the "Admin" group can see the "Pictures" folder // Users in the "All" group can also see the "Pictures" folder // Users who are in the "Admin" group and in the "All" group at the same time can see the "Pictures" folder. >> Works

3.

  • The "Admin" group gets all rights for the main folder "Pictures"
  • The "All" group gets only read rights
  • The "All" group is denied the "Read" right for a subfolder "Tablet". Result Users in the "Admin" group can see the "Pictures" folder and the "Tablet" subfolder // Users in the "All" group can also see the "Pictures" folder, but not the "Tablet" subfolder // Users who are in the "Admin" group and the "All" group at the same time can see the "Pictures" folder but not the "Tablet" subfolder >> Does not work as intended

Expectation: Users who are in the "Admin" and "All" groups at the same time should see the "Tablet" subfolder. So to speak, the "Admin" group's read permission should be higher than denying read rights to the "All" group.

I suspect that the permissions are not being passed on to the subfolders correctly!?

I hope I made it clear.

@XueSheng-GIT
Copy link

I assume you are mixing up general Access rights via Admin settings and ACL.
Please be more precise where you did those access restrictions. For me is also not clear whether you created nested groupfolders or not.
Maybe screenshots for the non-working example would be helpful (admin settings + ACL).

@Mululu
Copy link
Author

Mululu commented May 6, 2024

The main rights are assigned in the settings. However, since there are also subfolders in the group folder that not everyone should see, the ACL permission is required there. image

What you can also see in the following image is that rights are displayed incorrectly. The general rights from the settings (image above) are not displayed as standard (inherited) but are generally shown as denied. Main folder
image

But then shown as allowed again in the subfolder
image

Variant 2 (as described above)

Here the folder is visible to the Admin and Everyone groups, as defined in the basic settings. However, the display is incorrect as described above. image
Subfolder "Tablet" visible according to basic setting rights
Display group "Admin" correct, display group All incorrect >> according to basic setting no share and no deletion rights. Read, write, create >> correct
image

Variant 3 (as described above)
Read rights for group All on subfolder Tablet denied
328163928-0d5bb3ec-4896-4c1c-8830-b5d4baa70ce1

Result
Users who are in the group All can no longer see the folder >> so far so good. Users who are only in the group Admin can still see the folder >> just fine. However, if I add the user to the group All, the folder is no longer visible, even though they are still in the admin group and have reading rights.

Basically, the rights should be displayed correctly, otherwise it's stupid. And maybe you should be able to define the group's ranking. But that's not the main problem. Everything worked before the update to Nextcloud 29.

@XueSheng-GIT
Copy link

XueSheng-GIT commented May 6, 2024

On the frist view I would say as expected. ACL deny does overwrite general permit. Try to add a ACL grant to read for Admin and double check again. But I assume that also deny acl does overwrite grand acl.
Maybe a more correct description: #2828

Regarding displaying of inheritance, I think there's already an issue for this (probably also a fix).

@XueSheng-GIT
Copy link

Please also double check whether your issue is a duplicate of #2812.

@fschrempf fschrempf added the feature: acl Items related to the groupfolders ACL or "Advanced Permissions" label May 6, 2024
@fschrempf
Copy link
Contributor

Maybe related or even fixed by: #2870

@Mululu
Copy link
Author

Mululu commented May 6, 2024

On the frist view I would say as expected. ACL deny does overwrite general permit. Try to add a ACL grant to read for Admin and double check again. But I assume that also deny acl does overwrite grand acl. Maybe a more correct description: #2828

Regarding displaying of inheritance, I think there's already an issue for this (probably also a fix).

Unfortunately, I have to disagree with you because the regulations are such that approvals override refusals.

image

Maybe related or even fixed by: #2870

I'll look at it tomorrow

@Mululu
Copy link
Author

Mululu commented May 7, 2024

So I have no idea what has changed in Nextcloud 29 or in the Groupfolder app, but something is wrong here. According to the documentation, the allow right should override the deny right. This has worked for me in the past and we have been using Nextcloud since version 25. And from now on it no longer works without any changes to the rights.

Maybe something has been messed up here? @#2902

What I can't find anywhere is what does occ config:app:set groupfolders acl-inherit-per-user --value true do?
Is this new or when was it introduced?
What happens if it's true? What happens if it's false? I couldn't see any change.

@chaosgrid
Copy link

chaosgrid commented May 14, 2024

I think this is the same issue as #2812 because in general, inherited "allow" permissions of a group A do not take precedence over explicit "deny" permissions of a different group B in the current folder while you are a member of both groups. This behaviour changed somewhere within the last year.

@Jerome-Herbinet
Copy link
Member

@icewind1991 I would like to draw your attention to this thread.

@fschrempf
Copy link
Contributor

What I can't find anywhere is what does occ config:app:set groupfolders acl-inherit-per-user --value true do?
Is this new or when was it introduced?
What happens if it's true? What happens if it's false? I couldn't see any change.

Once again some feature was added without documentation. You have to read through the description in the PR here: #2870 (comment). This should give you an idea what the new option is about. This was only added a few weeks ago and it seems like it has been backported to NC 26 to 29.

@Mululu
Copy link
Author

Mululu commented May 17, 2024

I do not get it.

Do I now have to set acl-inherit-per-user to true or false to restore the previous behavior or is that no longer possible? Such a change is totally annoying!

@dorianne-arawa
Copy link

To restore old ACLs (prior to Groupfolders versions 14.0.6, 15.3.3, 16.0.2 and 17.0.0), you need to set the occ config:app:set groupfolders acl-inherit-per-user --value parameter to true. In this way, the old ACL operating mode remains available. I hope this information will help you @Mululu and @fschrempf

This setting is the result of Nextcloud correcting a bug in Groupfolder. This bug was being exploited by many users (as it was not easy to identify). This correction therefore implies a change in the expected behavior of Groupfolder ACLs on the part of users who were exploiting this bug. (intentionally or not)
Following a collaboration between Nextcloud and Arawa, this setting has been implemented to allow both modes of operation, according to user preference.

To my knowledge, there is no documentation on this yet. Arawa plans to write one as soon as possible to provide a clear view of these changes and the impact of this new configuration.

Damour Dorianne
Arawa

@Mululu
Copy link
Author

Mululu commented May 17, 2024

I have now executed the occ config:app:set groupfolders acl-inherit-per-user --value true. However, it doesn't work properly, here are 3 videos:

User is in group "admin" & group "Alle"

Video 1: General assignment of rights for groups, locking the "Tablet" folder for the "Alle" group, folder no longer visible even though users are also in the "admin" group

Video 2. Inheritance should be basically on, but selected again, folder not visible.

Video 3. "admin" group explicitly given read rights, folder not visible

Video 4. Inheritance display error?

In general, it seems to me that there is a bug with inheritance!?

https://vimeo.com/947520766?share=copy
https://vimeo.com/947520830?share=copy
https://vimeo.com/947520766?share=copy
https://vimeo.com/947523572?share=copy

EDIT:

With Nextcloud Hub 8 (29.0.1 RC1) the display error (video 4) seems to have been resolved.

image

@chaosgrid
Copy link

What I can't find anywhere is what does occ config:app:set groupfolders acl-inherit-per-user --value true do?
Is this new or when was it introduced?
What happens if it's true? What happens if it's false? I couldn't see any change.

Once again some feature was added without documentation. You have to read through the description in the PR here: #2870 (comment). This should give you an idea what the new option is about. This was only added a few weeks ago and it seems like it has been backported to NC 26 to 29.

Thanks, setting this to true re-activates the old behaviour. In the future, it would be good to document these changes since this is a critical permission behaviour and I think many people were locked out of folders due to this change. In general, old behaviour should have priority over activating a new critical permission behaviour for everybody by default.

@Mululu
Copy link
Author

Mululu commented May 22, 2024

But the problem is not yet solved! True doesn't seem to perform the old behavior correctly see my post with the videos...

#2934 (comment)_

The display error has been fixed but the folder still does not remain visible to the "admin" if the folder is locked for the "Everyone" group.

@Mululu
Copy link
Author

Mululu commented May 27, 2024

!?!?

@Jazz7584
Copy link

Jazz7584 commented Jun 5, 2024

To restore old ACLs (prior to Groupfolders versions 14.0.6, 15.3.3, 16.0.2 and 17.0.0), you need to set the occ config:app:set groupfolders acl-inherit-per-user --value parameter to true. In this way, the old ACL operating mode remains available. I hope this information will help you @Mululu and @fschrempf

This setting is the result of Nextcloud correcting a bug in Groupfolder. This bug was being exploited by many users (as it was not easy to identify). This correction therefore implies a change in the expected behavior of Groupfolder ACLs on the part of users who were exploiting this bug. (intentionally or not) Following a collaboration between Nextcloud and Arawa, this setting has been implemented to allow both modes of operation, according to user preference.

To my knowledge, there is no documentation on this yet. Arawa plans to write one as soon as possible to provide a clear view of these changes and the impact of this new configuration.

Damour Dorianne Arawa

Worked for me thx.

@dorianne-arawa
Copy link

dorianne-arawa commented Aug 21, 2024

Hi @Mululu ,

I don't know if your problem has been solved.

Could you please tell me which version of the Group folders application you had installed when you wrote your comment containing the 4 videos?
Reading your videos, I don't see the “ dashes ” for inherited rights, never set manually, which makes me wonder about the version.

Could you also confirm that the “occ config:app:set groupfolders acl-inherit-per-user --value” parameter was set to “true”? The problems seen in the videos seem to be solved if the parameter is set to “true”.

In video 1: I've reproduced the problem as best I can on an instance, and my user being in “admin” and “Alle” can see the “Tablet” folder after reloading. But if the “false” mode is activated for the new parameter, the result is that this same user can no longer see “Tablet” (in the case of inherited and manual rights, it's the manual right that takes priority when the mode is set to “wrong”).

In video 2, my thoughts are similar: the manual “false” read right of “Alle” overrides the “true” read right of “admin” if the “occ config:app:set groupfolders acl-inherit-per-user --value” parameter is set to “false”. At “true”, “admin” should see the folder. The same applies to video 3.

After various tests, I noticed that when the parameter “occ config:app:set groupfolders acl-inherit-per-user --value ”is set to :

  • "false": we have the new operating mode, following a bug fix (which should have been the initial operating mode, but was not effective because of the fixed bug in question):

    • if you only have inherited rights, the most permissive right applies.
    • if you only have manual rights, the right of the nearest relative set manually (by going up the tree) applies.
    • if you have a mixture of both inherited and manual rights, the manual rights apply, and if two inherited rights are different, then the MANUALLY SET NEAREST PARENT right (going up the tree) applies.
  • "true": we're back to the way things worked before, but without the bug and with proper inheritance of rights:

    • whether permissions are inherited or manual, or both mixed on a folder, the most permissive right applies.
    • if there are dashes somewhere, it means that the right has never been manually entered in the database (but a “true” or “false” right still applies, but I don't know which).

I hope this will help you or others.

@level420
Copy link

level420 commented Aug 26, 2024

@dorianne-arawa but a dash means inherited in both cases, indifferent from occ config:app:set groupfolders acl-inherit-per-user --value set to true or false, or am I wrong here?

@dorianne-arawa
Copy link

dorianne-arawa commented Aug 26, 2024

I think you're right @level420 . It means inherited in both case. But it's inherited rights when you only click on the user or group in the advanced permission, and define nothing. I suppose the applying rule is taken from the group folder interface (nearest parent here) and the three rights (delete, share, write). The Files interface displays a dash, even if a right "true" or "false" applied.

groupfolder interface:
image

advanced permissions interface:
image

final user rights interface:
image

Maybe those 2 links can help you too : #3067 and https://github.com/arawa/nextcloud-groupfolder-acl-flattener (trying to fix the dashed with script, but it's dedicated this is dedicated to the transition from the old version of GF without dashes to the new version with dashes, to be done in one time)

@provokateurin
Copy link
Member

Duplicate of #598

@provokateurin provokateurin marked this as a duplicate of #598 Sep 17, 2024
@provokateurin provokateurin closed this as not planned Won't fix, can't repro, duplicate, stale Sep 17, 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 discussion feature: acl Items related to the groupfolders ACL or "Advanced Permissions"
Projects
None yet
Development

No branches or pull requests

9 participants