From e563c828bed2034102c51c5f87f5a079c6d63cf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 31 Mar 2026 14:59:25 +0200 Subject: [PATCH 1/3] fix: Fix group creation and management MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/LDAPGroupManager.php | 115 +++++++++++++++++++-------------------- 1 file changed, 55 insertions(+), 60 deletions(-) diff --git a/lib/LDAPGroupManager.php b/lib/LDAPGroupManager.php index 5b4bab7f..3dfafd25 100644 --- a/lib/LDAPGroupManager.php +++ b/lib/LDAPGroupManager.php @@ -9,7 +9,6 @@ namespace OCA\LdapWriteSupport; use Exception; -use OCA\LdapWriteSupport\AppInfo\Application; use OCA\User_LDAP\Group_Proxy; use OCA\User_LDAP\ILDAPGroupPlugin; use OCP\GroupInterface; @@ -18,21 +17,12 @@ use Psr\Log\LoggerInterface; class LDAPGroupManager implements ILDAPGroupPlugin { - /** @var ILDAPProvider */ - private $ldapProvider; - - /** @var IGroupManager */ - private $groupManager; - public function __construct( - IGroupManager $groupManager, + private IGroupManager $groupManager, private LDAPConnect $ldapConnect, private LoggerInterface $logger, - ILDAPProvider $LDAPProvider, + private ILDAPProvider $ldapProvider, ) { - $this->groupManager = $groupManager; - $this->ldapProvider = $LDAPProvider; - if ($this->ldapConnect->groupsEnabled()) { $this->makeLdapBackendFirst(); } @@ -44,7 +34,7 @@ public function __construct( * * @return int bitwise-or'ed actions */ - public function respondToActions() { + public function respondToActions(): int { if (!$this->ldapConnect->groupsEnabled()) { return 0; } @@ -56,9 +46,8 @@ public function respondToActions() { /** * @param string $gid - * @return string|null */ - public function createGroup($gid) { + public function createGroup($gid): ?string { /** * FIXME could not create group using LDAPProvider, because its methods rely * on passing an already inserted [ug]id, which we do not have at this point. @@ -70,12 +59,10 @@ public function createGroup($gid) { $newGroupDN = $this->ldapProvider->sanitizeDN([$newGroupDN])[0]; if ($connection && ($ret = ldap_add($connection, $newGroupDN, $newGroupEntry))) { - $message = "Create LDAP group '$gid' ($newGroupDN)"; - $this->logger->notice($message, ['app' => Application::APP_ID]); + $this->logger->notice("Create LDAP group '$gid' ($newGroupDN)"); return $newGroupDN; } else { - $message = "Unable to create LDAP group '$gid' ($newGroupDN)"; - $this->logger->error($message, ['app' => Application::APP_ID]); + $this->logger->error("Unable to create LDAP group '$gid' ($newGroupDN)"); return null; } } @@ -84,19 +71,16 @@ public function createGroup($gid) { * delete a group * * @param string $gid gid of the group to delete - * @return bool * @throws Exception */ - public function deleteGroup($gid) { + public function deleteGroup($gid): bool { $connection = $this->ldapProvider->getGroupLDAPConnection($gid); $groupDN = $this->ldapProvider->getGroupDN($gid); if (!$ret = ldap_delete($connection, $groupDN)) { - $message = 'Unable to delete LDAP Group: ' . $gid; - $this->logger->error($message, ['app' => Application::APP_ID]); + $this->logger->error('Unable to delete LDAP Group: ' . $gid); } else { - $message = 'Delete LDAP Group: ' . $gid; - $this->logger->notice($message, ['app' => Application::APP_ID]); + $this->logger->notice('Delete LDAP Group: ' . $gid); } return $ret; } @@ -106,37 +90,35 @@ public function deleteGroup($gid) { * * @param string $uid Name of the user to add to group * @param string $gid Name of the group in which add the user - * @return bool * * Adds a LDAP user to a LDAP group. * @throws Exception */ - public function addToGroup($uid, $gid) { + public function addToGroup($uid, $gid): bool { $connection = $this->ldapProvider->getGroupLDAPConnection($gid); $groupDN = $this->ldapProvider->getGroupDN($gid); $entry = []; - switch ($this->ldapProvider->getLDAPGroupMemberAssoc($gid)) { - case 'memberUid': - $entry['memberuid'] = $uid; + $attribute = strtolower($this->ldapProvider->getLDAPGroupMemberAssoc($gid)); + switch ($attribute) { + case 'memberuid': + $entry[$attribute] = $uid; break; - case 'uniqueMember': - $entry['uniquemember'] = $this->ldapProvider->getUserDN($uid); + case 'gidnumber': + throw new Exception('Cannot add to group when gidNumber is used as relation'); break; + default: + $this->logger->notice('Unexpected attribute {attribute} as group member association.', ['attribute' => $attribute]); + case 'uniquemember': case 'member': - $entry['member'] = $this->ldapProvider->getUserDN($uid); - break; - case 'gidNumber': - throw new Exception('Cannot add to group when gidNumber is used as relation'); + $entry[$attribute] = $this->ldapProvider->getUserDN($uid); break; } if (!$ret = ldap_mod_add($connection, $groupDN, $entry)) { - $message = 'Unable to add user ' . $uid . ' to group ' . $gid; - $this->logger->error($message, ['app' => Application::APP_ID]); + $this->logger->error('Unable to add user ' . $uid . ' to group ' . $gid); } else { - $message = 'Add user: ' . $uid . ' to group: ' . $gid; - $this->logger->notice($message, ['app' => Application::APP_ID]); + $this->logger->notice('Add user: ' . $uid . ' to group: ' . $gid); } return $ret; } @@ -146,46 +128,45 @@ public function addToGroup($uid, $gid) { * * @param string $uid Name of the user to remove from group * @param string $gid Name of the group from which remove the user - * @return bool * * removes the user from a group. * @throws Exception */ - public function removeFromGroup($uid, $gid) { + public function removeFromGroup($uid, $gid): bool { $connection = $this->ldapProvider->getGroupLDAPConnection($gid); $groupDN = $this->ldapProvider->getGroupDN($gid); $entry = []; - switch ($this->ldapProvider->getLDAPGroupMemberAssoc($gid)) { - case 'memberUid': - $entry['memberuid'] = $uid; + $attribute = strtolower($this->ldapProvider->getLDAPGroupMemberAssoc($gid)); + switch ($attribute) { + case 'memberuid': + $entry[$attribute] = $uid; break; - case 'uniqueMember': - $entry['uniquemember'] = $this->ldapProvider->getUserDN($uid); + case 'gidnumber': + throw new Exception('Cannot remove from group when gidNumber is used as relation'); break; + default: + $this->logger->notice('Unexpected attribute {attribute} as group member association.', ['attribute' => $attribute]); + case 'uniquemember': case 'member': - $entry['member'] = $this->ldapProvider->getUserDN($uid); + $entry[$attribute] = $this->ldapProvider->getUserDN($uid); break; - case 'gidNumber': - throw new Exception('Cannot remove from group when gidNumber is used as relation'); } if (!$ret = ldap_mod_del($connection, $groupDN, $entry)) { - $message = 'Unable to remove user: ' . $uid . ' from group: ' . $gid; - $this->logger->error($message, ['app' => Application::APP_ID]); + $this->logger->error('Unable to remove user: ' . $uid . ' from group: ' . $gid); } else { - $message = 'Remove user: ' . $uid . ' from group: ' . $gid; - $this->logger->notice($message, ['app' => Application::APP_ID]); + $this->logger->notice('Remove user: ' . $uid . ' from group: ' . $gid); } return $ret; } - public function countUsersInGroup($gid, $search = '') { + public function countUsersInGroup($gid, $search = ''): bool { return false; } - public function getGroupDetails($gid) { + public function getGroupDetails($gid): bool { return false; } @@ -197,12 +178,26 @@ public function isLDAPGroup($gid): bool { } } - private function buildNewEntry($gid): array { - return [ - 'objectClass' => ['groupOfNames', 'top'], + private function buildNewEntry(string $gid): array { + $entry = [ + 'objectClass' => [], 'cn' => $gid, - 'member' => [''] ]; + $attribute = strtolower($this->ldapProvider->getLDAPGroupMemberAssoc($gid)); + switch ($attribute) { + case 'memberuid': + case 'gidnumber': + $entry['objectClass'][] = 'posixGroup'; + break; + default: + $this->logger->notice('Unexpected attribute {attribute} as group member association.', ['attribute' => $attribute]); + case 'uniquemember': + case 'member': + $entry['objectClass'][] = 'groupOfNames'; + $entry[$attribute] = ['']; + break; + } + return $entry; } public function makeLdapBackendFirst(): void { From 08e77dd4aa9d8aeb6bb214928485beb0657cc5ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 31 Mar 2026 15:29:47 +0200 Subject: [PATCH 2/3] fix: Cannot guess groupMemberAssocAttr from gid before creating group MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/LDAPConnect.php | 4 ++++ lib/LDAPGroupManager.php | 5 ++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/LDAPConnect.php b/lib/LDAPConnect.php index 31b89b2f..3da3cebf 100644 --- a/lib/LDAPConnect.php +++ b/lib/LDAPConnect.php @@ -122,6 +122,10 @@ public function getDisplayNameAttribute(): string { return $this->ldapConfig->ldapUserDisplayName; } + public function getGroupMemberAssocAttribute(): string { + return strtolower($this->ldapConfig->ldapGroupMemberAssocAttr); + } + public function groupsEnabled(): bool { $filter = trim((string)$this->ldapConfig->ldapGroupFilter); $gAssoc = trim((string)$this->ldapConfig->ldapGroupMemberAssocAttr); diff --git a/lib/LDAPGroupManager.php b/lib/LDAPGroupManager.php index 3dfafd25..3b1a03d4 100644 --- a/lib/LDAPGroupManager.php +++ b/lib/LDAPGroupManager.php @@ -53,7 +53,7 @@ public function createGroup($gid): ?string { * on passing an already inserted [ug]id, which we do not have at this point. */ - $newGroupEntry = $this->buildNewEntry($gid); + $newGroupEntry = $this->buildNewEntry($gid, $this->ldapConnect->getGroupMemberAssocAttribute()); $connection = $this->ldapConnect->getLDAPConnection(); $newGroupDN = "cn=$gid," . $this->ldapConnect->getLDAPBaseGroups()[0]; $newGroupDN = $this->ldapProvider->sanitizeDN([$newGroupDN])[0]; @@ -178,12 +178,11 @@ public function isLDAPGroup($gid): bool { } } - private function buildNewEntry(string $gid): array { + private function buildNewEntry(string $gid, string $attribute): array { $entry = [ 'objectClass' => [], 'cn' => $gid, ]; - $attribute = strtolower($this->ldapProvider->getLDAPGroupMemberAssoc($gid)); switch ($attribute) { case 'memberuid': case 'gidnumber': From 371b880e8a08b442273fdcf394d0e02f22ba7a98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 31 Mar 2026 15:38:41 +0200 Subject: [PATCH 3/3] chore: composer run cs:fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/LDAPGroupManager.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/LDAPGroupManager.php b/lib/LDAPGroupManager.php index 3b1a03d4..02af2e9c 100644 --- a/lib/LDAPGroupManager.php +++ b/lib/LDAPGroupManager.php @@ -109,6 +109,7 @@ public function addToGroup($uid, $gid): bool { break; default: $this->logger->notice('Unexpected attribute {attribute} as group member association.', ['attribute' => $attribute]); + // no break case 'uniquemember': case 'member': $entry[$attribute] = $this->ldapProvider->getUserDN($uid); @@ -147,6 +148,7 @@ public function removeFromGroup($uid, $gid): bool { break; default: $this->logger->notice('Unexpected attribute {attribute} as group member association.', ['attribute' => $attribute]); + // no break case 'uniquemember': case 'member': $entry[$attribute] = $this->ldapProvider->getUserDN($uid); @@ -190,6 +192,7 @@ private function buildNewEntry(string $gid, string $attribute): array { break; default: $this->logger->notice('Unexpected attribute {attribute} as group member association.', ['attribute' => $attribute]); + // no break case 'uniquemember': case 'member': $entry['objectClass'][] = 'groupOfNames';