Add interface methods for permission check

Instead of checking for admin perm, use interface method
canUserAssignTag and canUserSeeTag to check for permissions.
Allows for more flexible implementation.

5 files changed, 91 insertions, 77 deletions

diff --git a/apps/dav/lib/SystemTag/SystemTagMappingNode.php b/apps/dav/lib/SystemTag/SystemTagMappingNode.php
index bb2936c13dc..83e10e5bfb2 100644
--- a/apps/dav/lib/SystemTag/SystemTagMappingNode.php
+++ b/apps/dav/lib/SystemTag/SystemTagMappingNode.php
@@ -56,7 +56,7 @@ class SystemTagMappingNode extends SystemTagNode {
 	 * @param ISystemTag $tag system tag
 	 * @param string $objectId
 	 * @param string $objectType
-	 * @param bool $isAdmin whether to allow permissions for admin
+	 * @param string $userId user id
 	 * @param ISystemTagManager $tagManager
 	 * @param ISystemTagObjectMapper $tagMapper
 	 */
@@ -64,14 +64,14 @@ class SystemTagMappingNode extends SystemTagNode {
 		ISystemTag $tag,
 		$objectId,
 		$objectType,
-		$isAdmin,
+		$userId,
 		ISystemTagManager $tagManager,
 		ISystemTagObjectMapper $tagMapper
 	) {
 		$this->objectId = $objectId;
 		$this->objectType = $objectType;
 		$this->tagMapper = $tagMapper;
-		parent::__construct($tag, $isAdmin, $tagManager);
+		parent::__construct($tag, $userId, $tagManager);
 	}
 
 	/**
@@ -97,13 +97,11 @@ class SystemTagMappingNode extends SystemTagNode {
 	 */
 	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->userId)) {
+				throw new NotFound('Tag with id ' . $this->tag->getId() . ' not found');
+			}
+			if (!$this->tagManager->canUserAssignTag($this->tag, $this->userId)) {
+				throw new Forbidden('No permission to unassign tag ' . $this->tag->getId());
 			}
 			$this->tagMapper->unassignTags($this->objectId, $this->objectType, $this->tag->getId());
 		} catch (TagNotFoundException $e) {
diff --git a/apps/dav/lib/SystemTag/SystemTagNode.php b/apps/dav/lib/SystemTag/SystemTagNode.php
index 500e1a3adea..7de80696f59 100644
--- a/apps/dav/lib/SystemTag/SystemTagNode.php
+++ b/apps/dav/lib/SystemTag/SystemTagNode.php
@@ -49,22 +49,22 @@ class SystemTagNode implements \Sabre\DAV\INode {
 	protected $tagManager;
 
 	/**
-	 * Whether to allow permissions for admins
+	 * User id
 	 *
-	 * @var bool
+	 * @var string
 	 */
-	protected $isAdmin;
+	protected $userId;
 
 	/**
 	 * Sets up the node, expects a full path name
 	 *
 	 * @param ISystemTag $tag system tag
-	 * @param bool $isAdmin whether to allow operations for admins
-	 * @param ISystemTagManager $tagManager
+	 * @param string $userId user id
+	 * @param ISystemTagManager $tagManager tag manager
 	 */
-	public function __construct(ISystemTag $tag, $isAdmin, ISystemTagManager $tagManager) {
+	public function __construct(ISystemTag $tag, $userId, ISystemTagManager $tagManager) {
 		$this->tag = $tag;
-		$this->isAdmin = $isAdmin;
+		$this->userId = $userId;
 		$this->tagManager = $tagManager;
 	}
 
@@ -109,21 +109,22 @@ 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());
-				}
-
-				// only renaming is allowed for regular users
-				if ($userVisible !== $this->tag->isUserVisible()
-					|| $userAssignable !== $this->tag->isUserAssignable()
-				) {
-					throw new Forbidden('No permission to update permissions for tag ' . $this->tag->getId());
-				}
+			if (!$this->tagManager->canUserSeeTag($this->tag, $this->userId)) {
+				throw new NotFound('Tag with id ' . $this->tag->getId() . ' does not exist');
 			}
+			if (!$this->tagManager->canUserAssignTag($this->tag, $this->userId)) {
+				throw new Forbidden('No permission to update tag ' . $this->tag->getId());
+			}
+
+			// FIXME: admin should be able to change permissions still
+
+			// only renaming is allowed for regular users
+			if ($userVisible !== $this->tag->isUserVisible()
+				|| $userAssignable !== $this->tag->isUserAssignable()
+			) {
+				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 +146,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->userId)) {
+				throw new NotFound('Tag with id ' . $this->tag->getId() . ' not found');
+			}
+			if (!$this->tagManager->canUserAssignTag($this->tag, $this->userId)) {
+				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
diff --git a/apps/dav/lib/SystemTag/SystemTagsByIdCollection.php b/apps/dav/lib/SystemTag/SystemTagsByIdCollection.php
index 298902501ab..73b595b4e4a 100644
--- a/apps/dav/lib/SystemTag/SystemTagsByIdCollection.php
+++ b/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();
@@ -79,6 +82,21 @@ class SystemTagsByIdCollection implements ICollection {
 	}
 
 	/**
+	 * Returns the user id
+	 *
+	 * @return string user id
+	 *
+	 * @throws NoUserException if no user exists in the session
+	 */
+	private function getUserId() {
+		$user = $this->userSession->getUser();
+		if ($user !== null) {
+			return $user->getUID();
+		}
+		throw new NoUserException();
+	}
+
+	/**
 	 * @param string $name
 	 * @param resource|string $data Initial payload
 	 * @throws Forbidden
@@ -101,7 +119,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->getUserId())) {
 				throw new NotFound('Tag with id ' . $name . ' not found');
 			}
 			return $this->makeNode($tag);
@@ -131,7 +149,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->getUserId())) {
 				return false;
 			}
 			return true;
@@ -171,6 +189,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->getUserId(), $this->tagManager);
 	}
 }
diff --git a/apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php b/apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php
index eb75ed06393..b87b51dffa9 100644
--- a/apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php
+++ b/apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php
@@ -58,11 +58,11 @@ class SystemTagsObjectMappingCollection implements ICollection {
 	private $tagMapper;
 
 	/**
-	 * Whether to return results only visible for admins
+	 * User id
 	 *
-	 * @var bool
+	 * @var string
 	 */
-	private $isAdmin;
+	private $userId;
 
 
 	/**
@@ -70,30 +70,29 @@ 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 string $userId user id
 	 * @param ISystemTagManager $tagManager
 	 * @param ISystemTagObjectMapper $tagMapper
 	 */
-	public function __construct($objectId, $objectType, $isAdmin, $tagManager, $tagMapper) {
+	public function __construct($objectId, $objectType, $userId, $tagManager, $tagMapper) {
 		$this->tagManager = $tagManager;
 		$this->tagMapper = $tagMapper;
 		$this->objectId = $objectId;
 		$this->objectType = $objectType;
-		$this->isAdmin = $isAdmin;
+		$this->userId = $userId;
 	}
 
 	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->userId)) {
+				throw new PreconditionFailed('Tag with id ' . $tagId . ' does not exist, cannot assign');
+			}
+			if (!$this->tagManager->canUserAssignTag($tag, $this->userId)) {
+				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 +108,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->userId)) {
 					return $this->makeNode($tag);
 				}
 			}
