Merge pull request #20135 from owncloud/check-if-null-subadmin

Drop OC_SubAdmin and replace usages

11 files changed, 124 insertions, 96 deletions

diff --git a/settings/ajax/changedisplayname.php b/settings/ajax/changedisplayname.php
index 36c36e73184..380cbac43da 100644
--- a/settings/ajax/changedisplayname.php
+++ b/settings/ajax/changedisplayname.php
@@ -37,7 +37,15 @@ $userstatus = null;
 if(OC_User::isAdminUser(OC_User::getUser())) {
 	$userstatus = 'admin';
 }
-if(OC_SubAdmin::isUserAccessible(OC_User::getUser(), $username)) {
+
+$isUserAccessible = false;
+$subadminUserObject = \OC::$server->getUserManager()->get(\OC_User::getUser());
+$targetUserObject = \OC::$server->getUserManager()->get($username);
+if($subadminUserObject !== null && $targetUserObject !== null) {
+	$isUserAccessible = \OC::$server->getGroupManager()->getSubAdmin()->isUserAccessible($subadminUserObject, $targetUserObject);
+}
+
+if($isUserAccessible) {
 	$userstatus = 'subadmin';
 }
 
diff --git a/settings/ajax/geteveryonecount.php b/settings/ajax/geteveryonecount.php
index 659c8466f72..002c849fd39 100644
--- a/settings/ajax/geteveryonecount.php
+++ b/settings/ajax/geteveryonecount.php
@@ -29,7 +29,12 @@ $userCount = 0;
 $currentUser = \OC::$server->getUserSession()->getUser()->getUID();
 
 if (!OC_User::isAdminUser($currentUser)) {
-	$groups = OC_SubAdmin::getSubAdminsGroups($currentUser);
+	$groups = \OC::$server->getGroupManager()->getSubAdmin()->getSubAdminsGroups(\OC::$server->getUserSession()->getUser());
+	// New class returns IGroup[] so convert back
+	foreach ($groups as $key => $group) {
+		$groups[$key] = $group->getGID();
+	}
+
 
 	foreach ($groups as $group) {
 		$userCount += count(OC_Group::usersInGroup($group));
diff --git a/settings/ajax/setquota.php b/settings/ajax/setquota.php
index a9012a55345..8e6d44c2d8d 100644
--- a/settings/ajax/setquota.php
+++ b/settings/ajax/setquota.php
@@ -32,9 +32,16 @@ OCP\JSON::callCheck();
 
 $username = isset($_POST["username"]) ? (string)$_POST["username"] : '';
 
+$isUserAccessible = false;
+$currentUserObject = \OC::$server->getUserSession()->getUser();
+$targetUserObject = \OC::$server->getUserManager()->get($username);
+if($targetUserObject !== null && $currentUserObject !== null) {
+	$isUserAccessible = \OC::$server->getGroupManager()->getSubAdmin()->isUserAccessible($currentUserObject, $targetUserObject);
+}
+
 if(($username === '' && !OC_User::isAdminUser(OC_User::getUser()))
 	|| (!OC_User::isAdminUser(OC_User::getUser())
-		&& !OC_SubAdmin::isUserAccessible(OC_User::getUser(), $username))) {
+		&& !$isUserAccessible)) {
 	$l = \OC::$server->getL10N('core');
 	OC_JSON::error(array( 'data' => array( 'message' => $l->t('Authentication error') )));
 	exit();
diff --git a/settings/ajax/togglegroups.php b/settings/ajax/togglegroups.php
index 4d248408db0..f46fa356549 100644
--- a/settings/ajax/togglegroups.php
+++ b/settings/ajax/togglegroups.php
@@ -37,9 +37,19 @@ if($username === OC_User::getUser() && $group === "admin" &&  OC_User::isAdminUs
 	exit();
 }
 
+$isUserAccessible = false;
+$isGroupAccessible = false;
+$currentUserObject = \OC::$server->getUserSession()->getUser();
+$targetUserObject = \OC::$server->getUserManager()->get($username);
+$targetGroupObject = \OC::$server->getGroupManager()->get($group);
+if($targetUserObject !== null && $currentUserObject !== null && $targetGroupObject !== null) {
+	$isUserAccessible = \OC::$server->getGroupManager()->getSubAdmin()->isUserAccessible($currentUserObject, $targetUserObject);
+	$isGroupAccessible = \OC::$server->getGroupManager()->getSubAdmin()->isSubAdminofGroup($currentUserObject, $targetGroupObject);
+}
+
 if(!OC_User::isAdminUser(OC_User::getUser())
-	&& (!OC_SubAdmin::isUserAccessible(OC_User::getUser(), $username)
-		|| !OC_SubAdmin::isGroupAccessible(OC_User::getUser(), $group))) {
+	&& (!$isUserAccessible
+		|| !$isGroupAccessible)) {
 	$l = \OC::$server->getL10N('core');
 	OC_JSON::error(array( 'data' => array( 'message' => $l->t('Authentication error') )));
 	exit();
diff --git a/settings/ajax/togglesubadmins.php b/settings/ajax/togglesubadmins.php
index 767baa9f3ee..6d6bf6d6724 100644
--- a/settings/ajax/togglesubadmins.php
+++ b/settings/ajax/togglesubadmins.php
@@ -27,11 +27,20 @@ OCP\JSON::callCheck();
 $username = (string)$_POST['username'];
 $group = (string)$_POST['group'];
 
+$subAdminManager = \OC::$server->getGroupManager()->getSubAdmin();
+$targetUserObject = \OC::$server->getUserManager()->get($username);
+$targetGroupObject = \OC::$server->getGroupManager()->get($group);
+
+$isSubAdminOfGroup = false;
+if($targetUserObject !== null && $targetUserObject !== null) {
+	$isSubAdminOfGroup = $subAdminManager->isSubAdminofGroup($targetUserObject, $targetGroupObject);
+}
+
 // Toggle group
-if(OC_SubAdmin::isSubAdminofGroup($username, $group)) {
-	OC_SubAdmin::deleteSubAdmin($username, $group);
-}else{
-	OC_SubAdmin::createSubAdmin($username, $group);
+if($isSubAdminOfGroup) {
+	$subAdminManager->deleteSubAdmin($targetUserObject, $targetGroupObject);
+} else {
+	$subAdminManager->createSubAdmin($targetUserObject, $targetGroupObject);
 }
 
 OC_JSON::success();
diff --git a/settings/application.php b/settings/application.php
index 155cc39d041..c876065fffa 100644
--- a/settings/application.php
+++ b/settings/application.php
@@ -36,7 +36,6 @@ 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;
@@ -136,8 +135,7 @@ class Application extends App {
 				$c->query('Mailer'),
 				$c->query('DefaultMailAddress'),
 				$c->query('URLGenerator'),
-				$c->query('OCP\\App\\IAppManager'),
-				$c->query('SubAdminFactory')
+				$c->query('OCP\\App\\IAppManager')
 			);
 		});
 		$container->registerService('LogSettingsController', function(IContainer $c) {
@@ -199,11 +197,12 @@ class Application extends App {
 		});
 		/** FIXME: Remove once OC_SubAdmin is non-static and mockable */
 		$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();
+			$userObject = \OC::$server->getUserSession()->getUser();
+			$isSubAdmin = false;
+			if($userObject !== null) {
+				$isSubAdmin = \OC::$server->getGroupManager()->getSubAdmin()->isSubAdmin($userObject);
+			}
+			return $isSubAdmin;
 		});
 		$container->registerService('Mailer', function(IContainer $c) {
 			return $c->query('ServerContainer')->getMailer();
diff --git a/settings/changepassword/controller.php b/settings/changepassword/controller.php
index 3fa3ac67edb..695914683db 100644
--- a/settings/changepassword/controller.php
+++ b/settings/changepassword/controller.php
@@ -66,9 +66,16 @@ class Controller {
 		$password = isset($_POST['password']) ? $_POST['password'] : null;
 		$recoveryPassword = isset($_POST['recoveryPassword']) ? $_POST['recoveryPassword'] : null;
 
+		$isUserAccessible = false;
+		$currentUserObject = \OC::$server->getUserSession()->getUser();
+		$targetUserObject = \OC::$server->getUserManager()->get($username);
+		if($currentUserObject !== null && $targetUserObject !== null) {
+			$isUserAccessible = \OC::$server->getGroupManager()->getSubAdmin()->isUserAccessible($currentUserObject, $targetUserObject);
+		}
+
 		if (\OC_User::isAdminUser(\OC_User::getUser())) {
 			$userstatus = 'admin';
-		} elseif (\OC_SubAdmin::isUserAccessible(\OC_User::getUser(), $username)) {
+		} elseif ($isUserAccessible) {
 			$userstatus = 'subadmin';
 		} else {
 			$l = new \OC_L10n('settings');
diff --git a/settings/controller/groupscontroller.php b/settings/controller/groupscontroller.php
index 218a9c0d51c..93dcc7358a9 100644
--- a/settings/controller/groupscontroller.php
+++ b/settings/controller/groupscontroller.php
@@ -77,8 +77,12 @@ class GroupsController extends Controller {
 	public function index($pattern = '', $filterGroups = false, $sortGroups = MetaData::SORT_USERCOUNT) {
 		$groupPattern = $filterGroups ? $pattern : '';
 
-		$groupsInfo = new MetaData($this->userSession->getUser()->getUID(),
-			$this->isAdmin, $this->groupManager);
+		$groupsInfo = new MetaData(
+			$this->userSession->getUser()->getUID(),
+			$this->isAdmin,
+			$this->groupManager,
+			$this->userSession
+		);
 		$groupsInfo->setSorting($sortGroups);
 		list($adminGroups, $groups) = $groupsInfo->get($groupPattern, $pattern);
 
diff --git a/settings/controller/userscontroller.php b/settings/controller/userscontroller.php
index 1a77172db9b..8183bc4739b 100644
--- a/settings/controller/userscontroller.php
+++ b/settings/controller/userscontroller.php
@@ -74,8 +74,6 @@ 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
@@ -92,7 +90,6 @@ class UsersController extends Controller {
 	 * @param string $fromMailAddress
 	 * @param IURLGenerator $urlGenerator
 	 * @param IAppManager $appManager
-	 * @param SubAdminFactory $subAdminFactory
 	 */
 	public function __construct($appName,
 								IRequest $request,
@@ -107,8 +104,7 @@ class UsersController extends Controller {
 								IMailer $mailer,
 								$fromMailAddress,
 								IURLGenerator $urlGenerator,
-								IAppManager $appManager,
-								SubAdminFactory $subAdminFactory) {
+								IAppManager $appManager) {
 		parent::__construct($appName, $request);
 		$this->userManager = $userManager;
 		$this->groupManager = $groupManager;
@@ -121,7 +117,6 @@ class UsersController extends Controller {
 		$this->mailer = $mailer;
 		$this->fromMailAddress = $fromMailAddress;
 		$this->urlGenerator = $urlGenerator;
-		$this->subAdminFactory = $subAdminFactory;
 
 		// check for encryption state - TODO see formatUserForIndex
 		$this->isEncryptionAppEnabled = $appManager->isEnabledForUser('encryption');
@@ -164,11 +159,16 @@ class UsersController extends Controller {
 			$restorePossible = true;
 		}
 
+		$subAdminGroups = $this->groupManager->getSubAdmin()->getSubAdminsGroups($user);
+		foreach($subAdminGroups as $key => $subAdminGroup) {
+			$subAdminGroups[$key] = $subAdminGroup->getGID();
+		}
+
 		return [
 			'name' => $user->getUID(),
 			'displayname' => $user->getDisplayName(),
 			'groups' => (empty($userGroups)) ? $this->groupManager->getUserGroupIds($user) : $userGroups,
-			'subadmin' => $this->subAdminFactory->getSubAdminsOfGroups($user->getUID()),
+			'subadmin' => $subAdminGroups,
 			'quota' => $this->config->getUserValue($user->getUID(), 'files', 'quota', 'default'),
 			'storageLocation' => $user->getHome(),
 			'lastLogin' => $user->getLastLogin() * 1000,
@@ -234,9 +234,14 @@ class UsersController extends Controller {
 			}
 
 		} else {
-			$subAdminOfGroups = $this->subAdminFactory->getSubAdminsOfGroups(
-				$this->userSession->getUser()->getUID()
-			);
+			$subAdminOfGroups = $this->groupManager->getSubAdmin()->getSubAdminsGroups($this->userSession->getUser());
+			// New class returns IGroup[] so convert back
+			$gids = [];
+			foreach ($subAdminOfGroups as $group) {
+				$gids[] = $group->getGID();
+			}
+			$subAdminOfGroups = $gids;
+
 			// Set the $gid parameter to an empty value if the subadmin has no rights to access a specific group
 			if($gid !== '' && !in_array($gid, $subAdminOfGroups)) {
 				$gid = '';
@@ -247,6 +252,7 @@ class UsersController extends Controller {
 			if($gid === '') {
 				foreach($subAdminOfGroups as $group) {
 					$groupUsers = $this->groupManager->displayNamesInGroup($group, $pattern, $limit, $offset);
+
 					foreach($groupUsers as $uid => $displayName) {
 						$batch[$uid] = $displayName;
 					}
@@ -288,17 +294,31 @@ class UsersController extends Controller {
 			);
 		}
 
+		$currentUser = $this->userSession->getUser();
+
 		if (!$this->isAdmin) {
-			$userId = $this->userSession->getUser()->getUID();
 			if (!empty($groups)) {
 				foreach ($groups as $key => $group) {
-					if (!$this->subAdminFactory->isGroupAccessible($userId, $group)) {
+					$groupObject = $this->groupManager->get($group);
+					if($groupObject === null) {
+						unset($groups[$key]);
+						continue;
+					}
+
+					if (!$this->groupManager->getSubAdmin()->isSubAdminofGroup($currentUser, $groupObject)) {
 						unset($groups[$key]);
 					}
 				}
 			}
+
 			if (empty($groups)) {
-				$groups = $this->subAdminFactory->getSubAdminsOfGroups($userId);
+				$groups = $this->groupManager->getSubAdmin()->getSubAdminsGroups($currentUser);
+				// New class returns IGroup[] so convert back
+				$gids = [];
+				foreach ($groups as $group) {
+					$gids[] = $group->getGID();
+				}
+				$groups = $gids;
 			}
 		}
 
@@ -391,6 +411,8 @@ class UsersController extends Controller {
 	 */
 	public function destroy($id) {
 		$userId = $this->userSession->getUser()->getUID();
+		$user = $this->userManager->get($id);
+
 		if($userId === $id) {
 			return new DataResponse(
 				array(
@@ -403,7 +425,7 @@ class UsersController extends Controller {
 			);
 		}
 
-		if(!$this->isAdmin && !$this->subAdminFactory->isUserAccessible($userId, $id)) {
+		if(!$this->isAdmin && !$this->groupManager->getSubAdmin()->isUserAccessible($this->userSession->getUser(), $user)) {
 			return new DataResponse(
 				array(
 					'status' => 'error',
@@ -415,7 +437,6 @@ class UsersController extends Controller {
 			);
 		}
 
-		$user = $this->userManager->get($id);
 		if($user) {
 			if($user->delete()) {
 				return new DataResponse(
@@ -453,9 +474,11 @@ class UsersController extends Controller {
 	 */
 	public function setMailAddress($id, $mailAddress) {
 		$userId = $this->userSession->getUser()->getUID();
+		$user = $this->userManager->get($id);
+
 		if($userId !== $id
 			&& !$this->isAdmin
-			&& !$this->subAdminFactory->isUserAccessible($userId, $id)) {
+			&& !$this->groupManager->getSubAdmin()->isUserAccessible($this->userSession->getUser(), $user)) {
 			return new DataResponse(
 				array(
 					'status' => 'error',
@@ -479,7 +502,6 @@ class UsersController extends Controller {
 			);
 		}
 
-		$user = $this->userManager->get($id);
 		if(!$user){
 			return new DataResponse(
 				array(
diff --git a/settings/factory/subadminfactory.php b/settings/factory/subadminfactory.php
deleted file mode 100644
index 5a0f6e4e1e4..00000000000
--- a/settings/factory/subadminfactory.php
+++ /dev/null
@@ -1,57 +0,0 @@
-<?php
-/**
- * @author Lukas Reschke <lukas@owncloud.com>
- * @author Morris Jobke <hey@morrisjobke.de>
- *
- * @copyright Copyright (c) 2015, ownCloud, Inc.
- * @license AGPL-3.0
- *
- * This code is free software: you can redistribute it and/or modify
- * it under the terms of the GNU Affero General Public License, version 3,
- * as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU Affero General Public License for more details.
- *
- * You should have received a copy of the GNU Affero General Public License, version 3,
- * along with this program.  If not, see <http://www.gnu.org/licenses/>
- *
- */
-
-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);
-	}
-}
diff --git a/settings/users.php b/settings/users.php
index 59cc94f5023..dac649aa828 100644
--- a/settings/users.php
+++ b/settings/users.php
@@ -57,7 +57,12 @@ $config = \OC::$server->getConfig();
 
 $isAdmin = OC_User::isAdminUser(OC_User::getUser());
 
-$groupsInfo = new \OC\Group\MetaData(OC_User::getUser(), $isAdmin, $groupManager);
+$groupsInfo = new \OC\Group\MetaData(
+	OC_User::getUser(),
+	$isAdmin,
+	$groupManager,
+	\OC::$server->getUserSession()
+);
 $groupsInfo->setSorting($sortGroupsBy);
 list($adminGroup, $groups) = $groupsInfo->get();
 
@@ -65,7 +70,16 @@ $recoveryAdminEnabled = OC_App::isEnabled('encryption') &&
 					    $config->getAppValue( 'encryption', 'recoveryAdminEnabled', null );
 
 if($isAdmin) {
-	$subadmins = OC_SubAdmin::getAllSubAdmins();
+	$subadmins = \OC::$server->getGroupManager()->getSubAdmin()->getAllSubAdmins();
+	// New class returns IUser[] so convert back
+	$result = [];
+	foreach ($subAdmins as $subAdmin) {
+		$result[] = [
+			'gid' => $subAdmin['group']->getGID(),
+			'uid' => $subAdmin['user']->getUID(),
+		];
+	}
+	$subadmins = $result;
 }else{
 	/* Retrieve group IDs from $groups array, so we can pass that information into OC_Group::displayNamesInGroups() */
 	$gids = array();