-
Notifications
You must be signed in to change notification settings - Fork 87
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
feat: add option to resolve inherited option per-user instead of per-path #2870
Conversation
169c4d1
to
12a0c75
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you confirm that the goal is to have a switch to toggle between:
- Restrict down the user based on the rules
- Give as many permissions to the user based on the rules.
The difference is more complex than that. The changes in how rules are inherited/merged will lead to more permissions in some setups but it's dependent on the specifics of the configured rules. The main difference is that, with the new setup, if you're member of one group which has access to a folder and member of a group which has access blocked you will always have access. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving to get it merged, but I would like to properly understand the logic:
Considering the following hierarchy: /a/b
Without inheritMergePerUser
(old): merge(b/g1
, b/g2
)->apply(merge(a/g1
, a/g2
))
With inheritMergePerUser
(new): merge(b/g1
->apply(a/g1
), b/g2
->apply(a/g2
))?
applyPermissions
: if the rule does not have a value for a permission, use the provided one
mergeRules
: do an "or" with both permissions set
Folder | Read | Update | Share | Delete |
---|---|---|---|---|
a/g1 | 1 | 1 | 1 | 1 |
a/g2 | - | - | - | - |
b/g1 | - | - | - | - |
b/g2 | 0 | - | - | - |
b/u1 (old) | 0 | 1 | 1 | 1 |
b/u1 (new) | 1 | 1 | 1 | 1 |
If that's correct, then perfect :).
Do you think it would make sense to add a comment with the above?
12a0c75
to
d9b324a
Compare
Yes, I've added a comment, please let me know if that explanation is clear enough |
…path Signed-off-by: Robin Appelman <robin@icewind.nl>
d9b324a
to
c4b9374
Compare
/backport to stable28 |
/backport to stable27 |
/backport to stable26 |
Instead of always have subfolder rules overwrite parent folder rules, first resolve the inherited rules for each user/group before combining the rules from the different users/groups.
With the default behavior, if any rule applicable to the user denies permission for a subfolder, the permission will be denied, even if the user is member of a group that has "allow" permissions.
With the new behavior, if any group the user is a member for has "allow" permissions, the user has access.
To test:
group1
andgroup2
user
test ingroup1
andgroup2
gf
accessible togroup1
andgroup2
an ACLs enabledgf/a
and create a rule on it forgroup1
giving full permissionsgf/a/b
and create a rule on it forgroup2
, denying read permissionsuser
won't have access togf/a/b
because of the rule from 5.occ config:app:set groupfolders acl-inherit-per-user --value true
user
does have access togf/a/b
becausegroup1
has access, users only ingroup2
won't have access