@@ -127,12 +126,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->userId);
+		});
+
 		return array_values(array_map(function($tag) {
 			return $this->makeNode($tag);
 		}, $tags));
@@ -141,17 +140,12 @@ class SystemTagsObjectMappingCollection implements ICollection {
 	function childExists($tagId) {
 		try {
 			$result = ($this->tagMapper->haveTag([$this->objectId], $this->objectType, $tagId, true));
-			if ($this->isAdmin || !$result) {
-				return $result;
-			}
 
-			// verify if user is allowed to see this tag
-			$tag = $this->tagManager->getTagsByIds($tagId);
-			$tag = current($tag);
-			if (!$tag->isUserVisible()) {
+			if ($result && !$this->tagManager->canUserSeeTag($tagId, $this->userId)) {
 				return false;
 			}
-			return true;
+
+			return $result;
 		} catch (\InvalidArgumentException $e) {
 			throw new BadRequest('Invalid tag id', 0, $e);
 		} catch (TagNotFoundException $e) {
@@ -193,7 +187,7 @@ class SystemTagsObjectMappingCollection implements ICollection {
 			$tag,
 			$this->objectId,
 			$this->objectType,
-			$this->isAdmin,
+			$this->userId,
 			$this->tagManager,
 			$this->tagMapper
 		);
diff --git a/apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php b/apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php
index bdbc73c4e32..02c9995f7c5 100644
--- a/apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php
+++ b/apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php
@@ -95,14 +95,18 @@ class SystemTagsObjectTypeCollection implements ICollection {
 	}
 
 	/**
-	 * Returns whether the currently logged in user is an administrator
+	 * Returns the user id
+	 *
+	 * @return string user id
+	 *
+	 * @throws NoUserException if no user exists in the session
 	 */
-	private function isAdmin() {
+	private function getUserId() {
 		$user = $this->userSession->getUser();
 		if ($user !== null) {
-			return $this->groupManager->isAdmin($user->getUID());
+			return $user->getUID();
 		}
-		return false;
+		throw new NoUserException();
 	}
 
 	/**
@@ -132,7 +136,7 @@ class SystemTagsObjectTypeCollection implements ICollection {
 		return new SystemTagsObjectMappingCollection(
 			$objectId,
 			$this->objectType,
-			$this->isAdmin(),
+			$this->getUserId(),
 			$this->tagManager,
 			$this->tagMapper
 		);