Skip to content

fix: Fix group creation and management #1200

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
come-nc merged 3 commits into from
Mar 31, 2026
Merged

fix: Fix group creation and management #1200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
28893a0
fix: Fix group creation and management
come-nc Mar 31, 2026
728d5de
fix: Cannot guess groupMemberAssocAttr from gid before creating group
come-nc Mar 31, 2026
ef584fa
chore: composer run cs:fix
come-nc Mar 31, 2026
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
4 changes: 4 additions & 0 deletions lib/LDAPConnect.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
119 changes: 58 additions & 61 deletions lib/LDAPGroupManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
}
Expand All @@ -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;
}
Expand All @@ -56,26 +46,23 @@ 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.
*/

$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];

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;
}
}
Expand All @@ -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;
}
Expand All @@ -106,37 +90,36 @@ 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]);
// no break
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;
}
Expand All @@ -146,46 +129,46 @@ 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]);
// no break
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;
}

Expand All @@ -197,12 +180,26 @@ public function isLDAPGroup($gid): bool {
}
}

private function buildNewEntry($gid): array {
return [
'objectClass' => ['groupOfNames', 'top'],
private function buildNewEntry(string $gid, string $attribute): array {
$entry = [
'objectClass' => [],
'cn' => $gid,
'member' => ['']
];
switch ($attribute) {
case 'memberuid':
case 'gidnumber':
$entry['objectClass'][] = 'posixGroup';
break;
default:
$this->logger->notice('Unexpected attribute {attribute} as group member association.', ['attribute' => $attribute]);
// no break
case 'uniquemember':
case 'member':
$entry['objectClass'][] = 'groupOfNames';
$entry[$attribute] = [''];
break;
}
return $entry;
}

public function makeLdapBackendFirst(): void {
Expand Down
Loading