Merge pull request #4143 from nextcloud/ldap-downstream-64

Correctly apply quota

2 files changed, 366 insertions, 32 deletions

diff --git a/apps/user_ldap/lib/User/User.php b/apps/user_ldap/lib/User/User.php
index e29b10616ca..5d4af1fd09c 100644
--- a/apps/user_ldap/lib/User/User.php
+++ b/apps/user_ldap/lib/User/User.php
@@ -169,6 +169,10 @@ class User {
 		$attr = strtolower($this->connection->ldapQuotaAttribute);
 		if(isset($ldapEntry[$attr])) {
 			$this->updateQuota($ldapEntry[$attr][0]);
+		} else {
+			if ($this->connection->ldapQuotaDefault !== '') {
+				$this->updateQuota();
+			}
 		}
 		unset($attr);
 
@@ -455,6 +459,20 @@ class User {
 	}
 
 	/**
+	 * Overall process goes as follow:
+	 * 1. fetch the quota from LDAP and check if it's parseable with the "verifyQuotaValue" function
+	 * 2. if the value can't be fetched, is empty or not parseable, use the default LDAP quota
+	 * 3. if the default LDAP quota can't be parsed, use the ownCloud's default quota (use 'default')
+	 * 4. check if the target user exists and set the quota for the user.
+	 *
+	 * In order to improve performance and prevent an unwanted extra LDAP call, the $valueFromLDAP
+	 * parameter can be passed with the value of the attribute. This value will be considered as the
+	 * quota for the user coming from the LDAP server (step 1 of the process) It can be useful to
+	 * fetch all the user's attributes in one call and use the fetched values in this function.
+	 * The expected value for that parameter is a string describing the quota for the user. Valid
+	 * values are 'none' (unlimited), 'default' (the ownCloud's default quota), '1234' (quota in
+	 * bytes), '1234 MB' (quota in MB - check the \OC_Helper::computerFileSize method for more info)
+	 *
 	 * fetches the quota from LDAP and stores it as ownCloud user value
 	 * @param string $valueFromLDAP the quota attribute's value can be passed,
 	 * to save the readAttribute request
@@ -464,23 +482,51 @@ class User {
 		if($this->wasRefreshed('quota')) {
 			return;
 		}
-		//can be null
-		$quotaDefault = $this->connection->ldapQuotaDefault;
-		$quota = $quotaDefault !== '' ? $quotaDefault : null;
-		$quota = !is_null($valueFromLDAP) ? $valueFromLDAP : $quota;
 
+		$quota = false;
 		if(is_null($valueFromLDAP)) {
 			$quotaAttribute = $this->connection->ldapQuotaAttribute;
 			if ($quotaAttribute !== '') {
 				$aQuota = $this->access->readAttribute($this->dn, $quotaAttribute);
 				if($aQuota && (count($aQuota) > 0)) {
-					$quota = $aQuota[0];
+					if ($this->verifyQuotaValue($aQuota[0])) {
+						$quota = $aQuota[0];
+					} else {
+						$this->log->log('not suitable LDAP quota found for user ' . $this->uid . ': [' . $aQuota[0] . ']', \OCP\Util::WARN);
+					}
 				}
 			}
+		} else {
+			if ($this->verifyQuotaValue($valueFromLDAP)) {
+				$quota = $valueFromLDAP;
+			} else {
+				$this->log->log('not suitable LDAP quota found for user ' . $this->uid . ': [' . $valueFromLDAP . ']', \OCP\Util::WARN);
+			}
 		}
-		if(!is_null($quota)) {
-			$this->userManager->get($this->uid)->setQuota($quota);
+
+		if ($quota === false) {
+			// quota not found using the LDAP attribute (or not parseable). Try the default quota
+			$defaultQuota = $this->connection->ldapQuotaDefault;
+			if ($this->verifyQuotaValue($defaultQuota)) {
+				$quota = $defaultQuota;
+			}
 		}
+
+		$targetUser = $this->userManager->get($this->uid);
+		if ($targetUser) {
+			if($quota !== false) {
+				$targetUser->setQuota($quota);
+			} else {
+				$this->log->log('not suitable default quota found for user ' . $this->uid . ': [' . $defaultQuota . ']', \OCP\Util::WARN);
+				$targetUser->setQuota('default');
+			}
+		} else {
+			$this->log->log('trying to set a quota for user ' . $this->uid . ' but the user is missing', \OCP\Util::ERROR);
+		}
+	}
+
+	private function verifyQuotaValue($quotaValue) {
+		return $quotaValue === 'none' || $quotaValue === 'default' || \OC_Helper::computerFileSize($quotaValue) !== false;
 	}
 
 	/**
diff --git a/apps/user_ldap/tests/User/UserTest.php b/apps/user_ldap/tests/User/UserTest.php
index c261a2abaa2..6961e39d9f0 100644
--- a/apps/user_ldap/tests/User/UserTest.php
+++ b/apps/user_ldap/tests/User/UserTest.php
@@ -211,15 +211,17 @@ class UserTest extends \Test\TestCase {
 
 		$connection->expects($this->at(0))
 			->method('__get')
-			->with($this->equalTo('ldapQuotaDefault'))
-			->will($this->returnValue('23 GB'));
+			->with($this->equalTo('ldapQuotaAttribute'))
+			->will($this->returnValue('myquota'));
 
+		/* Having a quota defined, the ldapQuotaDefault won't be used
 		$connection->expects($this->at(1))
 			->method('__get')
-			->with($this->equalTo('ldapQuotaAttribute'))
-			->will($this->returnValue('myquota'));
+			->with($this->equalTo('ldapQuotaDefault'))
+			->will($this->returnValue('23 GB'));
+		*/
 
-		$connection->expects($this->exactly(2))
+		$connection->expects($this->exactly(1))
 			->method('__get');
 
 		$access->expects($this->once())
@@ -247,7 +249,7 @@ class UserTest extends \Test\TestCase {
 		$user->updateQuota();
 	}
 
-	public function testUpdateQuotaDefaultProvided() {
+	public function testUpdateQuotaToDefaultAllProvided() {
 		list($access, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr) =
 			$this->getTestInstances();
 
@@ -256,14 +258,94 @@ class UserTest extends \Test\TestCase {
 
 		$connection->expects($this->at(0))
 			->method('__get')
-			->with($this->equalTo('ldapQuotaDefault'))
-			->will($this->returnValue('25 GB'));
+			->with($this->equalTo('ldapQuotaAttribute'))
+			->will($this->returnValue('myquota'));
 
-		$connection->expects($this->at(1))
+		$connection->expects($this->exactly(1))
+			->method('__get');
+
+		$access->expects($this->once())
+			->method('readAttribute')
+			->with($this->equalTo('uid=alice,dc=foo,dc=bar'),
+				$this->equalTo('myquota'))
+			->will($this->returnValue(array('default')));
+
+		$user = $this->createMock('\OCP\IUser');
+		$user->expects($this->once())
+			->method('setQuota')
+			->with('default');
+
+		$userMgr->expects($this->once())
+			->method('get')
+			->with('alice')
+			->will($this->returnValue($user));
+
+		$uid = 'alice';
+		$dn  = 'uid=alice,dc=foo,dc=bar';
+
+		$user = new User(
+			$uid, $dn, $access, $config, $filesys, $image, $log, $avaMgr, $userMgr);
+
+		$user->updateQuota();
+	}
+
+	public function testUpdateQuotaToNoneAllProvided() {
+		list($access, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr) =
+			$this->getTestInstances();
+
+		list($access, $connection) =
+			$this->getAdvancedMocks($config, $filesys, $log, $avaMgr, $dbc);
+
+		$connection->expects($this->at(0))
 			->method('__get')
 			->with($this->equalTo('ldapQuotaAttribute'))
 			->will($this->returnValue('myquota'));
 
+		$connection->expects($this->exactly(1))
+			->method('__get');
+
+		$access->expects($this->once())
+			->method('readAttribute')
+			->with($this->equalTo('uid=alice,dc=foo,dc=bar'),
+				$this->equalTo('myquota'))
+			->will($this->returnValue(array('none')));
+
+		$user = $this->createMock('\OCP\IUser');
+		$user->expects($this->once())
+			->method('setQuota')
+			->with('none');
+
+		$userMgr->expects($this->once())
+			->method('get')
+			->with('alice')
+			->will($this->returnValue($user));
+
+		$uid = 'alice';
+		$dn  = 'uid=alice,dc=foo,dc=bar';
+
+		$user = new User(
+			$uid, $dn, $access, $config, $filesys, $image, $log, $avaMgr, $userMgr);
+
+		$user->updateQuota();
+	}
+
+	public function testUpdateQuotaDefaultProvided() {
+		list($access, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr) =
+			$this->getTestInstances();
+
+		list($access, $connection) =
+			$this->getAdvancedMocks($config, $filesys, $log, $avaMgr, $dbc);
+
+		$connection->expects($this->at(0))
+			->method('__get')
+			->with($this->equalTo('ldapQuotaAttribute'))
+			->will($this->returnValue('myquota'));
+
+		$connection->expects($this->at(1))
+			->method('__get')
+			->with($this->equalTo('ldapQuotaDefault'))
+			->will($this->returnValue('25 GB'));
+
 		$connection->expects($this->exactly(2))
 			->method('__get');
 
@@ -301,15 +383,17 @@ class UserTest extends \Test\TestCase {
 
 		$connection->expects($this->at(0))
 			->method('__get')
-			->with($this->equalTo('ldapQuotaDefault'))
-			->will($this->returnValue(''));
+			->with($this->equalTo('ldapQuotaAttribute'))
+			->will($this->returnValue('myquota'));
 
+		/* Having a quota set this won't be used
 		$connection->expects($this->at(1))
 			->method('__get')
-			->with($this->equalTo('ldapQuotaAttribute'))
-			->will($this->returnValue('myquota'));
+			->with($this->equalTo('ldapQuotaDefault'))
+			->will($this->returnValue(''));
+		*/
 
-		$connection->expects($this->exactly(2))
+		$connection->expects($this->exactly(1))
 			->method('__get');
 
 		$access->expects($this->once())
@@ -346,13 +430,13 @@ class UserTest extends \Test\TestCase {
 
 		$connection->expects($this->at(0))
 			->method('__get')
-			->with($this->equalTo('ldapQuotaDefault'))
-			->will($this->returnValue(''));
+			->with($this->equalTo('ldapQuotaAttribute'))
+			->will($this->returnValue('myquota'));
 
 		$connection->expects($this->at(1))
 			->method('__get')
-			->with($this->equalTo('ldapQuotaAttribute'))
-			->will($this->returnValue('myquota'));
+			->with($this->equalTo('ldapQuotaDefault'))
+			->will($this->returnValue(''));
 
 		$connection->expects($this->exactly(2))
 			->method('__get');
@@ -363,6 +447,16 @@ class UserTest extends \Test\TestCase {
 				$this->equalTo('myquota'))
 			->will($this->returnValue(false));
 
+		$user = $this->createMock('\OCP\IUser');
+		$user->expects($this->once())
+			->method('setQuota')
+			->with('default');
+
+		$userMgr->expects($this->once())
+			->method('get')
+			->with('alice')
+			->will($this->returnValue($user));
+
 		$config->expects($this->never())
 			->method('setUserValue');
 
@@ -384,17 +478,28 @@ class UserTest extends \Test\TestCase {
 
 		$connection->expects($this->at(0))
 			->method('__get')
-			->with($this->equalTo('ldapQuotaDefault'))
+			->with($this->equalTo('ldapQuotaAttribute'))
 			->will($this->returnValue(''));
 
 		$connection->expects($this->at(1))
 			->method('__get')
-			->with($this->equalTo('ldapQuotaAttribute'))
+			->with($this->equalTo('ldapQuotaDefault'))
 			->will($this->returnValue(''));
 
 		$connection->expects($this->exactly(2))
 			->method('__get');
 
+		$user = $this->createMock('\OCP\IUser');
+		$user->expects($this->once())
+			->method('setQuota')
+			->with('default');
+
+		$userMgr->expects($this->once())
+			->method('get')
+			->with('alice')
+			->will($this->returnValue($user));
+
+
 		$access->expects($this->never())
 			->method('readAttribute');
 
@@ -419,23 +524,206 @@ class UserTest extends \Test\TestCase {
 
 		$readQuota = '19 GB';
 
+		$connection->expects($this->never())
+			->method('__get')
+			->with($this->equalTo('ldapQuotaDefault'));
+
+		$access->expects($this->never())
+			->method('readAttribute');
+
+		$user = $this->createMock(IUser::class);
+		$user->expects($this->once())
+			->method('setQuota')
+			->with($readQuota);
+
+		$userMgr->expects($this->once())
+			->method('get')
+			->with('alice')
+			->will($this->returnValue($user));
+
+		$uid = 'alice';
+		$dn  = 'uid=alice,dc=foo,dc=bar';
+
+		$user = new User(
+			$uid, $dn, $access, $config, $filesys, $image, $log, $avaMgr, $userMgr);
+
+		$user->updateQuota($readQuota);
+	}
+
+	/**
+	 * Unparseable quota will fallback to use the LDAP default
+	 */
+	public function testUpdateWrongQuotaAllProvided() {
+		list($access, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr) =
+			$this->getTestInstances();
+
+		list($access, $connection) =
+			$this->getAdvancedMocks($config, $filesys, $log, $avaMgr, $dbc);
+
+		$connection->expects($this->at(0))
+			->method('__get')
+			->with($this->equalTo('ldapQuotaAttribute'))
+			->will($this->returnValue('myquota'));
+
+		$connection->expects($this->at(1))
+			->method('__get')
+			->with($this->equalTo('ldapQuotaDefault'))
+			->will($this->returnValue('23 GB'));
+
+		$connection->expects($this->exactly(2))
+			->method('__get');
+
+		$access->expects($this->once())
+			->method('readAttribute')
+			->with($this->equalTo('uid=alice,dc=foo,dc=bar'),
+				$this->equalTo('myquota'))
+			->will($this->returnValue(array('42 GBwos')));
+
+		$user = $this->createMock('\OCP\IUser');
+		$user->expects($this->once())
+			->method('setQuota')
+			->with('23 GB');
+
+		$userMgr->expects($this->once())
+			->method('get')
+			->with('alice')
+			->will($this->returnValue($user));
+
+		$uid = 'alice';
+		$dn  = 'uid=alice,dc=foo,dc=bar';
+
+		$user = new User(
+			$uid, $dn, $access, $config, $filesys, $image, $log, $avaMgr, $userMgr);
+
+		$user->updateQuota();
+	}
+
+	/**
+	 * No user quota and wrong default will set 'default' as quota
+	 */
+	public function testUpdateWrongDefaultQuotaProvided() {
+		list($access, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr) =
+			$this->getTestInstances();
+
+		list($access, $connection) =
+			$this->getAdvancedMocks($config, $filesys, $log, $avaMgr, $dbc);
+
+		$connection->expects($this->at(0))
+			->method('__get')
+			->with($this->equalTo('ldapQuotaAttribute'))
+			->will($this->returnValue('myquota'));
+
+		$connection->expects($this->at(1))
+			->method('__get')
+			->with($this->equalTo('ldapQuotaDefault'))
+			->will($this->returnValue('23 GBwowowo'));
+
+		$connection->expects($this->exactly(2))
+			->method('__get');
+
+		$access->expects($this->once())
+			->method('readAttribute')
+			->with($this->equalTo('uid=alice,dc=foo,dc=bar'),
+				$this->equalTo('myquota'))
+			->will($this->returnValue(false));
+
+		$user = $this->createMock('\OCP\IUser');
+		$user->expects($this->once())
+			->method('setQuota')
+			->with('default');
+
+		$userMgr->expects($this->once())
+			->method('get')
+			->with('alice')
+			->will($this->returnValue($user));
+
+		$uid = 'alice';
+		$dn  = 'uid=alice,dc=foo,dc=bar';
+
+		$user = new User(
+			$uid, $dn, $access, $config, $filesys, $image, $log, $avaMgr, $userMgr);
+
+		$user->updateQuota();
+	}
+
+	/**
+	 * Wrong user quota and wrong default will set 'default' as quota
+	 */
+	public function testUpdateWrongQuotaAndDefaultAllProvided() {
+		list($access, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr) =
+			$this->getTestInstances();
+
+		list($access, $connection) =
+			$this->getAdvancedMocks($config, $filesys, $log, $avaMgr, $dbc);
+
 		$connection->expects($this->at(0))
 			->method('__get')
+			->with($this->equalTo('ldapQuotaAttribute'))
+			->will($this->returnValue('myquota'));
+
+		$connection->expects($this->at(1))
+			->method('__get')
 			->with($this->equalTo('ldapQuotaDefault'))
+			->will($this->returnValue('23 GBwowowo'));
+
+		$connection->expects($this->exactly(2))
+			->method('__get');
+
+		$access->expects($this->once())
+			->method('readAttribute')
+			->with($this->equalTo('uid=alice,dc=foo,dc=bar'),
+				$this->equalTo('myquota'))
+			->will($this->returnValue(array('23 flush')));
+
+		$user = $this->createMock('\OCP\IUser');
+		$user->expects($this->once())
+			->method('setQuota')
+			->with('default');
+
+		$userMgr->expects($this->once())
+			->method('get')
+			->with('alice')
+			->will($this->returnValue($user));
+
+		$uid = 'alice';
+		$dn  = 'uid=alice,dc=foo,dc=bar';
+
+		$user = new User(
+			$uid, $dn, $access, $config, $filesys, $image, $log, $avaMgr, $userMgr);
+
+		$user->updateQuota();
+	}
+
+	/**
+	 * No quota attribute set and wrong default will set 'default' as quota
+	 */
+	public function testUpdateWrongDefaultQuotaNotProvided() {
+		list($access, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr) =
+			$this->getTestInstances();
+
+		list($access, $connection) =
+			$this->getAdvancedMocks($config, $filesys, $log, $avaMgr, $dbc);
+
+		$connection->expects($this->at(0))
+			->method('__get')
+			->with($this->equalTo('ldapQuotaAttribute'))
 			->will($this->returnValue(''));
 
-		$connection->expects($this->once(1))
+		$connection->expects($this->at(1))
 			->method('__get')
 			->with($this->equalTo('ldapQuotaDefault'))
-			->will($this->returnValue(null));
+			->will($this->returnValue('23 GBwowowo'));
+
+		$connection->expects($this->exactly(2))
+			->method('__get');
 
 		$access->expects($this->never())
 			->method('readAttribute');
 
-		$user = $this->createMock(IUser::class);
+		$user = $this->createMock('\OCP\IUser');
 		$user->expects($this->once())
 			->method('setQuota')
-			->with($readQuota);
+			->with('default');
 
 		$userMgr->expects($this->once())
 			->method('get')
@@ -448,7 +736,7 @@ class UserTest extends \Test\TestCase {
 		$user = new User(
 			$uid, $dn, $access, $config, $filesys, $image, $log, $avaMgr, $userMgr);
 
-		$user->updateQuota($readQuota);
+		$user->updateQuota();
 	}
 
 	//the testUpdateAvatar series also implicitely tests getAvatarImage