Merge pull request #134 from owncloud/allow-allgroupadmins

Add option for all group admins to impersonate their group memebers
owncloud · Feb 12, 2019 · 7341a80 · 7341a80
2 parents 37445b6 + 3a3e44e
commit 7341a80
Show file tree

Hide file tree

Showing 5 changed files with 141 additions and 57 deletions.
diff --git a/controller/settingscontroller.php b/controller/settingscontroller.php
@@ -23,6 +23,7 @@
 use OCP\IRequest;
 use OCP\AppFramework\Controller;
 use OCP\ISession;
+use OCP\IUser;
 use OCP\IUserManager;
 use OCP\IUserSession;
 use Symfony\Component\EventDispatcher\EventDispatcher;
@@ -98,17 +99,37 @@ public function __construct($appName, IRequest $request, IUserManager $userManag
 	public function getDataForImpersonateApp() {
 		$isEnabled = $this->config->getValue('impersonate', 'impersonate_include_groups', false);
 		$includedGroups = $this->config->getValue('impersonate', 'impersonate_include_groups_list', '[]');
+		$allowFullAccessSubAdmins = $this->config->getValue('impersonate', 'impersonate_all_groupadmins', false);
 		$currentUser = $this->userSession->getUser();
 		if ($currentUser === null) {
 			return new JSONResponse([$includedGroups, $isEnabled,
-				false, false]);
+				$allowFullAccessSubAdmins, false, false]);
 		}
 
 		return new JSONResponse([$includedGroups, $isEnabled,
+			$allowFullAccessSubAdmins,
 			$this->groupManager->isAdmin($currentUser->getUID()),
 			$this->subAdmin->isSubAdmin($currentUser)]);
 	}
 
