From ea5b51325a85fb61b2b7f2217bbc186099a8d82c Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Fri, 4 Sep 2020 10:27:25 +0200 Subject: [PATCH 1/2] these code bits were part of old logic that was already refactored out - only references were in unit tests Signed-off-by: Arthur Schiwon --- apps/user_ldap/lib/User/User.php | 1 - apps/user_ldap/tests/User/UserTest.php | 1 - 2 files changed, 2 deletions(-) diff --git a/apps/user_ldap/lib/User/User.php b/apps/user_ldap/lib/User/User.php index 467d5ca025ba6..89e5e54d26dfd 100644 --- a/apps/user_ldap/lib/User/User.php +++ b/apps/user_ldap/lib/User/User.php @@ -195,7 +195,6 @@ public function markUser() { * @param array $ldapEntry the user entry as retrieved from LDAP */ public function processAttributes($ldapEntry) { - $this->markRefreshTime(); //Quota $attr = strtolower($this->connection->ldapQuotaAttribute); if(isset($ldapEntry[$attr])) { diff --git a/apps/user_ldap/tests/User/UserTest.php b/apps/user_ldap/tests/User/UserTest.php index 36a312172c1ef..66ba43f2e7dd5 100644 --- a/apps/user_ldap/tests/User/UserTest.php +++ b/apps/user_ldap/tests/User/UserTest.php @@ -1005,7 +1005,6 @@ public function imageDataProvider() { public function testProcessAttributes() { $requiredMethods = [ - 'markRefreshTime', 'updateQuota', 'updateEmail', 'composeAndStoreDisplayName', From 24d464c5cdd2cebbf71b43b6819bbf39df385805 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Fri, 4 Sep 2020 10:51:41 +0200 Subject: [PATCH 2/2] add repair step to clean up DB off lastFeatureRefresh entries in user prefs - also removes related app setting "updateAttributesInterval" Signed-off-by: Arthur Schiwon --- apps/user_ldap/appinfo/info.xml | 3 +- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + .../lib/Migration/RemoveRefreshTime.php | 65 ++++++++++++++++ apps/user_ldap/lib/User/User.php | 52 ------------- apps/user_ldap/tests/User/UserTest.php | 78 ------------------- 6 files changed, 69 insertions(+), 131 deletions(-) create mode 100644 apps/user_ldap/lib/Migration/RemoveRefreshTime.php diff --git a/apps/user_ldap/appinfo/info.xml b/apps/user_ldap/appinfo/info.xml index e6a46163af80f..2d51b4680c2a5 100644 --- a/apps/user_ldap/appinfo/info.xml +++ b/apps/user_ldap/appinfo/info.xml @@ -9,7 +9,7 @@ A user logs into Nextcloud with their LDAP or AD credentials, and is granted access based on an authentication request handled by the LDAP or AD server. Nextcloud does not store LDAP or AD passwords, rather these credentials are used to authenticate a user and then Nextcloud uses a session for the user ID. More information is available in the LDAP User and Group Backend documentation. - 1.8.0 + 1.8.1 agpl Dominik Schmidt Arthur Schiwon @@ -36,6 +36,7 @@ A user logs into Nextcloud with their LDAP or AD credentials, and is granted acc OCA\User_LDAP\Migration\UUIDFixInsert + OCA\User_LDAP\Migration\RemoveRefreshTime diff --git a/apps/user_ldap/composer/composer/autoload_classmap.php b/apps/user_ldap/composer/composer/autoload_classmap.php index fadbc701ec036..7e255e78a9e53 100644 --- a/apps/user_ldap/composer/composer/autoload_classmap.php +++ b/apps/user_ldap/composer/composer/autoload_classmap.php @@ -48,6 +48,7 @@ 'OCA\\User_LDAP\\Mapping\\AbstractMapping' => $baseDir . '/../lib/Mapping/AbstractMapping.php', 'OCA\\User_LDAP\\Mapping\\GroupMapping' => $baseDir . '/../lib/Mapping/GroupMapping.php', 'OCA\\User_LDAP\\Mapping\\UserMapping' => $baseDir . '/../lib/Mapping/UserMapping.php', + 'OCA\\User_LDAP\\Migration\\RemoveRefreshTime' => $baseDir . '/../lib/Migration/RemoveRefreshTime.php', 'OCA\\User_LDAP\\Migration\\UUIDFix' => $baseDir . '/../lib/Migration/UUIDFix.php', 'OCA\\User_LDAP\\Migration\\UUIDFixGroup' => $baseDir . '/../lib/Migration/UUIDFixGroup.php', 'OCA\\User_LDAP\\Migration\\UUIDFixInsert' => $baseDir . '/../lib/Migration/UUIDFixInsert.php', diff --git a/apps/user_ldap/composer/composer/autoload_static.php b/apps/user_ldap/composer/composer/autoload_static.php index d40df6e483625..e21f7afcc23a6 100644 --- a/apps/user_ldap/composer/composer/autoload_static.php +++ b/apps/user_ldap/composer/composer/autoload_static.php @@ -63,6 +63,7 @@ class ComposerStaticInitUser_LDAP 'OCA\\User_LDAP\\Mapping\\AbstractMapping' => __DIR__ . '/..' . '/../lib/Mapping/AbstractMapping.php', 'OCA\\User_LDAP\\Mapping\\GroupMapping' => __DIR__ . '/..' . '/../lib/Mapping/GroupMapping.php', 'OCA\\User_LDAP\\Mapping\\UserMapping' => __DIR__ . '/..' . '/../lib/Mapping/UserMapping.php', + 'OCA\\User_LDAP\\Migration\\RemoveRefreshTime' => __DIR__ . '/..' . '/../lib/Migration/RemoveRefreshTime.php', 'OCA\\User_LDAP\\Migration\\UUIDFix' => __DIR__ . '/..' . '/../lib/Migration/UUIDFix.php', 'OCA\\User_LDAP\\Migration\\UUIDFixGroup' => __DIR__ . '/..' . '/../lib/Migration/UUIDFixGroup.php', 'OCA\\User_LDAP\\Migration\\UUIDFixInsert' => __DIR__ . '/..' . '/../lib/Migration/UUIDFixInsert.php', diff --git a/apps/user_ldap/lib/Migration/RemoveRefreshTime.php b/apps/user_ldap/lib/Migration/RemoveRefreshTime.php new file mode 100644 index 0000000000000..b4b4e2c262812 --- /dev/null +++ b/apps/user_ldap/lib/Migration/RemoveRefreshTime.php @@ -0,0 +1,65 @@ + + * + * @author Arthur Schiwon + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\User_LDAP\Migration; + +use OCP\IConfig; +use OCP\IDBConnection; +use OCP\Migration\IOutput; +use OCP\Migration\IRepairStep; + +/** + * Class RmRefreshTime + * + * this can be removed with Nextcloud 21 + * + * @package OCA\User_LDAP\Migration + */ +class RemoveRefreshTime implements IRepairStep { + + /** @var IDBConnection */ + private $dbc; + /** @var IConfig */ + private $config; + + public function __construct(IDBConnection $dbc, IConfig $config) { + $this->dbc = $dbc; + $this->config = $config; + } + + public function getName() { + return 'Remove deprecated refresh time markers for LDAP user records'; + } + + public function run(IOutput $output) { + $this->config->deleteAppValue('user_ldap', 'updateAttributesInterval'); + + $qb = $this->dbc->getQueryBuilder(); + $qb->delete('preferences') + ->where($qb->expr()->eq('appid', $qb->createNamedParameter('user_ldap'))) + ->andWhere($qb->expr()->eq('configkey', $qb->createNamedParameter('lastFeatureRefresh'))) + ->execute(); + } +} diff --git a/apps/user_ldap/lib/User/User.php b/apps/user_ldap/lib/User/User.php index 89e5e54d26dfd..78948625c5ebb 100644 --- a/apps/user_ldap/lib/User/User.php +++ b/apps/user_ldap/lib/User/User.php @@ -106,7 +106,6 @@ class User { * DB config keys for user preferences */ const USER_PREFKEY_FIRSTLOGIN = 'firstLoginAccomplished'; - const USER_PREFKEY_LASTREFRESH = 'lastFeatureRefresh'; /** * @brief constructor, make sure the subclasses call this one! @@ -149,32 +148,6 @@ public function __construct($username, $dn, Access $access, \OCP\Util::connectHook('OC_User', 'post_login', $this, 'handlePasswordExpiry'); } - /** - * @brief updates properties like email, quota or avatar provided by LDAP - * @return null - */ - public function update() { - if(is_null($this->dn)) { - return null; - } - - $hasLoggedIn = $this->config->getUserValue($this->uid, 'user_ldap', - self::USER_PREFKEY_FIRSTLOGIN, 0); - - if($this->needsRefresh()) { - $this->updateEmail(); - $this->updateQuota(); - if($hasLoggedIn !== 0) { - //we do not need to try it, when the user has not been logged in - //before, because the file system will not be ready. - $this->updateAvatar(); - //in order to get an avatar as soon as possible, mark the user - //as refreshed only when updating the avatar did happen - $this->markRefreshTime(); - } - } - } - /** * marks a user as deleted * @@ -395,31 +368,6 @@ public function markLogin() { $this->uid, 'user_ldap', self::USER_PREFKEY_FIRSTLOGIN, 1); } - /** - * @brief marks the time when user features like email have been updated - * @return null - */ - public function markRefreshTime() { - $this->config->setUserValue( - $this->uid, 'user_ldap', self::USER_PREFKEY_LASTREFRESH, time()); - } - - /** - * @brief checks whether user features needs to be updated again by - * comparing the difference of time of the last refresh to now with the - * desired interval - * @return bool - */ - private function needsRefresh() { - $lastChecked = $this->config->getUserValue($this->uid, 'user_ldap', - self::USER_PREFKEY_LASTREFRESH, 0); - - if((time() - (int)$lastChecked) < (int)$this->config->getAppValue('user_ldap', 'updateAttributesInterval', 86400)) { - return false; - } - return true; - } - /** * Stores a key-value pair in relation to this user * diff --git a/apps/user_ldap/tests/User/UserTest.php b/apps/user_ldap/tests/User/UserTest.php index 66ba43f2e7dd5..8a32e17c24a70 100644 --- a/apps/user_ldap/tests/User/UserTest.php +++ b/apps/user_ldap/tests/User/UserTest.php @@ -831,57 +831,6 @@ public function testUpdateAvatarNotProvided() { $this->user->updateAvatar(); } - public function testUpdateBeforeFirstLogin() { - $this->config->expects($this->at(0)) - ->method('getUserValue') - ->with($this->equalTo($this->uid), $this->equalTo('user_ldap'), - $this->equalTo(User::USER_PREFKEY_FIRSTLOGIN), - $this->equalTo(0)) - ->will($this->returnValue(0)); - $this->config->expects($this->at(1)) - ->method('getUserValue') - ->with($this->equalTo($this->uid), $this->equalTo('user_ldap'), - $this->equalTo(User::USER_PREFKEY_LASTREFRESH), - $this->equalTo(0)) - ->will($this->returnValue(0)); - $this->config->expects($this->exactly(2)) - ->method('getUserValue'); - $this->config->expects($this->never()) - ->method('setUserValue'); - - $this->user->update(); - } - - public function testUpdateAfterFirstLogin() { - $this->config->expects($this->at(0)) - ->method('getUserValue') - ->with($this->equalTo($this->uid), $this->equalTo('user_ldap'), - $this->equalTo(User::USER_PREFKEY_FIRSTLOGIN), - $this->equalTo(0)) - ->will($this->returnValue(1)); - $this->config->expects($this->at(1)) - ->method('getUserValue') - ->with($this->equalTo($this->uid), $this->equalTo('user_ldap'), - $this->equalTo(User::USER_PREFKEY_LASTREFRESH), - $this->equalTo(0)) - ->will($this->returnValue(0)); - $this->config->expects($this->exactly(2)) - ->method('getUserValue'); - $this->config->expects($this->once()) - ->method('setUserValue') - ->with($this->equalTo($this->uid), $this->equalTo('user_ldap'), - $this->equalTo(User::USER_PREFKEY_LASTREFRESH), - $this->anything()) - ->will($this->returnValue(true)); - - $this->connection->expects($this->any()) - ->method('resolveRule') - ->with('avatar') - ->willReturn(['jpegphoto', 'thumbnailphoto']); - - $this->user->update(); - } - public function extStorageHomeDataProvider() { return [ [ 'myFolder', null ], @@ -926,33 +875,6 @@ public function testUpdateExtStorageHome(string $expected, string $valueFromLDAP } - public function testUpdateNoRefresh() { - $this->config->expects($this->at(0)) - ->method('getUserValue') - ->with($this->equalTo($this->uid), $this->equalTo('user_ldap'), - $this->equalTo(User::USER_PREFKEY_FIRSTLOGIN), - $this->equalTo(0)) - ->will($this->returnValue(1)); - $this->config->expects($this->at(1)) - ->method('getUserValue') - ->with($this->equalTo($this->uid), $this->equalTo('user_ldap'), - $this->equalTo(User::USER_PREFKEY_LASTREFRESH), - $this->equalTo(0)) - ->will($this->returnValue(time() - 10)); - $this->config->expects($this->once()) - ->method('getAppValue') - ->with($this->equalTo('user_ldap'), - $this->equalTo('updateAttributesInterval'), - $this->anything()) - ->will($this->returnValue(1800)); - $this->config->expects($this->exactly(2)) - ->method('getUserValue'); - $this->config->expects($this->never()) - ->method('setUserValue'); - - $this->user->update(); - } - public function testMarkLogin() { $this->config->expects($this->once()) ->method('setUserValue')