From 41c3f5e7cfb4d27f39825eb4a9bcba7a237424b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 5 Mar 2024 12:57:04 +0100 Subject: [PATCH] fix(user_ldap): Catch DB Exceptions when updating group memberships MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/user_ldap/lib/LoginListener.php | 37 +++++++++++- .../lib/Service/UpdateGroupsService.php | 56 ++++++++++++++++++- 2 files changed, 88 insertions(+), 5 deletions(-) diff --git a/apps/user_ldap/lib/LoginListener.php b/apps/user_ldap/lib/LoginListener.php index ac5b32635c8f5..f67bdcbd16709 100644 --- a/apps/user_ldap/lib/LoginListener.php +++ b/apps/user_ldap/lib/LoginListener.php @@ -27,6 +27,7 @@ use OCA\User_LDAP\Db\GroupMembership; use OCA\User_LDAP\Db\GroupMembershipMapper; +use OCP\DB\Exception; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventDispatcher; use OCP\EventDispatcher\IEventListener; @@ -92,7 +93,23 @@ private function updateGroups(IUser $userObject): void { ); continue; } - $this->groupMembershipMapper->insert(GroupMembership::fromParams(['groupid' => $groupId,'userid' => $userId])); + try { + $this->groupMembershipMapper->insert(GroupMembership::fromParams(['groupid' => $groupId,'userid' => $userId])); + } catch (Exception $e) { + if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { + $this->logger->error( + __CLASS__ . ' – group {group} membership failed to be added (user {user})', + [ + 'app' => 'user_ldap', + 'user' => $userId, + 'group' => $groupId, + 'exception' => $e, + ] + ); + } + /* We failed to insert the groupmembership so we do not want to advertise it */ + continue; + } $this->groupBackend->addRelationshipToCaches($userId, null, $groupId); $this->dispatcher->dispatchTyped(new UserAddedEvent($groupObject, $userObject)); $this->logger->info( @@ -105,7 +122,23 @@ private function updateGroups(IUser $userObject): void { ); } foreach ($oldGroups as $groupId) { - $this->groupMembershipMapper->delete($groupMemberships[$groupId]); + try { + $this->groupMembershipMapper->delete($groupMemberships[$groupId]); + } catch (Exception $e) { + if ($e->getReason() !== Exception::REASON_DATABASE_OBJECT_NOT_FOUND) { + $this->logger->error( + __CLASS__ . ' – group {group} membership failed to be removed (user {user})', + [ + 'app' => 'user_ldap', + 'user' => $userId, + 'group' => $groupId, + 'exception' => $e, + ] + ); + } + /* We failed to delete the groupmembership so we do not want to advertise it */ + continue; + } $groupObject = $this->groupManager->get($groupId); if ($groupObject === null) { $this->logger->error( diff --git a/apps/user_ldap/lib/Service/UpdateGroupsService.php b/apps/user_ldap/lib/Service/UpdateGroupsService.php index faee092a37256..85f803ac25a4a 100644 --- a/apps/user_ldap/lib/Service/UpdateGroupsService.php +++ b/apps/user_ldap/lib/Service/UpdateGroupsService.php @@ -107,7 +107,24 @@ public function handleKnownGroups(array $groups): void { continue; } foreach (array_diff($knownUsers, $actualUsers) as $removedUser) { - $this->groupMembershipMapper->delete($groupMemberships[$removedUser]); + try { + $this->groupMembershipMapper->delete($groupMemberships[$removedUser]); + } catch (Exception $e) { + if ($e->getReason() !== Exception::REASON_DATABASE_OBJECT_NOT_FOUND) { + /* If reason is not found something else removed the membership, that’s fine */ + $this->logger->error( + __CLASS__ . ' – group {group} membership failed to be removed (user {user})', + [ + 'app' => 'user_ldap', + 'user' => $removedUser, + 'group' => $group, + 'exception' => $e, + ] + ); + } + /* We failed to delete the groupmembership so we do not want to advertise it */ + continue; + } $userObject = $this->userManager->get($removedUser); if ($userObject instanceof IUser) { $this->dispatcher->dispatchTyped(new UserRemovedEvent($groupObject, $userObject)); @@ -121,7 +138,24 @@ public function handleKnownGroups(array $groups): void { ); } foreach (array_diff($actualUsers, $knownUsers) as $addedUser) { - $this->groupMembershipMapper->insert(GroupMembership::fromParams(['groupid' => $group,'userid' => $addedUser])); + try { + $this->groupMembershipMapper->insert(GroupMembership::fromParams(['groupid' => $group,'userid' => $addedUser])); + } catch (Exception $e) { + if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { + /* If reason is unique constraint something else added the membership, that’s fine */ + $this->logger->error( + __CLASS__ . ' – group {group} membership failed to be added (user {user})', + [ + 'app' => 'user_ldap', + 'user' => $addedUser, + 'group' => $group, + 'exception' => $e, + ] + ); + } + /* We failed to insert the groupmembership so we do not want to advertise it */ + continue; + } $userObject = $this->userManager->get($addedUser); if ($userObject instanceof IUser) { $this->dispatcher->dispatchTyped(new UserAddedEvent($groupObject, $userObject)); @@ -151,7 +185,23 @@ public function handleCreatedGroups(array $createdGroups): void { $users = $this->groupBackend->usersInGroup($createdGroup); $groupObject = $this->groupManager->get($createdGroup); foreach ($users as $user) { - $this->groupMembershipMapper->insert(GroupMembership::fromParams(['groupid' => $createdGroup,'userid' => $user])); + try { + $this->groupMembershipMapper->insert(GroupMembership::fromParams(['groupid' => $createdGroup,'userid' => $user])); + } catch (Exception $e) { + if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { + $this->logger->error( + __CLASS__ . ' – group {group} membership failed to be added (user {user})', + [ + 'app' => 'user_ldap', + 'user' => $user, + 'group' => $createdGroup, + 'exception' => $e, + ] + ); + } + /* We failed to insert the groupmembership so we do not want to advertise it */ + continue; + } if ($groupObject instanceof IGroup) { $userObject = $this->userManager->get($user); if ($userObject instanceof IUser) {