+	/**
+	 * Impersonate the user
+	 * This method is called after the users capability to impersonate is decided
+	 * in the method impersonate($target).
+	 *
+	 * @param string $impersonator, the current user
+	 * @param string $target, the target user
+	 * @param IUser $user, target user object
+	 * @return JSONResponse
+	 */
+	private function impersonateUser($impersonator, $target, $user) {
+		$this->logger->info("User $impersonator impersonated user $target", ['app' => 'impersonate']);
+		$this->util->switchUser($user, $impersonator);
+		$startEvent = new GenericEvent(null, ['impersonator' => $impersonator, 'targetUser' => $target]);
+		$this->eventDispatcher->dispatch('user.afterimpersonate', $startEvent);
+		return new JSONResponse();
+	}
+
 	/**
 	 * become another user
 	 * @param string $target
@@ -164,25 +185,20 @@ public function impersonate($target) {
 			], http::STATUS_NOT_FOUND);
 		} else {
 			if ($this->groupManager->isAdmin($currentUser->getUID())) {
-				$this->logger->info("User $impersonator impersonated user $target", ['app' => 'impersonate']);
-				$this->util->switchUser($user, $impersonator);
-				$startEvent = new GenericEvent(null, ['impersonator' => $impersonator, 'targetUser' => $target]);
-				$this->eventDispatcher->dispatch('user.afterimpersonate', $startEvent);
-				return new JSONResponse();
+				return $this->impersonateUser($impersonator, $target, $user);
 			}
 
 			$includedGroups = $this->config->getValue('impersonate', 'impersonate_include_groups_list', '');
-			if ($includedGroups !== '') {
+			$allowSubAdminsImpersonate = $this->config->getValue('impersonate', 'impersonate_all_groupadmins', "false");
+			if ($allowSubAdminsImpersonate === "true") {
+				return $this->impersonateUser($impersonator, $target, $user);
+			} elseif ($includedGroups !== '') {
 				$includedGroups = \json_decode($includedGroups);
 
 				foreach ($includedGroups as $group) {
 					if ($this->groupManager->isInGroup($user->getUID(), $group)
 						&& $this->subAdmin->isSubAdminofGroup($this->userSession->getUser(), $this->groupManager->get($group))) {
-						$this->logger->info("User $impersonator impersonated user $target", ['app' => 'impersonate']);
-						$this->util->switchUser($user, $impersonator);
-						$startEvent = new GenericEvent(null, ['impersonator' => $impersonator, 'targetUser' => $target]);
-						$this->eventDispatcher->dispatch('user.afterimpersonate', $startEvent);
-						return new JSONResponse();
+						return $this->impersonateUser($impersonator, $target, $user);
 					}
 				}
 			}

diff --git a/js/impersonate.js b/js/impersonate.js
@@ -26,7 +26,7 @@
 	var includedGroups,
 	groupEnabled,
 	adminUser,
-	subadminUser;
+	subadminUser, subAdminFullAccess;
 
 	var currentUser = OC.getCurrentUser().uid;
 
@@ -41,8 +41,9 @@
 				promiseGetData.then(function (result) {
 					includedGroups = $.parseJSON(result[0]);
 					groupEnabled = JSON.parse($.trim(result[1]));
-					adminUser = JSON.parse($.trim(result[2]));
-					subadminUser = JSON.parse($.trim(result[3]));
+					subAdminFullAccess = JSON.parse($.trim(result[2]));
+					adminUser = JSON.parse($.trim(result[3]));
+					subadminUser = JSON.parse($.trim(result[4]));
 					if (!$tr) {
 						return;
 					}
@@ -59,21 +60,25 @@
 						addImpersonateIcon($tr);
 
 					} else if (subadminUser) {
-						var found = false;
-						if(groupEnabled) {
-							for (var i = 0; i < includedGroups.length; i++) {
-								if ($.inArray($.trim(includedGroups[i]), groupsSelectedByUser) !== -1) {
-									found = true;
-									addImpersonateIcon($tr);
-									break;
+						if (subAdminFullAccess) {
+							addImpersonateIcon($tr);
+						} else {
+							var found = false;
+							if (groupEnabled) {
+								for (var i = 0; i < includedGroups.length; i++) {
+									if ($.inArray($.trim(includedGroups[i]), groupsSelectedByUser) !== -1) {
+										found = true;
+										addImpersonateIcon($tr);
+										break;
+									}
 								}
+							} else {
+								addImpersonateIcon($tr);
+								found = true;
+							}
+							if (found === false) {
+								removeImpersonateIcon($tr);
 							}
-						} else {
-							addImpersonateIcon($tr);
-							found = true;
-						}
-						if (found === false) {
-							removeImpersonateIcon($tr);
 						}
 					}
 					return $tr;

diff --git a/js/settings-admin.js b/js/settings-admin.js
@@ -1,8 +1,17 @@
 $(document).ready(function () {
 	$("#impersonateIncludeGroups").change(function () {
-		$("#selectIncludedGroups").toggleClass('hidden', !this.checked);
+		$("#selectIncludedGroups").removeClass('hidden');
 		var val = $("#impersonateIncludeGroups").is(":checked");
-		OC.AppConfig.setValue('impersonate',$(this).attr('name'),val);
+		OC.AppConfig.setValue('impersonate', 'impersonate_include_groups', val);
+		OC.AppConfig.setValue('impersonate', 'impersonate_all_groupadmins', 'false')
+	});
+
+	$("#impersonateAllGroupAdmins").change(function () {
+		var val = $("#impersonateAllGroupAdmins").is(":checked");
+		$("#selectIncludedGroups").addClass('hidden');
+		OC.AppConfig.setValue('impersonate', 'impersonate_all_groupadmins', val);
+		OC.AppConfig.setValue('impersonate', 'impersonate_include_groups', 'false')
+		OC.AppConfig.setValue('impersonate', 'impersonate_include_groups_list', JSON.stringify([]));
 	});
 
 	$('#includedGroups').each(function (index, element) {

diff --git a/templates/settings-admin.php b/templates/settings-admin.php
@@ -9,11 +9,18 @@
 	<p class="<?php if (\OC::$server->getAppConfig()->getValue('impersonate', 'enabled', 'no') === 'no') {
 	p('hidden');
 }?>">
-		<input type="checkbox" name="impersonate_include_groups" id="impersonateIncludeGroups" class="checkbox"
-			value="1" <?php if (\OC::$server->getAppConfig()->getValue('impersonate', 'impersonate_include_groups', 'false') !== 'false') {
+		<input type="radio" name="groupadmins" id="impersonateAllGroupAdmins" class="radio"
+			<?php if (\OC::$server->getAppConfig()->getValue('impersonate', 'impersonate_all_groupadmins', 'false') !== 'false') {
 	print_unescaped('checked="checked"');
 } ?> />
-		<label for="impersonateIncludeGroups"><?php p($l->t('Allow group admins to impersonate users from these groups'));?></label><br/>
+		<label for="impersonateAllGroupAdmins"><?php p($l->t('Allow all group admins to impersonate users within the groups they are admins of'))?></label><br/>
+
+		<input type="radio" name="groupadmins" id="impersonateIncludeGroups" class="radio"
+			<?php if (\OC::$server->getAppConfig()->getValue('impersonate', 'impersonate_include_groups', 'false') !== 'false') {
+	print_unescaped('checked="checked"');
+} ?> />
+		<label for="impersonateIncludeGroups"><?php p($l->t('Allow group admins of specific groups to impersonate the users within those groups'));?></label><br/>
+
 	</p>
 	<p id="selectIncludedGroups" class="indent <?php if (\OC::$server->getAppConfig()->getValue('impersonate', 'impersonate_include_groups', 'false') === 'false') {
 	p('hidden');

diff --git a/tests/controller/SettingsControllerTest.php b/tests/controller/SettingsControllerTest.php
@@ -22,6 +22,7 @@
 use OCP\IL10N;
 use OCP\IRequest;
 use OCP\ILogger;
+use OCP\IUser;
 use OCP\IUserManager;
 use OCP\IUserSession;
 use OCP\ISession;
@@ -173,10 +174,12 @@ public function testImpersonate($query, $uid, $group) {
 				$this->controller->impersonate($query)
 			);
 		} elseif ($group === 'groupadmin') {
-			$this->config->expects($this->once())
+			$this->config
 				->method('getValue')
-				->with('impersonate', 'impersonate_include_groups_list', "")
-				->willReturn(\json_encode([$group]));
+				->will($this->returnValueMap([
+					['impersonate', 'impersonate_include_groups_list', "", \json_encode([$group])],
+					['impersonate', 'impersonate_all_groupadmins', "false", "false"]
+				]));
 
 			$iGroup = $this->createMock(IGroup::class);
 
@@ -209,10 +212,12 @@ function (GenericEvent $event) use (&$calledAfterImpersonate) {
 			$this->assertEquals('username', $calledAfterImpersonate[1]->getArgument('impersonator'));
 			$this->assertEquals('Username', $calledAfterImpersonate[1]->getArgument('targetUser'));
 		} elseif ($group === 'normaluser') {
-			$this->config->expects($this->once())
+			$this->config
 				->method('getValue')
-				->with('impersonate', 'impersonate_include_groups_list', "")
-				->willReturn("");
+				->will($this->returnValueMap([
+					['impersonate', 'impersonate_include_groups_list', "", ""],
+					['impersonate', 'impersonate_all_groupadmins', "false", "false"]
+				]));
 
 			$this->groupManger->expects($this->any())
 				->method('isAdmin')
@@ -253,10 +258,12 @@ public function testUserNotPartOfGroup() {
 			->method('getLastLogin')
 			->willReturn(1);
 
-		$this->config->expects($this->once())
+		$this->config
 			->method('getValue')
-			->with('impersonate', 'impersonate_include_groups_list', "")
-			->willReturn(\json_encode(['testgroup']));
+			->will($this->returnValueMap([
+				['impersonate', 'impersonate_include_groups_list', "", \json_encode(['testgroup'])],
+				['impersonate', 'impersonate_all_groupadmins', "false", "false"]
+			]));
 
 		$iGroup = $this->createMock(IGroup::class);
 
@@ -304,10 +311,12 @@ public function testWronglyConfiguredGroupListAllowsImpersonation() {
 			->method('getLastLogin')
 			->willReturn(1);
 
-		$this->config->expects($this->once())
+		$this->config
 			->method('getValue')
-			->with('impersonate', 'impersonate_include_groups_list', "")
-			->willReturn(\json_encode(['testgroup','testgroup2']));
+			->will($this->returnValueMap([
+				['impersonate', 'impersonate_include_groups_list', "", \json_encode(['testgroup','testgroup2'])],
+				['impersonate', 'impersonate_all_groupadmins', "false", "false"]
+			]));
 
 		$iGroup = $this->createMock(IGroup::class);
 
@@ -492,23 +501,29 @@ public function testRestrictNestedImpersonate($subadminUser, $subadminUid) {
 	/**
 	 * @dataProvider providesGetDataForImpersonateApp
 	 */
