Skip to content

Commit d6d98d4

Browse files
blizzzblizzz
committed
fix LDAP User deletion (cleanup)
discovered a bug in the integration test which lead to following a different code path and giving a false-positive success feedback. Also listens now to the evendispatcher instead of old hook system Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
1 parent 113fd47 commit d6d98d4

17 files changed

Lines changed: 115 additions & 81 deletions

‎apps/user_ldap/appinfo/app.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,10 @@
4444
'name' => $l->t('LDAP user and group backend'),
4545
];
4646
});
47+
$userSession = \OC::$server->getUserSession();
4748

4849
$userBackend = new OCA\User_LDAP\User_Proxy(
49-
$configPrefixes, $ldapWrapper, $ocConfig, $notificationManager
50+
$configPrefixes, $ldapWrapper, $ocConfig, $notificationManager, $userSession
5051
);
5152
$groupBackend = new OCA\User_LDAP\Group_Proxy($configPrefixes, $ldapWrapper);
5253
// register user backend

‎apps/user_ldap/appinfo/register_command.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@
3636
$helper->getServerConfigurationPrefixes(true),
3737
new LDAP(),
3838
$ocConfig,
39-
\OC::$server->getNotificationManager()
39+
\OC::$server->getNotificationManager(),
40+
\OC::$server->getUserSession()
4041
);
4142
$deletedUsersIndex = new DeletedUsersIndex(
4243
$ocConfig, $dbConnection, $userMapping

‎apps/user_ldap/lib/Command/Search.php‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,13 @@ protected function execute(InputInterface $input, OutputInterface $output) {
120120
$limit = null;
121121
}
122122
} else {
123-
$proxy = new User_Proxy($configPrefixes, $ldapWrapper, $this->ocConfig, \OC::$server->getNotificationManager());
123+
$proxy = new User_Proxy(
124+
$configPrefixes,
125+
$ldapWrapper,
126+
$this->ocConfig,
127+
\OC::$server->getNotificationManager(),
128+
\OC::$server->getUserSession()
129+
);
124130
$getMethod = 'getDisplayNames';
125131
$printID = true;
126132
}

‎apps/user_ldap/lib/Helper.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -294,9 +294,10 @@ public static function loginName2UserName($param) {
294294
$ldapWrapper = new LDAP();
295295
$ocConfig = \OC::$server->getConfig();
296296
$notificationManager = \OC::$server->getNotificationManager();
297+
$userSession = \OC::$server->getUserSession();
297298

298299
$userBackend = new User_Proxy(
299-
$configPrefixes, $ldapWrapper, $ocConfig, $notificationManager
300+
$configPrefixes, $ldapWrapper, $ocConfig, $notificationManager, $userSession
300301
);
301302
$uid = $userBackend->loginName2UserName($param['uid'] );
302303
if($uid !== false) {

‎apps/user_ldap/lib/Jobs/CleanUp.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,8 @@ public function setArguments($arguments) {
9999
$this->ldapHelper->getServerConfigurationPrefixes(true),
100100
new LDAP(),
101101
$this->ocConfig,
102-
\OC::$server->getNotificationManager()
102+
\OC::$server->getNotificationManager(),
103+
\OC::$server->getUserSession()
103104
);
104105
}
105106

‎apps/user_ldap/lib/Migration/UUIDFixGroup.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,6 @@ class UUIDFixGroup extends UUIDFix {
3333
public function __construct(GroupMapping $mapper, LDAP $ldap, IConfig $config, Helper $helper) {
3434
$this->mapper = $mapper;
3535
$this->proxy = new User_Proxy($helper->getServerConfigurationPrefixes(true), $ldap, $config,
36-
\OC::$server->getNotificationManager());
36+
\OC::$server->getNotificationManager(), \OC::$server->getUserSession());
3737
}
3838
}

‎apps/user_ldap/lib/User_LDAP.php‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
use OCA\User_LDAP\User\User;
4343
use OCP\IConfig;
4444
use OCP\IUser;
45+
use OCP\IUserSession;
4546
use OCP\Notification\IManager as INotificationManager;
4647
use OCP\Util;
4748

