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: Return only administered groups of a user for subadmins #44141

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Pytal
Copy link
Member

@Pytal Pytal commented Mar 12, 2024

Summary

  • Fixes a bug when disabling a user as a subadmin who does not have access to all a users groups shows an error
  • Prevents returning extraneous user information to subadmins

Checklist

@Pytal Pytal added this to the Nextcloud 29 milestone Mar 12, 2024
@Pytal Pytal requested review from nickvergessen and blizzz March 12, 2024 00:56
@Pytal Pytal self-assigned this Mar 12, 2024
This was referenced Mar 12, 2024
@emoral435
Copy link
Contributor

LGMT :)

This was referenced Mar 18, 2024
@skjnldsv skjnldsv mentioned this pull request Mar 28, 2024
81 tasks
This was referenced Apr 4, 2024
@blizzz blizzz modified the milestones: Nextcloud 29, Nextcloud 30 Apr 8, 2024
@Pytal Pytal force-pushed the fix/subadmin-user-groups branch from 6c4e4a4 to 3c74e02 Compare May 22, 2024 23:13
@Pytal Pytal requested review from nfebe and come-nc May 22, 2024 23:14
@Pytal Pytal enabled auto-merge May 22, 2024 23:14
@emoral435
Copy link
Contributor

Haha, just noticed that my review no longer works ^*^ but LGTM!

@@ -137,6 +137,9 @@ protected function getUserData(string $userId, bool $includeScopes = false): ?ar
$groups = $this->groupManager->getUserGroups($targetUserObject);
$gids = [];
foreach ($groups as $group) {
if (!$this->groupManager->getSubAdmin()->isSubAdminOfGroup($currentLoggedInUser, $group)) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (!$this->groupManager->getSubAdmin()->isSubAdminOfGroup($currentLoggedInUser, $group)) {
if (!$isAdmin && !$this->groupManager->getSubAdmin()->isSubAdminOfGroup($currentLoggedInUser, $group)) {

But not sure we should do that.
It might be important to see that a user is in more/other groups?

This was referenced Jul 30, 2024
@Altahrim Altahrim mentioned this pull request Aug 5, 2024
@skjnldsv skjnldsv added the 2. developing Work in progress label Aug 6, 2024
@skjnldsv skjnldsv added stale Ticket or PR with no recent activity and removed 3. to review Waiting for reviews labels Aug 6, 2024
@Altahrim Altahrim mentioned this pull request Aug 7, 2024
@skjnldsv skjnldsv mentioned this pull request Aug 13, 2024
@skjnldsv skjnldsv modified the milestones: Nextcloud 30, Nextcloud 31 Aug 14, 2024
@skjnldsv skjnldsv disabled auto-merge November 15, 2024 13:10
@skjnldsv skjnldsv marked this pull request as draft November 15, 2024 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress bug feature: users and groups stale Ticket or PR with no recent activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants