From dbb4be903c52db5a241aa5765412a3bb875097c2 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Sun, 10 Feb 2013 21:52:57 +0100 Subject: [PATCH 01/16] LDAP: change generation of internal names. Use UUID for users. Change to sequential numbers for groups as they are still used as display names --- apps/user_ldap/lib/access.php | 119 ++++++++++++++++++++++++++++------ 1 file changed, 100 insertions(+), 19 deletions(-) diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index 68cbe4a5e75..ab6915d839b 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -293,6 +293,10 @@ abstract class Access { $query->execute(array($dn, $uuid)); return $component; } + } else { + //If the UUID can't be detected something is foul. + \OCP\Util::writeLog('user_ldap', 'Cannot determine UUID for '.$dn.'. Skipping.', \OCP\Util::INFO); + return false; } if(is_null($ldapname)) { @@ -303,21 +307,24 @@ abstract class Access { } $ldapname = $ldapname[0]; } - $ldapname = $this->sanitizeUsername($ldapname); + $intname = $isUser ? $this->sanitizeUsername($uuid) : $this->sanitizeUsername($ldapname); //a new user/group! Add it only if it doesn't conflict with other backend's users or existing groups - if(($isUser && !\OCP\User::userExists($ldapname, 'OCA\\user_ldap\\USER_LDAP')) || (!$isUser && !\OC_Group::groupExists($ldapname))) { - if($this->mapComponent($dn, $ldapname, $isUser)) { - return $ldapname; + //disabling Cache is required to avoid that the new user is cached as not-existing in fooExists check + $originalTTL = $this->connection->ldapCacheTTL; + $this->connection->setConfiguration(array('ldapCacheTTL' => 0)); + if(($isUser && !\OCP\User::userExists($intname)) + || (!$isUser && !\OC_Group::groupExists($intname))) { + if($this->mapComponent($dn, $intname, $isUser)) { + $this->connection->setConfiguration(array('ldapCacheTTL' => $originalTTL)); + return $intname; } } + $this->connection->setConfiguration(array('ldapCacheTTL' => $originalTTL)); - //doh! There is a conflict. We need to distinguish between users/groups. Adding indexes is an idea, but not much of a help for the user. The DN is ugly, but for now the only reasonable way. But we transform it to a readable format and remove the first part to only give the path where this object is located. - $oc_name = $this->alternateOwnCloudName($ldapname, $dn); - if(($isUser && !\OCP\User::userExists($oc_name)) || (!$isUser && !\OC_Group::groupExists($oc_name))) { - if($this->mapComponent($dn, $oc_name, $isUser)) { - return $oc_name; - } + $altname = $this->createAltInternalOwnCloudName($intname, $isUser); + if($this->mapComponent($dn, $altname, $isUser)) { + return $altname; } //if everything else did not help.. @@ -400,18 +407,92 @@ abstract class Access { } /** - * @brief creates a hopefully unique name for owncloud based on the display name and the dn of the LDAP object + * @brief creates a unique name for internal ownCloud use for users. Don't call it directly. * @param $name the display name of the object - * @param $dn the dn of the object - * @returns string with with the name to use in ownCloud + * @returns string with with the name to use in ownCloud or false if unsuccessful * - * creates a hopefully unique name for owncloud based on the display name and the dn of the LDAP object + * Instead of using this method directly, call + * createAltInternalOwnCloudName($name, true) */ - private function alternateOwnCloudName($name, $dn) { - $ufn = ldap_dn2ufn($dn); - $name = $name . '@' . trim(\OCP\Util::mb_substr_replace($ufn, '', 0, mb_strpos($ufn, ',', 0, 'UTF-8'), 'UTF-8')); - $name = $this->sanitizeUsername($name); - return $name; + private function _createAltInternalOwnCloudNameForUsers($name) { + $attempts = 0; + //while loop is just a precaution. If a name is not generated within + //20 attempts, something else is very wrong. Avoids infinite loop. + while($attempts < 20){ + $altName = $name . '_' . uniqid(); + if(\OCP\User::userExists($altName)) { + return $altName; + } + $attempts++; + } + return false; + } + + /** + * @brief creates a unique name for internal ownCloud use for groups. Don't call it directly. + * @param $name the display name of the object + * @returns string with with the name to use in ownCloud or false if unsuccessful. + * + * Instead of using this method directly, call + * createAltInternalOwnCloudName($name, false) + * + * Group names are also used as display names, so we do a sequential + * numbering, e.g. Developers_42 when there are 41 other groups called + * "Developers" + */ + private function _createAltInternalOwnCloudNameForGroups($name) { + $query = \OCP\DB::prepare(' + SELECT `owncloud_name` + FROM `'.$this->getMapTable(false).'` + WHERE `owncloud_name` LIKE ? + '); + + $usedNames = array(); + $res = $query->execute(array($name.'_%')); + while($row = $res->fetchRow()) { + $usedNames[] = $row['owncloud_name']; + } + if(!($usedNames) || count($usedNames) == 0) { + $lastNo = 1; //will become name_2 + } else { + natsort($usedNames); + $lastname = array_pop($usedNames); + $lastNo = intval(substr($lastname, strrpos($lastname, '_') + 1)); + } + $altName = $name.'_'.strval($lastNo+1); + unset($usedNames); + + $attempts = 1; + while($attempts < 21){ + //Pro forma check to be really sure it is unique + //while loop is just a precaution. If a name is not generated within + //20 attempts, something else is very wrong. Avoids infinite loop. + if(!\OC_Group::groupExists($altName)) { + return $altName; + } + $altName = $name . '_' . $lastNo + $attempts; + $attempts++; + } + return false; + } + + /** + * @brief creates a unique name for internal ownCloud use. + * @param $name the display name of the object + * @param $isUser boolean, whether name should be created for a user (true) or a group (false) + * @returns string with with the name to use in ownCloud or false if unsuccessful + */ + private function createAltInternalOwnCloudName($name, $isUser) { + $originalTTL = $this->connection->ldapCacheTTL; + $this->connection->setConfiguration(array('ldapCacheTTL' => 0)); + if($isUser) { + $altName = $this->_createAltInternalOwnCloudNameForUsers($name); + } else { + $altName = $this->_createAltInternalOwnCloudNameForGroups($name); + } + $this->connection->setConfiguration(array('ldapCacheTTL' => $originalTTL)); + + return $altName; } /** From afc9fe419aee034999fcaa9ace05c0043189154d Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Sun, 10 Feb 2013 21:53:27 +0100 Subject: [PATCH 02/16] adjust copyright --- apps/user_ldap/lib/access.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index ab6915d839b..057ae17c308 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -4,7 +4,7 @@ * ownCloud – LDAP Access * * @author Arthur Schiwon - * @copyright 2012 Arthur Schiwon blizzz@owncloud.com + * @copyright 2012, 2013 Arthur Schiwon blizzz@owncloud.com * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE From 56d10e9054c5f2699e3e0df00bd71a40f53be738 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Sun, 10 Feb 2013 18:15:23 +0100 Subject: [PATCH 03/16] Cache: simplify scanner logic a bit when handeling with unknown folder sizes --- lib/files/cache/scanner.php | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/lib/files/cache/scanner.php b/lib/files/cache/scanner.php index 5a9a119458e..ff37c944240 100644 --- a/lib/files/cache/scanner.php +++ b/lib/files/cache/scanner.php @@ -101,18 +101,16 @@ class Scanner { $child = ($path) ? $path . '/' . $file : $file; $data = $this->scanFile($child); if ($data) { - if ($data['mimetype'] === 'httpd/unix-directory') { + if ($data['size'] === -1) { if ($recursive === self::SCAN_RECURSIVE) { $childQueue[] = $child; $data['size'] = 0; } else { - $data['size'] = -1; + $size = -1; } - } else { } - if ($data['size'] === -1) { - $size = -1; - } elseif ($size !== -1) { + + if ($size !== -1) { $size += $data['size']; } } @@ -133,7 +131,7 @@ class Scanner { } return $size; } - + /** * @brief check if the file should be ignored when scanning * NOTE: files with a '.part' extension are ignored as well! @@ -143,8 +141,8 @@ class Scanner { */ private function isIgnoredFile($file) { if ($file === '.' || $file === '..' - || pathinfo($file,PATHINFO_EXTENSION) === 'part') - { + || pathinfo($file, PATHINFO_EXTENSION) === 'part' + ) { return true; } return false; From 299649b40e1a87eee7bcead74b269fe8c452e04d Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Sun, 10 Feb 2013 18:24:24 +0100 Subject: [PATCH 04/16] Cache: reuse known folder sizes when doing a shallow scan --- lib/files/cache/scanner.php | 10 +++++++--- tests/lib/files/cache/watcher.php | 1 - 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/files/cache/scanner.php b/lib/files/cache/scanner.php index ff37c944240..b27b3555c91 100644 --- a/lib/files/cache/scanner.php +++ b/lib/files/cache/scanner.php @@ -58,9 +58,10 @@ class Scanner { * scan a single file and store it in the cache * * @param string $file + * @param bool $checkExisting check existing folder sizes in the cache instead of always using -1 for folder size * @return array with metadata of the scanned file */ - public function scanFile($file) { + public function scanFile($file, $checkExisting = false) { \OC_Hook::emit('\OC\Files\Cache\Scanner', 'scan_file', array('path' => $file, 'storage' => $this->storageId)); $data = $this->getData($file); if ($data) { @@ -73,7 +74,10 @@ class Scanner { $this->scanFile($parent); } } - $id = $this->cache->put($file, $data); + if ($data['size'] === -1 and $cacheData = $this->cache->get($file)) { + $data['size'] = $cacheData['size']; + } + $this->cache->put($file, $data); } return $data; } @@ -99,7 +103,7 @@ class Scanner { while ($file = readdir($dh)) { if (!$this->isIgnoredFile($file)) { $child = ($path) ? $path . '/' . $file : $file; - $data = $this->scanFile($child); + $data = $this->scanFile($child, $recursive === self::SCAN_SHALLOW); if ($data) { if ($data['size'] === -1) { if ($recursive === self::SCAN_RECURSIVE) { diff --git a/tests/lib/files/cache/watcher.php b/tests/lib/files/cache/watcher.php index e8a1689cab0..8ef6ab44d10 100644 --- a/tests/lib/files/cache/watcher.php +++ b/tests/lib/files/cache/watcher.php @@ -76,7 +76,6 @@ class Watcher extends \PHPUnit_Framework_TestCase { $updater->checkUpdate(''); $entry = $cache->get('foo.txt'); - $this->assertEquals(-1, $entry['size']); $this->assertEquals('httpd/unix-directory', $entry['mimetype']); $this->assertFalse($cache->inCache('folder')); $this->assertFalse($cache->inCache('folder/bar.txt')); From d84c3cd014fd73930cd15aee64d57aad3383b2aa Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 11 Feb 2013 13:24:56 +0100 Subject: [PATCH 05/16] Cache: actually use parameter --- lib/files/cache/scanner.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/files/cache/scanner.php b/lib/files/cache/scanner.php index b27b3555c91..ec216264429 100644 --- a/lib/files/cache/scanner.php +++ b/lib/files/cache/scanner.php @@ -74,7 +74,7 @@ class Scanner { $this->scanFile($parent); } } - if ($data['size'] === -1 and $cacheData = $this->cache->get($file)) { + if ($checkExisting and $data['size'] === -1 and $cacheData = $this->cache->get($file)) { $data['size'] = $cacheData['size']; } $this->cache->put($file, $data); From 2921d2fb785f265bb3bf17873ada304145d49aec Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 11 Feb 2013 13:27:34 +0100 Subject: [PATCH 06/16] Cache: don't create a new etag when the mtime hasn't changed --- lib/files/cache/scanner.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/files/cache/scanner.php b/lib/files/cache/scanner.php index ec216264429..7f19261d972 100644 --- a/lib/files/cache/scanner.php +++ b/lib/files/cache/scanner.php @@ -76,6 +76,9 @@ class Scanner { } if ($checkExisting and $data['size'] === -1 and $cacheData = $this->cache->get($file)) { $data['size'] = $cacheData['size']; + if ($data['mtime'] === $cacheData['mtime']) { + $data['etag'] = $cacheData['etag']; + } } $this->cache->put($file, $data); } From e68e5cc849b625dccc38b903fd19e418d62d1c22 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 11 Feb 2013 15:18:14 +0100 Subject: [PATCH 07/16] Split editable select code used for quota selection into a jquery plugin --- core/js/singleselect.js | 76 ++++ settings/css/settings.css | 1 - settings/js/users.js | 792 ++++++++++++++++------------------- settings/templates/users.php | 62 ++- settings/users.php | 1 + 5 files changed, 475 insertions(+), 457 deletions(-) create mode 100644 core/js/singleselect.js diff --git a/core/js/singleselect.js b/core/js/singleselect.js new file mode 100644 index 00000000000..1a018b74148 --- /dev/null +++ b/core/js/singleselect.js @@ -0,0 +1,76 @@ +(function ($) { + $.fn.singleSelect = function () { + return this.each(function (i, select) { + var input = $(''); + select = $(select); + input.css('position', 'absolute'); + input.css(select.offset()); + input.css({ + 'box-sizing': 'border-box', + '-moz-box-sizing': 'border-box', + 'margin': 0, + 'width': (select.width() - 5) + 'px', + 'height': (select.outerHeight() - 2) + 'px', + 'border': 'none', + 'box-shadow': 'none', + 'margin-top': '1px', + 'margin-left': '1px', + 'z-index': 1000 + }); + input.hide(); + $('body').append(input); + + select.on('change', function (event) { + var value = $(this).val(), + newAttr = $('option:selected', $(this)).attr('data-new'); + if (!(typeof newAttr !== 'undefined' && newAttr !== false)) { + input.hide(); + select.data('previous', value); + } else { + event.stopImmediatePropagation(); + input.show(); + select.css('background-color', 'white'); + input.focus(); + } + }); + + $(select).data('previous', $(select).val()); + + input.on('change', function () { + var value = $(this).val(); + if (value) { + select.children().attr('selected', null); + var existingOption = select.children().filter(function (i, option) { + return ($(option).val() == value); + }); + if (existingOption.length) { + existingOption.attr('selected', 'selected'); + } else { + var option = $('