@@ -59,24 +60,21 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn
5960
* @param Access $access
6061
* @param \OCP\IConfig $ocConfig
6162
* @param \OCP\Notification\IManager $notificationManager
63+
* @param IUserSession $userSession
6264
*/
63-
public function __construct(Access $access, IConfig $ocConfig, INotificationManager $notificationManager) {
65+
public function __construct(Access $access, IConfig $ocConfig, INotificationManager $notificationManager, IUserSession $userSession) {
6466
parent::__construct($access);
6567
$this->ocConfig = $ocConfig;
6668
$this->notificationManager = $notificationManager;
67-
$this->registerHooks();
69+
$this->registerHooks($userSession);
6870
}
6971

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

75-
public function preDeleteUser(array $param) {
76-
$user = $param[0];
77-
if(!$user instanceof IUser) {
78-
throw new \RuntimeException('IUser expected');
79-
}
77+
public function preDeleteUser(IUser $user) {
8078
$this->currentUserInDeletionProcess = $user->getUID();
8179
}
8280

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

379-
//Get Home Directory out of user preferences so we can return it later,
380-
//necessary for removing directories as done by OC_User.
381377
$this->access->getUserMapper()->unmap($uid);
382378
$this->access->userManager->invalidate($uid);
383379
return true;
@@ -406,7 +402,9 @@ public function getHome($uid) {
406402
// early return path if it is a deleted user
407403
$user = $this->access->userManager->get($uid);
408404
if($user instanceof OfflineUser) {
409-
if($this->currentUserInDeletionProcess === $user->getUID()) {
405+
if($this->currentUserInDeletionProcess !== null
406+
&& $this->currentUserInDeletionProcess === $user->getOCName()
407+
) {
410408
return $user->getHomePath();
411409
} else {
412410
throw new NoUserException($uid . ' is not a valid user anymore');

‎apps/user_ldap/lib/User_Proxy.php‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131

3232
use OCA\User_LDAP\User\User;
3333
use OCP\IConfig;
34+
use OCP\IUserSession;
3435
use OCP\Notification\IManager as INotificationManager;
3536

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

4041
/**
4142
* Constructor
43+
*
4244
* @param array $serverConfigPrefixes array containing the config Prefixes
45+
* @param ILDAPWrapper $ldap
46+
* @param IConfig $ocConfig
47+
* @param INotificationManager $notificationManager
48+
* @param IUserSession $userSession
4349
*/
4450
public function __construct(array $serverConfigPrefixes, ILDAPWrapper $ldap, IConfig $ocConfig,
45-
INotificationManager $notificationManager) {
51+
INotificationManager $notificationManager, IUserSession $userSession) {
4652
parent::__construct($ldap);
4753
foreach($serverConfigPrefixes as $configPrefix) {
4854
$this->backends[$configPrefix] =
49-
new User_LDAP($this->getAccess($configPrefix), $ocConfig, $notificationManager);
55+
new User_LDAP($this->getAccess($configPrefix), $ocConfig, $notificationManager, $userSession);
5056
if(is_null($this->refBackend)) {
5157
$this->refBackend = &$this->backends[$configPrefix];
5258
}

‎apps/user_ldap/tests/Integration/AbstractIntegrationTest.php‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -144,11 +144,17 @@ public function run() {
144144
foreach($methods as $method) {
145145
if(strpos($method, 'case') === 0) {
146146
print("running $method " . PHP_EOL);
147-
if(!$this->$method()) {
148-
print(PHP_EOL . '>>> !!! Test ' . $method . ' FAILED !!! <<<' . PHP_EOL . PHP_EOL);
147+
try {
148+
if(!$this->$method()) {
149+
print(PHP_EOL . '>>> !!! Test ' . $method . ' FAILED !!! <<<' . PHP_EOL . PHP_EOL);
150+
exit(1);
151+
}
152+
$atLeastOneCaseRan = true;
153+
} catch(\Exception $e) {
154+
print(PHP_EOL . '>>> !!! Test ' . $method . ' RAISED AN EXCEPTION !!! <<<' . PHP_EOL);
155+
print($e->getMessage() . PHP_EOL . PHP_EOL);
149156
exit(1);
150157
}
151-
$atLeastOneCaseRan = true;
152158
}
153159
}
154160
if($atLeastOneCaseRan) {

‎apps/user_ldap/tests/Integration/Lib/IntegrationTestAttributeDetection.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public function init() {
4949
$groupMapper->clear();
5050
$this->access->setGroupMapper($groupMapper);
5151

52-
$userBackend = new User_LDAP($this->access, \OC::$server->getConfig(), \OC::$server->getNotificationManager());
52+
$userBackend = new User_LDAP($this->access, \OC::$server->getConfig(), \OC::$server->getNotificationManager(), \OC::$server->getUserSession());
5353
$userManager = \OC::$server->getUserManager();
5454
$userManager->clearBackends();
5555
$userManager->registerBackend($userBackend);

0 commit comments

Comments
 (0)