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

[stable12] fix LDAP User deletion (cleanup), fixes #3365 #6699

Merged
merged 1 commit into from Nov 7, 2017
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 2 additions & 1 deletion apps/user_ldap/appinfo/app.php
Expand Up @@ -44,9 +44,10 @@
'name' => $l->t('LDAP user and group backend'),
];
});
$userSession = \OC::$server->getUserSession();

$userBackend = new OCA\User_LDAP\User_Proxy(
$configPrefixes, $ldapWrapper, $ocConfig, $notificationManager
$configPrefixes, $ldapWrapper, $ocConfig, $notificationManager, $userSession
);
$groupBackend = new OCA\User_LDAP\Group_Proxy($configPrefixes, $ldapWrapper);
// register user backend
Expand Down
3 changes: 2 additions & 1 deletion apps/user_ldap/appinfo/register_command.php
Expand Up @@ -36,7 +36,8 @@
$helper->getServerConfigurationPrefixes(true),
new LDAP(),
$ocConfig,
\OC::$server->getNotificationManager()
\OC::$server->getNotificationManager(),
\OC::$server->getUserSession()
);
$deletedUsersIndex = new DeletedUsersIndex(
$ocConfig, $dbConnection, $userMapping
Expand Down
8 changes: 7 additions & 1 deletion apps/user_ldap/lib/Command/Search.php
Expand Up @@ -120,7 +120,13 @@ protected function execute(InputInterface $input, OutputInterface $output) {
$limit = null;
}
} else {
$proxy = new User_Proxy($configPrefixes, $ldapWrapper, $this->ocConfig, \OC::$server->getNotificationManager());
$proxy = new User_Proxy(
$configPrefixes,
$ldapWrapper,
$this->ocConfig,
\OC::$server->getNotificationManager(),
\OC::$server->getUserSession()
);
$getMethod = 'getDisplayNames';
$printID = true;
}
Expand Down
3 changes: 2 additions & 1 deletion apps/user_ldap/lib/Helper.php
Expand Up @@ -294,9 +294,10 @@ public static function loginName2UserName($param) {
$ldapWrapper = new LDAP();
$ocConfig = \OC::$server->getConfig();
$notificationManager = \OC::$server->getNotificationManager();
$userSession = \OC::$server->getUserSession();

$userBackend = new User_Proxy(
$configPrefixes, $ldapWrapper, $ocConfig, $notificationManager
$configPrefixes, $ldapWrapper, $ocConfig, $notificationManager, $userSession
);
$uid = $userBackend->loginName2UserName($param['uid'] );
if($uid !== false) {
Expand Down
3 changes: 2 additions & 1 deletion apps/user_ldap/lib/Jobs/CleanUp.php
Expand Up @@ -99,7 +99,8 @@ public function setArguments($arguments) {
$this->ldapHelper->getServerConfigurationPrefixes(true),
new LDAP(),
$this->ocConfig,
\OC::$server->getNotificationManager()
\OC::$server->getNotificationManager(),
\OC::$server->getUserSession()
);
}

Expand Down
2 changes: 1 addition & 1 deletion apps/user_ldap/lib/Migration/UUIDFixGroup.php
Expand Up @@ -33,6 +33,6 @@ class UUIDFixGroup extends UUIDFix {
public function __construct(GroupMapping $mapper, LDAP $ldap, IConfig $config, Helper $helper) {
$this->mapper = $mapper;
$this->proxy = new User_Proxy($helper->getServerConfigurationPrefixes(true), $ldap, $config,
\OC::$server->getNotificationManager());
\OC::$server->getNotificationManager(), \OC::$server->getUserSession());
}
}
24 changes: 11 additions & 13 deletions apps/user_ldap/lib/User_LDAP.php
Expand Up @@ -42,6 +42,7 @@
use OCA\User_LDAP\User\User;
use OCP\IConfig;
use OCP\IUser;
use OCP\IUserSession;
use OCP\Notification\IManager as INotificationManager;
use OCP\Util;

