From 0d72089e014d27fbd31c177c25fcd20b6e27544c Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 7 Dec 2022 12:44:44 +0100 Subject: [PATCH] allow local group modifications in some cases - must not be about admin group - must be about local group backend - must be in allow migration list - must not have users from mixed backends Signed-off-by: Arthur Schiwon --- lib/GroupManager.php | 85 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 69 insertions(+), 16 deletions(-) diff --git a/lib/GroupManager.php b/lib/GroupManager.php index 1f593d24e..82b4d24bc 100644 --- a/lib/GroupManager.php +++ b/lib/GroupManager.php @@ -86,40 +86,51 @@ public function __construct( $this->settings = $settings; } - private function getGroupsToRemove(array $samlGroups, array $assignedGroups): array { + /** + * @param string[] $samlGroupNames + * @param IGroup[] $assignedGroups + * @return string[] + */ + private function getGroupsToRemove(array $samlGroupNames, array $assignedGroups): array { $groupsToRemove = []; + $this->config->getAppValue('user_saml', self::LOCAL_GROUPS_CHECK_FOR_MIGRATION, null); foreach ($assignedGroups as $group) { // if group is not supplied by SAML and group has SAML backend - if (!in_array($group->getGID(), $samlGroups) && $this->hasSamlBackend($group)) { + if (!in_array($group->getGID(), $samlGroupNames) && $this->hasSamlBackend($group)) { + $groupsToRemove[] = $group->getGID(); + } else if ($this->mayModifyGroup($group)) { $groupsToRemove[] = $group->getGID(); } } return $groupsToRemove; } - private function getGroupsToAdd(array $samlGroups, array $assignedGroupIds): array { + private function getGroupsToAdd(array $samlGroupNames, array $assignedGroupIds): array { $groupsToAdd = []; - foreach ($samlGroups as $group) { - // if user is not assigend to the group or the provided group has a non SAML backend - if (!in_array($group, $assignedGroupIds) || !$this->hasSamlBackend($this->groupManager->get($group))) { - $groupsToAdd[] = $group; + foreach ($samlGroupNames as $groupName) { + $group = $this->groupManager->get($groupName); + // if user is not assigned to the group or the provided group has a non SAML backend + if (!in_array($groupName, $assignedGroupIds) || !$this->hasSamlBackend($group)) { + $groupsToAdd[] = $groupName; + } else if ($this->mayModifyGroup($group)) { + $groupsToAdd[] = $groupName->getGID(); } } return $groupsToAdd; } - public function replaceGroups(string $uid, array $samlGroups): void { + public function replaceGroups(string $uid, array $samlGroupNames): void { $user = $this->userManager->get($uid); if ($user === null) { return; } - $this->translateGroupToIds($samlGroups); + $this->translateGroupToIds($samlGroupNames); $assignedGroups = $this->groupManager->getUserGroups($user); $assignedGroupIds = array_map(function (IGroup $group) { return $group->getGID(); }, $assignedGroups); - $groupsToRemove = $this->getGroupsToRemove($samlGroups, $assignedGroups); - $groupsToAdd = $this->getGroupsToAdd($samlGroups, $assignedGroupIds); + $groupsToRemove = $this->getGroupsToRemove($samlGroupNames, $assignedGroups); + $groupsToAdd = $this->getGroupsToAdd($samlGroupNames, $assignedGroupIds); $this->removeGroups($user, $groupsToRemove); $this->addGroups($user, $groupsToAdd); } @@ -144,11 +155,16 @@ public function removeGroup(IUser $user, string $gid): void { if ($group === null) { return; } - $this->ownGroupBackend->removeFromGroup($user->getUID(), $group->getGID()); - if ($this->ownGroupBackend->countUsersInGroup($gid) === 0) { - $this->groupManager->emit('\OC\Group', 'preDelete', [$group]); - $this->ownGroupBackend->deleteGroup($group->getGID()); - $this->groupManager->emit('\OC\Group', 'postDelete', [$group]); + + if ($this->hasSamlBackend($group)) { + $this->ownGroupBackend->removeFromGroup($user->getUID(), $group->getGID()); + if ($this->ownGroupBackend->countUsersInGroup($gid) === 0) { + $this->groupManager->emit('\OC\Group', 'preDelete', [$group]); + $this->ownGroupBackend->deleteGroup($group->getGID()); + $this->groupManager->emit('\OC\Group', 'postDelete', [$group]); + } + } else { + $group->removeUser($user); } } @@ -239,4 +255,41 @@ public function evaluateGroupMigrations(array $groups): void { $this->jobList->add(MigrateGroups::class, ['gids' => $groups]); } + + protected function isGroupInTransitionList(string $groupId): bool { + $candidateInfo = $this->config->getAppValue('user_saml', self::LOCAL_GROUPS_CHECK_FOR_MIGRATION, null); + if ($candidateInfo === null) { + return false; + } + $candidateInfo = \json_decode($candidateInfo, true); + if (!isset($candidateInfo['dropAfter']) || $candidateInfo['dropAfter'] < time()) { + $this->config->deleteAppValue('user_saml', self::LOCAL_GROUPS_CHECK_FOR_MIGRATION); + return false; + } + + return in_array($groupId, $candidateInfo['groups']); + } + + protected function hasGroupForeignMembers(IGroup $group): bool { + foreach ($group->getUsers() as $user) { + if ($user->getBackendClassName() !== 'user_saml') { + return true; + } + } + return false; + } + + /** + * In the transition phase, update group memberships of local groups only + * under very specific conditions. Otherwise, membership modifications are + * allowed only for groups owned by the SAML backend. + */ + protected function mayModifyGroup(?IGroup $group): bool { + return + $group !== null + && $group->getGID() !== 'admin' + && in_array('Database', $group->getBackendNames()) + && $this->isGroupInTransitionList($group->getGID()) + && !$this->hasGroupForeignMembers($group); + } }