diff options
author | C. Montero Luque <cmonteroluque@users.noreply.github.com> | 2015-12-15 20:14:58 +0300 |
---|---|---|
committer | C. Montero Luque <cmonteroluque@users.noreply.github.com> | 2015-12-15 20:14:58 +0300 |
commit | a376fec98416304320cdadab17f85e3c00b61072 (patch) | |
tree | 24e1a8ca71bee58f7ff0ee80185ef8b47baf4df4 /apps | |
parent | 1ecda59c2412164e95db452b43d11b28f1b7c64a (diff) | |
parent | e39415c946338c3093257a21d09a7360e0c9ffd4 (diff) |
Merge pull request #21133 from owncloud/fix-shared-files-of-deleted-users
Fix shared files of deleted users, detect DN change when checking for existence on LDAP
Diffstat (limited to 'apps')
-rw-r--r-- | apps/user_ldap/lib/access.php | 95 | ||||
-rw-r--r-- | apps/user_ldap/lib/mapping/abstractmapping.php | 12 | ||||
-rw-r--r-- | apps/user_ldap/tests/user_ldap.php | 62 | ||||
-rw-r--r-- | apps/user_ldap/user_ldap.php | 43 |
4 files changed, 203 insertions, 9 deletions
diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index 667f1076235..693a420a74d 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -1277,6 +1277,54 @@ class Access extends LDAPUtility implements user\IUserTools { } /** + * reverse lookup of a DN given a known UUID + * + * @param string $uuid + * @return string + * @throws \Exception + */ + public function getUserDnByUuid($uuid) { + $uuidOverride = $this->connection->ldapExpertUUIDUserAttr; + $filter = $this->connection->ldapUserFilter; + $base = $this->connection->ldapBaseUsers; + + if($this->connection->ldapUuidUserAttribute === 'auto' && empty($uuidOverride)) { + // Sacrebleu! The UUID attribute is unknown :( We need first an + // existing DN to be able to reliably detect it. + $result = $this->search($filter, $base, ['dn'], 1); + if(!isset($result[0]) || !isset($result[0]['dn'])) { + throw new \Exception('Cannot determine UUID attribute'); + } + $dn = $result[0]['dn'][0]; + if(!$this->detectUuidAttribute($dn, true)) { + throw new \Exception('Cannot determine UUID attribute'); + } + } else { + // The UUID attribute is either known or an override is given. + // By calling this method we ensure that $this->connection->$uuidAttr + // is definitely set + if(!$this->detectUuidAttribute('', true)) { + throw new \Exception('Cannot determine UUID attribute'); + } + } + + $uuidAttr = $this->connection->ldapUuidUserAttribute; + if($uuidAttr === 'guid' || $uuidAttr === 'objectguid') { + $uuid = $this->formatGuid2ForFilterUser($uuid); + } + + $filter = $uuidAttr . '=' . $uuid; + $result = $this->searchUsers($filter, ['dn'], 2); + if(is_array($result) && isset($result[0]) && isset($result[0]['dn']) && count($result) === 1) { + // we put the count into account to make sure that this is + // really unique + return $result[0]['dn'][0]; + } + + throw new \Exception('Cannot determine UUID attribute'); + } + + /** * auto-detects the directory's UUID attribute * @param string $dn a known DN used to check against * @param bool $isUser @@ -1379,6 +1427,53 @@ class Access extends LDAPUtility implements user\IUserTools { } /** + * the first three blocks of the string-converted GUID happen to be in + * reverse order. In order to use it in a filter, this needs to be + * corrected. Furthermore the dashes need to be replaced and \\ preprended + * to every two hax figures. + * + * If an invalid string is passed, it will be returned without change. + * + * @param string $guid + * @return string + */ + public function formatGuid2ForFilterUser($guid) { + if(!is_string($guid)) { + throw new \InvalidArgumentException('String expected'); + } + $blocks = explode('-', $guid); + if(count($blocks) !== 5) { + /* + * Why not throw an Exception instead? This method is a utility + * called only when trying to figure out whether a "missing" known + * LDAP user was or was not renamed on the LDAP server. And this + * even on the use case that a reverse lookup is needed (UUID known, + * not DN), i.e. when finding users (search dialog, users page, + * login, …) this will not be fired. This occurs only if shares from + * a users are supposed to be mounted who cannot be found. Throwing + * an exception here would kill the experience for a valid, acting + * user. Instead we write a log message. + */ + \OC::$server->getLogger()->info( + 'Passed string does not resemble a valid GUID. Known UUID ' . + '({uuid}) probably does not match UUID configuration.', + [ 'app' => 'user_ldap', 'uuid' => $guid ] + ); + return $guid; + } + for($i=0; $i < 3; $i++) { + $pairs = str_split($blocks[$i], 2); + $pairs = array_reverse($pairs); + $blocks[$i] = implode('', $pairs); + } + for($i=0; $i < 5; $i++) { + $pairs = str_split($blocks[$i], 2); + $blocks[$i] = '\\' . implode('\\', $pairs); + } + return implode('', $blocks); + } + + /** * gets a SID of the domain of the given dn * @param string $dn * @return string|bool diff --git a/apps/user_ldap/lib/mapping/abstractmapping.php b/apps/user_ldap/lib/mapping/abstractmapping.php index f0f0f6df75e..c3d38ce8b71 100644 --- a/apps/user_ldap/lib/mapping/abstractmapping.php +++ b/apps/user_ldap/lib/mapping/abstractmapping.php @@ -158,7 +158,7 @@ abstract class AbstractMapping { } /** - * Gets the name based on the provided LDAP DN. + * Gets the name based on the provided LDAP UUID. * @param string $uuid * @return string|false */ @@ -167,6 +167,16 @@ abstract class AbstractMapping { } /** + * Gets the UUID based on the provided LDAP DN + * @param string $dn + * @return false|string + * @throws \Exception + */ + public function getUUIDByDN($dn) { + return $this->getXbyY('directory_uuid', 'ldap_dn', $dn); + } + + /** * gets a piece of the mapping list * @param int $offset * @param int $limit diff --git a/apps/user_ldap/tests/user_ldap.php b/apps/user_ldap/tests/user_ldap.php index 7593371d85d..3aec2a5ce57 100644 --- a/apps/user_ldap/tests/user_ldap.php +++ b/apps/user_ldap/tests/user_ldap.php @@ -70,14 +70,26 @@ class Test_User_Ldap_Direct extends \Test\TestCase { array($lw, null, null)); $this->configMock = $this->getMock('\OCP\IConfig'); - $um = new \OCA\user_ldap\lib\user\Manager( + + $offlineUser = $this->getMockBuilder('\OCA\user_ldap\lib\user\OfflineUser') + ->disableOriginalConstructor() + ->getMock(); + + $um = $this->getMockBuilder('\OCA\user_ldap\lib\user\Manager') + ->setMethods(['getDeletedUser']) + ->setConstructorArgs([ $this->configMock, $this->getMock('\OCA\user_ldap\lib\FilesystemHelper'), $this->getMock('\OCA\user_ldap\lib\LogWrapper'), $this->getMock('\OCP\IAvatarManager'), $this->getMock('\OCP\Image'), $this->getMock('\OCP\IDBConnection') - ); + ]) + ->getMock(); + + $um->expects($this->any()) + ->method('getDeletedUser') + ->will($this->returnValue($offlineUser)); $access = $this->getMock('\OCA\user_ldap\lib\Access', $accMethods, @@ -661,6 +673,44 @@ class Test_User_Ldap_Direct extends \Test\TestCase { $this->assertFalse($result); } + /** + * @expectedException \OC\User\NoUserException + */ + public function testGetHomeDeletedUser() { + $access = $this->getAccessMock(); + $backend = new UserLDAP($access, $this->getMock('\OCP\IConfig')); + $this->prepareMockForUserExists($access); + + $access->connection->expects($this->any()) + ->method('__get') + ->will($this->returnCallback(function($name) { + if($name === 'homeFolderNamingRule') { + return 'attr:testAttribute'; + } + return null; + })); + + $access->expects($this->any()) + ->method('readAttribute') + ->will($this->returnValue([])); + + $userMapper = $this->getMockBuilder('\OCA\User_LDAP\Mapping\UserMapping') + ->disableOriginalConstructor() + ->getMock(); + + $access->expects($this->any()) + ->method('getUserMapper') + ->will($this->returnValue($userMapper)); + + $this->configMock->expects($this->any()) + ->method('getUserValue') + ->will($this->returnValue(true)); + + //no path at all – triggers OC default behaviour + $result = $backend->getHome('newyorker'); + $this->assertFalse($result); + } + private function prepareAccessForGetDisplayName(&$access) { $access->connection->expects($this->any()) ->method('__get') @@ -686,6 +736,14 @@ class Test_User_Ldap_Direct extends \Test\TestCase { return false; } })); + + $userMapper = $this->getMockBuilder('\OCA\User_LDAP\Mapping\UserMapping') + ->disableOriginalConstructor() + ->getMock(); + + $access->expects($this->any()) + ->method('getUserMapper') + ->will($this->returnValue($userMapper)); } public function testGetDisplayName() { diff --git a/apps/user_ldap/user_ldap.php b/apps/user_ldap/user_ldap.php index 0097dda89b5..a266be7b7f7 100644 --- a/apps/user_ldap/user_ldap.php +++ b/apps/user_ldap/user_ldap.php @@ -30,6 +30,7 @@ namespace OCA\user_ldap; +use OC\User\NoUserException; use OCA\user_ldap\lib\BackendUtility; use OCA\user_ldap\lib\Access; use OCA\user_ldap\lib\user\OfflineUser; @@ -190,15 +191,18 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn /** * checks whether a user is still available on LDAP + * * @param string|\OCA\User_LDAP\lib\user\User $user either the ownCloud user * name or an instance of that user * @return bool + * @throws \Exception + * @throws \OC\ServerNotAvailableException */ public function userExistsOnLDAP($user) { if(is_string($user)) { $user = $this->access->userManager->get($user); } - if(!$user instanceof User) { + if(is_null($user)) { return false; } @@ -209,7 +213,22 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn if(is_null($lcr)) { throw new \Exception('No LDAP Connection to server ' . $this->access->connection->ldapHost); } - return false; + + try { + $uuid = $this->access->getUserMapper()->getUUIDByDN($dn); + if(!$uuid) { + return false; + } + $newDn = $this->access->getUserDnByUuid($uuid); + $this->access->getUserMapper()->setDNbyUUID($newDn, $uuid); + return true; + } catch (\Exception $e) { + return false; + } + } + + if($user instanceof OfflineUser) { + $user->unmark(); } return true; @@ -274,10 +293,13 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn } /** - * get the user's home directory - * @param string $uid the username - * @return string|bool - */ + * get the user's home directory + * + * @param string $uid the username + * @return bool|string + * @throws NoUserException + * @throws \Exception + */ public function getHome($uid) { if(isset($this->homesToKill[$uid]) && !empty($this->homesToKill[$uid])) { //a deleted user who needs some clean up @@ -295,6 +317,15 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn } $user = $this->access->userManager->get($uid); + if(is_null($user) || ($user instanceof OfflineUser && !$this->userExistsOnLDAP($user->getOCName()))) { + throw new NoUserException($uid . ' is not a valid user anymore'); + } + if($user instanceof OfflineUser) { + // apparently this user survived the userExistsOnLDAP check, + // we request the user instance again in order to retrieve a User + // instance instead + $user = $this->access->userManager->get($uid); + } $path = $user->getHomePath(); $this->access->cacheUserHome($uid, $path); |