From 84ed1ed198f042dd832c262da0998a0a1129998d Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Mon, 28 Oct 2019 15:11:41 +0100 Subject: [PATCH] relax strict getHome behaviour for LDAP users in a shadow state * simplifies deletion process * less strange behaviour when looking up home storage (as long as it is local) * thus could enable transfer ownerships after user went invisible on ldap Signed-off-by: Arthur Schiwon --- apps/user_ldap/lib/User_LDAP.php | 32 +++----------------------- apps/user_ldap/tests/User_LDAPTest.php | 21 ++++------------- 2 files changed, 8 insertions(+), 45 deletions(-) diff --git a/apps/user_ldap/lib/User_LDAP.php b/apps/user_ldap/lib/User_LDAP.php index 85b11acfc5766..390b6afca3818 100644 --- a/apps/user_ldap/lib/User_LDAP.php +++ b/apps/user_ldap/lib/User_LDAP.php @@ -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; @@ -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; @@ -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; } /** @@ -429,21 +411,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; } diff --git a/apps/user_ldap/tests/User_LDAPTest.php b/apps/user_ldap/tests/User_LDAPTest.php index 7517994b34add..aac264ac7d1bc 100644 --- a/apps/user_ldap/tests/User_LDAPTest.php +++ b/apps/user_ldap/tests/User_LDAPTest.php @@ -314,22 +314,12 @@ public function testDeleteUserSuccess() { $offlineUser->expects($this->once()) ->method('getHomePath') ->willReturn($home); - $offlineUser->expects($this->once()) - ->method('getOCName') - ->willReturn($uid); $this->userManager->expects($this->atLeastOnce()) ->method('get') ->willReturn($offlineUser); $backend = new UserLDAP($this->access, $this->config, $this->notificationManager, $this->session, $this->pluginManager); - /** @var IUser|\PHPUnit_Framework_MockObject_MockObject $user */ - $user = $this->createMock(IUser::class); - $user->expects($this->once()) - ->method('getUID') - ->willReturn($uid); - - $backend->preDeleteUser($user); $result = $backend->deleteUser($uid); $this->assertTrue($result); /** @noinspection PhpUnhandledExceptionInspection */ @@ -836,9 +826,6 @@ public function testGetHomeNoPath() { $this->assertFalse($result); } - /** - * @expectedException \OC\User\NoUserException - */ public function testGetHomeDeletedUser() { $uid = 'newyorker'; @@ -869,14 +856,16 @@ public function testGetHomeDeletedUser() { ->will($this->returnValue(true)); $offlineUser = $this->createMock(OfflineUser::class); - $offlineUser->expects($this->never()) - ->method('getHomePath'); + $offlineUser->expects($this->atLeastOnce()) + ->method('getHomePath') + ->willReturn(''); $this->userManager->expects($this->atLeastOnce()) ->method('get') ->willReturn($offlineUser); - $backend->getHome($uid); + $result = $backend->getHome($uid); + $this->assertFalse($result); } public function testGetHomeWithPlugin() {