-	public function testGetDataForImpersonateApp($enabled, $includedGroups, $currentUser, $isAdmin = false, $isSubAdmin = false) {
+	public function testGetDataForImpersonateApp($enabled, $includedGroups, $allowAllSubadminGroups, $currentUser, $isAdmin = false, $isSubAdmin = false) {
 		$map = [
-			['impersonate','impersonate_include_groups',false, $enabled],
-			['impersonate','impersonate_include_groups_list', '[]', $includedGroups]
+			['impersonate','impersonate_include_groups', false, $enabled],
+			['impersonate','impersonate_include_groups_list', '[]', $includedGroups],
+			['impersonate', 'impersonate_all_groupadmins', false, $allowAllSubadminGroups]
 		];
 		$this->config
 			->method('getValue')
 			->will($this->returnValueMap($map));
 
-		$user = $this->createMock('\OCP\IUser');
+		if ($currentUser === null) {
+			$user = null;
+		} else {
+			$user = $this->createMock('\OCP\IUser');
+		}
 		$this->userSession
 			->method('getUser')
 			->willReturn($user);
 		if ($currentUser === null) {
+			$user = null;
 			$this->assertEquals(
 				new JSONResponse([
-					$includedGroups, $enabled, false, false
+					$includedGroups, $enabled, $allowAllSubadminGroups, false, false
 				]),
 				$this->controller->getDataForImpersonateApp('test')
 			);
@@ -524,7 +539,7 @@ public function testGetDataForImpersonateApp($enabled, $includedGroups, $current
 				->willReturn($isSubAdmin);
 			$this->assertEquals(
 				new JSONResponse([
-					$includedGroups, $enabled, $isAdmin, $isSubAdmin
+					$includedGroups, $enabled, $allowAllSubadminGroups, $isAdmin, $isSubAdmin
 				]),
 				$this->controller->getDataForImpersonateApp('test')
 			);
@@ -533,12 +548,44 @@ public function testGetDataForImpersonateApp($enabled, $includedGroups, $current
 
 	public function providesGetDataForImpersonateApp() {
 		return [
-			[true, ['hello', 'world'], null],
-			[true, ['hello', 'world'], 'user', true, true],
-			[true, ['hello', 'world'], 'user', true, false],
-			[true, ['hello', 'world'], 'user', false, true],
-			[false, ['hello', 'world'], 'user', false, true],
-			[false, [], 'user', false, true],
+			[true, ['hello', 'world'], "true", null],
+			[true, ['hello', 'world'], "false", 'user', true, true],
+			[true, ['hello', 'world'], "false",'user', true, false],
+			[true, ['hello', 'world'], "false", 'user', false, true],
+			[false, ['hello', 'world'], "false", 'user', false, true],
+			[false, [], "true", 'user', false, true],
 		];
 	}
+
+	public function testImpersonateGrantAllSubadmin() {
+		$user = $this->createMock(IUser::class);
+		$user->method('getUID')
+			->willReturn('foo');
+
+		$this->userSession
+			->method('getUser')
+			->willReturn($user);
+
+		$targetUser = $this->createMock(IUser::class);
+		$targetUser->method('getLastLogin')
+			->willReturn(1);
+
+		$this->userManager->method('get')
+			->willReturn($targetUser);
+
+		$this->groupManger->method('isAdmin')
+			->willReturn(false);
+
+		$this->config
+			->method('getValue')
+			->will($this->returnValueMap([
+				['impersonate', 'impersonate_include_groups_list', "", ""],
+				['impersonate', 'impersonate_all_groupadmins', "false", "true"]
+			]));
+
+		$this->assertEquals(
+			new JSONResponse(),
+			$this->controller->impersonate('foo')
+		);
+	}
 }