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

Cascading usergroup moderator permissions broken #3233

Open
Ben-MyBB opened this issue Jun 3, 2018 · 0 comments · May be fixed by #3421
Open

Cascading usergroup moderator permissions broken #3233

Ben-MyBB opened this issue Jun 3, 2018 · 0 comments · May be fixed by #3421
Assignees
Labels
b:1.8 Branch: 1.8.x s:in-progress Status: In Progress. Some work completed t:bug Type: Bug. An issue causing error / flaw / malfunction

Comments

@Ben-MyBB
Copy link
Member

Ben-MyBB commented Jun 3, 2018

If you create a user moderator on a category, and enable, say, open/close threads and stick/unstick threads, the user will have these powers in all forums inside the category. If you then add them as a moderator again to one of the forums inside this category, and only give them the ability to stick/unstick, then they lose the ability to open/close threads in this specific forum. This is correct.

However, the same does not apply to usergroup moderators. Using the same example as above, the moderators would still be able to open/close threads in the forum were specific moderator permissions were set, where they should only be granted stick/unstick permissions.

The issue is that in get_moderator_permissions, when setting permissions for users, it checks if the value is 0:

PHP Code:// Figure out the user permissionsif($value == 0){    // The user doesn't have permission to set this action    $perms[$action] = 0;}else{    $perms[$action] = max($perm[$action], $perms[$action]);} 
This will override the 1 set at category level with the 0 set at forum level, thus removing the ability to open/close threads (again, correct).

However, a few lines down when setting permissions for usergroups, it doesn't do this:

PHP Code:$perms[$action] = max($perm[$action], $perms[$action]); 
Simply using the same code for usergroups as we do for users fixes the issue.

Usually in MyBB, we say a yes overrides a no - i.e. you can be in 10 groups that disallow a permission, but 1 that allows it, so you will be allowed. However, I think moderator permissions are a bit different.

Personally I think usergroups should behave the same as users, as it is reasonable to want to override permissions for a specific forum, maybe 3 levels deep, while setting everything else on a category level. This already works for users, just not usergroups.

Ideally, these permissions wouldn't be set up with a simple yes/no, they really need an 'inherit' option. This way, when you edit the moderators on specific forums, you could choose whether to specifically restrict a permission, or keep it in line with the permissions set on the category. However, this is best suited for 2.0 :D

Original thread: Cascading usergroup moderator permissions broken

@Ben-MyBB Ben-MyBB added b:1.8 Branch: 1.8.x t:bug Type: Bug. An issue causing error / flaw / malfunction s:confirmed Status: Confirmed. Retested and found the issue exists labels Jun 3, 2018
@Starpaul20 Starpaul20 self-assigned this Aug 30, 2018
@Starpaul20 Starpaul20 added this to the 1.8.19 milestone Aug 30, 2018
@dvz dvz added s:review-needed Status: Review Needed. Possible solution submitted s:in-progress Status: In Progress. Some work completed and removed s:confirmed Status: Confirmed. Retested and found the issue exists s:review-needed Status: Review Needed. Possible solution submitted labels Sep 4, 2018
@dvz dvz removed this from the 1.8.19 milestone Sep 6, 2018
@effone effone added this to the 1.8.20 milestone Nov 8, 2018
@dvz dvz removed this from the 1.8.20 milestone Jan 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
b:1.8 Branch: 1.8.x s:in-progress Status: In Progress. Some work completed t:bug Type: Bug. An issue causing error / flaw / malfunction
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants