Merge pull request #13790 from owncloud/fix-subadmin-group

Fix subadmin listing of group
2015-02-02 09:45:28 +01:00 · 2015-02-02 09:45:28 +01:00 · 359abca50c
parent 800829d93d fcd5056376
commit 359abca50c
5 changed files with 657 additions and 66 deletions
--- a/settings/application.php
+++ b/settings/application.php
@ -1,7 +1,7 @@
 <?php
 /**
 * @author Lukas Reschke
- * @copyright 2014 Lukas Reschke lukas@owncloud.com
+ * @copyright 2014-2015 Lukas Reschke lukas@owncloud.com
 *
 * This file is licensed under the Affero General Public License version 3 or
 * later.
@ -16,6 +16,7 @@ use OC\Settings\Controller\LogSettingsController;
 use OC\Settings\Controller\MailSettingsController;
 use OC\Settings\Controller\SecuritySettingsController;
 use OC\Settings\Controller\UsersController;
+use OC\Settings\Factory\SubAdminFactory;
 use OC\Settings\Middleware\SubadminMiddleware;
 use \OCP\AppFramework\App;
 use OCP\IContainer;
@ -91,7 +92,8 @@ class Application extends App {
 				$c->query('Mail'),
 				$c->query('DefaultMailAddress'),
 				$c->query('URLGenerator'),
-				$c->query('OCP\\App\\IAppManager')
+				$c->query('OCP\\App\\IAppManager'),
+				$c->query('SubAdminFactory')
 			);
 		});
 		$container->registerService('LogSettingsController', function(IContainer $c) {
@ -145,6 +147,10 @@ class Application extends App {
 		$container->registerService('IsSubAdmin', function(IContainer $c) {
 			return \OC_Subadmin::isSubAdmin(\OC_User::getUser());
 		});
+		/** FIXME: Remove once OC_SubAdmin is non-static and mockable */
+		$container->registerService('SubAdminFactory', function(IContainer $c) {
+			return new SubAdminFactory();
+		});
 		$container->registerService('Mail', function(IContainer $c) {
 			return new \OC_Mail;
 		});
--- a/settings/controller/userscontroller.php
+++ b/settings/controller/userscontroller.php
@ -1,7 +1,7 @@
 <?php
 /**
 * @author Lukas Reschke
- * @copyright 2014 Lukas Reschke lukas@owncloud.com
+ * @copyright 2014-2015 Lukas Reschke lukas@owncloud.com
 *
 * This file is licensed under the Affero General Public License version 3 or
 * later.
@ -11,6 +11,7 @@
 namespace OC\Settings\Controller;

 use OC\AppFramework\Http;
+use OC\Settings\Factory\SubAdminFactory;
 use OC\User\User;
 use OCP\App\IAppManager;
 use OCP\AppFramework\Controller;
@ -56,6 +57,8 @@ class UsersController extends Controller {
 	private $isEncryptionAppEnabled;
 	/** @var bool contains the state of the admin recovery setting */
 	private $isRestoreEnabled = false;
+	/** @var SubAdminFactory */
+	private $subAdminFactory;

 	/**
 	 * @param string $appName
@ -70,7 +73,9 @@ class UsersController extends Controller {
 	 * @param \OC_Defaults $defaults
 	 * @param \OC_Mail $mail
 	 * @param string $fromMailAddress
+	 * @param IURLGenerator $urlGenerator
 	 * @param IAppManager $appManager
+	 * @param SubAdminFactory $subAdminFactory
 	 */
 	public function __construct($appName,
 								IRequest $request,
@ -85,7 +90,8 @@ class UsersController extends Controller {
 								\OC_Mail $mail,
 								$fromMailAddress,
 								IURLGenerator $urlGenerator,
-								IAppManager $appManager) {
+								IAppManager $appManager,
+								SubAdminFactory $subAdminFactory) {
 		parent::__construct($appName, $request);
 		$this->userManager = $userManager;
 		$this->groupManager = $groupManager;
@ -98,6 +104,7 @@ class UsersController extends Controller {
 		$this->mail = $mail;
 		$this->fromMailAddress = $fromMailAddress;
 		$this->urlGenerator = $urlGenerator;
+		$this->subAdminFactory = $subAdminFactory;

 		// check for encryption state - TODO see formatUserForIndex
 		$this->isEncryptionAppEnabled = $appManager->isEnabledForUser('files_encryption');
@ -161,7 +168,7 @@ class UsersController extends Controller {
 	private function getUsersForUID(array $userIDs) {
 		$users = [];
 		foreach ($userIDs as $uid => $displayName) {
-			$users[] = $this->userManager->get($uid);
+			$users[$uid] = $this->userManager->get($uid);
 		}
 		return $users;
 	}
@ -196,7 +203,7 @@ class UsersController extends Controller {
 			}
 		}

-		$users = array();
+		$users = [];
 		if ($this->isAdmin) {

 			if($gid !== '') {
@ -210,16 +217,34 @@ class UsersController extends Controller {
 			}

 		} else {
+			$subAdminOfGroups = $this->subAdminFactory->getSubAdminsOfGroups(
+				$this->userSession->getUser()->getUID()
+			);
 			// Set the $gid parameter to an empty value if the subadmin has no rights to access a specific group
-			if($gid !== '' && !in_array($gid, \OC_SubAdmin::getSubAdminsGroups($this->userSession->getUser()->getUID()))) {
+			if($gid !== '' && !in_array($gid, $subAdminOfGroups)) {
 				$gid = '';
 			}

-			$batch = $this->getUsersForUID($this->groupManager->displayNamesInGroup($gid, $pattern, $limit, $offset));
+			// Batch all groups the user is subadmin of when a group is specified
+			$batch = [];
+			if($gid === '') {
+				foreach($subAdminOfGroups as $group) {
+					$groupUsers = $this->groupManager->displayNamesInGroup($group, $pattern, $limit, $offset);
+					foreach($groupUsers as $uid => $displayName) {
+						$batch[$uid] = $displayName;
+					}
+				}
+			} else {
+				$batch = $this->groupManager->displayNamesInGroup($gid, $pattern, $limit, $offset);
+			}
+			$batch = $this->getUsersForUID($batch);
+
 			foreach ($batch as $user) {
 				// Only add the groups, this user is a subadmin of
-				$userGroups = array_intersect($this->groupManager->getUserGroupIds($user),
-					\OC_SubAdmin::getSubAdminsGroups($this->userSession->getUser()->getUID()));
+				$userGroups = array_values(array_intersect(
+					$this->groupManager->getUserGroupIds($user),
+					$subAdminOfGroups
+				));
 				$users[] = $this->formatUserForIndex($user, $userGroups);
 			}
 		}
@ -235,8 +260,6 @@ class UsersController extends Controller {
 	 * @param array $groups
 	 * @param string $email
 	 * @return DataResponse
-	 *
-	 * TODO: Tidy up and write unit tests - code is mainly static method calls
 	 */
 	public function create($username, $password, array $groups=array(), $email='') {

@ -249,17 +272,17 @@ class UsersController extends Controller {
 			);
 		}

-		// TODO FIXME get rid of the static calls to OC_Subadmin
 		if (!$this->isAdmin) {
+			$userId = $this->userSession->getUser()->getUID();
 			if (!empty($groups)) {
 				foreach ($groups as $key => $group) {
-					if (!\OC_SubAdmin::isGroupAccessible($this->userSession->getUser()->getUID(), $group)) {
+					if (!$this->subAdminFactory->isGroupAccessible($userId, $group)) {
 						unset($groups[$key]);
 					}
 				}
 			}
 			if (empty($groups)) {
-				$groups = \OC_SubAdmin::getSubAdminsGroups($this->userSession->getUser()->getUID());
+				$groups = $this->subAdminFactory->getSubAdminsOfGroups($userId);
 			}
 		}

@ -276,7 +299,7 @@ class UsersController extends Controller {

 		if($user instanceof User) {
 			if($groups !== null) {
-				foreach( $groups as $groupName ) {
+				foreach($groups as $groupName) {
 					$group = $this->groupManager->get($groupName);

 					if(empty($group)) {
@ -342,11 +365,10 @@ class UsersController extends Controller {
 	 *
 	 * @param string $id
 	 * @return DataResponse
-	 *
-	 * TODO: Tidy up and write unit tests - code is mainly static method calls
 	 */
 	public function destroy($id) {
-		if($this->userSession->getUser()->getUID() === $id) {
+		$userId = $this->userSession->getUser()->getUID();
+		if($userId === $id) {
 			return new DataResponse(
 				array(
 					'status' => 'error',
@ -358,8 +380,7 @@ class UsersController extends Controller {
 			);
 		}

-		// FIXME: Remove this static function call at some point…
-		if(!$this->isAdmin && !\OC_SubAdmin::isUserAccessible($this->userSession->getUser()->getUID(), $id)) {
+		if(!$this->isAdmin && !$this->subAdminFactory->isUserAccessible($userId, $id)) {
 			return new DataResponse(
 				array(
 					'status' => 'error',
@ -406,14 +427,12 @@ class UsersController extends Controller {
 	 * @param string $id
 	 * @param string $mailAddress
 	 * @return DataResponse
-	 *
-	 * TODO: Tidy up and write unit tests - code is mainly static method calls
 	 */
 	public function setMailAddress($id, $mailAddress) {
-		// FIXME: Remove this static function call at some point…
-		if($this->userSession->getUser()->getUID() !== $id
+		$userId = $this->userSession->getUser()->getUID();
+		if($userId !== $id
 			&& !$this->isAdmin
-			&& !\OC_SubAdmin::isUserAccessible($this->userSession->getUser()->getUID(), $id)) {
+			&& !$this->subAdminFactory->isUserAccessible($userId, $id)) {
 			return new DataResponse(
 				array(
 					'status' => 'error',
--- a/settings/factory/subadminfactory.php
+++ b/settings/factory/subadminfactory.php
@ -0,0 +1,45 @@
+<?php
+/**
+ * @author Lukas Reschke
+ * @copyright 2015 Lukas Reschke lukas@owncloud.com
+ *
+ * This file is licensed under the Affero General Public License version 3 or
+ * later.
+ * See the COPYING-README file.
+ */
+
+namespace OC\Settings\Factory;
+
+/**
+ * @package OC\Settings\Factory
+ */
+class SubAdminFactory {
+	/**
+	 * Get the groups $uid is SubAdmin of
+	 * @param string $uid
+	 * @return array Array of groups that $uid is subadmin of
+	 */
+	function getSubAdminsOfGroups($uid) {
+		return \OC_SubAdmin::getSubAdminsGroups($uid);
+	}
+
+	/**
+	 * Whether the $group is accessible to $uid as subadmin
+	 * @param string $uid
+	 * @param string $group
+	 * @return bool
+	 */
+	function isGroupAccessible($uid, $group) {
+		return \OC_SubAdmin::isGroupAccessible($uid, $group);
+	}
+
+	/**
+	 * Whether $uid is accessible to $subAdmin
+	 * @param string $subAdmin
+	 * @param string $uid
+	 * @return bool
+	 */
+	function isUserAccessible($subAdmin, $uid) {
+		return \OC_SubAdmin::isUserAccessible($subAdmin, $uid);
+	}
+}
--- a/tests/settings/controller/logsettingscontrollertest.php
+++ b/tests/settings/controller/logsettingscontrollertest.php
@ -10,6 +10,7 @@
 namespace Test\Settings\Controller;

 use \OC\Settings\Application;
+use OC\Settings\Controller\LogSettingsController;

 /**
 * @package OC\Settings\Controller
--- a/tests/settings/controller/userscontrollertest.php
+++ b/tests/settings/controller/userscontrollertest.php
@ -1,7 +1,7 @@
 <?php
 /**
 * @author Lukas Reschke
- * @copyright 2014 Lukas Reschke lukas@owncloud.com
+ * @copyright 2014-2015 Lukas Reschke lukas@owncloud.com
 *
 * This file is licensed under the Affero General Public License version 3 or
 * later.
@ -33,9 +33,10 @@ class UsersControllerTest extends \Test\TestCase {
 			->disableOriginalConstructor()->getMock();
 		$this->container['L10N'] = $this->getMockBuilder('\OCP\IL10N')
 			->disableOriginalConstructor()->getMock();
+		$this->container['SubAdminFactory'] = $this->getMockBuilder('\OC\Settings\Factory\SubAdminFactory')
+			->disableOriginalConstructor()->getMock();
 		$this->container['Config'] = $this->getMockBuilder('\OCP\IConfig')
 			->disableOriginalConstructor()->getMock();
-		$this->container['IsAdmin'] = true;
 		$this->container['L10N']
 			->expects($this->any())
 			->method('t')
@ -55,11 +56,9 @@ class UsersControllerTest extends \Test\TestCase {
 			->disableOriginalConstructor()->getMock();
 	}

-	/**
-	 * TODO: Since the function uses the static OC_Subadmin class it can't be mocked
-	 * to test for subadmins. Thus the test always assumes you have admin permissions...
-	 */
-	public function testIndex() {
+	public function testIndexAdmin() {
+		$this->container['IsAdmin'] = true;
+
 		$foo = $this->getMockBuilder('\OC\User\User')
 			->disableOriginalConstructor()->getMock();
 		$foo
@ -198,11 +197,182 @@ class UsersControllerTest extends \Test\TestCase {
 		$this->assertEquals($expectedResponse, $response);
 	}

+	public function testIndexSubAdmin() {
+		$this->container['IsAdmin'] = false;
+		$this->container['SubAdminFactory']
+			->expects($this->once())
+			->method('getSubAdminsOfGroups')
+			->with('username')
+			->will($this->returnValue(['SubGroup1', 'SubGroup2']));
+
+		$user = $this->getMockBuilder('\OC\User\User')
+			->disableOriginalConstructor()->getMock();
+		$user
+			->expects($this->once())
+			->method('getUID')
+			->will($this->returnValue('username'));
+		$this->container['UserSession']
+			->expects($this->once())
+			->method('getUser')
+			->will($this->returnValue($user));
+
+		$foo = $this->getMockBuilder('\OC\User\User')
+			->disableOriginalConstructor()->getMock();
+		$foo
+			->expects($this->exactly(4))
+			->method('getUID')
+			->will($this->returnValue('foo'));
+		$foo
+			->expects($this->once())
+			->method('getDisplayName')
+			->will($this->returnValue('M. Foo'));
+		$foo
+			->method('getLastLogin')
+			->will($this->returnValue(500));
+		$foo
+			->method('getHome')
+			->will($this->returnValue('/home/foo'));
+		$foo
+			->expects($this->once())
+			->method('getBackendClassName')
+			->will($this->returnValue('OC_User_Database'));
+		$admin = $this->getMockBuilder('\OC\User\User')
+			->disableOriginalConstructor()->getMock();
+		$admin
+			->expects($this->exactly(4))
+			->method('getUID')
+			->will($this->returnValue('admin'));
+		$admin
+			->expects($this->once())
+			->method('getDisplayName')
+			->will($this->returnValue('S. Admin'));
+		$admin
+			->expects($this->once())
+			->method('getLastLogin')
+			->will($this->returnValue(12));
+		$admin
+			->expects($this->once())
+			->method('getHome')
+			->will($this->returnValue('/home/admin'));
+		$admin
+			->expects($this->once())
+			->method('getBackendClassName')
+			->will($this->returnValue('OC_User_Dummy'));
+		$bar = $this->getMockBuilder('\OC\User\User')
+			->disableOriginalConstructor()->getMock();
+		$bar
+			->expects($this->exactly(4))
+			->method('getUID')
+			->will($this->returnValue('bar'));
+		$bar
+			->expects($this->once())
+			->method('getDisplayName')
+			->will($this->returnValue('B. Ar'));
+		$bar
+			->method('getLastLogin')
+			->will($this->returnValue(3999));
+		$bar
+			->method('getHome')
+			->will($this->returnValue('/home/bar'));
+		$bar
+			->expects($this->once())
+			->method('getBackendClassName')
+			->will($this->returnValue('OC_User_Dummy'));
+
+		$this->container['GroupManager']
+			->expects($this->at(0))
+			->method('displayNamesInGroup')
+			->with('SubGroup1', 'pattern')
+			->will($this->returnValue(['foo' => 'M. Foo', 'admin' => 'S. Admin']));
+		$this->container['GroupManager']
+			->expects($this->at(1))
+			->method('displayNamesInGroup')
+			->with('SubGroup2', 'pattern')
+			->will($this->returnValue(['bar' => 'B. Ar']));
+		$this->container['GroupManager']
+			->expects($this->exactly(3))
+			->method('getUserGroupIds')
+			->will($this->onConsecutiveCalls(
+				['SubGroup2', 'SubGroup1'],
+				['SubGroup2', 'Foo'],
+				['admin', 'SubGroup1', 'testGroup']
+			));
+		$this->container['UserManager']
+			->expects($this->at(0))
+			->method('get')
+			->with('foo')
+			->will($this->returnValue($foo));
+		$this->container['UserManager']
+			->expects($this->at(1))
+			->method('get')
+			->with('admin')
+			->will($this->returnValue($admin));
+		$this->container['UserManager']
+			->expects($this->at(2))
+			->method('get')
+			->with('bar')
+			->will($this->returnValue($bar));
+		$this->container['Config']
+			->expects($this->exactly(6))
+			->method('getUserValue')
+			->will($this->onConsecutiveCalls(
+				1024, 'foo@bar.com',
+				404, 'admin@bar.com',
+				2323, 'bar@dummy.com'
+			));
+
+		$expectedResponse = new DataResponse(
+			[
+				0 => [
+					'name' => 'foo',
+					'displayname' => 'M. Foo',
+					'groups' => ['SubGroup2', 'SubGroup1'],
+					'subadmin' => [],
+					'quota' => 1024,
+					'storageLocation' => '/home/foo',
+					'lastLogin' => 500,
+					'backend' => 'OC_User_Database',
+					'email' => 'foo@bar.com',
+					'isRestoreDisabled' => false,
+				],
+				1 => [
+					'name' => 'admin',
+					'displayname' => 'S. Admin',
+					'groups' => ['SubGroup2'],
+					'subadmin' => [],
+					'quota' => 404,
+					'storageLocation' => '/home/admin',
+					'lastLogin' => 12,
+					'backend' => 'OC_User_Dummy',
+					'email' => 'admin@bar.com',
+					'isRestoreDisabled' => false,
+				],
+				2 => [
+					'name' => 'bar',
+					'displayname' => 'B. Ar',
+					'groups' => ['SubGroup1'],
+					'subadmin' => [],
+					'quota' => 2323,
+					'storageLocation' => '/home/bar',
+					'lastLogin' => 3999,
+					'backend' => 'OC_User_Dummy',
+					'email' => 'bar@dummy.com',
+					'isRestoreDisabled' => false,
+				],
+			]
+		);
+
+		$response = $this->container['UsersController']->index(0, 10, '', 'pattern');
+		$this->assertEquals($expectedResponse, $response);
+	}
+
 	/**
 	 * TODO: Since the function uses the static OC_Subadmin class it can't be mocked
 	 * to test for subadmins. Thus the test always assumes you have admin permissions...
 	 */
 	public function testIndexWithSearch() {
+		$this->container['IsAdmin'] = true;
+
 		$foo = $this->getMockBuilder('\OC\User\User')
 			->disableOriginalConstructor()->getMock();
 		$foo
@ -326,8 +496,9 @@ class UsersControllerTest extends \Test\TestCase {
 		$this->assertEquals($expectedResponse, $response);
 	}

-
 	public function testIndexWithBackend() {
+		$this->container['IsAdmin'] = true;
+
 		$user = $this->getMockBuilder('\OC\User\User')
 			->disableOriginalConstructor()->getMock();
 		$user
@ -386,6 +557,8 @@ class UsersControllerTest extends \Test\TestCase {
 	}

 	public function testIndexWithBackendNoUser() {
+		$this->container['IsAdmin'] = true;
+
 		$this->container['UserManager']
 			->expects($this->once())
 			->method('getBackends')
@ -401,11 +574,9 @@ class UsersControllerTest extends \Test\TestCase {
 		$this->assertEquals($expectedResponse, $response);
 	}

-	/**
-	 * TODO: Since the function uses the static OC_Subadmin class it can't be mocked
-	 * to test for subadmins. Thus the test always assumes you have admin permissions...
-	 */
-	public function testCreateSuccessfulWithoutGroup() {
+	public function testCreateSuccessfulWithoutGroupAdmin() {
+		$this->container['IsAdmin'] = true;
+
 		$user = $this->getMockBuilder('\OC\User\User')
 			->disableOriginalConstructor()->getMock();
 		$user
@ -444,11 +615,88 @@ class UsersControllerTest extends \Test\TestCase {
 		$this->assertEquals($expectedResponse, $response);
 	}

-	/**
-	 * TODO: Since the function uses the static OC_Subadmin class it can't be mocked
-	 * to test for subadmins. Thus the test always assumes you have admin permissions...
-	 */
-	public function testCreateSuccessfulWithGroup() {
+	public function testCreateSuccessfulWithoutGroupSubAdmin() {
+		$this->container['IsAdmin'] = false;
+		$this->container['SubAdminFactory']
+			->expects($this->once())
+			->method('getSubAdminsOfGroups')
+			->with('username')
+			->will($this->returnValue(['SubGroup1', 'SubGroup2']));
+		$user = $this->getMockBuilder('\OC\User\User')
+			->disableOriginalConstructor()->getMock();
+		$user
+			->expects($this->once())
+			->method('getUID')
+			->will($this->returnValue('username'));
+		$this->container['UserSession']
+			->expects($this->once())
+			->method('getUser')
+			->will($this->returnValue($user));
+
+		$user = $this->getMockBuilder('\OC\User\User')
+			->disableOriginalConstructor()->getMock();
+		$user
+			->method('getHome')
+			->will($this->returnValue('/home/user'));
+		$user
+			->method('getHome')
+			->will($this->returnValue('/home/user'));
+		$user
+			->method('getUID')
+			->will($this->returnValue('foo'));
+		$user
+			->expects($this->once())
+			->method('getBackendClassName')
+			->will($this->returnValue('bar'));
+		$subGroup1 = $this->getMockBuilder('\OCP\IGroup')
+			->disableOriginalConstructor()->getMock();
+		$subGroup1
+			->expects($this->once())
+			->method('addUser')
+			->with($user);
+		$subGroup2 = $this->getMockBuilder('\OCP\IGroup')
+			->disableOriginalConstructor()->getMock();
+		$subGroup2
+			->expects($this->once())
+			->method('addUser')
+			->with($user);
+
+		$this->container['UserManager']
+			->expects($this->once())
+			->method('createUser')
+			->will($this->onConsecutiveCalls($user));
+		$this->container['GroupManager']
+			->expects($this->exactly(2))
+			->method('get')
+			->will($this->onConsecutiveCalls($subGroup1, $subGroup2));
+		$this->container['GroupManager']
+			->expects($this->once())
+			->method('getUserGroupIds')
+			->with($user)
+			->will($this->onConsecutiveCalls(['SubGroup1', 'SubGroup2']));
+
+		$expectedResponse = new DataResponse(
+			array(
+				'name' => 'foo',
+				'groups' => ['SubGroup1', 'SubGroup2'],
+				'storageLocation' => '/home/user',
+				'backend' => 'bar',
+				'lastLogin' => null,
+				'displayname' => null,
+				'quota' => null,
+				'subadmin' => [],
+				'email' => null,
+				'isRestoreDisabled' => false,
+			),
+			Http::STATUS_CREATED
+		);
+		$response = $this->container['UsersController']->create('foo', 'password');
+		$this->assertEquals($expectedResponse, $response);
+	}
+
+	public function testCreateSuccessfulWithGroupAdmin() {
+		$this->container['IsAdmin'] = true;
+
 		$user = $this->getMockBuilder('\OC\User\User')
 			->disableOriginalConstructor()->getMock();
 		$user
@ -515,11 +763,88 @@ class UsersControllerTest extends \Test\TestCase {
 		$this->assertEquals($expectedResponse, $response);
 	}

-	/**
-	 * TODO: Since the function uses the static OC_Subadmin class it can't be mocked
-	 * to test for subadmins. Thus the test always assumes you have admin permissions...
-	 */
-	public function testCreateUnsuccessful() {
+	public function testCreateSuccessfulWithGroupSubAdmin() {
+		$this->container['IsAdmin'] = false;
+		$this->container['SubAdminFactory']
+			->expects($this->once())
+			->method('getSubAdminsOfGroups')
+			->with('username')
+			->will($this->returnValue(['SubGroup1', 'SubGroup2']));
+		$user = $this->getMockBuilder('\OC\User\User')
+			->disableOriginalConstructor()->getMock();
+		$user
+			->expects($this->once())
+			->method('getUID')
+			->will($this->returnValue('username'));
+		$this->container['UserSession']
+			->expects($this->once())
+			->method('getUser')
+			->will($this->returnValue($user));
+
+		$user = $this->getMockBuilder('\OC\User\User')
+			->disableOriginalConstructor()->getMock();
+		$user
+			->method('getHome')
+			->will($this->returnValue('/home/user'));
+		$user
+			->method('getHome')
+			->will($this->returnValue('/home/user'));
+		$user
+			->method('getUID')
+			->will($this->returnValue('foo'));
+		$user
+			->expects($this->once())
+			->method('getBackendClassName')
+			->will($this->returnValue('bar'));
+		$subGroup1 = $this->getMockBuilder('\OCP\IGroup')
+			->disableOriginalConstructor()->getMock();
+		$subGroup1
+			->expects($this->once())
+			->method('addUser')
+			->with($user);
+		$subGroup2 = $this->getMockBuilder('\OCP\IGroup')
+			->disableOriginalConstructor()->getMock();
+		$subGroup2
+			->expects($this->once())
+			->method('addUser')
+			->with($user);
+
+		$this->container['UserManager']
+			->expects($this->once())
+			->method('createUser')
+			->will($this->onConsecutiveCalls($user));
+		$this->container['GroupManager']
+			->expects($this->exactly(2))
+			->method('get')
+			->will($this->onConsecutiveCalls($subGroup1, $subGroup2));
+		$this->container['GroupManager']
+			->expects($this->once())
+			->method('getUserGroupIds')
+			->with($user)
+			->will($this->onConsecutiveCalls(['SubGroup1']));
+
+		$expectedResponse = new DataResponse(
+			array(
+				'name' => 'foo',
+				'groups' => ['SubGroup1'],
+				'storageLocation' => '/home/user',
+				'backend' => 'bar',
+				'lastLogin' => null,
+				'displayname' => null,
+				'quota' => null,
+				'subadmin' => [],
+				'email' => null,
+				'isRestoreDisabled' => false,
+			),
+			Http::STATUS_CREATED
+		);
+		$response = $this->container['UsersController']->create('foo', 'password', ['SubGroup1', 'ExistingGroup']);
+		$this->assertEquals($expectedResponse, $response);
+	}
+
+	public function testCreateUnsuccessfulAdmin() {
+		$this->container['IsAdmin'] = true;
+
 		$this->container['UserManager']
 			->method('createUser')
 			->will($this->throwException(new \Exception()));
@ -534,11 +859,41 @@ class UsersControllerTest extends \Test\TestCase {
 		$this->assertEquals($expectedResponse, $response);
 	}

-	/**
-	 * TODO: Since the function uses the static OC_Subadmin class it can't be mocked
-	 * to test for subadmins. Thus the test always assumes you have admin permissions...
-	 */
-	public function testDestroySelf() {
+	public function testCreateUnsuccessfulSubAdmin() {
+		$this->container['IsAdmin'] = false;
+		$this->container['SubAdminFactory']
+			->expects($this->once())
+			->method('getSubAdminsOfGroups')
+			->with('username')
+			->will($this->returnValue(['SubGroup1', 'SubGroup2']));
+		$user = $this->getMockBuilder('\OC\User\User')
+			->disableOriginalConstructor()->getMock();
+		$user
+			->expects($this->once())
+			->method('getUID')
+			->will($this->returnValue('username'));
+		$this->container['UserSession']
+			->expects($this->once())
+			->method('getUser')
+			->will($this->returnValue($user));
+
+		$this->container['UserManager']
+			->method('createUser')
+			->will($this->throwException(new \Exception()));
+
+		$expectedResponse = new DataResponse(
+			[
+				'message' => 'Unable to create user.'
+			],
+			Http::STATUS_FORBIDDEN
+		);
+		$response = $this->container['UsersController']->create('foo', 'password', array());
+		$this->assertEquals($expectedResponse, $response);
+	}
+
+	public function testDestroySelfAdmin() {
+		$this->container['IsAdmin'] = true;
+
 		$user = $this->getMockBuilder('\OC\User\User')
 			->disableOriginalConstructor()->getMock();
 		$user
@ -562,11 +917,35 @@ class UsersControllerTest extends \Test\TestCase {
 		$this->assertEquals($expectedResponse, $response);
 	}

-	/**
-	 * TODO: Since the function uses the static OC_Subadmin class it can't be mocked
-	 * to test for subadmins. Thus the test always assumes you have admin permissions...
-	 */
-	public function testDestroy() {
+	public function testDestroySelfSubadmin() {
+		$this->container['IsAdmin'] = false;
+
+		$user = $this->getMockBuilder('\OC\User\User')
+			->disableOriginalConstructor()->getMock();
+		$user
+			->expects($this->once())
+			->method('getUID')
+			->will($this->returnValue('myself'));
+		$this->container['UserSession']
+			->method('getUser')
+			->will($this->returnValue($user));
+
+		$expectedResponse = new DataResponse(
+			array(
+				'status' => 'error',
+				'data' => array(
+					'message' => 'Unable to delete user.'
+				)
+			),
+			Http::STATUS_FORBIDDEN
+		);
+		$response = $this->container['UsersController']->destroy('myself');
+		$this->assertEquals($expectedResponse, $response);
+	}
+
+	public function testDestroyAdmin() {
+		$this->container['IsAdmin'] = true;
+
 		$user = $this->getMockBuilder('\OC\User\User')
 			->disableOriginalConstructor()->getMock();
 		$user
@ -599,11 +978,56 @@ class UsersControllerTest extends \Test\TestCase {
 		$response = $this->container['UsersController']->destroy('UserToDelete');
 		$this->assertEquals($expectedResponse, $response);
 	}
-	/**
-	 * TODO: Since the function uses the static OC_Subadmin class it can't be mocked
-	 * to test for subadmins. Thus the test always assumes you have admin permissions...
-	 */
-	public function testDestroyUnsuccessful() {
+
+	public function testDestroySubAdmin() {
+		$this->container['IsAdmin'] = false;
+		$this->container['SubAdminFactory']
+			->expects($this->once())
+			->method('isUserAccessible')
+			->with('myself', 'UserToDelete')
+			->will($this->returnValue(true));
+		$user = $this->getMockBuilder('\OC\User\User')
+			->disableOriginalConstructor()->getMock();
+		$user
+			->expects($this->once())
+			->method('getUID')
+			->will($this->returnValue('myself'));
+		$this->container['UserSession']
+			->method('getUser')
+			->will($this->returnValue($user));
+
+		$user = $this->getMockBuilder('\OC\User\User')
+			->disableOriginalConstructor()->getMock();
+		$toDeleteUser = $this->getMockBuilder('\OC\User\User')
+			->disableOriginalConstructor()->getMock();
+		$toDeleteUser
+			->expects($this->once())
+			->method('delete')
+			->will($this->returnValue(true));
+		$this->container['UserSession']
+			->method('getUser')
+			->will($this->returnValue($user));
+		$this->container['UserManager']
+			->method('get')
+			->with('UserToDelete')
+			->will($this->returnValue($toDeleteUser));
+
+		$expectedResponse = new DataResponse(
+			[
+				'status' => 'success',
+				'data' => [
+					'username' => 'UserToDelete'
+				]
+			],
+			Http::STATUS_NO_CONTENT
+		);
+		$response = $this->container['UsersController']->destroy('UserToDelete');
+		$this->assertEquals($expectedResponse, $response);
+	}
+
+	public function testDestroyUnsuccessfulAdmin() {
+		$this->container['IsAdmin'] = true;
+
 		$user = $this->getMockBuilder('\OC\User\User')
 			->disableOriginalConstructor()->getMock();
 		$user
@ -637,10 +1061,96 @@ class UsersControllerTest extends \Test\TestCase {
 		$this->assertEquals($expectedResponse, $response);
 	}

+	public function testDestroyUnsuccessfulSubAdmin() {
+		$this->container['IsAdmin'] = false;
+		$this->container['SubAdminFactory']
+			->expects($this->once())
+			->method('isUserAccessible')
+			->with('myself', 'UserToDelete')
+			->will($this->returnValue(true));
+		$user = $this->getMockBuilder('\OC\User\User')
+			->disableOriginalConstructor()->getMock();
+		$user
+			->expects($this->once())
+			->method('getUID')
+			->will($this->returnValue('myself'));
+		$this->container['UserSession']
+			->method('getUser')
+			->will($this->returnValue($user));
+
+		$toDeleteUser = $this->getMockBuilder('\OC\User\User')
+			->disableOriginalConstructor()->getMock();
+		$toDeleteUser
+			->expects($this->once())
+			->method('delete')
+			->will($this->returnValue(false));
+		$this->container['UserSession']
+			->method('getUser')
+			->will($this->returnValue($user));
+		$this->container['UserManager']
+			->method('get')
+			->with('UserToDelete')
+			->will($this->returnValue($toDeleteUser));
+
+		$expectedResponse = new DataResponse(
+			[
+				'status' => 'error',
+				'data' => [
+					'message' => 'Unable to delete user.'
+				]
+			],
+			Http::STATUS_FORBIDDEN
+		);
+		$response = $this->container['UsersController']->destroy('UserToDelete');
+		$this->assertEquals($expectedResponse, $response);
+	}
+
+	public function testDestroyNotAccessibleToSubAdmin() {
+		$this->container['IsAdmin'] = false;
+		$this->container['SubAdminFactory']
+			->expects($this->once())
+			->method('isUserAccessible')
+			->with('myself', 'UserToDelete')
+			->will($this->returnValue(false));
+		$user = $this->getMockBuilder('\OC\User\User')
+			->disableOriginalConstructor()->getMock();
+		$user
+			->expects($this->once())
+			->method('getUID')
+			->will($this->returnValue('myself'));
+		$this->container['UserSession']
+			->method('getUser')
+			->will($this->returnValue($user));
+
+		$toDeleteUser = $this->getMockBuilder('\OC\User\User')
+			->disableOriginalConstructor()->getMock();
+		$this->container['UserSession']
+			->method('getUser')
+			->will($this->returnValue($user));
+		$this->container['UserManager']
+			->method('get')
+			->with('UserToDelete')
+			->will($this->returnValue($toDeleteUser));
+
+		$expectedResponse = new DataResponse(
+			[
+				'status' => 'error',
+				'data' => [
+					'message' => 'Authentication error'
+				]
+			],
+			Http::STATUS_FORBIDDEN
+		);
+		$response = $this->container['UsersController']->destroy('UserToDelete');
+		$this->assertEquals($expectedResponse, $response);
+	}
+
 	/**
 	 * test if an invalid mail result in a failure response
 	 */
-	public function testCreateUnsuccessfulWithInvalidEMail() {
+	public function testCreateUnsuccessfulWithInvalidEmailAdmin() {
+		$this->container['IsAdmin'] = true;
+
 		/**
 		 * FIXME: Disabled due to missing DI on mail class.
 		 * TODO: Re-enable when https://github.com/owncloud/core/pull/12085 is merged.
@ -665,7 +1175,9 @@ class UsersControllerTest extends \Test\TestCase {
 	/**
 	 * test if a valid mail result in a successful mail send
 	 */
-	public function testCreateSuccessfulWithValidEMail() {
+	public function testCreateSuccessfulWithValidEmailAdmin() {
+		$this->container['IsAdmin'] = true;
+
 		/**
 		 * FIXME: Disabled due to missing DI on mail class.
 		 * TODO: Re-enable when https://github.com/owncloud/core/pull/12085 is merged.
@ -737,6 +1249,8 @@ class UsersControllerTest extends \Test\TestCase {
 	}

 	public function testRestorePossibleWithoutEncryption() {
+		$this->container['IsAdmin'] = true;
+
 		list($user, $expectedResult) = $this->mockUser();

 		$result = \Test_Helper::invokePrivate($this->container['UsersController'], 'formatUserForIndex', [$user]);
@ -744,6 +1258,8 @@ class UsersControllerTest extends \Test\TestCase {
 	}

 	public function testRestorePossibleWithAdminAndUserRestore() {
+		$this->container['IsAdmin'] = true;
+
 		list($user, $expectedResult) = $this->mockUser();

 		$this->container['OCP\\App\\IAppManager']
@ -779,6 +1295,8 @@ class UsersControllerTest extends \Test\TestCase {
 	}

 	public function testRestoreNotPossibleWithoutAdminRestore() {
+		$this->container['IsAdmin'] = true;
+
 		list($user, $expectedResult) = $this->mockUser();

 		$this->container['OCP\\App\\IAppManager']
@ -795,6 +1313,8 @@ class UsersControllerTest extends \Test\TestCase {
 	}

 	public function testRestoreNotPossibleWithoutUserRestore() {
+		$this->container['IsAdmin'] = true;
+
 		list($user, $expectedResult) = $this->mockUser();

 		$this->container['OCP\\App\\IAppManager']