diff options
author | Vincent Petry <pvince81@owncloud.com> | 2015-12-15 22:52:58 +0300 |
---|---|---|
committer | Vincent Petry <pvince81@owncloud.com> | 2015-12-15 22:52:58 +0300 |
commit | 87eb48275bd1161ddab9bb4b8dc4f01f4584602f (patch) | |
tree | 1f3dfd1e8e0cc09c51cd12397efd4764fb23638e | |
parent | 4d1bee93081f7e642d70c42958487e62c98e1abd (diff) | |
parent | 1ec2f3233ef07f8d14ce7e230081a91003d0dbbb (diff) |
Merge pull request #21214 from owncloud/backport-21133-stable8
[backport] [stable8] Fix shared files of deleted users, detect DN change when checking for existence on LDAP
-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/lib/user/offlineuser.php | 7 | ||||
-rw-r--r-- | apps/user_ldap/tests/user_ldap.php | 67 | ||||
-rw-r--r-- | apps/user_ldap/user_ldap.php | 52 |
5 files changed, 223 insertions, 10 deletions
diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index d6d9743f7bd..1f7b4c152cf 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -1131,6 +1131,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])) { + throw new \Exception('Cannot determine UUID attribute'); + } + $dn = $result[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]) && count($result) === 1) { + // we put the count into account to make sure that this is + // really unique + return $result[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 @@ -1233,6 +1281,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 cb9db83f683..ed31202b125 100644 --- a/apps/user_ldap/lib/mapping/abstractmapping.php +++ b/apps/user_ldap/lib/mapping/abstractmapping.php @@ -144,7 +144,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 */ @@ -153,6 +153,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/lib/user/offlineuser.php b/apps/user_ldap/lib/user/offlineuser.php index 1833f4be968..17b86bfcf5a 100644 --- a/apps/user_ldap/lib/user/offlineuser.php +++ b/apps/user_ldap/lib/user/offlineuser.php @@ -86,6 +86,13 @@ class OfflineUser { } /** + * remove the Delete-flag from the user. + */ + public function unmark() { + $this->config->setUserValue($this->ocName, 'user_ldap', 'isDeleted', '0'); + } + + /** * exports the user details in an assoc array * @return array */ diff --git a/apps/user_ldap/tests/user_ldap.php b/apps/user_ldap/tests/user_ldap.php index f8ac4a11c6a..a2a9e46d19c 100644 --- a/apps/user_ldap/tests/user_ldap.php +++ b/apps/user_ldap/tests/user_ldap.php @@ -30,6 +30,7 @@ use \OCA\user_ldap\lib\ILDAPWrapper; class Test_User_Ldap_Direct extends \Test\TestCase { protected $backend; protected $access; + protected $configMock; protected function setUp() { parent::setUp(); @@ -57,14 +58,28 @@ class Test_User_Ldap_Direct extends \Test\TestCase { $conMethods, array($lw, null, null)); - $um = new \OCA\user_ldap\lib\user\Manager( - $this->getMock('\OCP\IConfig'), + + $offlineUser = $this->getMockBuilder('\OCA\user_ldap\lib\user\OfflineUser') + ->disableOriginalConstructor() + ->getMock(); + + $this->configMock = $this->getMock('\OCP\IConfig'); + + $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, @@ -643,6 +658,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') @@ -668,6 +721,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 6f00ba13a77..3fe00ac988e 100644 --- a/apps/user_ldap/user_ldap.php +++ b/apps/user_ldap/user_ldap.php @@ -25,6 +25,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; @@ -159,15 +160,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; } @@ -178,7 +182,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; @@ -243,10 +262,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 @@ -262,6 +284,24 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn if($this->access->connection->isCached($cacheKey)) { return $this->access->connection->getFromCache($cacheKey); } + + $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); + if($user instanceof OfflineUser) { + // should not happen, still be better safe than sorry + $path = $user->getHomePath(); + $this->access->connection->writeToCache($cacheKey, $path); + return $path; + } + } + if(strpos($this->access->connection->homeFolderNamingRule, 'attr:') === 0 && $this->access->connection->homeFolderNamingRule !== 'attr:') { $attr = substr($this->access->connection->homeFolderNamingRule, strlen('attr:')); |