Merge pull request #24307 from owncloud/systemtags-perminterface

Add interface methods for permission check for system tags

apps/dav/lib/SystemTag/SystemTagMappingNode.php

@@ -24,21 +24,22 @@ namespace OCA\DAV\SystemTag;
 
 use Sabre\DAV\Exception\NotFound;
 use Sabre\DAV\Exception\Forbidden;
+use Sabre\DAV\Exception\MethodNotAllowed;
 
 use OCP\SystemTag\ISystemTag;
 use OCP\SystemTag\ISystemTagManager;
 use OCP\SystemTag\ISystemTagObjectMapper;
 use OCP\SystemTag\TagNotFoundException;
+use OCP\IUser;
 
 /**
  * Mapping node for system tag to object id
  */
-class SystemTagMappingNode extends SystemTagNode {
-
+class SystemTagMappingNode implements \Sabre\DAV\INode {
 	/**
-	 * @var ISystemTagObjectMapper
+	 * @var ISystemTag
 	 */
-	private $tagMapper;
+	protected $tag;
 
 	/**
 	 * @var string
@@ -50,13 +51,30 @@ class SystemTagMappingNode extends SystemTagNode {
 	 */
 	private $objectType;
 
+	/**
+	 * User
+	 *
+	 * @var IUser
+	 */
+	protected $user;
+
+	/**
+	 * @var ISystemTagManager
+	 */
+	protected $tagManager;
+
+	/**
+	 * @var ISystemTagObjectMapper
+	 */
+	private $tagMapper;
+
 	/**
 	 * Sets up the node, expects a full path name
 	 *
 	 * @param ISystemTag $tag system tag
 	 * @param string $objectId
 	 * @param string $objectType
-	 * @param bool $isAdmin whether to allow permissions for admin
+	 * @param IUser $user user
 	 * @param ISystemTagManager $tagManager
 	 * @param ISystemTagObjectMapper $tagMapper
 	 */
@@ -64,14 +82,16 @@ class SystemTagMappingNode extends SystemTagNode {
 		ISystemTag $tag,
 		$objectId,
 		$objectType,
-		$isAdmin,
+		IUser $user,
 		ISystemTagManager $tagManager,
 		ISystemTagObjectMapper $tagMapper
 	) {
+		$this->tag = $tag;
 		$this->objectId = $objectId;
 		$this->objectType = $objectType;
+		$this->user = $user;
+		$this->tagManager = $tagManager;
 		$this->tagMapper = $tagMapper;
-		parent::__construct($tag, $isAdmin, $tagManager);
 	}
 
 	/**
@@ -92,18 +112,53 @@ class SystemTagMappingNode extends SystemTagNode {
 		return $this->objectType;
 	}
 
+	/**
+	 * Returns the system tag represented by this node
+	 *
+	 * @return ISystemTag system tag
+	 */
+	public function getSystemTag() {
+		return $this->tag;
+	}
+
+	/**
+	 *  Returns the id of the tag
+	 *
+	 * @return string
+	 */
+	public function getName() {
+		return $this->tag->getId();
+	}
+
+	/**
+	 * Renames the node
+	 *
+	 * @param string $name The new name
+	 *
+	 * @throws MethodNotAllowed not allowed to rename node
+	 */
+	public function setName($name) {
+		throw new MethodNotAllowed();
+	}
+
+	/**
+	 * Returns null, not supported
+	 *
+	 */
+	public function getLastModified() {
+		return null;
+	}
+
 	/**
 	 * Delete tag to object association
 	 */
 	public function delete() {
 		try {
-			if (!$this->isAdmin) {
-				if (!$this->tag->isUserVisible()) {
-					throw new NotFound('Tag with id ' . $this->tag->getId() . ' not found');
-				}
-				if (!$this->tag->isUserAssignable()) {
-					throw new Forbidden('No permission to unassign tag ' . $this->tag->getId());
-				}
+			if (!$this->tagManager->canUserSeeTag($this->tag, $this->user)) {
+				throw new NotFound('Tag with id ' . $this->tag->getId() . ' not found');
+			}
+			if (!$this->tagManager->canUserAssignTag($this->tag, $this->user)) {
+				throw new Forbidden('No permission to unassign tag ' . $this->tag->getId());
 			}
 			$this->tagMapper->unassignTags($this->objectId, $this->objectType, $this->tag->getId());
 		} catch (TagNotFoundException $e) {

apps/dav/lib/SystemTag/SystemTagNode.php

@@ -32,6 +32,7 @@ use OCP\SystemTag\ISystemTag;
 use OCP\SystemTag\ISystemTagManager;
 use OCP\SystemTag\TagNotFoundException;
 use OCP\SystemTag\TagAlreadyExistsException;
+use OCP\IUser;
 
 /**
  * DAV node representing a system tag, with the name being the tag id.
@@ -48,6 +49,13 @@ class SystemTagNode implements \Sabre\DAV\INode {
 	 */
 	protected $tagManager;
 
+	/**
+	 * User
+	 *
+	 * @var IUser
+	 */
+	protected $user;
+
 	/**
 	 * Whether to allow permissions for admins
 	 *
@@ -59,11 +67,13 @@ class SystemTagNode implements \Sabre\DAV\INode {
 	 * Sets up the node, expects a full path name
 	 *
 	 * @param ISystemTag $tag system tag
+	 * @param IUser $user user
 	 * @param bool $isAdmin whether to allow operations for admins
-	 * @param ISystemTagManager $tagManager
+	 * @param ISystemTagManager $tagManager tag manager
 	 */
-	public function __construct(ISystemTag $tag, $isAdmin, ISystemTagManager $tagManager) {
+	public function __construct(ISystemTag $tag, IUser $user, $isAdmin, ISystemTagManager $tagManager) {
 		$this->tag = $tag;
+		$this->user = $user;
 		$this->isAdmin = $isAdmin;
 		$this->tagManager = $tagManager;
 	}
@@ -109,14 +119,15 @@ class SystemTagNode implements \Sabre\DAV\INode {
 	 */
 	public function update($name, $userVisible, $userAssignable) {
 		try {
-			if (!$this->isAdmin) {
-				if (!$this->tag->isUserVisible()) {
-					throw new NotFound('Tag with id ' . $this->tag->getId() . ' does not exist');
-				}
-				if (!$this->tag->isUserAssignable()) {
-					throw new Forbidden('No permission to update tag ' . $this->tag->getId());
-				}
+			if (!$this->tagManager->canUserSeeTag($this->tag, $this->user)) {
+				throw new NotFound('Tag with id ' . $this->tag->getId() . ' does not exist');
+			}
+			if (!$this->tagManager->canUserAssignTag($this->tag, $this->user)) {
+				throw new Forbidden('No permission to update tag ' . $this->tag->getId());
+			}
 
+			// only admin is able to change permissions, regular users can only rename
+			if (!$this->isAdmin) {
 				// only renaming is allowed for regular users
 				if ($userVisible !== $this->tag->isUserVisible()
 					|| $userAssignable !== $this->tag->isUserAssignable()
@@ -124,6 +135,7 @@ class SystemTagNode implements \Sabre\DAV\INode {
 					throw new Forbidden('No permission to update permissions for tag ' . $this->tag->getId());
 				}
 			}
+
 			$this->tagManager->updateTag($this->tag->getId(), $name, $userVisible, $userAssignable);
 		} catch (TagNotFoundException $e) {
 			throw new NotFound('Tag with id ' . $this->tag->getId() . ' does not exist');
@@ -145,14 +157,13 @@ class SystemTagNode implements \Sabre\DAV\INode {
 
 	public function delete() {
 		try {
-			if (!$this->isAdmin) {
-				if (!$this->tag->isUserVisible()) {
-					throw new NotFound('Tag with id ' . $this->tag->getId() . ' not found');
-				}
-				if (!$this->tag->isUserAssignable()) {
-					throw new Forbidden('No permission to delete tag ' . $this->tag->getId());
-				}
+			if (!$this->tagManager->canUserSeeTag($this->tag, $this->user)) {
+				throw new NotFound('Tag with id ' . $this->tag->getId() . ' not found');
 			}
+			if (!$this->tagManager->canUserAssignTag($this->tag, $this->user)) {
+				throw new Forbidden('No permission to delete tag ' . $this->tag->getId());
+			}
+
 			$this->tagManager->deleteTags($this->tag->getId());
 		} catch (TagNotFoundException $e) {
 			// can happen if concurrent deletion occurred

apps/dav/lib/SystemTag/SystemTagPlugin.php

@@ -24,18 +24,20 @@ namespace OCA\DAV\SystemTag;
 
 use OCP\IGroupManager;
 use OCP\IUserSession;
-use Sabre\DAV\Exception\NotFound;
 use Sabre\DAV\PropFind;
 use Sabre\DAV\PropPatch;
 use Sabre\DAV\Exception\BadRequest;
-use Sabre\DAV\Exception\UnsupportedMediaType;
 use Sabre\DAV\Exception\Conflict;
+use Sabre\DAV\Exception\Forbidden;
+use Sabre\DAV\Exception\NotFound;
+use Sabre\DAV\Exception\UnsupportedMediaType;
 
 use OCP\SystemTag\ISystemTag;
 use OCP\SystemTag\ISystemTagManager;
 use OCP\SystemTag\TagAlreadyExistsException;
 use Sabre\HTTP\RequestInterface;
 use Sabre\HTTP\ResponseInterface;
+use OCA\DAV\SystemTag\SystemTagMappingNode;
 
 /**
  * Sabre plugin to handle system tags:
@@ -52,6 +54,8 @@ class SystemTagPlugin extends \Sabre\DAV\ServerPlugin {
 	const DISPLAYNAME_PROPERTYNAME = '{http://owncloud.org/ns}display-name';
 	const USERVISIBLE_PROPERTYNAME = '{http://owncloud.org/ns}user-visible';
 	const USERASSIGNABLE_PROPERTYNAME = '{http://owncloud.org/ns}user-assignable';
+	const GROUPS_PROPERTYNAME = '{http://owncloud.org/ns}groups';
+	const CANASSIGN_PROPERTYNAME = '{http://owncloud.org/ns}can-assign';
 
 	/**
 	 * @var \Sabre\DAV\Server $server
@@ -181,14 +185,26 @@ class SystemTagPlugin extends \Sabre\DAV\ServerPlugin {
 			$userAssignable = (bool)$data['userAssignable'];
 		}
 
-		if($userVisible === false || $userAssignable === false) {
+		$groups = [];
+		if (isset($data['groups'])) {
+			$groups = $data['groups'];
+			if (is_string($groups)) {
+				$groups = explode('|', $groups);
+			}
+		}
+
+		if($userVisible === false || $userAssignable === false || !empty($groups)) {
 			if(!$this->userSession->isLoggedIn() || !$this->groupManager->isAdmin($this->userSession->getUser()->getUID())) {
 				throw new BadRequest('Not sufficient permissions');
 			}
 		}
 
 		try {
-			return $this->tagManager->createTag($tagName, $userVisible, $userAssignable);
+			$tag = $this->tagManager->createTag($tagName, $userVisible, $userAssignable);
+			if (!empty($groups)) {
+				$this->tagManager->setTagGroups($tag, $groups);
+			}
+			return $tag;
 		} catch (TagAlreadyExistsException $e) {
 			throw new Conflict('Tag already exists', 0, $e);
 		}
@@ -205,7 +221,7 @@ class SystemTagPlugin extends \Sabre\DAV\ServerPlugin {
 		PropFind $propFind,
 		\Sabre\DAV\INode $node
 	) {
-		if (!($node instanceof SystemTagNode)) {
+		if (!($node instanceof SystemTagNode) && !($node instanceof SystemTagMappingNode)) {
 			return;
 		}
 
@@ -222,8 +238,27 @@ class SystemTagPlugin extends \Sabre\DAV\ServerPlugin {
 		});
 
 		$propFind->handle(self::USERASSIGNABLE_PROPERTYNAME, function() use ($node) {
+			// this is the tag's inherent property "is user assignable"
 			return $node->getSystemTag()->isUserAssignable() ? 'true' : 'false';
 		});
+
+		$propFind->handle(self::CANASSIGN_PROPERTYNAME, function() use ($node) {
+			// this is the effective permission for the current user
+			return $this->tagManager->canUserAssignTag($node->getSystemTag(), $this->userSession->getUser()) ? 'true' : 'false';
+		});
+
+		$propFind->handle(self::GROUPS_PROPERTYNAME, function() use ($node) {
+			if (!$this->groupManager->isAdmin($this->userSession->getUser()->getUID())) {
+				// property only available for admins
+				throw new Forbidden();
+			}
+			$groups = [];
+			// no need to retrieve groups for namespaces that don't qualify
+			if ($node->getSystemTag()->isUserVisible() && !$node->getSystemTag()->isUserAssignable()) {
+				$groups = $this->tagManager->getTagGroups($node->getSystemTag());
+			}
+			return implode('|', $groups);
+		});
 	}
 
 	/**
@@ -239,6 +274,7 @@ class SystemTagPlugin extends \Sabre\DAV\ServerPlugin {
 			self::DISPLAYNAME_PROPERTYNAME,
 			self::USERVISIBLE_PROPERTYNAME,
 			self::USERASSIGNABLE_PROPERTYNAME,
+			self::GROUPS_PROPERTYNAME,
 		], function($props) use ($path) {
 			$node = $this->server->tree->getNodeForPath($path);
 			if (!($node instanceof SystemTagNode)) {
@@ -250,22 +286,42 @@ class SystemTagPlugin extends \Sabre\DAV\ServerPlugin {
 			$userVisible = $tag->isUserVisible();
 			$userAssignable = $tag->isUserAssignable();
 
+			$updateTag = false;
+
 			if (isset($props[self::DISPLAYNAME_PROPERTYNAME])) {
 				$name = $props[self::DISPLAYNAME_PROPERTYNAME];
+				$updateTag = true;
 			}
 
 			if (isset($props[self::USERVISIBLE_PROPERTYNAME])) {
 				$propValue = $props[self::USERVISIBLE_PROPERTYNAME];
 				$userVisible = ($propValue !== 'false' && $propValue !== '0');
+				$updateTag = true;
 			}
 
 			if (isset($props[self::USERASSIGNABLE_PROPERTYNAME])) {
 				$propValue = $props[self::USERASSIGNABLE_PROPERTYNAME];
 				$userAssignable = ($propValue !== 'false' && $propValue !== '0');
+				$updateTag = true;
+			}
+
+			if (isset($props[self::GROUPS_PROPERTYNAME])) {
+				if (!$this->groupManager->isAdmin($this->userSession->getUser()->getUID())) {
+					// property only available for admins
+					throw new Forbidden();
+				}
+
+				$propValue = $props[self::GROUPS_PROPERTYNAME];
+				$groupIds = explode('|', $propValue);
+				$this->tagManager->setTagGroups($tag, $groupIds);
+			}
+
+			if ($updateTag) {
+				$node->update($name, $userVisible, $userAssignable);
 			}
 
-			$node->update($name, $userVisible, $userAssignable);
 			return true;
 		});
+
 	}
 }

apps/dav/lib/SystemTag/SystemTagsByIdCollection.php

@@ -32,6 +32,7 @@ use OCP\SystemTag\ISystemTag;
 use OCP\SystemTag\TagNotFoundException;
 use OCP\IGroupManager;
 use OCP\IUserSession;
+use OC\User\NoUserException;
 
 class SystemTagsByIdCollection implements ICollection {
 
@@ -69,6 +70,8 @@ class SystemTagsByIdCollection implements ICollection {
 
 	/**
 	 * Returns whether the currently logged in user is an administrator
+	 *
+	 * @return bool true if the user is an admin
 	 */
 	private function isAdmin() {
 		$user = $this->userSession->getUser();
@@ -101,7 +104,7 @@ class SystemTagsByIdCollection implements ICollection {
 		try {
 			$tag = $this->tagManager->getTagsByIds([$name]);
 			$tag = current($tag);
-			if (!$this->isAdmin() && !$tag->isUserVisible()) {
+			if (!$this->tagManager->canUserSeeTag($tag, $this->userSession->getUser())) {
 				throw new NotFound('Tag with id ' . $name . ' not found');
 			}
 			return $this->makeNode($tag);
@@ -131,7 +134,7 @@ class SystemTagsByIdCollection implements ICollection {
 		try {
 			$tag = $this->tagManager->getTagsByIds([$name]);
 			$tag = current($tag);
-			if (!$this->isAdmin() && !$tag->isUserVisible()) {
+			if (!$this->tagManager->canUserSeeTag($tag, $this->userSession->getUser())) {
 				return false;
 			}
 			return true;
@@ -171,6 +174,6 @@ class SystemTagsByIdCollection implements ICollection {
 	 * @return SystemTagNode
 	 */
 	private function makeNode(ISystemTag $tag) {
-		return new SystemTagNode($tag, $this->isAdmin(), $this->tagManager);
+		return new SystemTagNode($tag, $this->userSession->getUser(), $this->isAdmin(), $this->tagManager);
 	}
 }

apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php

@@ -31,6 +31,7 @@ use OCP\SystemTag\ISystemTagManager;
 use OCP\SystemTag\ISystemTagObjectMapper;
 use OCP\SystemTag\ISystemTag;
 use OCP\SystemTag\TagNotFoundException;
+use OCP\IUser;
 
 /**
  * Collection containing tags by object id
@@ -58,11 +59,11 @@ class SystemTagsObjectMappingCollection implements ICollection {
 	private $tagMapper;
 
 	/**
-	 * Whether to return results only visible for admins
+	 * User
 	 *
-	 * @var bool
+	 * @var IUser
 	 */
-	private $isAdmin;
+	private $user;
 
 
 	/**
@@ -70,30 +71,35 @@ class SystemTagsObjectMappingCollection implements ICollection {
 	 *
 	 * @param string $objectId object id
 	 * @param string $objectType object type
-	 * @param bool $isAdmin whether to return results visible only for admins
-	 * @param ISystemTagManager $tagManager
-	 * @param ISystemTagObjectMapper $tagMapper
+	 * @param IUser $user user
+	 * @param ISystemTagManager $tagManager tag manager
+	 * @param ISystemTagObjectMapper $tagMapper tag mapper
 	 */
-	public function __construct($objectId, $objectType, $isAdmin, $tagManager, $tagMapper) {
+	public function __construct(
+		$objectId,
+		$objectType,
+		IUser $user,
+		ISystemTagManager $tagManager,
+		ISystemTagObjectMapper $tagMapper
+	) {
 		$this->tagManager = $tagManager;
 		$this->tagMapper = $tagMapper;
 		$this->objectId = $objectId;
 		$this->objectType = $objectType;
-		$this->isAdmin = $isAdmin;
+		$this->user = $user;
 	}
 
 	function createFile($tagId, $data = null) {
 		try {
-			if (!$this->isAdmin) {
-				$tag = $this->tagManager->getTagsByIds($tagId);
-				$tag = current($tag);
-				if (!$tag->isUserVisible()) {
-					throw new PreconditionFailed('Tag with id ' . $tagId . ' does not exist, cannot assign');
-				}
-				if (!$tag->isUserAssignable()) {
-					throw new Forbidden('No permission to assign tag ' . $tag->getId());
-				}
+			$tags = $this->tagManager->getTagsByIds([$tagId]);
+			$tag = current($tags);
+			if (!$this->tagManager->canUserSeeTag($tag, $this->user)) {
+				throw new PreconditionFailed('Tag with id ' . $tagId . ' does not exist, cannot assign');
 			}
+			if (!$this->tagManager->canUserAssignTag($tag, $this->user)) {
+				throw new Forbidden('No permission to assign tag ' . $tagId);
+			}
+
 			$this->tagMapper->assignTags($this->objectId, $this->objectType, $tagId);
 		} catch (TagNotFoundException $e) {
 			throw new PreconditionFailed('Tag with id ' . $tagId . ' does not exist, cannot assign');
@@ -109,7 +115,7 @@ class SystemTagsObjectMappingCollection implements ICollection {
 			if ($this->tagMapper->haveTag([$this->objectId], $this->objectType, $tagId, true)) {
 				$tag = $this->tagManager->getTagsByIds([$tagId]);
 				$tag = current($tag);
-				if ($this->isAdmin || $tag->isUserVisible()) {
+				if ($this->tagManager->canUserSeeTag($tag, $this->user)) {
 					return $this->makeNode($tag);
 				}
 			}
@@ -127,12 +133,12 @@ class SystemTagsObjectMappingCollection implements ICollection {
 			return [];
 		}
 		$tags = $this->tagManager->getTagsByIds($tagIds);
-		if (!$this->isAdmin) {
-			// filter out non-visible tags
-			$tags = array_filter($tags, function($tag) {
-				return $tag->isUserVisible();
-			});
-		}
+
+		// filter out non-visible tags
+		$tags = array_filter($tags, function($tag) {
+			return $this->tagManager->canUserSeeTag($tag, $this->user);
+		});
+
 		return array_values(array_map(function($tag) {
 			return $this->makeNode($tag);
 		}, $tags));
@@ -141,17 +147,16 @@ class SystemTagsObjectMappingCollection implements ICollection {
 	function childExists($tagId) {
 		try {
 			$result = ($this->tagMapper->haveTag([$this->objectId], $this->objectType, $tagId, true));
-			if ($this->isAdmin || !$result) {
-				return $result;
+
+			if ($result) {
+				$tags = $this->tagManager->getTagsByIds([$tagId]);
+				$tag = current($tags);
+				if (!$this->tagManager->canUserSeeTag($tag, $this->user)) {
+					return false;
+				}
 			}
 
-			// verify if user is allowed to see this tag
-			$tag = $this->tagManager->getTagsByIds($tagId);
-			$tag = current($tag);
-			if (!$tag->isUserVisible()) {
-				return false;
-			}
-			return true;
+			return $result;
 		} catch (\InvalidArgumentException $e) {
 			throw new BadRequest('Invalid tag id', 0, $e);
 		} catch (TagNotFoundException $e) {
@@ -193,7 +198,7 @@ class SystemTagsObjectMappingCollection implements ICollection {
 			$tag,
 			$this->objectId,
 			$this->objectType,
-			$this->isAdmin,
+			$this->user,
 			$this->tagManager,
 			$this->tagMapper
 		);

apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php

@@ -94,17 +94,6 @@ class SystemTagsObjectTypeCollection implements ICollection {
 		$this->fileRoot = $fileRoot;
 	}
 
-	/**
-	 * Returns whether the currently logged in user is an administrator
-	 */
-	private function isAdmin() {
-		$user = $this->userSession->getUser();
-		if ($user !== null) {
-			return $this->groupManager->isAdmin($user->getUID());
-		}
-		return false;
-	}
-
 	/**
 	 * @param string $name
 	 * @param resource|string $data Initial payload
@@ -132,7 +121,7 @@ class SystemTagsObjectTypeCollection implements ICollection {
 		return new SystemTagsObjectMappingCollection(
 			$objectId,
 			$this->objectType,
-			$this->isAdmin(),
+			$this->userSession->getUser(),
 			$this->tagManager,
 			$this->tagMapper
 		);

apps/dav/tests/unit/systemtag/systemtagmappingnode.php

@@ -24,6 +24,7 @@ namespace OCA\DAV\Tests\Unit\SystemTag;
 use Sabre\DAV\Exception\NotFound;
 use OC\SystemTag\SystemTag;
 use OCP\SystemTag\TagNotFoundException;
+use OCP\SystemTag\ISystemTag;
 
 class SystemTagMappingNode extends \Test\TestCase {
 
@@ -37,14 +38,20 @@ class SystemTagMappingNode extends \Test\TestCase {
 	 */
 	private $tagMapper;
 
+	/**
+	 * @var \OCP\IUser
+	 */
+	private $user;
+
 	protected function setUp() {
 		parent::setUp();
 
 		$this->tagManager = $this->getMock('\OCP\SystemTag\ISystemTagManager');
 		$this->tagMapper = $this->getMock('\OCP\SystemTag\ISystemTagObjectMapper');
+		$this->user = $this->getMock('\OCP\IUser');
 	}
 
-	public function getMappingNode($isAdmin = true, $tag = null) {
+	public function getMappingNode($tag = null) {
 		if ($tag === null) {
 			$tag = new SystemTag(1, 'Test', true, true);
 		}
@@ -52,7 +59,7 @@ class SystemTagMappingNode extends \Test\TestCase {
 			$tag,
 			123,
 			'files',
-			$isAdmin,
+			$this->user,
 			$this->tagManager,
 			$this->tagMapper
 		);
@@ -60,28 +67,30 @@ class SystemTagMappingNode extends \Test\TestCase {
 
 	public function testGetters() {
 		$tag = new SystemTag(1, 'Test', true, false);
-		$node = $this->getMappingNode(true, $tag);
+		$node = $this->getMappingNode($tag);
 		$this->assertEquals('1', $node->getName());
 		$this->assertEquals($tag, $node->getSystemTag());
 		$this->assertEquals(123, $node->getObjectId());
 		$this->assertEquals('files', $node->getObjectType());
 	}
 
-	public function adminFlagProvider() {
-		return [[true], [false]];
-	}
-
-	/**
-	 * @dataProvider adminFlagProvider
-	 */
-	public function testDeleteTag($isAdmin) {
+	public function testDeleteTag() {
+		$node = $this->getMappingNode();
+		$this->tagManager->expects($this->once())
+			->method('canUserSeeTag')
+			->with($node->getSystemTag())
+			->will($this->returnValue(true));
+		$this->tagManager->expects($this->once())
+			->method('canUserAssignTag')
+			->with($node->getSystemTag())
+			->will($this->returnValue(true));
 		$this->tagManager->expects($this->never())
 			->method('deleteTags');
 		$this->tagMapper->expects($this->once())
 			->method('unassignTags')
 			->with(123, 'files', 1);
 
-		$this->getMappingNode($isAdmin)->delete();
+		$node->delete();
 	}
 
 	public function tagNodeDeleteProviderPermissionException() {
@@ -102,7 +111,15 @@ class SystemTagMappingNode extends \Test\TestCase {
 	/**
 	 * @dataProvider tagNodeDeleteProviderPermissionException
 	 */
-	public function testDeleteTagExpectedException($tag, $expectedException) {
+	public function testDeleteTagExpectedException(ISystemTag $tag, $expectedException) {
+		$this->tagManager->expects($this->any())
+			->method('canUserSeeTag')
+			->with($tag)
+			->will($this->returnValue($tag->isUserVisible()));
+		$this->tagManager->expects($this->any())
+			->method('canUserAssignTag')
+			->with($tag)
+			->will($this->returnValue($tag->isUserAssignable()));
 		$this->tagManager->expects($this->never())
 			->method('deleteTags');
 		$this->tagMapper->expects($this->never())
@@ -110,7 +127,7 @@ class SystemTagMappingNode extends \Test\TestCase {
 
 		$thrown = null;
 		try {
-			$this->getMappingNode(false, $tag)->delete();
+			$this->getMappingNode($tag)->delete();
 		} catch (\Exception $e) {
 			$thrown = $e;
 		}
@@ -122,11 +139,22 @@ class SystemTagMappingNode extends \Test\TestCase {
 	 * @expectedException Sabre\DAV\Exception\NotFound
 	 */
 	public function testDeleteTagNotFound() {
+		// assuming the tag existed at the time the node was created,
+		// but got deleted concurrently in the database
+		$tag = new SystemTag(1, 'Test', true, true);
+		$this->tagManager->expects($this->once())
+			->method('canUserSeeTag')
+			->with($tag)
+			->will($this->returnValue($tag->isUserVisible()));
+		$this->tagManager->expects($this->once())
+			->method('canUserAssignTag')
+			->with($tag)
+			->will($this->returnValue($tag->isUserAssignable()));
 		$this->tagMapper->expects($this->once())
 			->method('unassignTags')
 			->with(123, 'files', 1)
 			->will($this->throwException(new TagNotFoundException()));
 
-		$this->getMappingNode()->delete();
+		$this->getMappingNode($tag)->delete();
 	}
 }

apps/dav/tests/unit/systemtag/systemtagnode.php

@@ -28,6 +28,7 @@ use Sabre\DAV\Exception\Conflict;
 use OC\SystemTag\SystemTag;
 use OCP\SystemTag\TagNotFoundException;
 use OCP\SystemTag\TagAlreadyExistsException;
+use OCP\SystemTag\ISystemTag;
 
 class SystemTagNode extends \Test\TestCase {
 
@@ -36,10 +37,16 @@ class SystemTagNode extends \Test\TestCase {
 	 */
 	private $tagManager;
 
+	/**
+	 * @var \OCP\IUser
+	 */
+	private $user;
+
 	protected function setUp() {
 		parent::setUp();
 
 		$this->tagManager = $this->getMock('\OCP\SystemTag\ISystemTagManager');
+		$this->user = $this->getMock('\OCP\IUser');
 	}
 
 	protected function getTagNode($isAdmin = true, $tag = null) {
@@ -48,6 +55,7 @@ class SystemTagNode extends \Test\TestCase {
 		}
 		return new \OCA\DAV\SystemTag\SystemTagNode(
 			$tag,
+			$this->user,
 			$isAdmin,
 			$this->tagManager
 		);
@@ -101,6 +109,14 @@ class SystemTagNode extends \Test\TestCase {
 	 * @dataProvider tagNodeProvider
 	 */
 	public function testUpdateTag($isAdmin, $originalTag, $changedArgs) {
+		$this->tagManager->expects($this->once())
+			->method('canUserSeeTag')
+			->with($originalTag)
+			->will($this->returnValue($originalTag->isUserVisible() || $isAdmin));
+		$this->tagManager->expects($this->once())
+			->method('canUserAssignTag')
+			->with($originalTag)
+			->will($this->returnValue($originalTag->isUserAssignable() || $isAdmin));
 		$this->tagManager->expects($this->once())
 			->method('updateTag')
 			->with(1, $changedArgs[0], $changedArgs[1], $changedArgs[2]);
@@ -153,6 +169,14 @@ class SystemTagNode extends \Test\TestCase {
 	 * @dataProvider tagNodeProviderPermissionException
 	 */
 	public function testUpdateTagPermissionException($originalTag, $changedArgs, $expectedException = null) {
+		$this->tagManager->expects($this->any())
+			->method('canUserSeeTag')
+			->with($originalTag)
+			->will($this->returnValue($originalTag->isUserVisible()));
+		$this->tagManager->expects($this->any())
+			->method('canUserAssignTag')
+			->with($originalTag)
+			->will($this->returnValue($originalTag->isUserAssignable()));
 		$this->tagManager->expects($this->never())
 			->method('updateTag');
 
@@ -172,32 +196,59 @@ class SystemTagNode extends \Test\TestCase {
 	 * @expectedException Sabre\DAV\Exception\Conflict
 	 */
 	public function testUpdateTagAlreadyExists() {
+		$tag = new SystemTag(1, 'tag1', true, true);
+		$this->tagManager->expects($this->any())
+			->method('canUserSeeTag')
+			->with($tag)
+			->will($this->returnValue(true));
+		$this->tagManager->expects($this->any())
+			->method('canUserAssignTag')
+			->with($tag)
+			->will($this->returnValue(true));
 		$this->tagManager->expects($this->once())
 			->method('updateTag')
-			->with(1, 'Renamed', false, true)
+			->with(1, 'Renamed', true, true)
 			->will($this->throwException(new TagAlreadyExistsException()));
-		$this->getTagNode()->update('Renamed', false, true);
+		$this->getTagNode(false, $tag)->update('Renamed', true, true);
 	}
 
 	/**
 	 * @expectedException Sabre\DAV\Exception\NotFound
 	 */
 	public function testUpdateTagNotFound() {
+		$tag = new SystemTag(1, 'tag1', true, true);
+		$this->tagManager->expects($this->any())
+			->method('canUserSeeTag')
+			->with($tag)
+			->will($this->returnValue(true));
+		$this->tagManager->expects($this->any())
+			->method('canUserAssignTag')
+			->with($tag)
+			->will($this->returnValue(true));
 		$this->tagManager->expects($this->once())
 			->method('updateTag')
-			->with(1, 'Renamed', false, true)
+			->with(1, 'Renamed', true, true)
 			->will($this->throwException(new TagNotFoundException()));
-		$this->getTagNode()->update('Renamed', false, true);
+		$this->getTagNode(false, $tag)->update('Renamed', true, true);
 	}
 
 	/**
 	 * @dataProvider adminFlagProvider
 	 */
 	public function testDeleteTag($isAdmin) {
+		$tag = new SystemTag(1, 'tag1', true, true);
+		$this->tagManager->expects($this->once())
+			->method('canUserSeeTag')
+			->with($tag)
+			->will($this->returnValue(true));
+		$this->tagManager->expects($this->once())
+			->method('canUserAssignTag')
+			->with($tag)
+			->will($this->returnValue(true));
 		$this->tagManager->expects($this->once())
 			->method('deleteTags')
 			->with('1');
-		$this->getTagNode($isAdmin)->delete();
+		$this->getTagNode($isAdmin, $tag)->delete();
 	}
 
 	public function tagNodeDeleteProviderPermissionException() {
@@ -218,7 +269,15 @@ class SystemTagNode extends \Test\TestCase {
 	/**
 	 * @dataProvider tagNodeDeleteProviderPermissionException
 	 */
-	public function testDeleteTagPermissionException($tag, $expectedException) {
+	public function testDeleteTagPermissionException(ISystemTag $tag, $expectedException) {
+		$this->tagManager->expects($this->any())
+			->method('canUserSeeTag')
+			->with($tag)
+			->will($this->returnValue($tag->isUserVisible()));
+		$this->tagManager->expects($this->any())
+			->method('canUserAssignTag')
+			->with($tag)
+			->will($this->returnValue($tag->isUserAssignable()));
 		$this->tagManager->expects($this->never())
 			->method('deleteTags');
 
@@ -235,10 +294,19 @@ class SystemTagNode extends \Test\TestCase {
 	 * @expectedException Sabre\DAV\Exception\NotFound
 	 */
 	public function testDeleteTagNotFound() {
+		$tag = new SystemTag(1, 'tag1', true, true);
+		$this->tagManager->expects($this->any())
+			->method('canUserSeeTag')
+			->with($tag)
+			->will($this->returnValue($tag->isUserVisible()));
+		$this->tagManager->expects($this->any())
+			->method('canUserAssignTag')
+			->with($tag)
+			->will($this->returnValue($tag->isUserAssignable()));
 		$this->tagManager->expects($this->once())
 			->method('deleteTags')
 			->with('1')
 			->will($this->throwException(new TagNotFoundException()));
-		$this->getTagNode()->delete();
+		$this->getTagNode(false, $tag)->delete();
 	}
 }

apps/dav/tests/unit/systemtag/systemtagplugin.php

@@ -27,6 +27,8 @@ use OC\SystemTag\SystemTag;
 use OCP\IGroupManager;
 use OCP\IUserSession;
 use OCP\SystemTag\TagAlreadyExistsException;
+use OCP\IUser;
+use OCP\SystemTag\ISystemTag;
 
 class SystemTagPlugin extends \Test\TestCase {
 
@@ -34,6 +36,8 @@ class SystemTagPlugin extends \Test\TestCase {
 	const DISPLAYNAME_PROPERTYNAME = \OCA\DAV\SystemTag\SystemTagPlugin::DISPLAYNAME_PROPERTYNAME;
 	const USERVISIBLE_PROPERTYNAME = \OCA\DAV\SystemTag\SystemTagPlugin::USERVISIBLE_PROPERTYNAME;
 	const USERASSIGNABLE_PROPERTYNAME = \OCA\DAV\SystemTag\SystemTagPlugin::USERASSIGNABLE_PROPERTYNAME;
+	const CANASSIGN_PROPERTYNAME = \OCA\DAV\SystemTag\SystemTagPlugin::CANASSIGN_PROPERTYNAME;
+	const GROUPS_PROPERTYNAME = \OCA\DAV\SystemTag\SystemTagPlugin::GROUPS_PROPERTYNAME;
 
 	/**
 	 * @var \Sabre\DAV\Server
@@ -60,6 +64,11 @@ class SystemTagPlugin extends \Test\TestCase {
 	 */
 	private $userSession;
 
+	/**
+	 * @var IUser
+	 */
+	private $user;
+
 	/**
 	 * @var \OCA\DAV\SystemTag\SystemTagPlugin
 	 */
@@ -75,7 +84,16 @@ class SystemTagPlugin extends \Test\TestCase {
 
 		$this->tagManager = $this->getMock('\OCP\SystemTag\ISystemTagManager');
 		$this->groupManager = $this->getMock('\OCP\IGroupManager');
+		$this->user = $this->getMock('\OCP\IUser');
 		$this->userSession = $this->getMock('\OCP\IUserSession');
+		$this->userSession
+			->expects($this->any())
+			->method('getUser')
+			->willReturn($this->user);
+		$this->userSession
+			->expects($this->any())
+			->method('isLoggedIn')
+			->willReturn(true);
 
 		$this->plugin = new \OCA\DAV\SystemTag\SystemTagPlugin(
 			$this->tagManager,
@@ -85,22 +103,139 @@ class SystemTagPlugin extends \Test\TestCase {
 		$this->plugin->initialize($this->server);
 	}
 
-	public function testGetProperties() {
-		$systemTag = new SystemTag(1, 'Test', true, true);
+	public function getPropertiesDataProvider() {
+		return [
+			[
+				new SystemTag(1, 'Test', true, true),
+				[],
+				[
+					self::ID_PROPERTYNAME,
+					self::DISPLAYNAME_PROPERTYNAME,
+					self::USERVISIBLE_PROPERTYNAME,
+					self::USERASSIGNABLE_PROPERTYNAME,
+					self::CANASSIGN_PROPERTYNAME,
+				],
+				[
+					self::ID_PROPERTYNAME => '1',
+					self::DISPLAYNAME_PROPERTYNAME => 'Test',
+					self::USERVISIBLE_PROPERTYNAME => 'true',
+					self::USERASSIGNABLE_PROPERTYNAME => 'true',
+					self::CANASSIGN_PROPERTYNAME => 'true',
+				]
+			],
+			[
+				new SystemTag(1, 'Test', true, false),
+				[],
+				[
+					self::ID_PROPERTYNAME,
+					self::DISPLAYNAME_PROPERTYNAME,
+					self::USERVISIBLE_PROPERTYNAME,
+					self::USERASSIGNABLE_PROPERTYNAME,
+					self::CANASSIGN_PROPERTYNAME,
+				],
+				[
+					self::ID_PROPERTYNAME => '1',
+					self::DISPLAYNAME_PROPERTYNAME => 'Test',
+					self::USERVISIBLE_PROPERTYNAME => 'true',
+					self::USERASSIGNABLE_PROPERTYNAME => 'false',
+					self::CANASSIGN_PROPERTYNAME => 'false',
+				]
+			],
+			[
+				new SystemTag(1, 'Test', true, false),
+				['group1', 'group2'],
+				[
+					self::ID_PROPERTYNAME,
+					self::GROUPS_PROPERTYNAME,
+				],
+				[
+					self::ID_PROPERTYNAME => '1',
+					self::GROUPS_PROPERTYNAME => 'group1|group2',
+				]
+			],
+			[
+				new SystemTag(1, 'Test', true, true),
+				['group1', 'group2'],
+				[
+					self::ID_PROPERTYNAME,
+					self::GROUPS_PROPERTYNAME,
+				],
+				[
+					self::ID_PROPERTYNAME => '1',
+					// groups only returned when userAssignable is false
+					self::GROUPS_PROPERTYNAME => '',
+				]
+			],
+		];
+	}
+
+	/**
+	 * @dataProvider getPropertiesDataProvider
+	 */
+	public function testGetProperties(ISystemTag $systemTag, $groups, $requestedProperties, $expectedProperties) {
+		$this->user->expects($this->any())
+			->method('getUID')
+			->willReturn('admin');
+		$this->groupManager
+			->expects($this->any())
+			->method('isAdmin')
+			->with('admin')
+			->willReturn(true);
+
+		$node = $this->getMockBuilder('\OCA\DAV\SystemTag\SystemTagNode')
+			->disableOriginalConstructor()
+			->getMock();
+		$node->expects($this->any())
+			->method('getSystemTag')
+			->will($this->returnValue($systemTag));
+
+		$this->tagManager->expects($this->any())
+			->method('canUserAssignTag')
+			->will($this->returnValue($systemTag->isUserAssignable()));
+
+		$this->tagManager->expects($this->any())
+			->method('getTagGroups')
+			->will($this->returnValue($groups));
+
+		$this->tree->expects($this->any())
+			->method('getNodeForPath')
+			->with('/systemtag/1')
+			->will($this->returnValue($node));
+
+		$propFind = new \Sabre\DAV\PropFind(
+			'/systemtag/1',
+			$requestedProperties,
+			0
+		);
+
+		$this->plugin->handleGetProperties(
+			$propFind,
+			$node
+		);
+
+		$result = $propFind->getResultForMultiStatus();
+
+		$this->assertEmpty($result[404]);
+		$this->assertEquals($expectedProperties, $result[200]);
+	}
+
+	/**
+	 * @expectedException \Sabre\DAV\Exception\Forbidden
+	 */
+	public function testGetPropertiesForbidden() {
+		$systemTag = new SystemTag(1, 'Test', true, false);
 		$requestedProperties = [
 			self::ID_PROPERTYNAME,
-			self::DISPLAYNAME_PROPERTYNAME,
-			self::USERVISIBLE_PROPERTYNAME,
-			self::USERASSIGNABLE_PROPERTYNAME
-		];
-		$expectedProperties = [
-			200 => [
-				self::ID_PROPERTYNAME => '1',
-				self::DISPLAYNAME_PROPERTYNAME => 'Test',
-				self::USERVISIBLE_PROPERTYNAME => 'true',
-				self::USERASSIGNABLE_PROPERTYNAME => 'true',
-			]
+			self::GROUPS_PROPERTYNAME,
 		];
+		$this->user->expects($this->once())
+			->method('getUID')
+			->willReturn('admin');
+		$this->groupManager
+			->expects($this->once())
+			->method('isAdmin')
+			->with('admin')
+			->willReturn(false);
 
 		$node = $this->getMockBuilder('\OCA\DAV\SystemTag\SystemTagNode')
 			->disableOriginalConstructor()
@@ -124,16 +259,19 @@ class SystemTagPlugin extends \Test\TestCase {
 			$propFind,
 			$node
 		);
-
-		$result = $propFind->getResultForMultiStatus();
-
-		$this->assertEmpty($result[404]);
-		unset($result[404]);
-		$this->assertEquals($expectedProperties, $result);
 	}
 
-	public function testUpdateProperties() {
+	public function testUpdatePropertiesAdmin() {
 		$systemTag = new SystemTag(1, 'Test', true, false);
+		$this->user->expects($this->any())
+			->method('getUID')
+			->willReturn('admin');
+		$this->groupManager
+			->expects($this->any())
+			->method('isAdmin')
+			->with('admin')
+			->willReturn(true);
+
 		$node = $this->getMockBuilder('\OCA\DAV\SystemTag\SystemTagNode')
 			->disableOriginalConstructor()
 			->getMock();
@@ -150,11 +288,16 @@ class SystemTagPlugin extends \Test\TestCase {
 			->method('update')
 			->with('Test changed', false, true);
 
+		$this->tagManager->expects($this->once())
+			->method('setTagGroups')
+			->with($systemTag, ['group1', 'group2']);
+
 		// properties to set
 		$propPatch = new \Sabre\DAV\PropPatch(array(
 			self::DISPLAYNAME_PROPERTYNAME => 'Test changed',
 			self::USERVISIBLE_PROPERTYNAME => 'false',
 			self::USERASSIGNABLE_PROPERTYNAME => 'true',
+			self::GROUPS_PROPERTYNAME => 'group1|group2',
 		));
 
 		$this->plugin->handleUpdateProperties(
@@ -174,102 +317,89 @@ class SystemTagPlugin extends \Test\TestCase {
 	}
 
 	/**
-	 * @expectedException \Sabre\DAV\Exception\BadRequest
-	 * @expectedExceptionMessage Not sufficient permissions
+	 * @expectedException \Sabre\DAV\Exception\Forbidden
 	 */
-	public function testCreateNotAssignableTagAsRegularUser() {
-		$user = $this->getMock('\OCP\IUser');
-		$user->expects($this->once())
+	public function testUpdatePropertiesForbidden() {
+		$systemTag = new SystemTag(1, 'Test', true, false);
+		$this->user->expects($this->any())
 			->method('getUID')
 			->willReturn('admin');
-		$this->userSession
-			->expects($this->once())
-			->method('isLoggedIn')
-			->willReturn(true);
-		$this->userSession
-			->expects($this->once())
-			->method('getUser')
-			->willReturn($user);
 		$this->groupManager
-			->expects($this->once())
+			->expects($this->any())
 			->method('isAdmin')
 			->with('admin')
 			->willReturn(false);
 
-		$requestData = json_encode([
-			'name' => 'Test',
-			'userVisible' => true,
-			'userAssignable' => false,
-		]);
-
-		$node = $this->getMockBuilder('\OCA\DAV\SystemTag\SystemTagsByIdCollection')
+		$node = $this->getMockBuilder('\OCA\DAV\SystemTag\SystemTagNode')
 			->disableOriginalConstructor()
 			->getMock();
-		$this->tagManager->expects($this->never())
-			->method('createTag');
+		$node->expects($this->any())
+			->method('getSystemTag')
+			->will($this->returnValue($systemTag));
 
 		$this->tree->expects($this->any())
 			->method('getNodeForPath')
-			->with('/systemtags')
+			->with('/systemtag/1')
 			->will($this->returnValue($node));
 
-		$request = $this->getMockBuilder('Sabre\HTTP\RequestInterface')
-			->disableOriginalConstructor()
-			->getMock();
-		$response = $this->getMockBuilder('Sabre\HTTP\ResponseInterface')
-			->disableOriginalConstructor()
-			->getMock();
+		$node->expects($this->never())
+			->method('update');
 
-		$request->expects($this->once())
-			->method('getPath')
-			->will($this->returnValue('/systemtags'));
+		$this->tagManager->expects($this->never())
+			->method('setTagGroups');
 
-		$request->expects($this->once())
-			->method('getBodyAsString')
-			->will($this->returnValue($requestData));
+		// properties to set
+		$propPatch = new \Sabre\DAV\PropPatch(array(
+			self::GROUPS_PROPERTYNAME => 'group1|group2',
+		));
 
-		$request->expects($this->once())
-			->method('getHeader')
-			->with('Content-Type')
-			->will($this->returnValue('application/json'));
+		$this->plugin->handleUpdateProperties(
+			'/systemtag/1',
+			$propPatch
+		);
 
-		$this->plugin->httpPost($request, $response);
+		$propPatch->commit();
 	}
 
+	public function createTagInsufficientPermissionsProvider() {
+		return [
+			[true, false, ''],
+			[false, true, ''],
+			[true, true, 'group1|group2'],
+		];
+	}
 	/**
+	 * @dataProvider createTagInsufficientPermissionsProvider
 	 * @expectedException \Sabre\DAV\Exception\BadRequest
 	 * @expectedExceptionMessage Not sufficient permissions
 	 */
-	public function testCreateInvisibleTagAsRegularUser() {
-		$user = $this->getMock('\OCP\IUser');
-		$user->expects($this->once())
+	public function testCreateNotAssignableTagAsRegularUser($userVisible, $userAssignable, $groups) {
+		$this->user->expects($this->once())
 			->method('getUID')
 			->willReturn('admin');
-		$this->userSession
-			->expects($this->once())
-			->method('isLoggedIn')
-			->willReturn(true);
-		$this->userSession
-			->expects($this->once())
-			->method('getUser')
-			->willReturn($user);
 		$this->groupManager
 			->expects($this->once())
 			->method('isAdmin')
 			->with('admin')
 			->willReturn(false);
 
-		$requestData = json_encode([
+		$requestData = [
 			'name' => 'Test',
-			'userVisible' => false,
-			'userAssignable' => true,
-		]);
+			'userVisible' => $userVisible,
+			'userAssignable' => $userAssignable,
+		];
+		if (!empty($groups)) {
+			$requestData['groups'] = $groups;
+		}
+		$requestData = json_encode($requestData);
 
 		$node = $this->getMockBuilder('\OCA\DAV\SystemTag\SystemTagsByIdCollection')
 			->disableOriginalConstructor()
 			->getMock();
 		$this->tagManager->expects($this->never())
 			->method('createTag');
+		$this->tagManager->expects($this->never())
+			->method('setTagGroups');
 
 		$this->tree->expects($this->any())
 			->method('getNodeForPath')
@@ -352,19 +482,21 @@ class SystemTagPlugin extends \Test\TestCase {
 		$this->plugin->httpPost($request, $response);
 	}
 
-	public function testCreateTagInByIdCollection() {
-		$user = $this->getMock('\OCP\IUser');
-		$user->expects($this->once())
+	public function createTagProvider() {
+		return [
+			[true, false, ''],
+			[false, false, ''],
+			[true, false, 'group1|group2'],
+		];
+	}
+
+	/**
+	 * @dataProvider createTagProvider
+	 */
+	public function testCreateTagInByIdCollection($userVisible, $userAssignable, $groups) {
+		$this->user->expects($this->once())
 			->method('getUID')
 			->willReturn('admin');
-		$this->userSession
-			->expects($this->once())
-			->method('isLoggedIn')
-			->willReturn(true);
-		$this->userSession
-			->expects($this->once())
-			->method('getUser')
-			->willReturn($user);
 		$this->groupManager
 			->expects($this->once())
 			->method('isAdmin')
@@ -373,19 +505,33 @@ class SystemTagPlugin extends \Test\TestCase {
 
 		$systemTag = new SystemTag(1, 'Test', true, false);
 
-		$requestData = json_encode([
+		$requestData = [
 			'name' => 'Test',
-			'userVisible' => true,
-			'userAssignable' => false,
-		]);
+			'userVisible' => $userVisible,
+			'userAssignable' => $userAssignable,
+		];
+		if (!empty($groups)) {
+			$requestData['groups'] = $groups;
+		}
+		$requestData = json_encode($requestData);
 
 		$node = $this->getMockBuilder('\OCA\DAV\SystemTag\SystemTagsByIdCollection')
 			->disableOriginalConstructor()
 			->getMock();
 		$this->tagManager->expects($this->once())
 			->method('createTag')
-			->with('Test', true, false)
+			->with('Test', $userVisible, $userAssignable)
 			->will($this->returnValue($systemTag));
+		
+		if (!empty($groups)) {
+			$this->tagManager->expects($this->once())
+				->method('setTagGroups')
+				->with($systemTag, explode('|', $groups))
+				->will($this->returnValue($systemTag));
+		} else {
+			$this->tagManager->expects($this->never())
+				->method('setTagGroups');
+		}
 
 		$this->tree->expects($this->any())
 			->method('getNodeForPath')
@@ -431,18 +577,9 @@ class SystemTagPlugin extends \Test\TestCase {
 	}
 
 	public function testCreateTagInMappingCollection() {
-		$user = $this->getMock('\OCP\IUser');
-		$user->expects($this->once())
+		$this->user->expects($this->once())
 			->method('getUID')
 			->willReturn('admin');
-		$this->userSession
-			->expects($this->once())
-			->method('isLoggedIn')
-			->willReturn(true);
-		$this->userSession
-			->expects($this->once())
-			->method('getUser')
-			->willReturn($user);
 		$this->groupManager
 			->expects($this->once())
 			->method('isAdmin')
@@ -510,8 +647,6 @@ class SystemTagPlugin extends \Test\TestCase {
 	 * @expectedException \Sabre\DAV\Exception\NotFound
 	 */
 	public function testCreateTagToUnknownNode() {
-		$systemTag = new SystemTag(1, 'Test', true, false);
-
 		$node = $this->getMockBuilder('\OCA\DAV\SystemTag\SystemTagsObjectMappingCollection')
 			->disableOriginalConstructor()
 			->getMock();
@@ -545,18 +680,9 @@ class SystemTagPlugin extends \Test\TestCase {
 	 * @expectedException \Sabre\DAV\Exception\Conflict
 	 */
 	public function testCreateTagConflict($nodeClass) {
-		$user = $this->getMock('\OCP\IUser');
-		$user->expects($this->once())
+		$this->user->expects($this->once())
 			->method('getUID')
 			->willReturn('admin');
-		$this->userSession
-			->expects($this->once())
-			->method('isLoggedIn')
-			->willReturn(true);
-		$this->userSession
-			->expects($this->once())
-			->method('getUser')
-			->willReturn($user);
 		$this->groupManager
 			->expects($this->once())
 			->method('isAdmin')

apps/dav/tests/unit/systemtag/systemtagsbyidcollection.php

@@ -32,6 +32,11 @@ class SystemTagsByIdCollection extends \Test\TestCase {
 	 */
 	private $tagManager;
 
+	/**
+	 * @var \OCP\IUser
+	 */
+	private $user;
+
 	protected function setUp() {
 		parent::setUp();
 
@@ -39,14 +44,14 @@ class SystemTagsByIdCollection extends \Test\TestCase {
 	}
 
 	public function getNode($isAdmin = true) {
-		$user = $this->getMock('\OCP\IUser');
-		$user->expects($this->any())
+		$this->user = $this->getMock('\OCP\IUser');
+		$this->user->expects($this->any())
 			->method('getUID')
 			->will($this->returnValue('testuser'));
 		$userSession = $this->getMock('\OCP\IUserSession');
 		$userSession->expects($this->any())
 			->method('getUser')
-			->will($this->returnValue($user));
+			->will($this->returnValue($this->user));
 		$groupManager = $this->getMock('\OCP\IGroupManager');
 		$groupManager->expects($this->any())
 			->method('isAdmin')
@@ -77,35 +82,19 @@ class SystemTagsByIdCollection extends \Test\TestCase {
 		$this->getNode()->createDirectory('789');
 	}
 
-	public function getChildProvider() {
-		return [
-			[
-				true,
-				true,
-			],
-			[
-				true,
-				false,
-			],
-			[
-				false,
-				true,
-			],
-		];
-	}
-
-	/**
-	 * @dataProvider getChildProvider
-	 */
-	public function testGetChild($isAdmin, $userVisible) {
-		$tag = new SystemTag(123, 'Test', $userVisible, false);
+	public function testGetChild() {
+		$tag = new SystemTag(123, 'Test', true, false);
+		$this->tagManager->expects($this->once())
+			->method('canUserSeeTag')
+			->with($tag)
+			->will($this->returnValue(true));
 
 		$this->tagManager->expects($this->once())
 			->method('getTagsByIds')
 			->with(['123'])
 			->will($this->returnValue([$tag]));
 
-		$childNode = $this->getNode($isAdmin)->getChild('123');
+		$childNode = $this->getNode()->getChild('123');
 
 		$this->assertInstanceOf('\OCA\DAV\SystemTag\SystemTagNode', $childNode);
 		$this->assertEquals('123', $childNode->getName());
@@ -198,27 +187,27 @@ class SystemTagsByIdCollection extends \Test\TestCase {
 
 	public function childExistsProvider() {
 		return [
-			// admins, always visible
-			[true, true, true],
-			[true, false, true],
-			// non-admins, depends on flag
-			[false, true, true],
-			[false, false, false],
+			[true, true],
+			[false, false],
 		];
 	}
 
 	/**
 	 * @dataProvider childExistsProvider
 	 */
-	public function testChildExists($isAdmin, $userVisible, $expectedResult) {
+	public function testChildExists($userVisible, $expectedResult) {
 		$tag = new SystemTag(123, 'One', $userVisible, false);
+		$this->tagManager->expects($this->once())
+			->method('canUserSeeTag')
+			->with($tag)
+			->will($this->returnValue($userVisible));
 
 		$this->tagManager->expects($this->once())
 			->method('getTagsByIds')
 			->with(['123'])
 			->will($this->returnValue([$tag]));
 
-		$this->assertEquals($expectedResult, $this->getNode($isAdmin)->childExists('123'));
+		$this->assertEquals($expectedResult, $this->getNode()->childExists('123'));
 	}
 
 	public function testChildExistsNotFound() {

apps/dav/tests/unit/systemtag/systemtagsobjectmappingcollection.php

@@ -37,45 +37,50 @@ class SystemTagsObjectMappingCollection extends \Test\TestCase {
 	 */
 	private $tagMapper;
 
+	/**
+	 * @var \OCP\IUser
+	 */
+	private $user;
+
 	protected function setUp() {
 		parent::setUp();
 
 		$this->tagManager = $this->getMock('\OCP\SystemTag\ISystemTagManager');
 		$this->tagMapper = $this->getMock('\OCP\SystemTag\ISystemTagObjectMapper');
+
+		$this->user = $this->getMock('\OCP\IUser');
 	}
 
-	public function getNode($isAdmin = true) {
+	public function getNode() {
 		return new \OCA\DAV\SystemTag\SystemTagsObjectMappingCollection (
 			111,
 			'files',
-			$isAdmin,
+			$this->user,
 			$this->tagManager,
 			$this->tagMapper
 		);
 	}
 
-	public function testAssignTagAsAdmin() {
-		$this->tagManager->expects($this->never())
-			->method('getTagsByIds');
-		$this->tagMapper->expects($this->once())
-			->method('assignTags')
-			->with(111, 'files', '555');
-
-		$this->getNode(true)->createFile('555');
-	}
-
-	public function testAssignTagAsUser() {
+	public function testAssignTag() {
 		$tag = new SystemTag('1', 'Test', true, true);
+		$this->tagManager->expects($this->once())
+			->method('canUserSeeTag')
+			->with($tag)
+			->will($this->returnValue(true));
+		$this->tagManager->expects($this->once())
+			->method('canUserAssignTag')
+			->with($tag)
+			->will($this->returnValue(true));
 
 		$this->tagManager->expects($this->once())
 			->method('getTagsByIds')
-			->with('555')
+			->with(['555'])
 			->will($this->returnValue([$tag]));
 		$this->tagMapper->expects($this->once())
 			->method('assignTags')
 			->with(111, 'files', '555');
 
-		$this->getNode(false)->createFile('555');
+		$this->getNode()->createFile('555');
 	}
 
 	public function permissionsProvider() {
@@ -90,19 +95,27 @@ class SystemTagsObjectMappingCollection extends \Test\TestCase {
 	/**
 	 * @dataProvider permissionsProvider
 	 */
-	public function testAssignTagAsUserNoPermission($userVisible, $userAssignable, $expectedException) {
+	public function testAssignTagNoPermission($userVisible, $userAssignable, $expectedException) {
 		$tag = new SystemTag('1', 'Test', $userVisible, $userAssignable);
+		$this->tagManager->expects($this->once())
+			->method('canUserSeeTag')
+			->with($tag)
+			->will($this->returnValue($userVisible));
+		$this->tagManager->expects($this->any())
+			->method('canUserAssignTag')
+			->with($tag)
+			->will($this->returnValue($userAssignable));
 
 		$this->tagManager->expects($this->once())
 			->method('getTagsByIds')
-			->with('555')
+			->with(['555'])
 			->will($this->returnValue([$tag]));
 		$this->tagMapper->expects($this->never())
 			->method('assignTags');
 
 		$thrown = null;
 		try {
-			$this->getNode(false)->createFile('555');
+			$this->getNode()->createFile('555');
 		} catch (\Exception $e) {
 			$thrown = $e;
 		}
@@ -114,9 +127,9 @@ class SystemTagsObjectMappingCollection extends \Test\TestCase {
 	 * @expectedException Sabre\DAV\Exception\PreconditionFailed
 	 */
 	public function testAssignTagNotFound() {
-		$this->tagMapper->expects($this->once())
-			->method('assignTags')
-			->with(111, 'files', '555')
+		$this->tagManager->expects($this->once())
+			->method('getTagsByIds')
+			->with(['555'])
 			->will($this->throwException(new TagNotFoundException()));
 
 		$this->getNode()->createFile('555');
@@ -129,28 +142,12 @@ class SystemTagsObjectMappingCollection extends \Test\TestCase {
 		$this->getNode()->createDirectory('789');
 	}
 
-	public function getChildProvider() {
-		return [
-			[
-				true,
-				true,
-			],
-			[
-				true,
-				false,
-			],
-			[
-				false,
-				true,
-			],
-		];
-	}
-
-	/**
-	 * @dataProvider getChildProvider
-	 */
-	public function testGetChild($isAdmin, $userVisible) {
-		$tag = new SystemTag(555, 'TheTag', $userVisible, false);
+	public function testGetChild() {
+		$tag = new SystemTag(555, 'TheTag', true, false);
+		$this->tagManager->expects($this->once())
+			->method('canUserSeeTag')
+			->with($tag)
+			->will($this->returnValue(true));
 
 		$this->tagMapper->expects($this->once())
 			->method('haveTag')
@@ -162,17 +159,21 @@ class SystemTagsObjectMappingCollection extends \Test\TestCase {
 			->with(['555'])
 			->will($this->returnValue(['555' => $tag]));
 
-		$childNode = $this->getNode($isAdmin)->getChild('555');
+		$childNode = $this->getNode()->getChild('555');
 
-		$this->assertInstanceOf('\OCA\DAV\SystemTag\SystemTagNode', $childNode);
+		$this->assertInstanceOf('\OCA\DAV\SystemTag\SystemTagMappingNode', $childNode);
 		$this->assertEquals('555', $childNode->getName());
 	}
 
 	/**
 	 * @expectedException \Sabre\DAV\Exception\NotFound
 	 */
-	public function testGetChildUserNonVisible() {
+	public function testGetChildNonVisible() {
 		$tag = new SystemTag(555, 'TheTag', false, false);
+		$this->tagManager->expects($this->once())
+			->method('canUserSeeTag')
+			->with($tag)
+			->will($this->returnValue(false));
 
 		$this->tagMapper->expects($this->once())
 			->method('haveTag')
@@ -184,7 +185,7 @@ class SystemTagsObjectMappingCollection extends \Test\TestCase {
 			->with(['555'])
 			->will($this->returnValue(['555' => $tag]));
 
-		$this->getNode(false)->getChild('555');
+		$this->getNode()->getChild('555');
 	}
 
 	/**
@@ -223,7 +224,7 @@ class SystemTagsObjectMappingCollection extends \Test\TestCase {
 		$this->getNode()->getChild('777');
 	}
 
-	public function testGetChildrenAsAdmin() {
+	public function testGetChildren() {
 		$tag1 = new SystemTag(555, 'TagOne', true, false);
 		$tag2 = new SystemTag(556, 'TagTwo', true, true);
 		$tag3 = new SystemTag(557, 'InvisibleTag', false, true);
@@ -238,43 +239,13 @@ class SystemTagsObjectMappingCollection extends \Test\TestCase {
 			->with(['555', '556', '557'])
 			->will($this->returnValue(['555' => $tag1, '556' => $tag2, '557' => $tag3]));
 
-		$children = $this->getNode(true)->getChildren();
+		$this->tagManager->expects($this->exactly(3))
+			->method('canUserSeeTag')
+			->will($this->returnCallback(function($tag) {
+				return $tag->isUserVisible();
+			}));
 
-		$this->assertCount(3, $children);
-
-		$this->assertInstanceOf('\OCA\DAV\SystemTag\SystemTagMappingNode', $children[0]);
-		$this->assertInstanceOf('\OCA\DAV\SystemTag\SystemTagMappingNode', $children[1]);
-		$this->assertInstanceOf('\OCA\DAV\SystemTag\SystemTagMappingNode', $children[2]);
-
-		$this->assertEquals(111, $children[0]->getObjectId());
-		$this->assertEquals('files', $children[0]->getObjectType());
-		$this->assertEquals($tag1, $children[0]->getSystemTag());
-
-		$this->assertEquals(111, $children[1]->getObjectId());
-		$this->assertEquals('files', $children[1]->getObjectType());
-		$this->assertEquals($tag2, $children[1]->getSystemTag());
-
-		$this->assertEquals(111, $children[2]->getObjectId());
-		$this->assertEquals('files', $children[2]->getObjectType());
-		$this->assertEquals($tag3, $children[2]->getSystemTag());
-	}
-
-	public function testGetChildrenAsUser() {
-		$tag1 = new SystemTag(555, 'TagOne', true, false);
-		$tag2 = new SystemTag(556, 'TagTwo', true, true);
-		$tag3 = new SystemTag(557, 'InvisibleTag', false, true);
-
-		$this->tagMapper->expects($this->once())
-			->method('getTagIdsForObjects')
-			->with([111], 'files')
-			->will($this->returnValue(['111' => ['555', '556', '557']]));
-
-		$this->tagManager->expects($this->once())
-			->method('getTagsByIds')
-			->with(['555', '556', '557'])
-			->will($this->returnValue(['555' => $tag1, '556' => $tag2, '557' => $tag3]));
-
-		$children = $this->getNode(false)->getChildren();
+		$children = $this->getNode()->getChildren();
 
 		$this->assertCount(2, $children);
 
@@ -290,16 +261,7 @@ class SystemTagsObjectMappingCollection extends \Test\TestCase {
 		$this->assertEquals($tag2, $children[1]->getSystemTag());
 	}
 
-	public function testChildExistsAsAdmin() {
-		$this->tagMapper->expects($this->once())
-			->method('haveTag')
-			->with([111], 'files', '555')
-			->will($this->returnValue(true));
-
-		$this->assertTrue($this->getNode(true)->childExists('555'));
-	}
-
-	public function testChildExistsWithVisibleTagAsUser() {
+	public function testChildExistsWithVisibleTag() {
 		$tag = new SystemTag(555, 'TagOne', true, false);
 
 		$this->tagMapper->expects($this->once())
@@ -307,15 +269,20 @@ class SystemTagsObjectMappingCollection extends \Test\TestCase {
 			->with([111], 'files', '555')
 			->will($this->returnValue(true));
 
+		$this->tagManager->expects($this->once())
+			->method('canUserSeeTag')
+			->with($tag)
+			->will($this->returnValue(true));
+
 		$this->tagManager->expects($this->once())
 			->method('getTagsByIds')
-			->with('555')
+			->with(['555'])
 			->will($this->returnValue([$tag]));
 
-		$this->assertTrue($this->getNode(false)->childExists('555'));
+		$this->assertTrue($this->getNode()->childExists('555'));
 	}
 
-	public function testChildExistsWithInvisibleTagAsUser() {
+	public function testChildExistsWithInvisibleTag() {
 		$tag = new SystemTag(555, 'TagOne', false, false);
 
 		$this->tagMapper->expects($this->once())
@@ -325,10 +292,10 @@ class SystemTagsObjectMappingCollection extends \Test\TestCase {
 
 		$this->tagManager->expects($this->once())
 			->method('getTagsByIds')
-			->with('555')
+			->with(['555'])
 			->will($this->returnValue([$tag]));
 
-		$this->assertFalse($this->getNode(false)->childExists('555'));
+		$this->assertFalse($this->getNode()->childExists('555'));
 	}
 
 	public function testChildExistsNotFound() {

apps/systemtags/js/systemtagsinfoview.js

@@ -12,7 +12,7 @@
 
 	function modelToSelection(model) {
 		var data = model.toJSON();
-		if (!OC.isUserAdmin() && !data.userAssignable) {
+		if (!OC.isUserAdmin() && !data.canAssign) {
 			data.locked = true;
 		}
 		return data;

apps/systemtags/lib/Activity/Extension.php

@@ -324,7 +324,7 @@ class Extension implements IExtension {
 				case 'assignable':
 					return '<parameter>' . $matches[1] . '</parameter>';
 				case 'not-assignable':
-					return '<parameter>' . $l->t('%s (not-assignable)', $matches[1]) . '</parameter>';
+					return '<parameter>' . $l->t('%s (restricted)', $matches[1]) . '</parameter>';
 				case 'invisible':
 					return '<parameter>' . $l->t('%s (invisible)', $matches[1]) . '</parameter>';
 			}

apps/systemtags/tests/js/systemtagsinfoviewSpec.js

@@ -62,9 +62,9 @@ describe('OCA.SystemTags.SystemTagsInfoView tests', function() {
 			fetchStub.yieldTo('success', view.selectedTagsCollection);
 			expect(setDataStub.calledOnce).toEqual(true);
 			expect(setDataStub.getCall(0).args[0]).toEqual([{
-				id: '1', name: 'test1', userVisible: true, userAssignable: true
+				id: '1', name: 'test1', userVisible: true, userAssignable: true, canAssign: true
 			}, {
-				id: '3', name: 'test3', userVisible: true, userAssignable: true
+				id: '3', name: 'test3', userVisible: true, userAssignable: true, canAssign: true
 			}]);
 
 			expect(view.$el.hasClass('hidden')).toEqual(false);
@@ -79,7 +79,7 @@ describe('OCA.SystemTags.SystemTagsInfoView tests', function() {
 			view = new OCA.SystemTags.SystemTagsInfoView();
 			view.selectedTagsCollection.add([
 				{id: '1', name: 'test1'},
-				{id: '3', name: 'test3', userVisible: false, userAssignable: false}
+				{id: '3', name: 'test3', userVisible: false, userAssignable: false, canAssign: false}
 			]);
 
 			var callback = sinon.stub();
@@ -87,9 +87,9 @@ describe('OCA.SystemTags.SystemTagsInfoView tests', function() {
 
 			expect(callback.calledOnce).toEqual(true);
 			expect(callback.getCall(0).args[0]).toEqual([{
-				id: '1', name: 'test1', userVisible: true, userAssignable: true
+				id: '1', name: 'test1', userVisible: true, userAssignable: true, canAssign: true
 			}, {
-				id: '3', name: 'test3', userVisible: false, userAssignable: false
+				id: '3', name: 'test3', userVisible: false, userAssignable: false, canAssign: false
 			}]);
 
 			inputViewSpy.restore();
@@ -103,7 +103,7 @@ describe('OCA.SystemTags.SystemTagsInfoView tests', function() {
 			view = new OCA.SystemTags.SystemTagsInfoView();
 			view.selectedTagsCollection.add([
 				{id: '1', name: 'test1'},
-				{id: '3', name: 'test3', userAssignable: false}
+				{id: '3', name: 'test3', userAssignable: false, canAssign: false}
 			]);
 
 			var callback = sinon.stub();
@@ -111,9 +111,33 @@ describe('OCA.SystemTags.SystemTagsInfoView tests', function() {
 
 			expect(callback.calledOnce).toEqual(true);
 			expect(callback.getCall(0).args[0]).toEqual([{
-				id: '1', name: 'test1', userVisible: true, userAssignable: true
+				id: '1', name: 'test1', userVisible: true, userAssignable: true, canAssign: true
 			}, {
-				id: '3', name: 'test3', userVisible: true, userAssignable: false, locked: true
+				id: '3', name: 'test3', userVisible: true, userAssignable: false, canAssign: false, locked: true
+			}]);
+
+			inputViewSpy.restore();
+		});
+		it('does not set locked flag on non-assignable tags when canAssign overrides it with true', function() {
+			isAdminStub.returns(false);
+
+			var inputViewSpy = sinon.spy(OC.SystemTags, 'SystemTagsInputField');
+			var element = $('<input type="hidden" val="1,4"/>');
+			view.remove();
+			view = new OCA.SystemTags.SystemTagsInfoView();
+			view.selectedTagsCollection.add([
+				{id: '1', name: 'test1'},
+				{id: '4', name: 'test4', userAssignable: false, canAssign: true}
+			]);
+
+			var callback = sinon.stub();
+			inputViewSpy.getCall(0).args[0].initSelection(element, callback);
+
+			expect(callback.calledOnce).toEqual(true);
+			expect(callback.getCall(0).args[0]).toEqual([{
+				id: '1', name: 'test1', userVisible: true, userAssignable: true, canAssign: true
+			}, {
+				id: '4', name: 'test4', userVisible: true, userAssignable: false, canAssign: true
 			}]);
 
 			inputViewSpy.restore();
@@ -152,7 +176,8 @@ describe('OCA.SystemTags.SystemTagsInfoView tests', function() {
 				id: '2',
 				name: 'test2',
 				userVisible: true,
-				userAssignable: true
+				userAssignable: true,
+				canAssign: true
 			});
 
 			createStub.restore();

build/integration/features/bootstrap/TagsContext.php

@@ -97,28 +97,36 @@ class TagsContext implements \Behat\Behat\Context\Context {
 	}
 
 	/**
-	 * @When :user creates a :type tag with name :name
 	 * @param string $user
 	 * @param string $type
 	 * @param string $name
-	 * @throws \Exception
+	 * @param string $groups
 	 */
-	public function createsATagWithName($user, $type, $name) {
-		$userVisible = 'true';
-		$userAssignable = 'true';
+	private function createTag($user, $type, $name, $groups = null) {
+		$userVisible = true;
+		$userAssignable = true;
 		switch ($type) {
 			case 'normal':
 				break;
 			case 'not user-assignable':
-				$userAssignable = 'false';
+				$userAssignable = false;
 				break;
 			case 'not user-visible':
-				$userVisible = 'false';
+				$userVisible = false;
 				break;
 			default:
 				throw new \Exception('Unsupported type');
 		}
 
+		$body = [
+			'name' => $name,
+			'userVisible' => $userVisible,
+			'userAssignable' => $userAssignable,
+		];
+		if ($groups !== null) {
+			$body['groups'] = $groups;
+		}
+
 		try {
 			$this->response = $this->client->post(
 				$this->baseUrl . '/remote.php/dav/systemtags/',
@@ -130,14 +138,37 @@ class TagsContext implements \Behat\Behat\Context\Context {
 					'headers' => [
 						'Content-Type' => 'application/json',
 					],
-					'body' => '{"name":"'.$name.'","userVisible":'.$userVisible.',"userAssignable":'.$userAssignable.'}',
+					'body' => json_encode($body)
 				]
 			);
-		} catch (\GuzzleHttp\Exception\ClientException $e){
+		} catch (\GuzzleHttp\Exception\ClientException $e) {
 			$this->response = $e->getResponse();
 		}
 	}
 
+	/**
+	 * @When :user creates a :type tag with name :name
+	 * @param string $user
+	 * @param string $type
+	 * @param string $name
+	 * @throws \Exception
+	 */
+	public function createsATagWithName($user, $type, $name) {
+		$this->createTag($user, $type, $name);
+	}
+
+	/**
+	 * @When :user creates a :type tag with name :name and groups :groups
+	 * @param string $user
+	 * @param string $type
+	 * @param string $name
+	 * @param string $groups
+	 * @throws \Exception
+	 */
+	public function createsATagWithNameAndGroups($user, $type, $name, $groups) {
+		$this->createTag($user, $type, $name, $groups);
+	}
+
 	/**
 	 * @Then The response should have a status code :statusCode
 	 * @param int $statusCode
@@ -155,21 +186,30 @@ class TagsContext implements \Behat\Behat\Context\Context {
 	 * @param string $user
 	 * @return array
 	 */
-	private function requestTagsForUser($user) {
+	private function requestTagsForUser($user, $withGroups = false) {
 		try {
-			$request = $this->client->createRequest(
-				'PROPFIND',
-				$this->baseUrl . '/remote.php/dav/systemtags/',
-				[
-					'body' => '<?xml version="1.0"?>
+			$body = '<?xml version="1.0"?>
 <d:propfind  xmlns:d="DAV:" xmlns:oc="http://owncloud.org/ns">
   <d:prop>
     <oc:id />
     <oc:display-name />
     <oc:user-visible />
-    <oc:user-assignable />
+	<oc:user-assignable />
+	<oc:can-assign />
+';
+
+			if ($withGroups) {
+				$body .= '<oc:groups />';
+			}
+
+			$body .= '
   </d:prop>
-</d:propfind>',
+</d:propfind>';
+			$request = $this->client->createRequest(
+				'PROPFIND',
+				$this->baseUrl . '/remote.php/dav/systemtags/',
+				[
+					'body' => $body,
 					'auth' => [
 						$user,
 						$this->getPasswordForUser($user),
@@ -193,11 +233,16 @@ class TagsContext implements \Behat\Behat\Context\Context {
 				continue;
 			}
 
+			// FIXME: use actual property names instead of guessing index position
 			$tags[$singleEntry[0]['value']] = [
 				'display-name' => $singleEntry[1]['value'],
 				'user-visible' => $singleEntry[2]['value'],
 				'user-assignable' => $singleEntry[3]['value'],
+				'can-assign' => $singleEntry[4]['value'],
 			];
+			if (isset($singleEntry[5])) {
+				$tags[$singleEntry[0]['value']]['groups'] = $singleEntry[5]['value'];
+			}
 		}
 
 		return $tags;
@@ -238,6 +283,42 @@ class TagsContext implements \Behat\Behat\Context\Context {
 		}
 	}
 
+	/**
+	 * @Then the user :user :can assign The :type tag with name :tagName
+	 */
+	public function theUserCanAssignTheTag($user, $can, $type, $tagName) {
+		$foundTag = $this->findTag($type, $tagName, $user);
+		if ($foundTag === null) {
+			throw new \Exception('No matching tag found');
+		}
+
+		if ($can === 'can') {
+			$expected = 'true';
+		} else if ($can === 'cannot') {
+			$expected = 'false';
+		} else {
+			throw new \Exception('Invalid condition, must be "can" or "cannot"');
+		}
+
+		if ($foundTag['can-assign'] !== $expected) {
+			throw new \Exception('Tag cannot be assigned by user');
+		}
+	}
+
+	/**
+	 * @Then The :type tag with name :tagName has the groups :groups
+	 */
+	public function theTagHasGroup($type, $tagName, $groups) {
+		$foundTag = $this->findTag($type, $tagName, 'admin', true);
+		if ($foundTag === null) {
+			throw new \Exception('No matching tag found');
+		}
+
+		if ($foundTag['groups'] !== $groups) {
+			throw new \Exception('Tag has groups "' . $foundTag['group'] . '" instead of the expected "' . $groups . '"');
+		}
+	}
+
 	/**
 	 * @Then :count tags should exist for :user
 	 * @param int $count
@@ -250,6 +331,45 @@ class TagsContext implements \Behat\Behat\Context\Context {
 		}
 	}
 
+	/**
+	 * Find tag by type and name
+	 *
+	 * @param string $type tag type
+	 * @param string $tagName tag name
+	 * @param string $user retrieved from which user
+	 * @param bool $withGroups whether to also query the tag's groups
+	 *
+	 * @return array tag values or null if not found
+	 */
+	private function findTag($type, $tagName, $user = 'admin', $withGroups = false) {
+		$tags = $this->requestTagsForUser($user, $withGroups);
+		$userAssignable = 'true';
+		$userVisible = 'true';
+		switch ($type) {
+			case 'normal':
+				break;
+			case 'not user-assignable':
+				$userAssignable = 'false';
+				break;
+			case 'not user-visible':
+				$userVisible = 'false';
+				break;
+			default:
+				throw new \Exception('Unsupported type');
+		}
+
+		$foundTag = null;
+		foreach ($tags as $tag) {
+			if ($tag['display-name'] === $tagName
+				&& $tag['user-visible'] === $userVisible
+				&& $tag['user-assignable'] === $userAssignable) {
+					$foundTag = $tag;
+					break;
+			}
+		}
+		return $foundTag;
+	}
+
 	/**
 	 * @param string $name
 	 * @return int
@@ -304,6 +424,44 @@ class TagsContext implements \Behat\Behat\Context\Context {
 		}
 	}
 
+	/**
+	 * @When :user edits the tag with name :oldNmae and sets its groups to :groups
+	 * @param string $user
+	 * @param string $oldName
+	 * @param string $groups
+	 * @throws \Exception
+	 */
+	public function editsTheTagWithNameAndSetsItsGroupsTo($user, $oldName, $groups) {
+		$tagId = $this->findTagIdByName($oldName);
+		if($tagId === 0) {
+			throw new \Exception('Could not find tag to rename');
+		}
+
+		try {
+			$request = $this->client->createRequest(
+				'PROPPATCH',
+				$this->baseUrl . '/remote.php/dav/systemtags/' . $tagId,
+				[
+					'body' => '<?xml version="1.0"?>
+<d:propertyupdate  xmlns:d="DAV:" xmlns:oc="http://owncloud.org/ns">
+  <d:set>
+   <d:prop>
+      <oc:groups>' . $groups . '</oc:groups>
+    </d:prop>
+  </d:set>
+</d:propertyupdate>',
+					'auth' => [
+						$user,
+						$this->getPasswordForUser($user),
+					],
+				]
+			);
+			$this->response = $this->client->send($request);
+		} catch (\GuzzleHttp\Exception\ClientException $e) {
+			$this->response = $e->getResponse();
+		}
+	}
+
 	/**
 	 * @When :user deletes the tag with name :name
 	 * @param string $user

build/integration/features/tags.feature

@@ -21,6 +21,18 @@ Feature: tags
     Then The response should have a status code "400"
     And "0" tags should exist for "admin"
 
+  Scenario: Creating a not user-assignable tag with groups as admin should work
+    Given user "user0" exists
+    When "admin" creates a "not user-assignable" tag with name "TagWithGroups" and groups "group1|group2"
+    Then The response should have a status code "201"
+    And The "not user-assignable" tag with name "TagWithGroups" has the groups "group1|group2"
+
+  Scenario: Creating a normal tag with groups as regular user should fail
+    Given user "user0" exists
+    When "user0" creates a "normal" tag with name "MySuperAwesomeTagName" and groups "group1|group2"
+    Then The response should have a status code "400"
+    And "0" tags should exist for "user0"
+
   Scenario: Renaming a normal tag as regular user should work
     Given user "user0" exists
     Given "admin" creates a "normal" tag with name "MySuperAwesomeTagName"
@@ -45,6 +57,19 @@ Feature: tags
     And The following tags should exist for "admin"
       |MySuperAwesomeTagName|false|true|
 
+  Scenario: Editing tag groups as admin should work
+    Given user "user0" exists
+    Given "admin" creates a "not user-assignable" tag with name "TagWithGroups" and groups "group1|group2"
+    When "admin" edits the tag with name "TagWithGroups" and sets its groups to "group1|group3"
+    Then The response should have a status code "207"
+    And The "not user-assignable" tag with name "TagWithGroups" has the groups "group1|group3"
+
+  Scenario: Editing tag groups as regular user should fail
+    Given user "user0" exists
+    Given "admin" creates a "not user-assignable" tag with name "TagWithGroups"
+    When "user0" edits the tag with name "TagWithGroups" and sets its groups to "group1|group3"
+    Then The response should have a status code "403"
+
   Scenario: Deleting a normal tag as regular user should work
     Given user "user0" exists
     Given "admin" creates a "normal" tag with name "MySuperAwesomeTagName"
@@ -122,6 +147,23 @@ Feature: tags
     And "/myFileToTag.txt" shared by "user0" has the following tags
       |MyFirstTag|
 
+  Scenario: Assigning a not user-assignable tag to a file shared by someone else as regular user belongs to tag's groups should work
+    Given user "user0" exists
+    Given user "user1" exists
+    Given group "group1" exists
+    Given user "user1" belongs to group "group1"
+    Given "admin" creates a "not user-assignable" tag with name "MySuperAwesomeTagName" and groups "group1"
+    Given user "user0" uploads file "data/textfile.txt" to "/myFileToTag.txt"
+    Given As "user0" sending "POST" to "/apps/files_sharing/api/v1/shares" with
+      | path | myFileToTag.txt |
+      | shareWith | user1 |
+      | shareType | 0 |
+    When "user1" adds the tag "MySuperAwesomeTagName" to "/myFileToTag.txt" shared by "user0"
+    Then The response should have a status code "201"
+    And "/myFileToTag.txt" shared by "user0" has the following tags
+      |MySuperAwesomeTagName|
+
+
   Scenario: Assigning a not user-visible tag to a file shared by someone else as regular user should fail
     Given user "user0" exists
     Given user "user1" exists
@@ -368,3 +410,18 @@ Feature: tags
     And "/myFileToTag.txt" shared by "user0" has the following tags for "user1"
       ||
     And The response should have a status code "404"
+
+  Scenario: User can assign tags when in the tag's groups
+    Given user "user0" exists
+    Given group "group1" exists
+    Given user "user0" belongs to group "group1"
+    When "admin" creates a "not user-assignable" tag with name "TagWithGroups" and groups "group1|group2"
+    Then The response should have a status code "201"
+    And the user "user0" can assign the "not user-assignable" tag with name "TagWithGroups"
+
+  Scenario: User cannot assign tags when not in the tag's groups
+    Given user "user0" exists
+    When "admin" creates a "not user-assignable" tag with name "TagWithGroups" and groups "group1|group2"
+    Then The response should have a status code "201"
+    And the user "user0" cannot assign the "not user-assignable" tag with name "TagWithGroups"
+

core/js/systemtags/systemtagmodel.js

@@ -23,14 +23,17 @@
 
 		defaults: {
 			userVisible: true,
-			userAssignable: true
+			userAssignable: true,
+			canAssign: true
 		},
 
 		davProperties: {
 			'id': '{' + NS_OWNCLOUD + '}id',
 			'name': '{' + NS_OWNCLOUD + '}display-name',
 			'userVisible': '{' + NS_OWNCLOUD + '}user-visible',
-			'userAssignable': '{' + NS_OWNCLOUD + '}user-assignable'
+			'userAssignable': '{' + NS_OWNCLOUD + '}user-assignable',
+			// read-only, effective permissions computed by the server,
+			'canAssign': '{' + NS_OWNCLOUD + '}can-assign'
 		},
 
 		parse: function(data) {
@@ -38,7 +41,8 @@
 				id: data.id,
 				name: data.name,
 				userVisible: data.userVisible === true || data.userVisible === 'true',
-				userAssignable: data.userAssignable === true || data.userAssignable === 'true'
+				userAssignable: data.userAssignable === true || data.userAssignable === 'true',
+				canAssign: data.canAssign === true || data.canAssign === 'true'
 			};
 		}
 	});

core/js/systemtags/systemtags.js

@@ -36,7 +36,7 @@
 
 			var scope;
 			if (!tag.userAssignable) {
-				scope = t('core', 'not assignable');
+				scope = t('core', 'restricted');
 			}
 			if (!tag.userVisible) {
 				// invisible also implicitly means not assignable

core/js/systemtags/systemtagsinputfield.js

@@ -206,7 +206,8 @@
 				tag = this.collection.create({
 					name: e.object.name.trim(),
 					userVisible: true,
-					userAssignable: true
+					userAssignable: true,
+					canAssign: true
 				}, {
 					success: function(model) {
 						self._addToSelect2Selection(model.toJSON());
@@ -263,7 +264,7 @@
 					var tagModels = collection.filterByName(query.term.trim());
 					if (!self._isAdmin) {
 						tagModels = _.filter(tagModels, function(tagModel) {
-							return tagModel.get('userAssignable');
+							return tagModel.get('canAssign');
 						});
 					}
 					query.callback({
@@ -331,6 +332,7 @@
 					name: term,
 					userAssignable: true,
 					userVisible: true,
+					canAssign: true,
 					isNew: true
 				};
 			} else {
@@ -346,7 +348,7 @@
 
 			function modelToSelection(model) {
 				var data = model.toJSON();
-				if (!self._isAdmin && !data.userAssignable) {
+				if (!self._isAdmin && !data.canAssign) {
 					// lock static tags for non-admins
 					data.locked = true;
 				}

core/js/tests/specs/systemtags/systemtagsSpec.js

@@ -64,6 +64,6 @@ describe('OC.SystemTags tests', function() {
 		testScope(true, true, 'Fourty Two');
 		testScope(false, true, 'Fourty Two (invisible)');
 		testScope(false, false, 'Fourty Two (invisible)');
-		testScope(true, false, 'Fourty Two (not assignable)');
+		testScope(true, false, 'Fourty Two (restricted)');
 	});
 });

core/js/tests/specs/systemtags/systemtagsinputfieldSpec.js

@@ -68,7 +68,7 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
 				view.collection.add([
 					new OC.SystemTags.SystemTagModel({id: '1', name: 'abc'}),
 					new OC.SystemTags.SystemTagModel({id: '2', name: 'def'}),
-					new OC.SystemTags.SystemTagModel({id: '3', name: 'abd', userAssignable: false}),
+					new OC.SystemTags.SystemTagModel({id: '3', name: 'abd', userAssignable: false, canAssign: false}),
 				]);
 			});
 			it('does not create dummy tag when user types non-matching name', function() {
@@ -84,6 +84,7 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
 				expect(result.isNew).toEqual(true);
 				expect(result.userVisible).toEqual(true);
 				expect(result.userAssignable).toEqual(true);
+				expect(result.canAssign).toEqual(true);
 			});
 			it('creates dummy tag when user types non-matching name even with prefix of existing tag', function() {
 				var opts = select2Stub.getCall(0).args[0];
@@ -93,6 +94,7 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
 				expect(result.isNew).toEqual(true);
 				expect(result.userVisible).toEqual(true);
 				expect(result.userAssignable).toEqual(true);
+				expect(result.canAssign).toEqual(true);
 			});
 			it('creates the real tag and fires select event after user selects the dummy tag', function() {
 				var selectHandler = sinon.stub();
@@ -110,14 +112,16 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
 				expect(createStub.getCall(0).args[0]).toEqual({
 					name: 'newname',
 					userVisible: true,
-					userAssignable: true
+					userAssignable: true,
+					canAssign: true
 				});
 
 				var newModel = new OC.SystemTags.SystemTagModel({
 					id: '123',
 					name: 'newname',
 					userVisible: true,
-					userAssignable: true
+					userAssignable: true,
+					canAssign: true
 				});
 
 				// not called yet
@@ -186,7 +190,8 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
 				expect(createStub.getCall(0).args[0]).toEqual({
 					name: 'newname',
 					userVisible: true,
-					userAssignable: true
+					userAssignable: true,
+					canAssign: true
 				});
 
 				var newModel = new OC.SystemTags.SystemTagModel({
@@ -320,7 +325,8 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
 				testTags = [
 					new OC.SystemTags.SystemTagModel({id: '1', name: 'test1'}),
 					new OC.SystemTags.SystemTagModel({id: '2', name: 'test2'}),
-					new OC.SystemTags.SystemTagModel({id: '3', name: 'test3', userAssignable: false}),
+					new OC.SystemTags.SystemTagModel({id: '3', name: 'test3', userAssignable: false, canAssign: false}),
+					new OC.SystemTags.SystemTagModel({id: '4', name: 'test4', userAssignable: false, canAssign: true})
 				];
 			});
 			afterEach(function() {
@@ -328,7 +334,7 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
 			});
 			it('grabs values from the full collection', function() {
 				var $el = view.$el.find('input');
-				$el.val('1,3');
+				$el.val('1,3,4');
 				var opts = select2Stub.getCall(0).args[0];
 				var callback = sinon.stub();
 				opts.initSelection($el, callback);
@@ -339,11 +345,16 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
 
 				expect(callback.calledOnce).toEqual(true);
 				var models = callback.getCall(0).args[0];
-				expect(models.length).toEqual(2);
+				expect(models.length).toEqual(3);
 				expect(models[0].id).toEqual('1');
 				expect(models[0].name).toEqual('test1');
+				expect(models[0].locked).toBeFalsy();
 				expect(models[1].id).toEqual('3');
 				expect(models[1].name).toEqual('test3');
+				expect(models[1].locked).toBeFalsy();
+				expect(models[2].id).toEqual('4');
+				expect(models[2].name).toEqual('test4');
+				expect(models[2].locked).toBeFalsy();
 			});
 		});
 		describe('autocomplete', function() {
@@ -356,7 +367,7 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
 				view.collection.add([
 					new OC.SystemTags.SystemTagModel({id: '1', name: 'abc'}),
 					new OC.SystemTags.SystemTagModel({id: '2', name: 'def'}),
-					new OC.SystemTags.SystemTagModel({id: '3', name: 'abd', userAssignable: false}),
+					new OC.SystemTags.SystemTagModel({id: '3', name: 'abd', userAssignable: false, canAssign: false}),
 					new OC.SystemTags.SystemTagModel({id: '4', name: 'Deg'}),
 				]);
 			});
@@ -379,13 +390,15 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
 						id: '1',
 						name: 'abc',
 						userVisible: true,
-						userAssignable: true
+						userAssignable: true,
+						canAssign: true
 					},
 					{
 						id: '3',
 						name: 'abd',
 						userVisible: true,
-						userAssignable: false
+						userAssignable: false,
+						canAssign: false
 					}
 				]);
 			});
@@ -405,13 +418,15 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
 						id: '2',
 						name: 'def',
 						userVisible: true,
-						userAssignable: true
+						userAssignable: true,
+						canAssign: true
 					},
 					{
 						id: '4',
 						name: 'Deg',
 						userVisible: true,
-						userAssignable: true
+						userAssignable: true,
+						canAssign: true
 					}
 				]);
 			});
@@ -445,7 +460,8 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
 				testTags = [
 					new OC.SystemTags.SystemTagModel({id: '1', name: 'test1'}),
 					new OC.SystemTags.SystemTagModel({id: '2', name: 'test2'}),
-					new OC.SystemTags.SystemTagModel({id: '3', name: 'test3', userAssignable: false}),
+					new OC.SystemTags.SystemTagModel({id: '3', name: 'test3', userAssignable: false, canAssign: false}),
+					new OC.SystemTags.SystemTagModel({id: '4', name: 'test4', userAssignable: false, canAssign: true})
 				];
 				view.render();
 			});
@@ -454,7 +470,7 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
 			});
 			it('grabs values from the full collection', function() {
 				var $el = view.$el.find('input');
-				$el.val('1,3');
+				$el.val('1,3,4');
 				var opts = select2Stub.getCall(0).args[0];
 				var callback = sinon.stub();
 				opts.initSelection($el, callback);
@@ -465,11 +481,17 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
 
 				expect(callback.calledOnce).toEqual(true);
 				var models = callback.getCall(0).args[0];
-				expect(models.length).toEqual(2);
+				expect(models.length).toEqual(3);
 				expect(models[0].id).toEqual('1');
 				expect(models[0].name).toEqual('test1');
+				expect(models[0].locked).toBeFalsy();
 				expect(models[1].id).toEqual('3');
 				expect(models[1].name).toEqual('test3');
+				// restricted / cannot assign locks the entry
+				expect(models[1].locked).toEqual(true);
+				expect(models[2].id).toEqual('4');
+				expect(models[2].name).toEqual('test4');
+				expect(models[2].locked).toBeFalsy();
 			});
 		});
 		describe('autocomplete', function() {
@@ -483,8 +505,9 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
 				view.collection.add([
 					new OC.SystemTags.SystemTagModel({id: '1', name: 'abc'}),
 					new OC.SystemTags.SystemTagModel({id: '2', name: 'def'}),
-					new OC.SystemTags.SystemTagModel({id: '3', name: 'abd', userAssignable: false}),
+					new OC.SystemTags.SystemTagModel({id: '3', name: 'abd', userAssignable: false, canAssign: false}),
 					new OC.SystemTags.SystemTagModel({id: '4', name: 'Deg'}),
+					new OC.SystemTags.SystemTagModel({id: '5', name: 'abe', userAssignable: false, canAssign: true})
 				]);
 			});
 			afterEach(function() {
@@ -506,7 +529,15 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
 						id: '1',
 						name: 'abc',
 						userVisible: true,
-						userAssignable: true
+						userAssignable: true,
+						canAssign: true
+					},
+					{
+						id: '5',
+						name: 'abe',
+						userVisible: true,
+						userAssignable: false,
+						canAssign: true
 					}
 				]);
 			});
@@ -526,13 +557,15 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
 						id: '2',
 						name: 'def',
 						userVisible: true,
-						userAssignable: true
+						userAssignable: true,
+						canAssign: true
 					},
 					{
 						id: '4',
 						name: 'Deg',
 						userVisible: true,
-						userAssignable: true
+						userAssignable: true,
+						canAssign: true
 					}
 				]);
 			});

core/shipped.json

@@ -30,6 +30,7 @@
     "provisioning_api",
     "sharepoint",
     "systemtags",
+    "systemtags_management",
     "templateeditor",
     "updatenotification",
     "user_external",

db_structure.xml

@@ -1380,6 +1380,49 @@
 
 	</table>
 
+	<table>
+
+		<!--
+		System tag to group mapping
+		-->
+		<name>*dbprefix*systemtag_group</name>
+
+		<declaration>
+
+		<!-- Foreign Key systemtag::id -->
+		<field>
+			<name>systemtagid</name>
+			<type>integer</type>
+			<default>0</default>
+			<notnull>true</notnull>
+			<unsigned>true</unsigned>
+			<length>4</length>
+		</field>
+
+		<field>
+			<name>gid</name>
+			<type>string</type>
+			<notnull>true</notnull>
+		</field>
+
+		<index>
+			<name>systemtag_group</name>
+			<primary>true</primary>
+			<unique>true</unique>
+			<field>
+				<name>gid</name>
+				<sorting>ascending</sorting>
+			</field>
+			<field>
+				<name>systemtagid</name>
+				<sorting>ascending</sorting>
+			</field>
+		</index>
+
+		</declaration>
+
+	</table>
+
 	<table>
 
 		<!--

lib/private/SystemTag/ManagerFactory.php

@@ -59,6 +59,7 @@ class ManagerFactory implements ISystemTagManagerFactory {
 	public function getManager() {
 		return new SystemTagManager(
 			$this->serverContainer->getDatabaseConnection(),
+			$this->serverContainer->getGroupManager(),
 			$this->serverContainer->getEventDispatcher()
 		);
 	}

lib/private/SystemTag/SystemTagManager.php

@@ -30,10 +30,18 @@ use OCP\SystemTag\ManagerEvent;
 use OCP\SystemTag\TagAlreadyExistsException;
 use OCP\SystemTag\TagNotFoundException;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
+use OCP\IUserManager;
+use OCP\IGroupManager;
+use OCP\SystemTag\ISystemTag;
+use OCP\IUser;
 
+/**
+ * Manager class for system tags
+ */
 class SystemTagManager implements ISystemTagManager {
 
 	const TAG_TABLE = 'systemtag';
+	const TAG_GROUP_TABLE = 'systemtag_group';
 
 	/** @var IDBConnection */
 	protected $connection;
@@ -41,6 +49,9 @@ class SystemTagManager implements ISystemTagManager {
 	/** @var EventDispatcherInterface */
 	protected $dispatcher;
 
+	/** @var IGroupManager */
+	protected $groupManager;
+
 	/**
 	 * Prepared query for selecting tags directly
 	 *
@@ -54,8 +65,13 @@ class SystemTagManager implements ISystemTagManager {
 	 * @param IDBConnection $connection database connection
 	 * @param EventDispatcherInterface $dispatcher
 	 */
-	public function __construct(IDBConnection $connection, EventDispatcherInterface $dispatcher) {
+	public function __construct(
+		IDBConnection $connection,
+		IGroupManager $groupManager,
+		EventDispatcherInterface $dispatcher
+	) {
 		$this->connection = $connection;
+		$this->groupManager = $groupManager;
 		$this->dispatcher = $dispatcher;
 
 		$query = $this->connection->getQueryBuilder();
@@ -316,7 +332,102 @@ 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;
+		}
+
+		if ($this->groupManager->isAdmin($user->getUID())) {
+			return true;
+		}
+
+		if (!$tag->isUserVisible()) {
+			return false;
+		}
+
+		$groupIds = $this->groupManager->getUserGroupIds($user);
+		if (!empty($groupIds)) {
+			$matchingGroups = array_intersect($groupIds, $this->getTagGroups($tag));
+			if (!empty($matchingGroups)) {
+				return true;
+			}
+		}
+
+		return false;
+	}
+
+	/**
+	 * {@inheritdoc}
+	 */
+	public function canUserSeeTag(ISystemTag $tag, IUser $user) {
+		if ($tag->isUserVisible()) {
+			return true;
+		}
+
+		if ($this->groupManager->isAdmin($user->getUID())) {
+			return true;
+		}
+
+		return false;
+	}
+
 	private function createSystemTagFromRow($row) {
 		return new SystemTag((int)$row['id'], $row['name'], (bool)$row['visibility'], (bool)$row['editable']);
 	}
+
+	/**
+	 * {@inheritdoc}
+	 */
+	public function setTagGroups(ISystemTag $tag, $groupIds) {
+		// delete relationships first
+		$this->connection->beginTransaction();
+		try {
+			$query = $this->connection->getQueryBuilder();
+			$query->delete(self::TAG_GROUP_TABLE)
+				->where($query->expr()->eq('systemtagid', $query->createNamedParameter($tag->getId())))
+				->execute();
+
+			// add each group id
+			$query = $this->connection->getQueryBuilder();
+			$query->insert(self::TAG_GROUP_TABLE)
+				->values([
+					'systemtagid' => $query->createNamedParameter($tag->getId()),
+					'gid' => $query->createParameter('gid'),
+				]);
+			foreach ($groupIds as $groupId) {
+				$query->setParameter('gid', $groupId);
+				$query->execute();
+			}
+
+			$this->connection->commit();
+		} catch (\Exception $e) {
+			$this->connection->rollback();
+			throw $e;
+		}
+	}
+
+	/**
+	 * {@inheritdoc}
+	 */
+	public function getTagGroups(ISystemTag $tag) {
+		$groupIds = [];
+		$query = $this->connection->getQueryBuilder();
+		$query->select('gid')
+			->from(self::TAG_GROUP_TABLE)
+			->where($query->expr()->eq('systemtagid', $query->createNamedParameter($tag->getId())))
+			->orderBy('gid');
+
+		$result = $query->execute();
+		while ($row = $result->fetch()) {
+			$groupIds[] = $row['gid'];
+		}
+
+		$result->closeCursor();
+
+		return $groupIds;
+	}
 }

lib/public/SystemTag/ISystemTagManager.php

@@ -22,6 +22,9 @@
 
 namespace OCP\SystemTag;
 
+use OCP\IUser;
+use OCP\SystemTag\ISystemTag;
+
 /**
  * Public interface to access and manage system-wide tags.
  *
@@ -113,4 +116,49 @@ interface ISystemTagManager {
 	 */
 	public function deleteTags($tagIds);
 
+	/**
+	 * Checks whether the given user is allowed to assign/unassign the tag with the
+	 * given id.
+	 *
+	 * @param ISystemTag $tag tag to check permission for
+	 * @param IUser $user user to check permission for
+	 *
+	 * @return true if the user is allowed to assign/unassign the tag, false otherwise
+	 *
+	 * @since 9.1.0
+	 */
+	public function canUserAssignTag(ISystemTag $tag, IUser $user);
+
+	/**
+	 * Checks whether the given user is allowed to see the tag with the given id.
+	 *
+	 * @param ISystemTag $tag tag to check permission for
+	 * @param IUser $user user to check permission for
+	 *
+	 * @return true if the user can see the tag, false otherwise
+	 *
+	 * @since 9.1.0
+	 */
+	public function canUserSeeTag(ISystemTag $tag, IUser $userId);
+
+	/**
+	 * Set groups that can assign a given tag.
+	 *
+	 * @param ISystemTag $tag tag for group assignment
+	 * @param string[] $groupIds group ids of groups that can assign/unassign the tag
+	 *
+	 * @since 9.1.0
+	 */
+	public function setTagGroups(ISystemTag $tag, $groupIds);
+
+	/**
+	 * Get groups that can assign a given tag.
+	 *
+	 * @param ISystemTag $tag tag for group assignment
+	 *
+	 * @return string[] group ids of groups that can assign/unassign the tag
+	 *
+	 * @since 9.1.0
+	 */
+	public function getTagGroups(ISystemTag $tag);
 }

settings/js/settings.js

@@ -16,10 +16,13 @@ OC.Settings = _.extend(OC.Settings, {
 	 * for groups)
 	 *
 	 * @param $elements jQuery element (hidden input) to setup select2 on
-	 * @param [extraOptions] extra options hash to pass to select2
+	 * @param {Array} [extraOptions] extra options hash to pass to select2
+	 * @param {Array} [options] extra options
+	 * @param {Array} [options.excludeAdmins=false] flag whether to exclude admin groups
 	 */
-	setupGroupsSelect: function($elements, extraOptions) {
+	setupGroupsSelect: function($elements, extraOptions, options) {
 		var self = this;
+		options = options || {};
 		if ($elements.length > 0) {
 			// note: settings are saved through a "change" event registered
 			// on all input fields
@@ -48,9 +51,11 @@ OC.Settings = _.extend(OC.Settings, {
 							var results = [];
 
 							// add groups
-							$.each(data.data.adminGroups, function(i, group) {
-								results.push({id:group.id, displayname:group.name});
-							});
+							if (!options.excludeAdmins) {
+								$.each(data.data.adminGroups, function(i, group) {
+									results.push({id:group.id, displayname:group.name});
+								});
+							}
 							$.each(data.data.groups, function(i, group) {
 								results.push({id:group.id, displayname:group.name});
 							});

tests/lib/SystemTag/SystemTagManagerTest.php

@@ -17,6 +17,8 @@ use OCP\SystemTag\ISystemTag;
 use OCP\SystemTag\ISystemTagManager;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Test\TestCase;
+use OCP\IUserManager;
+use OCP\IGroupManager;
 
 /**
  * Class TestSystemTagManager
@@ -36,6 +38,11 @@ class SystemTagManagerTest extends TestCase {
 	 */
 	private $connection;
 
+	/**
+	 * @var IGroupManager
+	 */
+	private $groupManager;
+
 	/**
 	 * @var EventDispatcherInterface
 	 */
@@ -49,8 +56,11 @@ class SystemTagManagerTest extends TestCase {
 		$this->dispatcher = $this->getMockBuilder('Symfony\Component\EventDispatcher\EventDispatcherInterface')
 			->getMock();
 
+		$this->groupManager = $this->getMockBuilder('\OCP\IGroupManager')->getMock();
+
 		$this->tagManager = new SystemTagManager(
 			$this->connection,
+			$this->groupManager,
 			$this->dispatcher
 		);
 		$this->pruneTagsTables();
@@ -410,6 +420,102 @@ class SystemTagManagerTest extends TestCase {
 		], $tagIdMapping);
 	}
 
+	public function visibilityCheckProvider() {
+		return [
+			[false, false, false, false],
+			[true, false, false, true],
+			[false, false, true, true],
+			[true, false, true, true],
+		];
+	}
+
+	/**
+	 * @dataProvider visibilityCheckProvider
+	 */
+	public function testVisibilityCheck($userVisible, $userAssignable, $isAdmin, $expectedResult) {
+		$user = $this->getMockBuilder('\OCP\IUser')->getMock();
+		$user->expects($this->any())
+			->method('getUID')
+			->will($this->returnValue('test'));
+		$tag1 = $this->tagManager->createTag('one', $userVisible, $userAssignable);
+
+		$this->groupManager->expects($this->any())
+			->method('isAdmin')
+			->with('test')
+			->will($this->returnValue($isAdmin));
+
+		$this->assertEquals($expectedResult, $this->tagManager->canUserSeeTag($tag1, $user));
+	}
+
+	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, $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')
+			->with($user)
+			->will($this->returnValue($userGroupIds));
+
+		$this->assertEquals($expectedResult, $this->tagManager->canUserAssignTag($tag1, $user));
+	}
+
+	public function testTagGroups() {
+		$tag1 = $this->tagManager->createTag('tag1', true, false);
+		$tag2 = $this->tagManager->createTag('tag2', true, false);
+		$this->tagManager->setTagGroups($tag1, ['group1', 'group2']);
+		$this->tagManager->setTagGroups($tag2, ['group2', 'group3']);
+
+		$this->assertEquals(['group1', 'group2'], $this->tagManager->getTagGroups($tag1));
+		$this->assertEquals(['group2', 'group3'], $this->tagManager->getTagGroups($tag2));
+
+		// change groups
+		$this->tagManager->setTagGroups($tag1, ['group3', 'group4']);
+		$this->tagManager->setTagGroups($tag2, []);
+
+		$this->assertEquals(['group3', 'group4'], $this->tagManager->getTagGroups($tag1));
+		$this->assertEquals([], $this->tagManager->getTagGroups($tag2));
+	}
+
 	/**
 	 * @param ISystemTag $tag1
 	 * @param ISystemTag $tag2

version.php

@@ -26,7 +26,7 @@
 // We only can count up. The 4. digit is only for the internal patchlevel to trigger DB upgrades
 // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel
 // when updating major/minor version number.
-$OC_Version = array(9, 1, 0, 2);
+$OC_Version = array(9, 1, 0, 3);
 
 // The human readable string
 $OC_VersionString = '9.1.0 pre alpha';