Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
decouple userExists from userExistsOnLDAP check
allows to mark users as offline right away, avoids a gap of being not a
user and causing weird side effects

Signed-off-by: Arthur Schiwon <[email protected]>
  • Loading branch information
blizzz authored and ChristophWurst committed Jan 8, 2020
commit 5cae135b94d495449948978d5e2207335843697b
15 changes: 15 additions & 0 deletions apps/user_ldap/lib/User/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,21 @@ public function update() {
}
}

/**
* marks a user as deleted
*
* @throws \OCP\PreConditionNotMetException
*/
public function markUser() {
$curValue = $this->config->getUserValue($this->getUsername(), 'user_ldap', 'isDeleted', '0');
if($curValue === '1') {
// the user is already marked, do not write to DB again
return;
}
$this->config->setUserValue($this->getUsername(), 'user_ldap', 'isDeleted', '1');
$this->config->setUserValue($this->getUsername(), 'user_ldap', 'foundDeleted', (string)time());
}

/**
* processes results from LDAP for attributes as returned by getAttributesToRead()
* @param array $ldapEntry the user entry as retrieved from LDAP
Expand Down
20 changes: 13 additions & 7 deletions apps/user_ldap/lib/User_LDAP.php
Original file line number Diff line number Diff line change
Expand Up @@ -297,25 +297,35 @@ public function userExistsOnLDAP($user) {
if(is_null($user)) {
return false;
}
$uid = $user instanceof User ? $user->getUsername() : $user->getOCName();
$cacheKey = 'userExistsOnLDAP' . $uid;
$userExists = $this->access->connection->getFromCache($cacheKey);
if(!is_null($userExists)) {
return (bool)$userExists;
}

$dn = $user->getDN();
//check if user really still exists by reading its entry
if(!is_array($this->access->readAttribute($dn, '', $this->access->connection->ldapUserFilter))) {
try {
$uuid = $this->access->getUserMapper()->getUUIDByDN($dn);
if (!$uuid) {
$this->access->connection->writeToCache($cacheKey, false);
return false;
}
$newDn = $this->access->getUserDnByUuid($uuid);
//check if renamed user is still valid by reapplying the ldap filter
if ($newDn === $dn || !is_array($this->access->readAttribute($newDn, '', $this->access->connection->ldapUserFilter))) {
$this->access->connection->writeToCache($cacheKey, false);
return false;
}
$this->access->getUserMapper()->setDNbyUUID($newDn, $uuid);
$this->access->connection->writeToCache($cacheKey, true);
return true;
} catch (ServerNotAvailableException $e) {
throw $e;
} catch (\Exception $e) {
$this->access->connection->writeToCache($cacheKey, false);
return false;
}
}
Expand All @@ -324,6 +334,7 @@ public function userExistsOnLDAP($user) {
$user->unmark();
}

$this->access->connection->writeToCache($cacheKey, true);
return true;
}

Expand All @@ -346,15 +357,10 @@ public function userExists($uid) {
$this->access->connection->ldapHost, ILogger::DEBUG);
$this->access->connection->writeToCache('userExists'.$uid, false);
return false;
} else if($user instanceof OfflineUser) {
//express check for users marked as deleted. Returning true is
//necessary for cleanup
return true;
}

$result = $this->userExistsOnLDAP($user);
$this->access->connection->writeToCache('userExists'.$uid, $result);
return $result;
$this->access->connection->writeToCache('userExists'.$uid, true);
return true;
}

/**
Expand Down
37 changes: 29 additions & 8 deletions apps/user_ldap/lib/User_Proxy.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@
use OCP\Notification\IManager as INotificationManager;

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

/**
Expand All @@ -49,9 +50,14 @@ class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface,
* @param INotificationManager $notificationManager
* @param IUserSession $userSession
*/
public function __construct(array $serverConfigPrefixes, ILDAPWrapper $ldap, IConfig $ocConfig,
INotificationManager $notificationManager, IUserSession $userSession,
UserPluginManager $userPluginManager) {
public function __construct(
array $serverConfigPrefixes,
ILDAPWrapper $ldap,
IConfig $ocConfig,
INotificationManager $notificationManager,
IUserSession $userSession,
UserPluginManager $userPluginManager
) {
parent::__construct($ldap);
foreach($serverConfigPrefixes as $configPrefix) {
$this->backends[$configPrefix] =
Expand Down Expand Up @@ -105,13 +111,13 @@ protected function callOnLastSeenOn($uid, $method, $parameters, $passOnWhen) {
&& method_exists($this->getAccess($prefix), $method)) {
$instance = $this->getAccess($prefix);
}
$result = call_user_func_array(array($instance, $method), $parameters);
$result = call_user_func_array([$instance, $method], $parameters);
if($result === $passOnWhen) {
//not found here, reset cache to null if user vanished
//because sometimes methods return false with a reason
$userExists = call_user_func_array(
array($this->backends[$prefix], 'userExists'),
array($uid)
[$this->backends[$prefix], 'userExistsOnLDAP'],
[$uid]
);
if(!$userExists) {
$this->writeToCache($cacheKey, null);
Expand Down Expand Up @@ -170,7 +176,22 @@ public function getUsers($search = '', $limit = 10, $offset = 0) {
* @return boolean
*/
public function userExists($uid) {
return $this->handleRequest($uid, 'userExists', array($uid));
$existsOnLDAP = false;
$existsLocally = $this->handleRequest($uid, 'userExists', array($uid));
if($existsLocally) {
$existsOnLDAP = $this->userExistsOnLDAP($uid);
}
if($existsLocally && !$existsOnLDAP) {
try {
$user = $this->getLDAPAccess($uid)->userManager->get($uid);
if($user instanceof User) {
$user->markUser();
}
} catch (\Exception $e) {
// ignore
}
}
return $existsLocally;
}

/**
Expand Down