Skip to content

Commit b969380

Browse files
come-nccome-nc
authored
Merge pull request #1200 from nextcloud/fix/fix-group-management
fix: Fix group creation and management
2 parents cb34d6f + ef584fa commit b969380

2 files changed

Lines changed: 62 additions & 61 deletions

File tree

‎lib/LDAPConnect.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,10 @@ public function getDisplayNameAttribute(): string {
122122
return $this->ldapConfig->ldapUserDisplayName;
123123
}
124124

125+
public function getGroupMemberAssocAttribute(): string {
126+
return strtolower($this->ldapConfig->ldapGroupMemberAssocAttr);
127+
}
128+
125129
public function groupsEnabled(): bool {
126130
$filter = trim((string)$this->ldapConfig->ldapGroupFilter);
127131
$gAssoc = trim((string)$this->ldapConfig->ldapGroupMemberAssocAttr);

‎lib/LDAPGroupManager.php‎

Lines changed: 58 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
namespace OCA\LdapWriteSupport;
1010

1111
use Exception;
12-
use OCA\LdapWriteSupport\AppInfo\Application;
1312
use OCA\User_LDAP\Group_Proxy;
1413
use OCA\User_LDAP\ILDAPGroupPlugin;
1514
use OCP\GroupInterface;
@@ -18,21 +17,12 @@
1817
use Psr\Log\LoggerInterface;
1918

2019
class LDAPGroupManager implements ILDAPGroupPlugin {
21-
/** @var ILDAPProvider */
22-
private $ldapProvider;
23-
24-
/** @var IGroupManager */
25-
private $groupManager;
26-
2720
public function __construct(
28-
IGroupManager $groupManager,
21+
private IGroupManager $groupManager,
2922
private LDAPConnect $ldapConnect,
3023
private LoggerInterface $logger,
31-
ILDAPProvider $LDAPProvider,
24+
private ILDAPProvider $ldapProvider,
3225
) {
33-
$this->groupManager = $groupManager;
34-
$this->ldapProvider = $LDAPProvider;
35-
3626
if ($this->ldapConnect->groupsEnabled()) {
3727
$this->makeLdapBackendFirst();
3828
}
@@ -44,7 +34,7 @@ public function __construct(
4434
*
4535
* @return int bitwise-or'ed actions
4636
*/
47-
public function respondToActions() {
37+
public function respondToActions(): int {
4838
if (!$this->ldapConnect->groupsEnabled()) {
4939
return 0;
5040
}
@@ -56,26 +46,23 @@ public function respondToActions() {
5646

5747
/**
5848
* @param string $gid
59-
* @return string|null
6049
*/
61-
public function createGroup($gid) {
50+
public function createGroup($gid): ?string {
6251
/**
6352
* FIXME could not create group using LDAPProvider, because its methods rely
6453
* on passing an already inserted [ug]id, which we do not have at this point.
6554
*/
6655

67-
$newGroupEntry = $this->buildNewEntry($gid);
56+
$newGroupEntry = $this->buildNewEntry($gid, $this->ldapConnect->getGroupMemberAssocAttribute());
6857
$connection = $this->ldapConnect->getLDAPConnection();
6958
$newGroupDN = "cn=$gid," . $this->ldapConnect->getLDAPBaseGroups()[0];
7059
$newGroupDN = $this->ldapProvider->sanitizeDN([$newGroupDN])[0];
7160

7261
if ($connection && ($ret = ldap_add($connection, $newGroupDN, $newGroupEntry))) {
73-
$message = "Create LDAP group '$gid' ($newGroupDN)";
74-
$this->logger->notice($message, ['app' => Application::APP_ID]);
62+
$this->logger->notice("Create LDAP group '$gid' ($newGroupDN)");
7563
return $newGroupDN;
7664
} else {
77-
$message = "Unable to create LDAP group '$gid' ($newGroupDN)";
78-
$this->logger->error($message, ['app' => Application::APP_ID]);
65+
$this->logger->error("Unable to create LDAP group '$gid' ($newGroupDN)");
7966
return null;
8067
}
8168
}
@@ -84,19 +71,16 @@ public function createGroup($gid) {
8471
* delete a group
8572
*
8673
* @param string $gid gid of the group to delete
87-
* @return bool
8874
* @throws Exception
8975
*/
90-
public function deleteGroup($gid) {
76+
public function deleteGroup($gid): bool {
9177
$connection = $this->ldapProvider->getGroupLDAPConnection($gid);
9278
$groupDN = $this->ldapProvider->getGroupDN($gid);
9379

9480
if (!$ret = ldap_delete($connection, $groupDN)) {
95-
$message = 'Unable to delete LDAP Group: ' . $gid;
96-
$this->logger->error($message, ['app' => Application::APP_ID]);
81+
$this->logger->error('Unable to delete LDAP Group: ' . $gid);
9782
} else {
98-
$message = 'Delete LDAP Group: ' . $gid;
99-
$this->logger->notice($message, ['app' => Application::APP_ID]);
83+
$this->logger->notice('Delete LDAP Group: ' . $gid);
10084
}
10185
return $ret;
10286
}
@@ -106,37 +90,36 @@ public function deleteGroup($gid) {
10690
*
10791
* @param string $uid Name of the user to add to group
10892
* @param string $gid Name of the group in which add the user
109-
* @return bool
11093
*
11194
* Adds a LDAP user to a LDAP group.
11295
* @throws Exception
11396
*/
114-
public function addToGroup($uid, $gid) {
97+
public function addToGroup($uid, $gid): bool {
11598
$connection = $this->ldapProvider->getGroupLDAPConnection($gid);
11699
$groupDN = $this->ldapProvider->getGroupDN($gid);
117100

118101
$entry = [];
119-
switch ($this->ldapProvider->getLDAPGroupMemberAssoc($gid)) {
120-
case 'memberUid':
121-
$entry['memberuid'] = $uid;
102+
$attribute = strtolower($this->ldapProvider->getLDAPGroupMemberAssoc($gid));
103+
switch ($attribute) {
104+
case 'memberuid':
105+
$entry[$attribute] = $uid;
122106
break;
123-
case 'uniqueMember':
124-
$entry['uniquemember'] = $this->ldapProvider->getUserDN($uid);
107+
case 'gidnumber':
108+
throw new Exception('Cannot add to group when gidNumber is used as relation');
125109
break;
110+
default:
111+
$this->logger->notice('Unexpected attribute {attribute} as group member association.', ['attribute' => $attribute]);
112+
// no break
113+
case 'uniquemember':
126114
case 'member':
127-
$entry['member'] = $this->ldapProvider->getUserDN($uid);
128-
break;
129-
case 'gidNumber':
130-
throw new Exception('Cannot add to group when gidNumber is used as relation');
115+
$entry[$attribute] = $this->ldapProvider->getUserDN($uid);
131116
break;
132117
}
133118

134119
if (!$ret = ldap_mod_add($connection, $groupDN, $entry)) {
135-
$message = 'Unable to add user ' . $uid . ' to group ' . $gid;
136-
$this->logger->error($message, ['app' => Application::APP_ID]);
120+
$this->logger->error('Unable to add user ' . $uid . ' to group ' . $gid);
137121
} else {
138-
$message = 'Add user: ' . $uid . ' to group: ' . $gid;
139-
$this->logger->notice($message, ['app' => Application::APP_ID]);
122+
$this->logger->notice('Add user: ' . $uid . ' to group: ' . $gid);
140123
}
141124
return $ret;
142125
}
@@ -146,46 +129,46 @@ public function addToGroup($uid, $gid) {
146129
*
147130
* @param string $uid Name of the user to remove from group
148131
* @param string $gid Name of the group from which remove the user
149-
* @return bool
150132
*
151133
* removes the user from a group.
152134
* @throws Exception
153135
*/
154-
public function removeFromGroup($uid, $gid) {
136+
public function removeFromGroup($uid, $gid): bool {
155137
$connection = $this->ldapProvider->getGroupLDAPConnection($gid);
156138
$groupDN = $this->ldapProvider->getGroupDN($gid);
157139

158140
$entry = [];
159-
switch ($this->ldapProvider->getLDAPGroupMemberAssoc($gid)) {
160-
case 'memberUid':
161-
$entry['memberuid'] = $uid;
141+
$attribute = strtolower($this->ldapProvider->getLDAPGroupMemberAssoc($gid));
142+
switch ($attribute) {
143+
case 'memberuid':
144+
$entry[$attribute] = $uid;
162145
break;
163-
case 'uniqueMember':
164-
$entry['uniquemember'] = $this->ldapProvider->getUserDN($uid);
146+
case 'gidnumber':
147+
throw new Exception('Cannot remove from group when gidNumber is used as relation');
165148
break;
149+
default:
150+
$this->logger->notice('Unexpected attribute {attribute} as group member association.', ['attribute' => $attribute]);
151+
// no break
152+
case 'uniquemember':
166153
case 'member':
167-
$entry['member'] = $this->ldapProvider->getUserDN($uid);
154+
$entry[$attribute] = $this->ldapProvider->getUserDN($uid);
168155
break;
169-
case 'gidNumber':
170-
throw new Exception('Cannot remove from group when gidNumber is used as relation');
171156
}
172157

173158
if (!$ret = ldap_mod_del($connection, $groupDN, $entry)) {
174-
$message = 'Unable to remove user: ' . $uid . ' from group: ' . $gid;
175-
$this->logger->error($message, ['app' => Application::APP_ID]);
159+
$this->logger->error('Unable to remove user: ' . $uid . ' from group: ' . $gid);
176160
} else {
177-
$message = 'Remove user: ' . $uid . ' from group: ' . $gid;
178-
$this->logger->notice($message, ['app' => Application::APP_ID]);
161+
$this->logger->notice('Remove user: ' . $uid . ' from group: ' . $gid);
179162
}
180163
return $ret;
181164
}
182165

183166

184-
public function countUsersInGroup($gid, $search = '') {
167+
public function countUsersInGroup($gid, $search = ''): bool {
185168
return false;
186169
}
187170

188-
public function getGroupDetails($gid) {
171+
public function getGroupDetails($gid): bool {
189172
return false;
190173
}
191174

@@ -197,12 +180,26 @@ public function isLDAPGroup($gid): bool {
197180
}
198181
}
199182

200-
private function buildNewEntry($gid): array {
201-
return [
202-
'objectClass' => ['groupOfNames', 'top'],
183+
private function buildNewEntry(string $gid, string $attribute): array {
184+
$entry = [
185+
'objectClass' => [],
203186
'cn' => $gid,
204-
'member' => ['']
205187
];
188+
switch ($attribute) {
189+
case 'memberuid':
190+
case 'gidnumber':
191+
$entry['objectClass'][] = 'posixGroup';
192+
break;
193+
default:
194+
$this->logger->notice('Unexpected attribute {attribute} as group member association.', ['attribute' => $attribute]);
195+
// no break
196+
case 'uniquemember':
197+
case 'member':
198+
$entry['objectClass'][] = 'groupOfNames';
199+
$entry[$attribute] = [''];
200+
break;
201+
}
202+
return $entry;
206203
}
207204

208205
public function makeLdapBackendFirst(): void {

0 commit comments

Comments
 (0)