Skip to content

Commit 950856d

Browse files
blizzzblizzz
authored
Merge pull request #17717 from nextcloud/fix/noid/ldap-relax-getHome
relax strict getHome behaviour for LDAP users in a shadow state
2 parents 209e7ab + 489ed87 commit 950856d

6 files changed

Lines changed: 101 additions & 160 deletions

File tree

‎apps/user_ldap/lib/Group_LDAP.php‎

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -812,6 +812,7 @@ private function getGroupsByMember($dn, &$seen = null) {
812812
* @param int $limit
813813
* @param int $offset
814814
* @return array with user ids
815+
* @throws \Exception
815816
*/
816817
public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0) {
817818
if(!$this->enabled) {
@@ -863,7 +864,10 @@ public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0) {
863864
//we got uids, need to get their DNs to 'translate' them to user names
864865
$filter = $this->access->combineFilterWithAnd(array(
865866
str_replace('%uid', trim($member), $this->access->connection->ldapLoginFilter),
866-
$this->access->getFilterPartForUserSearch($search)
867+
$this->access->combineFilterWithAnd([
868+
$this->access->getFilterPartForUserSearch($search),
869+
$this->access->connection->ldapUserFilter
870+
])
867871
));
868872
$ldap_users = $this->access->fetchListOfUsers($filter, $attrs, 1);
869873
if(count($ldap_users) < 1) {
@@ -872,17 +876,32 @@ public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0) {
872876
$groupUsers[] = $this->access->dn2username($ldap_users[0]['dn'][0]);
873877
} else {
874878
//we got DNs, check if we need to filter by search or we can give back all of them
875-
if ($search !== '') {
876-
if(!$this->access->readAttribute($member,
879+
$uid = $this->access->dn2username($member);
880+
if(!$uid) {
881+
continue;
882+
}
883+
884+
$cacheKey = 'userExistsOnLDAP' . $uid;
885+
$userExists = $this->access->connection->getFromCache($cacheKey);
886+
if($userExists === false) {
887+
continue;
888+
}
889+
if($userExists === null || $search !== '') {
890+
if (!$this->access->readAttribute($member,
877891
$this->access->connection->ldapUserDisplayName,
878-
$this->access->getFilterPartForUserSearch($search))) {
892+
$this->access->combineFilterWithAnd([
893+
$this->access->getFilterPartForUserSearch($search),
894+
$this->access->connection->ldapUserFilter
895+
])))
896+
{
897+
if($search === '') {
898+
$this->access->connection->writeToCache($cacheKey, false);
899+
}
879900
continue;
880901
}
902+
$this->access->connection->writeToCache($cacheKey, true);
881903
}
882-
// dn2username will also check if the users belong to the allowed base
883-
if($ocname = $this->access->dn2username($member)) {
884-
$groupUsers[] = $ocname;
885-
}
904+
$groupUsers[] = $uid;
886905
}
887906
}
888907

‎apps/user_ldap/lib/User/User.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,21 @@ public function update() {
175175
}
176176
}
177177

178+
/**
179+
* marks a user as deleted
180+
*
181+
* @throws \OCP\PreConditionNotMetException
182+
*/
183+
public function markUser() {
184+
$curValue = $this->config->getUserValue($this->getUsername(), 'user_ldap', 'isDeleted', '0');
185+
if($curValue === '1') {
186+
// the user is already marked, do not write to DB again
187+
return;
188+
}
189+
$this->config->setUserValue($this->getUsername(), 'user_ldap', 'isDeleted', '1');
190+
$this->config->setUserValue($this->getUsername(), 'user_ldap', 'foundDeleted', (string)time());
191+
}
192+
178193
/**
179194
* processes results from LDAP for attributes as returned by getAttributesToRead()
180195
* @param array $ldapEntry the user entry as retrieved from LDAP

‎apps/user_ldap/lib/User_LDAP.php‎

Lines changed: 16 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@
4747
use OCA\User_LDAP\User\User;
4848
use OCP\IConfig;
4949
use OCP\ILogger;
50-
use OCP\IUser;
5150
use OCP\IUserSession;
5251
use OCP\Notification\IManager as INotificationManager;
5352
use OCP\Util;
@@ -59,9 +58,6 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn
5958
/** @var INotificationManager */
6059
protected $notificationManager;
6160

62-
/** @var string */
63-
protected $currentUserInDeletionProcess;
64-
6561
/** @var UserPluginManager */
6662
protected $userPluginManager;
6763

@@ -76,20 +72,6 @@ public function __construct(Access $access, IConfig $ocConfig, INotificationMana
7672
$this->ocConfig = $ocConfig;
7773
$this->notificationManager = $notificationManager;
7874
$this->userPluginManager = $userPluginManager;
79-
$this->registerHooks($userSession);
80-
}
81-
82-
protected function registerHooks(IUserSession $userSession) {
83-
$userSession->listen('\OC\User', 'preDelete', [$this, 'preDeleteUser']);
84-
$userSession->listen('\OC\User', 'postDelete', [$this, 'postDeleteUser']);
85-
}
86-
87-
public function preDeleteUser(IUser $user) {
88-
$this->currentUserInDeletionProcess = $user->getUID();
89-
}
90-
91-
public function postDeleteUser() {
92-
$this->currentUserInDeletionProcess = null;
9375
}
9476

9577
/**
@@ -315,25 +297,35 @@ public function userExistsOnLDAP($user) {
315297
if(is_null($user)) {
316298
return false;
317299
}
300+
$uid = $user instanceof User ? $user->getUsername() : $user->getOCName();
301+
$cacheKey = 'userExistsOnLDAP' . $uid;
302+
$userExists = $this->access->connection->getFromCache($cacheKey);
303+
if(!is_null($userExists)) {
304+
return (bool)$userExists;
305+
}
318306

319307
$dn = $user->getDN();
320308
//check if user really still exists by reading its entry
321309
if(!is_array($this->access->readAttribute($dn, '', $this->access->connection->ldapUserFilter))) {
322310
try {
323311
$uuid = $this->access->getUserMapper()->getUUIDByDN($dn);
324312
if (!$uuid) {
313+
$this->access->connection->writeToCache($cacheKey, false);
325314
return false;
326315
}
327316
$newDn = $this->access->getUserDnByUuid($uuid);
328317
//check if renamed user is still valid by reapplying the ldap filter
329318
if ($newDn === $dn || !is_array($this->access->readAttribute($newDn, '', $this->access->connection->ldapUserFilter))) {
319+
$this->access->connection->writeToCache($cacheKey, false);
330320
return false;
331321
}
332322
$this->access->getUserMapper()->setDNbyUUID($newDn, $uuid);
323+
$this->access->connection->writeToCache($cacheKey, true);
333324
return true;
334325
} catch (ServerNotAvailableException $e) {
335326
throw $e;
336327
} catch (\Exception $e) {
328+
$this->access->connection->writeToCache($cacheKey, false);
337329
return false;
338330
}
339331
}
@@ -342,6 +334,7 @@ public function userExistsOnLDAP($user) {
342334
$user->unmark();
343335
}
344336

337+
$this->access->connection->writeToCache($cacheKey, true);
345338
return true;
346339
}
347340

@@ -364,15 +357,10 @@ public function userExists($uid) {
364357
$this->access->connection->ldapHost, ILogger::DEBUG);
365358
$this->access->connection->writeToCache('userExists'.$uid, false);
366359
return false;
367-
} else if($user instanceof OfflineUser) {
368-
//express check for users marked as deleted. Returning true is
369-
//necessary for cleanup
370-
return true;
371360
}
372361

373-
$result = $this->userExistsOnLDAP($user);
374-
$this->access->connection->writeToCache('userExists'.$uid, $result);
375-
return $result;
362+
$this->access->connection->writeToCache('userExists'.$uid, true);
363+
return true;
376364
}
377365

378366
/**
@@ -430,21 +418,13 @@ public function getHome($uid) {
430418

431419
// early return path if it is a deleted user
432420
$user = $this->access->userManager->get($uid);
433-
if($user instanceof OfflineUser) {
434-
if($this->currentUserInDeletionProcess !== null
435-
&& $this->currentUserInDeletionProcess === $user->getOCName()
436-
) {
437-
return $user->getHomePath();
438-
} else {
439-
throw new NoUserException($uid . ' is not a valid user anymore');
440-
}
441-
} else if ($user === null) {
421+
if($user instanceof User || $user instanceof OfflineUser) {
422+
$path = $user->getHomePath() ?: false;
423+
} else {
442424
throw new NoUserException($uid . ' is not a valid user anymore');
443425
}
444426

445-
$path = $user->getHomePath();
446427
$this->access->cacheUserHome($uid, $path);
447-
448428
return $path;
449429
}
450430

‎apps/user_ldap/lib/User_Proxy.php‎

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@
3737
use OCP\Notification\IManager as INotificationManager;
3838

3939
class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface, IUserLDAP {
40-
private $backends = array();
40+
private $backends = [];
41+
/** @var User_LDAP */
4142
private $refBackend = null;
4243

4344
/**
@@ -49,9 +50,14 @@ class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface,
4950
* @param INotificationManager $notificationManager
5051
* @param IUserSession $userSession
5152
*/
52-
public function __construct(array $serverConfigPrefixes, ILDAPWrapper $ldap, IConfig $ocConfig,
53-
INotificationManager $notificationManager, IUserSession $userSession,
54-
UserPluginManager $userPluginManager) {
53+
public function __construct(
54+
array $serverConfigPrefixes,
55+
ILDAPWrapper $ldap,
56+
IConfig $ocConfig,
57+
INotificationManager $notificationManager,
58+
IUserSession $userSession,
59+
UserPluginManager $userPluginManager
60+
) {
5561
parent::__construct($ldap);
5662
foreach($serverConfigPrefixes as $configPrefix) {
5763
$this->backends[$configPrefix] =
@@ -105,13 +111,13 @@ protected function callOnLastSeenOn($uid, $method, $parameters, $passOnWhen) {
105111
&& method_exists($this->getAccess($prefix), $method)) {
106112
$instance = $this->getAccess($prefix);
107113
}
108-
$result = call_user_func_array(array($instance, $method), $parameters);
114+
$result = call_user_func_array([$instance, $method], $parameters);
109115
if($result === $passOnWhen) {
110116
//not found here, reset cache to null if user vanished
111117
//because sometimes methods return false with a reason
112118
$userExists = call_user_func_array(
113-
array($this->backends[$prefix], 'userExists'),
114-
array($uid)
119+
[$this->backends[$prefix], 'userExistsOnLDAP'],
120+
[$uid]
115121
);
116122
if(!$userExists) {
117123
$this->writeToCache($cacheKey, null);
@@ -170,7 +176,22 @@ public function getUsers($search = '', $limit = 10, $offset = 0) {
170176
* @return boolean
171177
*/
172178
public function userExists($uid) {
173-
return $this->handleRequest($uid, 'userExists', array($uid));
179+
$existsOnLDAP = false;
180+
$existsLocally = $this->handleRequest($uid, 'userExists', array($uid));
181+
if($existsLocally) {
182+
$existsOnLDAP = $this->userExistsOnLDAP($uid);
183+
}
184+
if($existsLocally && !$existsOnLDAP) {
185+
try {
186+
$user = $this->getLDAPAccess($uid)->userManager->get($uid);
187+
if($user instanceof User) {
188+
$user->markUser();
189+
}
190+
} catch (\Exception $e) {
191+
// ignore
192+
}
193+
}
194+
return $existsLocally;
174195
}
175196

176197
/**

‎apps/user_ldap/tests/Group_LDAPTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1054,7 +1054,7 @@ public function testGroupMembers($groupDN, $expectedMembers, $groupsInfo = null)
10541054
$ldap = new GroupLDAP($access, $pluginManager);
10551055
$resultingMembers = $this->invokePrivate($ldap, '_groupMembers', [$groupDN]);
10561056

1057-
$this->assertEquals($expectedMembers, $resultingMembers, '', 0.0, 10, true);
1057+
$this->assertEqualsCanonicalizing($expectedMembers, $resultingMembers);
10581058
}
10591059

10601060
public function displayNameProvider() {

0 commit comments

Comments
 (0)