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

Fix #4789: Group admins cannot see disabled users #7292

Merged
merged 1 commit into from
Nov 27, 2017

Conversation

Neraste
Copy link
Contributor

@Neraste Neraste commented Nov 26, 2017

This is a trivial fix for the issue #4789, pointing out that a sub admin user (not a super user) cannot see the disabled users of their own group.

The problem dwells in settings/Controller/UsersControllerlines, in the index method, line 342 :

if ($gid === '') {
    // list users of groups the current use is sub admin of
} else {
    // list users of provided group
}

This checks only if $gid is an empty string. If disabled users are listed, $gid's value is _disabledUsers and the else branch is executed, where no users are found, since there is not supposed to be a _disabledUsers group.

The PR simply mimics the same mechanics used some lines above if the current user is admin by adding to the check if $gid is _disabledUsers or _everyone.

I would like to test this modification, but I couldn't find how.

Copy link
Member

@LukasReschke LukasReschke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contibution! 🚀🐘

@LukasReschke LukasReschke merged commit 4d20085 into nextcloud:master Nov 27, 2017
@Neraste Neraste deleted the fix/4789_disabled_users branch November 28, 2017 20:33
@MorrisJobke
Copy link
Member

Hey @blizzz - @michag86 asked to backport this to stable12? What is your opinion here? Looks good from my side, just want to have your statement here.

@michag86
Copy link
Contributor

Backport works with stable12 (tested on 12.0.4)
BTW: without this fix the list of disabled users is empty for full admin too

@michag86
Copy link
Contributor

This can fix some parts of this issue #6217

@blizzz
Copy link
Member

blizzz commented Jan 23, 2018

@MorrisJobke OK with me, the patch is nice and not risky in any case.

@blizzz
Copy link
Member

blizzz commented Jan 23, 2018

BTW: without this fix the list of disabled users is empty for full admin too

works for me™

@blizzz
Copy link
Member

blizzz commented Jan 23, 2018

#8006

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants