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.

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

[stable17] relax strict getHome behaviour for LDAP users in a shadow state #18884

Merged
merged 1 commit into from
Feb 28, 2020
Merged

[stable17] relax strict getHome behaviour for LDAP users in a shadow state #18884

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
35 changes: 27 additions & 8 deletions apps/user_ldap/lib/Group_LDAP.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -811,6 +811,7 @@ private function getGroupsByMember($dn, &$seen = null) {
* @param int $limit
* @param int $offset
* @return array with user ids
* @throws \Exception
*/
public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0) {
if(!$this->enabled) {
Expand Down Expand Up @@ -862,7 +863,10 @@ public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0) {
//we got uids, need to get their DNs to 'translate' them to user names
$filter = $this->access->combineFilterWithAnd(array(
str_replace('%uid', trim($member), $this->access->connection->ldapLoginFilter),
$this->access->getFilterPartForUserSearch($search)
$this->access->combineFilterWithAnd([
$this->access->getFilterPartForUserSearch($search),
$this->access->connection->ldapUserFilter
])
));
$ldap_users = $this->access->fetchListOfUsers($filter, $attrs, 1);
if(count($ldap_users) < 1) {
Expand All @@ -871,17 +875,32 @@ public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0) {
$groupUsers[] = $this->access->dn2username($ldap_users[0]['dn'][0]);
} else {
//we got DNs, check if we need to filter by search or we can give back all of them
if ($search !== '') {
if(!$this->access->readAttribute($member,
$uid = $this->access->dn2username($member);
if(!$uid) {
continue;
}

$cacheKey = 'userExistsOnLDAP' . $uid;
$userExists = $this->access->connection->getFromCache($cacheKey);
if($userExists === false) {
continue;
}
if($userExists === null || $search !== '') {
if (!$this->access->readAttribute($member,
$this->access->connection->ldapUserDisplayName,
$this->access->getFilterPartForUserSearch($search))) {
$this->access->combineFilterWithAnd([
$this->access->getFilterPartForUserSearch($search),
$this->access->connection->ldapUserFilter
])))
{
if($search === '') {
$this->access->connection->writeToCache($cacheKey, false);
}
continue;
}
$this->access->connection->writeToCache($cacheKey, true);
}
// dn2username will also check if the users belong to the allowed base
if($ocname = $this->access->dn2username($member)) {
$groupUsers[] = $ocname;
}
$groupUsers[] = $uid;
}
}

Expand Down
15 changes: 15 additions & 0 deletions apps/user_ldap/lib/User/User.php
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,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 @@ -58,9 +57,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 @@ -75,20 +71,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 @@ -314,25 +296,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 @@ -341,6 +333,7 @@ public function userExistsOnLDAP($user) {
$user->unmark();
}

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

Expand All @@ -363,15 +356,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 @@ -429,21 +417,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
View file
Open in desktop
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
Loading