[Pkg-owncloud-commits] [owncloud] 96/107: throw NoUserException in getHome when the requested user does not exist anymore
David Prévot
taffit at moszumanska.debian.org
Thu Dec 17 19:40:41 UTC 2015
This is an automated email from the git hooks/post-receive script.
taffit pushed a commit to branch stable8
in repository owncloud.
commit 780b97d7b1fba014216c3b5ec8055e3873103e80
Author: Arthur Schiwon <blizzz at owncloud.com>
Date: Fri Dec 11 00:12:41 2015 +0100
throw NoUserException in getHome when the requested user does not exist anymore
look for DN changes before marking a user as deleted
---
apps/user_ldap/lib/access.php | 95 ++++++++++++++++++++++++++
apps/user_ldap/lib/mapping/abstractmapping.php | 12 +++-
apps/user_ldap/tests/user_ldap.php | 62 ++++++++++++++++-
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 7be9118..7df7994 100644
--- a/apps/user_ldap/lib/access.php
+++ b/apps/user_ldap/lib/access.php
@@ -1255,6 +1255,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
@@ -1357,6 +1405,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 f0f0f6d..c3d38ce 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 0f70c43..8be56c4 100644
--- a/apps/user_ldap/tests/user_ldap.php
+++ b/apps/user_ldap/tests/user_ldap.php
@@ -63,14 +63,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,
@@ -654,6 +666,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')
@@ -679,6 +729,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 fc8ce36..ad46313 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);
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-owncloud/owncloud.git
More information about the Pkg-owncloud-commits
mailing list