Skip to content

Commit 4a23308

Browse files
come-nccome-nc
authored
Merge pull request #43999 from nextcloud/fix/user_ldap-catch-db-errors-when-updating-group-memberships
fix(user_ldap): Catch DB Exceptions when updating group memberships
2 parents 782f808 + d163347 commit 4a23308

2 files changed

Lines changed: 88 additions & 5 deletions

File tree

‎apps/user_ldap/lib/LoginListener.php‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
use OCA\User_LDAP\Db\GroupMembership;
2929
use OCA\User_LDAP\Db\GroupMembershipMapper;
30+
use OCP\DB\Exception;
3031
use OCP\EventDispatcher\Event;
3132
use OCP\EventDispatcher\IEventDispatcher;
3233
use OCP\EventDispatcher\IEventListener;
@@ -92,7 +93,23 @@ private function updateGroups(IUser $userObject): void {
9293
);
9394
continue;
9495
}
95-
$this->groupMembershipMapper->insert(GroupMembership::fromParams(['groupid' => $groupId,'userid' => $userId]));
96+
try {
97+
$this->groupMembershipMapper->insert(GroupMembership::fromParams(['groupid' => $groupId,'userid' => $userId]));
98+
} catch (Exception $e) {
99+
if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
100+
$this->logger->error(
101+
__CLASS__ . ' – group {group} membership failed to be added (user {user})',
102+
[
103+
'app' => 'user_ldap',
104+
'user' => $userId,
105+
'group' => $groupId,
106+
'exception' => $e,
107+
]
108+
);
109+
}
110+
/* We failed to insert the groupmembership so we do not want to advertise it */
111+
continue;
112+
}
96113
$this->groupBackend->addRelationshipToCaches($userId, null, $groupId);
97114
$this->dispatcher->dispatchTyped(new UserAddedEvent($groupObject, $userObject));
98115
$this->logger->info(
@@ -105,7 +122,23 @@ private function updateGroups(IUser $userObject): void {
105122
);
106123
}
107124
foreach ($oldGroups as $groupId) {
108-
$this->groupMembershipMapper->delete($groupMemberships[$groupId]);
125+
try {
126+
$this->groupMembershipMapper->delete($groupMemberships[$groupId]);
127+
} catch (Exception $e) {
128+
if ($e->getReason() !== Exception::REASON_DATABASE_OBJECT_NOT_FOUND) {
129+
$this->logger->error(
130+
__CLASS__ . ' – group {group} membership failed to be removed (user {user})',
131+
[
132+
'app' => 'user_ldap',
133+
'user' => $userId,
134+
'group' => $groupId,
135+
'exception' => $e,
136+
]
137+
);
138+
}
139+
/* We failed to delete the groupmembership so we do not want to advertise it */
140+
continue;
141+
}
109142
$groupObject = $this->groupManager->get($groupId);
110143
if ($groupObject === null) {
111144
$this->logger->error(

‎apps/user_ldap/lib/Service/UpdateGroupsService.php‎

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,24 @@ public function handleKnownGroups(array $groups): void {
107107
continue;
108108
}
109109
foreach (array_diff($knownUsers, $actualUsers) as $removedUser) {
110-
$this->groupMembershipMapper->delete($groupMemberships[$removedUser]);
110+
try {
111+
$this->groupMembershipMapper->delete($groupMemberships[$removedUser]);
112+
} catch (Exception $e) {
113+
if ($e->getReason() !== Exception::REASON_DATABASE_OBJECT_NOT_FOUND) {
114+
/* If reason is not found something else removed the membership, that’s fine */
115+
$this->logger->error(
116+
__CLASS__ . ' – group {group} membership failed to be removed (user {user})',
117+
[
118+
'app' => 'user_ldap',
119+
'user' => $removedUser,
120+
'group' => $group,
121+
'exception' => $e,
122+
]
123+
);
124+
}
125+
/* We failed to delete the groupmembership so we do not want to advertise it */
126+
continue;
127+
}
111128
$userObject = $this->userManager->get($removedUser);
112129
if ($userObject instanceof IUser) {
113130
$this->dispatcher->dispatchTyped(new UserRemovedEvent($groupObject, $userObject));
@@ -121,7 +138,24 @@ public function handleKnownGroups(array $groups): void {
121138
);
122139
}
123140
foreach (array_diff($actualUsers, $knownUsers) as $addedUser) {
124-
$this->groupMembershipMapper->insert(GroupMembership::fromParams(['groupid' => $group,'userid' => $addedUser]));
141+
try {
142+
$this->groupMembershipMapper->insert(GroupMembership::fromParams(['groupid' => $group,'userid' => $addedUser]));
143+
} catch (Exception $e) {
144+
if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
145+
/* If reason is unique constraint something else added the membership, that’s fine */
146+
$this->logger->error(
147+
__CLASS__ . ' – group {group} membership failed to be added (user {user})',
148+
[
149+
'app' => 'user_ldap',
150+
'user' => $addedUser,
151+
'group' => $group,
152+
'exception' => $e,
153+
]
154+
);
155+
}
156+
/* We failed to insert the groupmembership so we do not want to advertise it */
157+
continue;
158+
}
125159
$userObject = $this->userManager->get($addedUser);
126160
if ($userObject instanceof IUser) {
127161
$this->dispatcher->dispatchTyped(new UserAddedEvent($groupObject, $userObject));
@@ -151,7 +185,23 @@ public function handleCreatedGroups(array $createdGroups): void {
151185
$users = $this->groupBackend->usersInGroup($createdGroup);
152186
$groupObject = $this->groupManager->get($createdGroup);
153187
foreach ($users as $user) {
154-
$this->groupMembershipMapper->insert(GroupMembership::fromParams(['groupid' => $createdGroup,'userid' => $user]));
188+
try {
189+
$this->groupMembershipMapper->insert(GroupMembership::fromParams(['groupid' => $createdGroup,'userid' => $user]));
190+
} catch (Exception $e) {
191+
if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
192+
$this->logger->error(
193+
__CLASS__ . ' – group {group} membership failed to be added (user {user})',
194+
[
195+
'app' => 'user_ldap',
196+
'user' => $user,
197+
'group' => $createdGroup,
198+
'exception' => $e,
199+
]
200+
);
201+
}
202+
/* We failed to insert the groupmembership so we do not want to advertise it */
203+
continue;
204+
}
155205
if ($groupObject instanceof IGroup) {
156206
$userObject = $this->userManager->get($user);
157207
if ($userObject instanceof IUser) {

0 commit comments

Comments
 (0)