Expand All @@ -59,24 +60,21 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn
* @param Access $access
* @param \OCP\IConfig $ocConfig
* @param \OCP\Notification\IManager $notificationManager
* @param IUserSession $userSession
*/
public function __construct(Access $access, IConfig $ocConfig, INotificationManager $notificationManager) {
public function __construct(Access $access, IConfig $ocConfig, INotificationManager $notificationManager, IUserSession $userSession) {
parent::__construct($access);
$this->ocConfig = $ocConfig;
$this->notificationManager = $notificationManager;
$this->registerHooks();
$this->registerHooks($userSession);
}

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

public function preDeleteUser(array $param) {
$user = $param[0];
if(!$user instanceof IUser) {
throw new \RuntimeException('IUser expected');
}
public function preDeleteUser(IUser $user) {
$this->currentUserInDeletionProcess = $user->getUID();
}

Expand Down Expand Up @@ -376,8 +374,6 @@ public function deleteUser($uid) {
\OC::$server->getLogger()->info('Cleaning up after user ' . $uid,
array('app' => 'user_ldap'));

//Get Home Directory out of user preferences so we can return it later,
//necessary for removing directories as done by OC_User.
$this->access->getUserMapper()->unmap($uid);
$this->access->userManager->invalidate($uid);
return true;
Expand Down Expand Up @@ -406,7 +402,9 @@ 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 === $user->getUID()) {
if($this->currentUserInDeletionProcess !== null
&& $this->currentUserInDeletionProcess === $user->getOCName()
) {
return $user->getHomePath();
} else {
throw new NoUserException($uid . ' is not a valid user anymore');
Expand Down
10 changes: 8 additions & 2 deletions apps/user_ldap/lib/User_Proxy.php
Expand Up @@ -31,6 +31,7 @@

use OCA\User_LDAP\User\User;
use OCP\IConfig;
use OCP\IUserSession;
use OCP\Notification\IManager as INotificationManager;

class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface, IUserLDAP {
Expand All @@ -39,14 +40,19 @@ class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface,

/**
* Constructor
*
* @param array $serverConfigPrefixes array containing the config Prefixes
* @param ILDAPWrapper $ldap
* @param IConfig $ocConfig
* @param INotificationManager $notificationManager
* @param IUserSession $userSession
*/
public function __construct(array $serverConfigPrefixes, ILDAPWrapper $ldap, IConfig $ocConfig,
INotificationManager $notificationManager) {
INotificationManager $notificationManager, IUserSession $userSession) {
parent::__construct($ldap);
foreach($serverConfigPrefixes as $configPrefix) {
$this->backends[$configPrefix] =
new User_LDAP($this->getAccess($configPrefix), $ocConfig, $notificationManager);
new User_LDAP($this->getAccess($configPrefix), $ocConfig, $notificationManager, $userSession);
if(is_null($this->refBackend)) {
$this->refBackend = &$this->backends[$configPrefix];
}
Expand Down
12 changes: 9 additions & 3 deletions apps/user_ldap/tests/Integration/AbstractIntegrationTest.php
Expand Up @@ -144,11 +144,17 @@ public function run() {
foreach($methods as $method) {
if(strpos($method, 'case') === 0) {
print("running $method " . PHP_EOL);
if(!$this->$method()) {
print(PHP_EOL . '>>> !!! Test ' . $method . ' FAILED !!! <<<' . PHP_EOL . PHP_EOL);
try {
if(!$this->$method()) {
print(PHP_EOL . '>>> !!! Test ' . $method . ' FAILED !!! <<<' . PHP_EOL . PHP_EOL);
exit(1);
}
$atLeastOneCaseRan = true;
} catch(\Exception $e) {
print(PHP_EOL . '>>> !!! Test ' . $method . ' RAISED AN EXCEPTION !!! <<<' . PHP_EOL);
print($e->getMessage() . PHP_EOL . PHP_EOL);
exit(1);
}
$atLeastOneCaseRan = true;
}
}
if($atLeastOneCaseRan) {
Expand Down
Expand Up @@ -49,7 +49,7 @@ public function init() {
$groupMapper->clear();
$this->access->setGroupMapper($groupMapper);

$userBackend = new User_LDAP($this->access, \OC::$server->getConfig(), \OC::$server->getNotificationManager());
$userBackend = new User_LDAP($this->access, \OC::$server->getConfig(), \OC::$server->getNotificationManager(), \OC::$server->getUserSession());
$userManager = \OC::$server->getUserManager();
$userManager->clearBackends();
$userManager->registerBackend($userBackend);
Expand Down
Expand Up @@ -47,7 +47,7 @@ public function init() {
$this->mapping = new UserMapping(\OC::$server->getDatabaseConnection());
$this->mapping->clear();
$this->access->setUserMapper($this->mapping);
$this->backend = new User_LDAP($this->access, \OC::$server->getConfig(), \OC::$server->getNotificationManager());
$this->backend = new User_LDAP($this->access, \OC::$server->getConfig(), \OC::$server->getNotificationManager(), \OC::$server->getUserSession());
}

/**
Expand Down
Expand Up @@ -47,7 +47,7 @@ public function init() {
require(__DIR__ . '/../setup-scripts/createExplicitUsers.php');
parent::init();

$this->backend = new User_LDAP($this->access, \OC::$server->getConfig(), \OC::$server->getNotificationManager());
$this->backend = new User_LDAP($this->access, \OC::$server->getConfig(), \OC::$server->getNotificationManager(), \OC::$server->getUserSession());
}

public function initConnection() {
Expand Down
Expand Up @@ -51,7 +51,7 @@ public function init() {
$this->mapping = new UserMapping(\OC::$server->getDatabaseConnection());
$this->mapping->clear();
$this->access->setUserMapper($this->mapping);
$this->backend = new User_LDAP($this->access, \OC::$server->getConfig(), \OC::$server->getNotificationManager());
$this->backend = new User_LDAP($this->access, \OC::$server->getConfig(), \OC::$server->getNotificationManager(), \OC::$server->getUserSession());
}

/**
Expand Down
Expand Up @@ -50,7 +50,7 @@ public function init() {
$this->mapping = new UserMapping(\OC::$server->getDatabaseConnection());
$this->mapping->clear();
$this->access->setUserMapper($this->mapping);
$userBackend = new User_LDAP($this->access, \OC::$server->getConfig(), \OC::$server->getNotificationManager());
$userBackend = new User_LDAP($this->access, \OC::$server->getConfig(), \OC::$server->getNotificationManager(), \OC::$server->getUserSession());
\OC_User::useBackend($userBackend);
}

Expand Down
Expand Up @@ -46,7 +46,7 @@ public function init() {
$this->mapping->clear();
$this->access->setUserMapper($this->mapping);

$userBackend = new User_LDAP($this->access, \OC::$server->getConfig(), \OC::$server->getNotificationManager());
$userBackend = new User_LDAP($this->access, \OC::$server->getConfig(), \OC::$server->getNotificationManager(), \OC::$server->getUserSession());
\OC_User::useBackend($userBackend);
}

Expand Down Expand Up @@ -76,16 +76,18 @@ protected function case1() {
$dn = 'uid=alice,ou=Users,' . $this->base;
$this->prepareUser($dn, $username);

$user = \OC::$server->getUserManager()->get($username);
if($user === null) {
return false;
}

$this->deleteUserFromLDAP($dn);

$job = new CleanUp();
$job->run([]);

// user instance must not be requested from global user manager, before
// it is deleted from the LDAP server. The instance will be returned
// from cache and may false-positively confirm the correctness.
$user = \OC::$server->getUserManager()->get($username);
if($user === null) {
return false;
}
$user->delete();

return null === \OC::$server->getUserManager()->get($username);
Expand Down
Expand Up @@ -43,7 +43,7 @@ public function init() {
$this->mapping = new UserMapping(\OC::$server->getDatabaseConnection());
$this->mapping->clear();
$this->access->setUserMapper($this->mapping);
$userBackend = new User_LDAP($this->access, \OC::$server->getConfig(), \OC::$server->getNotificationManager());
$userBackend = new User_LDAP($this->access, \OC::$server->getConfig(), \OC::$server->getNotificationManager(), \OC::$server->getUserSession());
\OC_User::useBackend($userBackend);
}

Expand Down