Fix the storage info and other checks when the user has wrong casing

Signed-off-by: Joas Schilling <coding@schilljs.com>

diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php
index 94d05195d160b5584031ed81ebab45c18a603ebe..c58c5f25e26474c3f06904859fccceb1ef41b407 100644 (file)
--- a/apps/provisioning_api/lib/Controller/UsersController.php
+++ b/apps/provisioning_api/lib/Controller/UsersController.php
@@ -243,10 +243,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);
                        }
                }
@@ -255,7 +255,7 @@ class UsersController extends OCSController {
 
                // Find the data
                $data['id'] = $targetUserObject->getUID();
-               $data['quota'] = $this->fillStorageInfo($userId);
+               $data['quota'] = $this->fillStorageInfo($targetUserObject->getUID());
                $data['email'] = $targetUserObject->getEMailAddress();
                $data['displayname'] = $targetUserObject->getDisplayName();
                $data['phone'] = $userAccount[\OC\Accounts\AccountManager::PROPERTY_PHONE]['value'];
@@ -289,7 +289,7 @@ class UsersController extends OCSController {
                }
 
                $permittedFields = [];
-               if($userId === $currentLoggedInUser->getUID()) {
+               if($targetUser->getUID() === $currentLoggedInUser->getUID()) {
                        // Editing self (display, email)
                        $permittedFields[] = 'display';
                        $permittedFields[] = 'email';
@@ -545,9 +545,9 @@ class UsersController extends OCSController {
                        throw new OCSException('', 104);
                }
                // Check they aren't removing themselves from 'admin' or their 'subadmin; group
-               if($userId === $loggedInUser->getUID()) {
-                       if($this->groupManager->isAdmin($loggedInUser->getUID())) {
-                               if($group->getGID() === 'admin') {
+               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);
                                }
                        } else {

diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php
index 6c05395243baf5025f9029c6047320ab618e5953..0dd8ecd485715a95e50e01e5beab764c5651e4cb 100644 (file)
--- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php
+++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php
@@ -673,19 +673,19 @@ class UsersControllerTest extends OriginalTest {
                $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'));
 
@@ -747,19 +747,19 @@ class UsersControllerTest extends OriginalTest {
                $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')
@@ -840,7 +840,7 @@ class UsersControllerTest extends OriginalTest {
                $loggedInUser
                        ->expects($this->exactly(2))
                        ->method('getUID')
-                       ->will($this->returnValue('subadmin'));
+                       ->will($this->returnValue('UID'));
                $targetUser = $this->getMockBuilder('OCP\IUser')
                        ->disableOriginalConstructor()
                        ->getMock();
@@ -856,7 +856,7 @@ class UsersControllerTest extends OriginalTest {
                $this->groupManager
                        ->expects($this->once())
                        ->method('isAdmin')
-                       ->with('subadmin')
+                       ->with('UID')
                        ->will($this->returnValue(false));
                $subAdminManager = $this->getMockBuilder('OC\SubAdmin')
                        ->disableOriginalConstructor()
@@ -873,7 +873,7 @@ class UsersControllerTest extends OriginalTest {
                $this->api
                        ->expects($this->once())
                        ->method('fillStorageInfo')
-                       ->with('subadmin')
+                       ->with('UID')
                        ->will($this->returnValue(['DummyValue']));
                $targetUser
                        ->expects($this->once())
@@ -884,7 +884,7 @@ class UsersControllerTest extends OriginalTest {
                        ->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')
@@ -918,7 +918,7 @@ class UsersControllerTest extends OriginalTest {
                $loggedInUser
                        ->expects($this->any())
                        ->method('getUID')
-                       ->will($this->returnValue('UserToEdit'));
+                       ->will($this->returnValue('UID'));
                $targetUser = $this->getMockBuilder('OCP\IUser')
                        ->disableOriginalConstructor()
                        ->getMock();
@@ -935,6 +935,10 @@ class UsersControllerTest extends OriginalTest {
                        ->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());
        }
@@ -946,7 +950,7 @@ class UsersControllerTest extends OriginalTest {
                $loggedInUser
                        ->expects($this->any())
                        ->method('getUID')
-                       ->will($this->returnValue('UserToEdit'));
+                       ->will($this->returnValue('UID'));
                $targetUser = $this->getMockBuilder('OCP\IUser')
                        ->disableOriginalConstructor()
                        ->getMock();
@@ -963,6 +967,10 @@ class UsersControllerTest extends OriginalTest {
                        ->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());
        }
@@ -979,7 +987,7 @@ class UsersControllerTest extends OriginalTest {
                $loggedInUser
                        ->expects($this->any())
                        ->method('getUID')
-                       ->will($this->returnValue('UserToEdit'));
+                       ->will($this->returnValue('UID'));
                $targetUser = $this->getMockBuilder('OCP\IUser')
                        ->disableOriginalConstructor()
                        ->getMock();
@@ -992,6 +1000,10 @@ class UsersControllerTest extends OriginalTest {
                        ->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');
        }
@@ -1003,7 +1015,7 @@ class UsersControllerTest extends OriginalTest {
                $loggedInUser
                        ->expects($this->any())
                        ->method('getUID')
-                       ->will($this->returnValue('UserToEdit'));
+                       ->will($this->returnValue('UID'));
                $targetUser = $this->getMockBuilder('OCP\IUser')
                        ->disableOriginalConstructor()
                        ->getMock();
@@ -1020,6 +1032,10 @@ class UsersControllerTest extends OriginalTest {
                        ->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());
        }
@@ -1036,7 +1052,7 @@ class UsersControllerTest extends OriginalTest {
                $loggedInUser
                        ->expects($this->any())
                        ->method('getUID')
-                       ->will($this->returnValue('UserToEdit'));
+                       ->will($this->returnValue('UID'));
                $targetUser = $this->getMockBuilder('OCP\IUser')
                        ->disableOriginalConstructor()
                        ->getMock();
@@ -1049,6 +1065,10 @@ class UsersControllerTest extends OriginalTest {
                        ->method('get')
                        ->with('UserToEdit')
                        ->will($this->returnValue($targetUser));
+               $targetUser
+                       ->expects($this->any())
+                       ->method('getUID')
+                       ->will($this->returnValue('UID'));
 
                $this->api->editUser('UserToEdit', 'quota', 'NewQuota');
        }
@@ -1058,7 +1078,7 @@ class UsersControllerTest extends OriginalTest {
                $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')
@@ -1075,8 +1095,12 @@ class UsersControllerTest extends OriginalTest {
                $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());
        }
@@ -1092,7 +1116,7 @@ class UsersControllerTest extends OriginalTest {
                $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())
@@ -1106,8 +1130,12 @@ class UsersControllerTest extends OriginalTest {
                $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');
        }
@@ -1143,6 +1171,10 @@ class UsersControllerTest extends OriginalTest {
                        ->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());
        }
@@ -1178,6 +1210,10 @@ class UsersControllerTest extends OriginalTest {
                        ->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());
        }
@@ -1214,6 +1250,10 @@ class UsersControllerTest extends OriginalTest {
                        ->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');
        }
@@ -1250,12 +1290,12 @@ class UsersControllerTest extends OriginalTest {
                $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')
@@ -1279,7 +1319,7 @@ class UsersControllerTest extends OriginalTest {
                $targetUser
                        ->expects($this->once())
                        ->method('getUID')
-                       ->will($this->returnValue('UserToDelete'));
+                       ->will($this->returnValue('UID'));
                $this->userSession
                        ->expects($this->once())
                        ->method('getUser')
@@ -1316,7 +1356,7 @@ class UsersControllerTest extends OriginalTest {
                $targetUser
                        ->expects($this->once())
                        ->method('getUID')
-                       ->will($this->returnValue('UserToDelete'));
+                       ->will($this->returnValue('UID'));
                $this->userSession
                        ->expects($this->once())
                        ->method('getUser')
@@ -1349,7 +1389,7 @@ class UsersControllerTest extends OriginalTest {
                $targetUser
                        ->expects($this->once())
                        ->method('getUID')
-                       ->will($this->returnValue('UserToDelete'));
+                       ->will($this->returnValue('UID'));
                $this->userSession
                        ->expects($this->once())
                        ->method('getUser')
@@ -1397,7 +1437,7 @@ class UsersControllerTest extends OriginalTest {
                $targetUser
                        ->expects($this->once())
                        ->method('getUID')
-                       ->will($this->returnValue('UserToDelete'));
+                       ->will($this->returnValue('UID'));
                $this->userSession
                        ->expects($this->once())
                        ->method('getUser')
@@ -1445,7 +1485,7 @@ class UsersControllerTest extends OriginalTest {
                $targetUser
                        ->expects($this->once())
                        ->method('getUID')
-                       ->will($this->returnValue('UserToDelete'));
+                       ->will($this->returnValue('UID'));
                $this->userSession
                        ->expects($this->once())
                        ->method('getUser')
@@ -1809,6 +1849,10 @@ class UsersControllerTest extends OriginalTest {
                        ->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())
@@ -1826,7 +1870,7 @@ class UsersControllerTest extends OriginalTest {
                $this->userManager
                        ->expects($this->once())
                        ->method('get')
-                       ->with('admin')
+                       ->with('Admin')
                        ->will($this->returnValue($targetUser));
                $subAdminManager = $this->getMockBuilder('OC\SubAdmin')
                        ->disableOriginalConstructor()->getMock();
@@ -1840,7 +1884,7 @@ class UsersControllerTest extends OriginalTest {
                        ->with('admin')
                        ->will($this->returnValue(true));
 
-               $this->api->removeFromGroup('admin', 'admin');
+               $this->api->removeFromGroup('Admin', 'admin');
        }
 
        /**
@@ -1855,6 +1899,10 @@ class UsersControllerTest extends OriginalTest {
                        ->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())
@@ -1872,7 +1920,7 @@ class UsersControllerTest extends OriginalTest {
                $this->userManager
                        ->expects($this->once())
                        ->method('get')
-                       ->with('subadmin')
+                       ->with('SubAdmin')
                        ->will($this->returnValue($targetUser));
                $subAdminManager = $this->getMockBuilder('OC\SubAdmin')
                        ->disableOriginalConstructor()->getMock();
@@ -1896,7 +1944,7 @@ class UsersControllerTest extends OriginalTest {
                        ->with('subadmin')
                        ->will($this->returnValue(false));
 
-               $this->api->removeFromGroup('subadmin', 'subadmin');
+               $this->api->removeFromGroup('SubAdmin', 'subadmin');
        }
 
        public function testRemoveFromGroupSuccessful() {
@@ -2364,7 +2412,7 @@ class UsersControllerTest extends OriginalTest {
 
        public function testGetCurrentUserLoggedIn() {
 
-               $user = $this->getMock(IUser::class);
+               $user = $this->getMockBuilder(IUser::class)->getMock();
                $user->expects($this->once())->method('getUID')->willReturn('UID');
 
                $this->userSession->expects($this->once())->method('getUser')