Skip to content

[stable28] fix(user_ldap): Catch DB Exceptions when updating group memberships #47335

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.

Sign up for GitHub

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

Merged
AndyScherzinger merged 1 commit into from
Aug 20, 2024
Merged

[stable28] fix(user_ldap): Catch DB Exceptions when updating group memberships #47335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 35 additions & 2 deletions apps/user_ldap/lib/LoginListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand Down
56 changes: 53 additions & 3 deletions apps/user_ldap/lib/Service/UpdateGroupsService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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));
Expand Down Expand Up @@ -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) {
Expand Down