Add system tag assignability check with groups

Whenever a user is not an admin, a tag is visible but not
user-assignable, check whether the user is a member of the allowed
groups.

2 files changed, 34 insertions, 1 deletions

diff --git a/lib/private/SystemTag/SystemTagManager.php b/lib/private/SystemTag/SystemTagManager.php
index 1c91ad1f578..832afc2a114 100644
--- a/lib/private/SystemTag/SystemTagManager.php
+++ b/lib/private/SystemTag/SystemTagManager.php
@@ -337,6 +337,7 @@ class SystemTagManager implements ISystemTagManager {
 	 * {@inheritdoc}
 	 */
 	public function canUserAssignTag(ISystemTag $tag, IUser $user) {
+		// early check to avoid unneeded group lookups
 		if ($tag->isUserAssignable() && $tag->isUserVisible()) {
 			return true;
 		}
@@ -345,6 +346,18 @@ class SystemTagManager implements ISystemTagManager {
 			return true;
 		}
 
+		if (!$tag->isUserVisible()) {
+			return false;
+		}
+
+		$groupIds = $this->groupManager->getUserGroupIds($user->getUID());
+		if (!empty($groupIds)) {
+			$matchingGroups = array_intersect($groupIds, $this->getTagGroups($tag));
+			if (!empty($matchingGroups)) {
+				return true;
+			}
+		}
+
 		return false;
 	}
 
diff --git a/tests/lib/SystemTag/SystemTagManagerTest.php b/tests/lib/SystemTag/SystemTagManagerTest.php
index 408134a8757..04f49eff963 100644
--- a/tests/lib/SystemTag/SystemTagManagerTest.php
+++ b/tests/lib/SystemTag/SystemTagManagerTest.php
@@ -449,31 +449,51 @@ class SystemTagManagerTest extends TestCase {
 
 	public function assignabilityCheckProvider() {
 		return [
+			// no groups
 			[false, false, false, false],
 			[true, false, false, false],
 			[true, true, false, true],
 			[false, true, false, false],
+			// admin rulez
 			[false, false, true, true],
 			[false, true, true, true],
 			[true, false, true, true],
 			[true, true, true, true],
+			// ignored groups
+			[false, false, false, false, ['group1'], ['group1']],
+			[true, true, false, true, ['group1'], ['group1']],
+			[true, true, false, true, ['group1'], ['anothergroup']],
+			[false, true, false, false, ['group1'], ['group1']],
+			// admin has precedence over groups
+			[false, false, true, true, ['group1'], ['anothergroup']],
+			[false, true, true, true, ['group1'], ['anothergroup']],
+			[true, false, true, true, ['group1'], ['anothergroup']],
+			[true, true, true, true, ['group1'], ['anothergroup']],
+			// groups only checked when visible and user non-assignable and non-admin
+			[true, false, false, false, ['group1'], ['anothergroup1']],
+			[true, false, false, true, ['group1'], ['group1']],
+			[true, false, false, true, ['group1', 'group2'], ['group2', 'group3']],
 		];
 	}
 
 	/**
 	 * @dataProvider assignabilityCheckProvider
 	 */
-	public function testAssignabilityCheck($userVisible, $userAssignable, $isAdmin, $expectedResult) {
+	public function testAssignabilityCheck($userVisible, $userAssignable, $isAdmin, $expectedResult, $userGroupIds = [], $tagGroupIds = []) {
 		$user = $this->getMockBuilder('\OCP\IUser')->getMock();
 		$user->expects($this->any())
 			->method('getUID')
 			->will($this->returnValue('test'));
 		$tag1 = $this->tagManager->createTag('one', $userVisible, $userAssignable);
+		$this->tagManager->setTagGroups($tag1, $tagGroupIds);
 
 		$this->groupManager->expects($this->any())
 			->method('isAdmin')
 			->with('test')
 			->will($this->returnValue($isAdmin));
+		$this->groupManager->expects($this->any())
+			->method('getUserGroupIds')
+			->will($this->returnValue($userGroupIds));
 
 		$this->assertEquals($expectedResult, $this->tagManager->canUserAssignTag($tag1, $user));
 	}