Skip to content
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.

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

relax strict getHome behaviour for LDAP users in a shadow state #17717

Merged
merged 5 commits into from
Jan 14, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
52 changes: 16 additions & 36 deletions apps/user_ldap/lib/User_LDAP.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
use OCA\User_LDAP\User\User;
use OCP\IConfig;
use OCP\ILogger;
use OCP\IUser;
use OCP\IUserSession;
use OCP\Notification\IManager as INotificationManager;
use OCP\Util;
Expand All @@ -59,9 +58,6 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn
/** @var INotificationManager */
protected $notificationManager;

/** @var string */
protected $currentUserInDeletionProcess;

/** @var UserPluginManager */
protected $userPluginManager;

Expand All @@ -76,20 +72,6 @@ public function __construct(Access $access, IConfig $ocConfig, INotificationMana
$this->ocConfig = $ocConfig;
$this->notificationManager = $notificationManager;
$this->userPluginManager = $userPluginManager;
$this->registerHooks($userSession);
}

protected function registerHooks(IUserSession $userSession) {
$userSession->listen('\OC\User', 'preDelete', [$this, 'preDeleteUser']);
$userSession->listen('\OC\User', 'postDelete', [$this, 'postDeleteUser']);
}

public function preDeleteUser(IUser $user) {
$this->currentUserInDeletionProcess = $user->getUID();
}

public function postDeleteUser() {
$this->currentUserInDeletionProcess = null;
}

/**
Expand Down Expand Up @@ -315,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 @@ -342,6 +334,7 @@ public function userExistsOnLDAP($user) {
$user->unmark();
}

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

Expand All @@ -364,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 Expand Up @@ -430,21 +418,13 @@ public function getHome($uid) {

// early return path if it is a deleted user
$user = $this->access->userManager->get($uid);
if($user instanceof OfflineUser) {
if($this->currentUserInDeletionProcess !== null
&& $this->currentUserInDeletionProcess === $user->getOCName()
) {
return $user->getHomePath();
} else {
throw new NoUserException($uid . ' is not a valid user anymore');
}
} else if ($user === null) {
if($user instanceof User || $user instanceof OfflineUser) {
$path = $user->getHomePath() ?: false;
} else {
throw new NoUserException($uid . ' is not a valid user anymore');
}

$path = $user->getHomePath();
$this->access->cacheUserHome($uid, $path);

return $path;
}

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
2 changes: 1 addition & 1 deletion apps/user_ldap/tests/Group_LDAPTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1054,7 +1054,7 @@ public function testGroupMembers($groupDN, $expectedMembers, $groupsInfo = null)
$ldap = new GroupLDAP($access, $pluginManager);
$resultingMembers = $this->invokePrivate($ldap, '_groupMembers', [$groupDN]);

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

public function displayNameProvider() {
Expand Down