Updated canUser* functions in SystemTagManager to accept objects

9 files changed, 77 insertions, 140 deletions

diff --git a/apps/dav/lib/SystemTag/SystemTagMappingNode.php b/apps/dav/lib/SystemTag/SystemTagMappingNode.php
index 83e10e5bfb2..b72b1de0183 100644
--- a/apps/dav/lib/SystemTag/SystemTagMappingNode.php
+++ b/apps/dav/lib/SystemTag/SystemTagMappingNode.php
@@ -29,6 +29,7 @@ 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
@@ -56,7 +57,7 @@ class SystemTagMappingNode extends SystemTagNode {
 	 * @param ISystemTag $tag system tag
 	 * @param string $objectId
 	 * @param string $objectType
-	 * @param string $userId user id
+	 * @param IUser $user user
 	 * @param ISystemTagManager $tagManager
 	 * @param ISystemTagObjectMapper $tagMapper
 	 */
@@ -64,14 +65,14 @@ class SystemTagMappingNode extends SystemTagNode {
 		ISystemTag $tag,
 		$objectId,
 		$objectType,
-		$userId,
+		IUser $user,
 		ISystemTagManager $tagManager,
 		ISystemTagObjectMapper $tagMapper
 	) {
 		$this->objectId = $objectId;
 		$this->objectType = $objectType;
 		$this->tagMapper = $tagMapper;
-		parent::__construct($tag, $userId, $tagManager);
+		parent::__construct($tag, $user, $tagManager);
 	}
 
 	/**
@@ -97,10 +98,10 @@ class SystemTagMappingNode extends SystemTagNode {
 	 */
 	public function delete() {
 		try {
-			if (!$this->tagManager->canUserSeeTag($this->tag, $this->userId)) {
+			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->userId)) {
+			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());
diff --git a/apps/dav/lib/SystemTag/SystemTagNode.php b/apps/dav/lib/SystemTag/SystemTagNode.php
index 7de80696f59..c8fe5f35239 100644
--- a/apps/dav/lib/SystemTag/SystemTagNode.php
+++ b/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.
@@ -49,22 +50,22 @@ class SystemTagNode implements \Sabre\DAV\INode {
 	protected $tagManager;
 
 	/**
-	 * User id
+	 * User
 	 *
-	 * @var string
+	 * @var IUser
 	 */
-	protected $userId;
+	protected $user;
 
 	/**
 	 * Sets up the node, expects a full path name
 	 *
 	 * @param ISystemTag $tag system tag
-	 * @param string $userId user id
+	 * @param IUser $user user
 	 * @param ISystemTagManager $tagManager tag manager
 	 */
-	public function __construct(ISystemTag $tag, $userId, ISystemTagManager $tagManager) {
+	public function __construct(ISystemTag $tag, IUser $user, ISystemTagManager $tagManager) {
 		$this->tag = $tag;
-		$this->userId = $userId;
+		$this->user = $user;
 		$this->tagManager = $tagManager;
 	}
 
@@ -109,10 +110,10 @@ class SystemTagNode implements \Sabre\DAV\INode {
 	 */
 	public function update($name, $userVisible, $userAssignable) {
 		try {
-			if (!$this->tagManager->canUserSeeTag($this->tag, $this->userId)) {
+			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->userId)) {
+			if (!$this->tagManager->canUserAssignTag($this->tag, $this->user)) {
 				throw new Forbidden('No permission to update tag ' . $this->tag->getId());
 			}
 
@@ -146,10 +147,10 @@ class SystemTagNode implements \Sabre\DAV\INode {
 
 	public function delete() {
 		try {
-			if (!$this->tagManager->canUserSeeTag($this->tag, $this->userId)) {
+			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->userId)) {
+			if (!$this->tagManager->canUserAssignTag($this->tag, $this->user)) {
 				throw new Forbidden('No permission to delete tag ' . $this->tag->getId());
 			}
 
diff --git a/apps/dav/lib/SystemTag/SystemTagsByIdCollection.php b/apps/dav/lib/SystemTag/SystemTagsByIdCollection.php
index 73b595b4e4a..deaa154d46f 100644
--- a/apps/dav/lib/SystemTag/SystemTagsByIdCollection.php
+++ b/apps/dav/lib/SystemTag/SystemTagsByIdCollection.php
@@ -82,21 +82,6 @@ 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
@@ -119,7 +104,7 @@ class SystemTagsByIdCollection implements ICollection {
 		try {
 			$tag = $this->tagManager->getTagsByIds([$name]);
 			$tag = current($tag);
-			if (!$this->tagManager->canUserSeeTag($tag, $this->getUserId())) {
+			if (!$this->tagManager->canUserSeeTag($tag, $this->userSession->getUser())) {
 				throw new NotFound('Tag with id ' . $name . ' not found');
 			}
 			return $this->makeNode($tag);
@@ -149,7 +134,7 @@ class SystemTagsByIdCollection implements ICollection {
 		try {
 			$tag = $this->tagManager->getTagsByIds([$name]);
 			$tag = current($tag);
-			if (!$this->tagManager->canUserSeeTag($tag, $this->getUserId())) {
+			if (!$this->tagManager->canUserSeeTag($tag, $this->userSession->getUser())) {
 				return false;
 			}
 			return true;
@@ -189,6 +174,6 @@ class SystemTagsByIdCollection implements ICollection {
 	 * @return SystemTagNode
 	 */
 	private function makeNode(ISystemTag $tag) {
-		return new SystemTagNode($tag, $this->getUserId(), $this->tagManager);
+		return new SystemTagNode($tag, $this->userSession->getUser(), $this->tagManager);
 	}
 }
diff --git a/apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php b/apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php
index b87b51dffa9..c4c45dc76d6 100644
--- a/apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php
+++ b/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;
 
 	/**
-	 * User id
+	 * User
 	 *
-	 * @var string
+	 * @var IUser
 	 */
-	private $userId;
+	private $user;
 
 
 	/**
@@ -70,26 +71,32 @@ class SystemTagsObjectMappingCollection implements ICollection {
 	 *
 	 * @param string $objectId object id
 	 * @param string $objectType object type
-	 * @param string $userId user id
-	 * @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, $userId, $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->userId = $userId;
+		$this->user = $user;
 	}
 
 	function createFile($tagId, $data = null) {
 		try {
 			$tags = $this->tagManager->getTagsByIds([$tagId]);
 			$tag = current($tags);
-			if (!$this->tagManager->canUserSeeTag($tag, $this->userId)) {
+			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->userId)) {
+			if (!$this->tagManager->canUserAssignTag($tag, $this->user)) {
 				throw new Forbidden('No permission to assign tag ' . $tagId);
 			}
 
@@ -108,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->tagManager->canUserSeeTag($tag, $this->userId)) {
+				if ($this->tagManager->canUserSeeTag($tag, $this->user)) {
 					return $this->makeNode($tag);
 				}
 			}
@@ -129,7 +136,7 @@ class SystemTagsObjectMappingCollection implements ICollection {
 
 		// filter out non-visible tags
 		$tags = array_filter($tags, function($tag) {
-			return $this->tagManager->canUserSeeTag($tag, $this->userId);
+			return $this->tagManager->canUserSeeTag($tag, $this->user);
 		});
 
 		return array_values(array_map(function($tag) {
@@ -141,8 +148,12 @@ class SystemTagsObjectMappingCollection implements ICollection {
 		try {
 			$result = ($this->tagMapper->haveTag([$this->objectId], $this->objectType, $tagId, true));
 
-			if ($result && !$this->tagManager->canUserSeeTag($tagId, $this->userId)) {
-				return false;
+			if ($result) {
+				$tags = $this->tagManager->getTagsByIds([$tagId]);
+				$tag = current($tags);
+				if (!$this->tagManager->canUserSeeTag($tag, $this->user)) {
+					return false;
+				}
 			}
 
 			return $result;
@@ -187,7 +198,7 @@ class SystemTagsObjectMappingCollection implements ICollection {
 			$tag,
 			$this->objectId,
 			$this->objectType,
-			$this->userId,
+			$this->user,
 			$this->tagManager,
 			$this->tagMapper
 		);
diff --git a/apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php b/apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php
index 02c9995f7c5..ae4b9d51a1b 100644
--- a/apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php
+++ b/apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php
@@ -95,21 +95,6 @@ class SystemTagsObjectTypeCollection 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
@@ -136,7 +121,7 @@ class SystemTagsObjectTypeCollection implements ICollection {
 		return new SystemTagsObjectMappingCollection(
 			$objectId,
 			$this->objectType,
-			$this->getUserId(),
+			$this->userSession->getUser(),
 			$this->tagManager,
 			$this->tagMapper
 		);
diff --git a/lib/private/SystemTag/ManagerFactory.php b/lib/private/SystemTag/ManagerFactory.php
index e6938e494bc..6b238e3c428 100644
--- a/lib/private/SystemTag/ManagerFactory.php
+++ b/lib/private/SystemTag/ManagerFactory.php
@@ -59,7 +59,6 @@ class ManagerFactory implements ISystemTagManagerFactory {
 	public function getManager() {
 		return new SystemTagManager(
 			$this->serverContainer->getDatabaseConnection(),
-			$this->serverContainer->getUserManager(),
 			$this->serverContainer->getGroupManager(),
 			$this->serverContainer->getEventDispatcher()
 		);
diff --git a/lib/private/SystemTag/SystemTagManager.php b/lib/private/SystemTag/SystemTagManager.php
index 0e4bdad078e..495df674a03 100644
--- a/lib/private/SystemTag/SystemTagManager.php
+++ b/lib/private/SystemTag/SystemTagManager.php
@@ -34,6 +34,7 @@ use OCP\IUserManager;
 use OCP\IGroupManager;
 use OCP\SystemTag\ISystemTag;
 use OCP\UserNotFoundException;
+use OCP\IUser;
 
 /**
  * Manager class for system tags
@@ -48,9 +49,6 @@ class SystemTagManager implements ISystemTagManager {
 	/** @var EventDispatcherInterface */
 	protected $dispatcher;
 
-	/** @var IUserManager */
-	protected $userManager;
-
 	/** @var IGroupManager */
 	protected $groupManager;
 
@@ -69,12 +67,10 @@ class SystemTagManager implements ISystemTagManager {
 	 */
 	public function __construct(
 		IDBConnection $connection,
-		IUserManager $userManager,
 		IGroupManager $groupManager,
 		EventDispatcherInterface $dispatcher
 	) {
 		$this->connection = $connection;
-		$this->userManager = $userManager;
 		$this->groupManager = $groupManager;
 		$this->dispatcher = $dispatcher;
 
@@ -339,23 +335,12 @@ class SystemTagManager implements ISystemTagManager {
 	/**
 	 * {@inheritdoc}
 	 */
-	public function canUserAssignTag($tag, $userId) {
-		if (!$tag instanceof ISystemTag) {
-			$tags = $this->getTagsByIds([$tag]);
-			/** @var ISystemTag $tag */
-			$tag = current($tags);
-		}
-
-		if ($tag->isUserAssignable()) {
+	public function canUserAssignTag(ISystemTag $tag, IUser $user) {
+		if ($tag->isUserAssignable() && $tag->isUserVisible()) {
 			return true;
 		}
 
-		$user = $this->userManager->get($userId);
-		if ($user === null) {
-			throw new UserNotFoundException($userId);
-		}
-
-		if ($this->groupManager->isAdmin($userId)) {
+		if ($this->groupManager->isAdmin($user->getUID())) {
 			return true;
 		}
 
@@ -365,23 +350,12 @@ class SystemTagManager implements ISystemTagManager {
 	/**
 	 * {@inheritdoc}
 	 */
-	public function canUserSeeTag($tag, $userId) {
-		if (!$tag instanceof ISystemTag) {
-			$tags = $this->getTagsByIds([$tag]);
-			/** @var ISystemTag $tag */
-			$tag = current($tags);
-		}
-
+	public function canUserSeeTag(ISystemTag $tag, IUser $user) {
 		if ($tag->isUserVisible()) {
 			return true;
 		}
 
-		$user = $this->userManager->get($userId);
-		if ($user === null) {
-			throw new UserNotFoundException($userId);
-		}
-
-		if ($this->groupManager->isAdmin($userId)) {
+		if ($this->groupManager->isAdmin($user->getUID())) {
 			return true;
 		}
 
diff --git a/lib/public/SystemTag/ISystemTagManager.php b/lib/public/SystemTag/ISystemTagManager.php
index 7fb0c21436c..283ca63e4f6 100644
--- a/lib/public/SystemTag/ISystemTagManager.php
+++ b/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.
  *
@@ -117,33 +120,25 @@ interface ISystemTagManager {
 	 * Checks whether the given user is allowed to assign/unassign the tag with the
 	 * given id.
 	 *
-	 * @param string|\OCP\SystemTag\ISystemTag $tag tag id or system tag
-	 * @param string $userId user 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
 	 *
-	 * @throws \OCP\SystemTag\TagNotFoundException if tag with the given id does not exist
-	 * @throws \OCP\UserNotFoundException if the given user id does not exist
-	 * @throws \InvalidArgumentException if the tag id is invalid (string instead of integer, etc.)
-	 *
 	 * @since 9.1.0
 	 */
-	public function canUserAssignTag($tag, $userId);
+	public function canUserAssignTag(ISystemTag $tag, IUser $user);
 
 	/**
 	 * Checks whether the given user is allowed to see the tag with the given id.
 	 *
-	 * @param string|\OCP\SystemTag\ISystemTag $tag tag id or system tag
-	 * @param string $userId user 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
 	 *
-	 * @throws \OCP\SystemTag\TagNotFoundException if tag with the given id does not exist
-	 * @throws \OCP\UserNotFoundException if the given user id does not exist
-	 * @throws \InvalidArgumentException if the tag id is invalid (string instead of integer, etc.)
-	 *
 	 * @since 9.1.0
 	 */
-	public function canUserSeeTag($tag, $userId);
+	public function canUserSeeTag(ISystemTag $tag, IUser $userId);
 
 }
diff --git a/tests/lib/SystemTag/SystemTagManagerTest.php b/tests/lib/SystemTag/SystemTagManagerTest.php
index 9bd4622c2be..fa13d287ed7 100644
--- a/tests/lib/SystemTag/SystemTagManagerTest.php
+++ b/tests/lib/SystemTag/SystemTagManagerTest.php
@@ -44,11 +44,6 @@ class SystemTagManagerTest extends TestCase {
 	private $groupManager;
 
 	/**
-	 * @var IUserManager
-	 */
-	private $userManager;
-
-	/**
 	 * @var EventDispatcherInterface
 	 */
 	private $dispatcher;
@@ -61,15 +56,10 @@ class SystemTagManagerTest extends TestCase {
 		$this->dispatcher = $this->getMockBuilder('Symfony\Component\EventDispatcher\EventDispatcherInterface')
 			->getMock();
 
-		$this->userManager = $this->getMockBuilder('\OCP\IUserManager')->getMock();
 		$this->groupManager = $this->getMockBuilder('\OCP\IGroupManager')->getMock();
-		$this->groupManager->expects($this->any())
-			->method('isAdmin')
-			->will($this->returnValue(false));
 
 		$this->tagManager = new SystemTagManager(
 			$this->connection,
-			$this->userManager,
 			$this->groupManager,
 			$this->dispatcher
 		);
@@ -443,20 +433,18 @@ class SystemTagManagerTest extends TestCase {
 	 * @dataProvider visibilityCheckProvider
 	 */
 	public function testVisibilityCheck($userVisible, $userAssignable, $isAdmin, $expectedResult) {
-		$userId = 'test';
+		$user = $this->getMockBuilder('\OCP\IUser')->getMock();
+		$user->expects($this->any())
+			->method('getUID')
+			->will($this->returnValue('test'));
 		$tag1 = $this->tagManager->createTag('one', $userVisible, $userAssignable);
 
-		$this->userManager->expects($this->once())
-			->method('get')
-			->with($userId)
-			->will($this->returnValue([]));
-		$this->groupManager->expects($this->once())
+		$this->groupManager->expects($this->any())
 			->method('isAdmin')
-			->with($userId)
+			->with('test')
 			->will($this->returnValue($isAdmin));
 
-		$this->assertEquals($expectedResult, $this->tagManager->canUserSeeTag($tag1, $userID));
-		$this->assertEquals($expectedResult, $this->tagManager->canUserSeeTag($tag1->getId(), $userID));
+		$this->assertEquals($expectedResult, $this->tagManager->canUserSeeTag($tag1, $user));
 	}
 
 	public function assignabilityCheckProvider() {
@@ -475,21 +463,19 @@ class SystemTagManagerTest extends TestCase {
 	/**
 	 * @dataProvider assignabilityCheckProvider
 	 */
-	public function testVisibilityCheck($userVisible, $userAssignable, $isAdmin, $expectedResult) {
-		$userId = 'test';
+	public function testAssignabilityCheck($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->userManager->expects($this->once())
-			->method('get')
-			->with($userId)
-			->will($this->returnValue([]));
-		$this->groupManager->expects($this->once())
+		$this->groupManager->expects($this->any())
 			->method('isAdmin')
-			->with($userId)
+			->with('test')
 			->will($this->returnValue($isAdmin));
 
-		$this->assertEquals($expectedResult, $this->tagManager->canUserAssignTag($tag1, $userID));
-		$this->assertEquals($expectedResult, $this->tagManager->canUserAssignTag($tag1->getId(), $userID));
+		$this->assertEquals($expectedResult, $this->tagManager->canUserAssignTag($tag1, $user));
 	}
 
 	/**