Merge pull request #4457 from nextcloud/fix-storage-info-with-wrong-casing

Fix storage info with wrong casing

2 files changed, 92 insertions, 84 deletions

diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php
index b1a1cf1c6b7..6e34fe53eb3 100644
--- a/apps/provisioning_api/lib/Controller/UsersController.php
+++ b/apps/provisioning_api/lib/Controller/UsersController.php
@@ -31,23 +31,20 @@ namespace OCA\Provisioning_API\Controller;
 
 use OC\Accounts\AccountManager;
 use OC\Settings\Mailer\NewUserMailHelper;
-use \OC_Helper;
+use OC_Helper;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\OCS\OCSException;
 use OCP\AppFramework\OCS\OCSForbiddenException;
 use OCP\AppFramework\OCSController;
-use OCP\Defaults;
 use OCP\Files\NotFoundException;
 use OCP\IConfig;
 use OCP\IGroup;
 use OCP\IGroupManager;
 use OCP\ILogger;
 use OCP\IRequest;
-use OCP\IURLGenerator;
 use OCP\IUserManager;
 use OCP\IUserSession;
 use OCP\L10N\IFactory;
-use OCP\Mail\IMailer;
 
 class UsersController extends OCSController {
 
@@ -63,14 +60,6 @@ class UsersController extends OCSController {
 	private $accountManager;
 	/** @var ILogger */
 	private $logger;
-	/** @var string */
-	private $fromMailAddress;
-	/** @var IURLGenerator */
-	private $urlGenerator;
-	/** @var IMailer */
-	private $mailer;
-	/** @var Defaults */
-	private $defaults;
 	/** @var IFactory */
 	private $l10nFactory;
 	/** @var NewUserMailHelper */
@@ -85,10 +74,6 @@ class UsersController extends OCSController {
 	 * @param IUserSession $userSession
 	 * @param AccountManager $accountManager
 	 * @param ILogger $logger
-	 * @param string $fromMailAddress
-	 * @param IURLGenerator $urlGenerator
-	 * @param IMailer $mailer
-	 * @param Defaults $defaults
 	 * @param IFactory $l10nFactory
 	 * @param NewUserMailHelper $newUserMailHelper
 	 */
@@ -100,10 +85,6 @@ class UsersController extends OCSController {
 								IUserSession $userSession,
 								AccountManager $accountManager,
 								ILogger $logger,
-								$fromMailAddress,
-								IURLGenerator $urlGenerator,
-								IMailer $mailer,
-								Defaults $defaults,
 								IFactory $l10nFactory,
 								NewUserMailHelper $newUserMailHelper) {
 		parent::__construct($appName, $request);
@@ -114,10 +95,6 @@ class UsersController extends OCSController {
 		$this->userSession = $userSession;
 		$this->accountManager = $accountManager;
 		$this->logger = $logger;
-		$this->fromMailAddress = $fromMailAddress;
-		$this->urlGenerator = $urlGenerator;
-		$this->mailer = $mailer;
-		$this->defaults = $defaults;
 		$this->l10nFactory = $l10nFactory;
 		$this->newUserMailHelper = $newUserMailHelper;
 	}
@@ -278,10 +255,10 @@ class UsersController extends OCSController {
 		// Admin? Or SubAdmin?
 		if($this->groupManager->isAdmin($currentLoggedInUser->getUID())
 			|| $this->groupManager->getSubAdmin()->isUserAccessible($currentLoggedInUser, $targetUserObject)) {
-			$data['enabled'] = $this->config->getUserValue($userId, 'core', 'enabled', 'true');
+			$data['enabled'] = $this->config->getUserValue($targetUserObject->getUID(), 'core', 'enabled', 'true');
 		} else {
 			// Check they are looking up themselves
-			if($currentLoggedInUser->getUID() !== $userId) {
+			if($currentLoggedInUser->getUID() !== $targetUserObject->getUID()) {
 				throw new OCSException('', \OCP\API::RESPOND_UNAUTHORISED);
 			}
 		}
@@ -295,7 +272,7 @@ class UsersController extends OCSController {
 
 		// Find the data
 		$data['id'] = $targetUserObject->getUID();
-		$data['quota'] = $this->fillStorageInfo($userId);
+		$data['quota'] = $this->fillStorageInfo($targetUserObject->getUID());
 		$data[AccountManager::PROPERTY_EMAIL] = $targetUserObject->getEMailAddress();
 		$data[AccountManager::PROPERTY_DISPLAYNAME] = $targetUserObject->getDisplayName();
 		$data[AccountManager::PROPERTY_PHONE] = $userAccount[AccountManager::PROPERTY_PHONE]['value'];
@@ -330,7 +307,7 @@ class UsersController extends OCSController {
 		}
 
 		$permittedFields = [];
-		if($userId === $currentLoggedInUser->getUID()) {
+		if($targetUser->getUID() === $currentLoggedInUser->getUID()) {
 			// Editing self (display, email)
 			$permittedFields[] = 'display';
 			$permittedFields[] = AccountManager::PROPERTY_DISPLAYNAME;
@@ -618,7 +595,7 @@ class UsersController extends OCSController {
 		}
 
 		// Check they aren't removing themselves from 'admin' or their 'subadmin; group
-		if ($userId === $loggedInUser->getUID()) {
+		if ($targetUser->getUID() === $loggedInUser->getUID()) {
 			if ($this->groupManager->isAdmin($loggedInUser->getUID())) {
 				if ($group->getGID() === 'admin') {
 					throw new OCSException('Cannot remove yourself from the admin group', 105);
@@ -668,10 +645,10 @@ class UsersController extends OCSController {
 		}
 		// Check if group exists
 		if($group === null) {
-			throw new OCSException('Group:'.$groupid.' does not exist',  102);
+			throw new OCSException('Group does not exist',  102);
 		}
 		// Check if trying to make subadmin of admin group
-		if(strtolower($groupid) === 'admin') {
+		if($group->getGID() === 'admin') {
 			throw new OCSException('Cannot create subadmins for admin group', 103);
 		}
 
@@ -713,7 +690,7 @@ class UsersController extends OCSController {
 			throw new OCSException('Group does not exist', 101);
 		}
 		// Check if they are a subadmin of this said group
-		if(!$subAdminManager->isSubAdminofGroup($user, $group)) {
+		if(!$subAdminManager->isSubAdminOfGroup($user, $group)) {
 			throw new OCSException('User is not a subadmin of this group', 102);
 		}
 
diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php
index 2eb3853d333..57e1d2eac66 100644
--- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php
+++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php
@@ -70,12 +70,6 @@ class UsersControllerTest extends TestCase {
 	protected $accountManager;
 	/** @var  IRequest|PHPUnit_Framework_MockObject_MockObject */
 	protected $request;
-	/** @var IURLGenerator|PHPUnit_Framework_MockObject_MockObject */
-	private $urlGenerator;
-	/** @var IMailer|PHPUnit_Framework_MockObject_MockObject */
-	private $mailer;
-	/** @var Defaults|PHPUnit_Framework_MockObject_MockObject */
-	private $defaults;
 	/** @var IFactory|PHPUnit_Framework_MockObject_MockObject */
 	private $l10nFactory;
 	/** @var NewUserMailHelper|PHPUnit_Framework_MockObject_MockObject */
@@ -91,9 +85,6 @@ class UsersControllerTest extends TestCase {
 		$this->logger = $this->createMock(ILogger::class);
 		$this->request = $this->createMock(IRequest::class);
 		$this->accountManager = $this->createMock(AccountManager::class);
-		$this->urlGenerator = $this->createMock(IURLGenerator::class);
-		$this->mailer = $this->createMock(IMailer::class);
-		$this->defaults = $this->createMock(Defaults::class);
 		$this->l10nFactory = $this->createMock(IFactory::class);
 		$this->newUserMailHelper = $this->createMock(NewUserMailHelper::class);
 
@@ -107,10 +98,6 @@ class UsersControllerTest extends TestCase {
 				$this->userSession,
 				$this->accountManager,
 				$this->logger,
-				'test@example.org',
-				$this->urlGenerator,
-				$this->mailer,
-				$this->defaults,
 				$this->l10nFactory,
 				$this->newUserMailHelper
 			])
@@ -705,19 +692,19 @@ class UsersControllerTest extends TestCase {
 		$this->config
 			->expects($this->at(0))
 			->method('getUserValue')
-			->with('UserToGet', 'core', 'enabled', 'true')
+			->with('UID', 'core', 'enabled', 'true')
 			->will($this->returnValue('true'));
 		$this->api
 			->expects($this->once())
 			->method('fillStorageInfo')
-			->with('UserToGet')
+			->with('UID')
 			->will($this->returnValue(['DummyValue']));
 		$targetUser
 			->expects($this->once())
 			->method('getDisplayName')
 			->will($this->returnValue('Demo User'));
 		$targetUser
-			->expects($this->once())
+			->expects($this->exactly(3))
 			->method('getUID')
 			->will($this->returnValue('UID'));
 
@@ -784,19 +771,19 @@ class UsersControllerTest extends TestCase {
 		$this->config
 			->expects($this->at(0))
 			->method('getUserValue')
-			->with('UserToGet', 'core', 'enabled', 'true')
+			->with('UID', 'core', 'enabled', 'true')
 			->will($this->returnValue('true'));
 		$this->api
 			->expects($this->once())
 			->method('fillStorageInfo')
-			->with('UserToGet')
+			->with('UID')
 			->will($this->returnValue(['DummyValue']));
 		$targetUser
 			->expects($this->once())
 			->method('getDisplayName')
 			->will($this->returnValue('Demo User'));
 		$targetUser
-			->expects($this->once())
+			->expects($this->exactly(3))
 			->method('getUID')
 			->will($this->returnValue('UID'));
 		$this->accountManager->expects($this->any())->method('getUser')
@@ -878,7 +865,7 @@ class UsersControllerTest extends TestCase {
 		$loggedInUser
 			->expects($this->exactly(2))
 			->method('getUID')
-			->will($this->returnValue('subadmin'));
+			->will($this->returnValue('UID'));
 		$targetUser = $this->getMockBuilder(IUser::class)
 			->disableOriginalConstructor()
 			->getMock();
@@ -894,7 +881,7 @@ class UsersControllerTest extends TestCase {
 		$this->groupManager
 			->expects($this->once())
 			->method('isAdmin')
-			->with('subadmin')
+			->with('UID')
 			->will($this->returnValue(false));
 		$subAdminManager = $this->getMockBuilder('OC\SubAdmin')
 			->disableOriginalConstructor()
@@ -915,7 +902,7 @@ class UsersControllerTest extends TestCase {
 		$this->api
 			->expects($this->once())
 			->method('fillStorageInfo')
-			->with('subadmin')
+			->with('UID')
 			->will($this->returnValue(['DummyValue']));
 		$targetUser
 			->expects($this->once())
@@ -926,7 +913,7 @@ class UsersControllerTest extends TestCase {
 			->method('getEMailAddress')
 			->will($this->returnValue('subadmin@owncloud.org'));
 		$targetUser
-			->expects($this->once())
+			->expects($this->exactly(3))
 			->method('getUID')
 			->will($this->returnValue('UID'));
 		$this->accountManager->expects($this->any())->method('getUser')
@@ -961,7 +948,7 @@ class UsersControllerTest extends TestCase {
 		$loggedInUser
 			->expects($this->any())
 			->method('getUID')
-			->will($this->returnValue('UserToEdit'));
+			->will($this->returnValue('UID'));
 		$targetUser = $this->getMockBuilder(IUser::class)
 			->disableOriginalConstructor()
 			->getMock();
@@ -978,6 +965,10 @@ class UsersControllerTest extends TestCase {
 			->expects($this->once())
 			->method('setDisplayName')
 			->with('NewDisplayName');
+		$targetUser
+			->expects($this->any())
+			->method('getUID')
+			->will($this->returnValue('UID'));
 
 		$this->assertEquals([], $this->api->editUser('UserToEdit', 'display', 'NewDisplayName')->getData());
 	}
@@ -989,7 +980,7 @@ class UsersControllerTest extends TestCase {
 		$loggedInUser
 			->expects($this->any())
 			->method('getUID')
-			->will($this->returnValue('UserToEdit'));
+			->will($this->returnValue('UID'));
 		$targetUser = $this->getMockBuilder(IUser::class)
 			->disableOriginalConstructor()
 			->getMock();
@@ -1006,6 +997,10 @@ class UsersControllerTest extends TestCase {
 			->expects($this->once())
 			->method('setEMailAddress')
 			->with('demo@owncloud.org');
+		$targetUser
+			->expects($this->any())
+			->method('getUID')
+			->will($this->returnValue('UID'));
 
 		$this->assertEquals([], $this->api->editUser('UserToEdit', 'email', 'demo@owncloud.org')->getData());
 	}
@@ -1022,7 +1017,7 @@ class UsersControllerTest extends TestCase {
 		$loggedInUser
 			->expects($this->any())
 			->method('getUID')
-			->will($this->returnValue('UserToEdit'));
+			->will($this->returnValue('UID'));
 		$targetUser = $this->getMockBuilder(IUser::class)
 			->disableOriginalConstructor()
 			->getMock();
@@ -1035,6 +1030,10 @@ class UsersControllerTest extends TestCase {
 			->method('get')
 			->with('UserToEdit')
 			->will($this->returnValue($targetUser));
+		$targetUser
+			->expects($this->any())
+			->method('getUID')
+			->will($this->returnValue('UID'));
 
 		$this->api->editUser('UserToEdit', 'email', 'demo.org');
 	}
@@ -1046,7 +1045,7 @@ class UsersControllerTest extends TestCase {
 		$loggedInUser
 			->expects($this->any())
 			->method('getUID')
-			->will($this->returnValue('UserToEdit'));
+			->will($this->returnValue('UID'));
 		$targetUser = $this->getMockBuilder(IUser::class)
 			->disableOriginalConstructor()
 			->getMock();
@@ -1063,6 +1062,10 @@ class UsersControllerTest extends TestCase {
 			->expects($this->once())
 			->method('setPassword')
 			->with('NewPassword');
+		$targetUser
+			->expects($this->any())
+			->method('getUID')
+			->will($this->returnValue('UID'));
 
 		$this->assertEquals([], $this->api->editUser('UserToEdit', 'password', 'NewPassword')->getData());
 	}
@@ -1079,7 +1082,7 @@ class UsersControllerTest extends TestCase {
 		$loggedInUser
 			->expects($this->any())
 			->method('getUID')
-			->will($this->returnValue('UserToEdit'));
+			->will($this->returnValue('UID'));
 		$targetUser = $this->getMockBuilder(IUser::class)
 			->disableOriginalConstructor()
 			->getMock();
@@ -1092,6 +1095,10 @@ class UsersControllerTest extends TestCase {
 			->method('get')
 			->with('UserToEdit')
 			->will($this->returnValue($targetUser));
+		$targetUser
+			->expects($this->any())
+			->method('getUID')
+			->will($this->returnValue('UID'));
 
 		$this->api->editUser('UserToEdit', 'quota', 'NewQuota');
 	}
@@ -1101,7 +1108,7 @@ class UsersControllerTest extends TestCase {
 		$loggedInUser
 			->expects($this->any())
 			->method('getUID')
-			->will($this->returnValue('UserToEdit'));
+			->will($this->returnValue('UID'));
 		$targetUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock();
 		$targetUser->expects($this->once())
 			->method('setQuota')
@@ -1118,8 +1125,12 @@ class UsersControllerTest extends TestCase {
 		$this->groupManager
 			->expects($this->once())
 			->method('isAdmin')
-			->with('UserToEdit')
+			->with('UID')
 			->will($this->returnValue(true));
+		$targetUser
+			->expects($this->any())
+			->method('getUID')
+			->will($this->returnValue('UID'));
 
 		$this->assertEquals([], $this->api->editUser('UserToEdit', 'quota', '3042824')->getData());
 	}
@@ -1135,7 +1146,7 @@ class UsersControllerTest extends TestCase {
 		$loggedInUser
 			->expects($this->any())
 			->method('getUID')
-			->will($this->returnValue('UserToEdit'));
+			->will($this->returnValue('UID'));
 		$targetUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock();
 		$this->userSession
 			->expects($this->once())
@@ -1149,8 +1160,12 @@ class UsersControllerTest extends TestCase {
 		$this->groupManager
 			->expects($this->once())
 			->method('isAdmin')
-			->with('UserToEdit')
+			->with('UID')
 			->will($this->returnValue(true));
+		$targetUser
+			->expects($this->any())
+			->method('getUID')
+			->will($this->returnValue('UID'));
 
 		$this->api->editUser('UserToEdit', 'quota', 'ABC');
 	}
@@ -1186,6 +1201,10 @@ class UsersControllerTest extends TestCase {
 			->expects($this->once())
 			->method('getSubAdmin')
 			->will($this->returnValue($subAdminManager));
+		$targetUser
+			->expects($this->any())
+			->method('getUID')
+			->will($this->returnValue('UID'));
 
 		$this->assertEquals([], $this->api->editUser('UserToEdit', 'quota', '3042824')->getData());
 	}
@@ -1221,6 +1240,10 @@ class UsersControllerTest extends TestCase {
 			->expects($this->once())
 			->method('getSubAdmin')
 			->will($this->returnValue($subAdminManager));
+		$targetUser
+			->expects($this->any())
+			->method('getUID')
+			->will($this->returnValue('UID'));
 
 		$this->assertEquals([], $this->api->editUser('UserToEdit', 'quota', '3042824')->getData());
 	}
@@ -1257,6 +1280,10 @@ class UsersControllerTest extends TestCase {
 			->expects($this->once())
 			->method('getSubAdmin')
 			->will($this->returnValue($subAdminManager));
+		$targetUser
+			->expects($this->any())
+			->method('getUID')
+			->will($this->returnValue('UID'));
 
 		$this->api->editUser('UserToEdit', 'quota', 'value');
 	}
@@ -1293,12 +1320,12 @@ class UsersControllerTest extends TestCase {
 		$loggedInUser
 			->expects($this->any())
 			->method('getUID')
-			->will($this->returnValue('UserToDelete'));
+			->will($this->returnValue('UID'));
 		$targetUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock();
 		$targetUser
 			->expects($this->once())
 			->method('getUID')
-			->will($this->returnValue('UserToDelete'));
+			->will($this->returnValue('UID'));
 		$this->userSession
 			->expects($this->once())
 			->method('getUser')
@@ -1322,7 +1349,7 @@ class UsersControllerTest extends TestCase {
 		$targetUser
 			->expects($this->once())
 			->method('getUID')
-			->will($this->returnValue('UserToDelete'));
+			->will($this->returnValue('UID'));
 		$this->userSession
 			->expects($this->once())
 			->method('getUser')
@@ -1359,7 +1386,7 @@ class UsersControllerTest extends TestCase {
 		$targetUser
 			->expects($this->once())
 			->method('getUID')
-			->will($this->returnValue('UserToDelete'));
+			->will($this->returnValue('UID'));
 		$this->userSession
 			->expects($this->once())
 			->method('getUser')
@@ -1392,7 +1419,7 @@ class UsersControllerTest extends TestCase {
 		$targetUser
 			->expects($this->once())
 			->method('getUID')
-			->will($this->returnValue('UserToDelete'));
+			->will($this->returnValue('UID'));
 		$this->userSession
 			->expects($this->once())
 			->method('getUser')
@@ -1440,7 +1467,7 @@ class UsersControllerTest extends TestCase {
 		$targetUser
 			->expects($this->once())
 			->method('getUID')
-			->will($this->returnValue('UserToDelete'));
+			->will($this->returnValue('UID'));
 		$this->userSession
 			->expects($this->once())
 			->method('getUser')
@@ -1488,7 +1515,7 @@ class UsersControllerTest extends TestCase {
 		$targetUser
 			->expects($this->once())
 			->method('getUID')
-			->will($this->returnValue('UserToDelete'));
+			->will($this->returnValue('UID'));
 		$this->userSession
 			->expects($this->once())
 			->method('getUser')
@@ -1984,6 +2011,10 @@ class UsersControllerTest extends TestCase {
 			->method('getUID')
 			->will($this->returnValue('admin'));
 		$targetUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock();
+		$targetUser
+			->expects($this->once())
+			->method('getUID')
+			->will($this->returnValue('admin'));
 		$targetGroup = $this->getMockBuilder('\OCP\IGroup')->disableOriginalConstructor()->getMock();
 		$targetGroup
 			->expects($this->once())
@@ -2001,7 +2032,7 @@ class UsersControllerTest extends TestCase {
 		$this->userManager
 			->expects($this->once())
 			->method('get')
-			->with('admin')
+			->with('Admin')
 			->will($this->returnValue($targetUser));
 		$subAdminManager = $this->getMockBuilder('OC\SubAdmin')
 			->disableOriginalConstructor()->getMock();
@@ -2015,7 +2046,7 @@ class UsersControllerTest extends TestCase {
 			->with('admin')
 			->will($this->returnValue(true));
 
-		$this->api->removeFromGroup('admin', 'admin');
+		$this->api->removeFromGroup('Admin', 'admin');
 	}
 
 	/**
@@ -2030,6 +2061,10 @@ class UsersControllerTest extends TestCase {
 			->method('getUID')
 			->will($this->returnValue('subadmin'));
 		$targetUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock();
+		$targetUser
+			->expects($this->once())
+			->method('getUID')
+			->will($this->returnValue('subadmin'));
 		$targetGroup = $this->getMockBuilder('\OCP\IGroup')->disableOriginalConstructor()->getMock();
 		$targetGroup
 			->expects($this->any())
@@ -2047,7 +2082,7 @@ class UsersControllerTest extends TestCase {
 		$this->userManager
 			->expects($this->once())
 			->method('get')
-			->with('subadmin')
+			->with('SubAdmin')
 			->will($this->returnValue($targetUser));
 		$subAdminManager = $this->getMockBuilder('OC\SubAdmin')
 			->disableOriginalConstructor()->getMock();
@@ -2066,7 +2101,7 @@ class UsersControllerTest extends TestCase {
 			->with('subadmin')
 			->will($this->returnValue(false));
 
-		$this->api->removeFromGroup('subadmin', 'subadmin');
+		$this->api->removeFromGroup('SubAdmin', 'subadmin');
 	}
 
 	/**
@@ -2190,7 +2225,7 @@ class UsersControllerTest extends TestCase {
 	/**
 	 * @expectedException \OCP\AppFramework\OCS\OCSException
 	 * @expectedExceptionCode 102
-	 * @expectedExceptionMessage Group:NotExistingGroup does not exist
+	 * @expectedExceptionMessage Group does not exist
 	 */
 	public function testAddSubAdminWithNotExistingTargetGroup() {
 
@@ -2217,6 +2252,10 @@ class UsersControllerTest extends TestCase {
 	public function testAddSubAdminToAdminGroup() {
 		$targetUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock();
 		$targetGroup = $this->getMockBuilder('\OCP\IGroup')->disableOriginalConstructor()->getMock();
+		$targetGroup
+			->expects($this->once())
+			->method('getGID')
+			->will($this->returnValue('admin'));
 		$this->userManager
 			->expects($this->once())
 			->method('get')
@@ -2612,10 +2651,6 @@ class UsersControllerTest extends TestCase {
 				$this->userSession,
 				$this->accountManager,
 				$this->logger,
-				'',
-				$this->urlGenerator,
-				$this->mailer,
-				$this->defaults,
 				$this->l10nFactory,
 				$this->newUserMailHelper
 			])
@@ -2676,10 +2711,6 @@ class UsersControllerTest extends TestCase {
 				$this->userSession,
 				$this->accountManager,
 				$this->logger,
-				'',
-				$this->urlGenerator,
-				$this->mailer,
-				$this->defaults,
 				$this->l10nFactory,
 				$this->newUserMailHelper
 			])