Updated canUser* functions in SystemTagManager to accept objects

This commit is contained in:
Vincent Petry 2016-04-27 15:24:28 +02:00
parent 8343cfb64b
commit 09b3883d9c
No known key found for this signature in database
GPG Key ID: AF8F9EFC56562186
9 changed files with 77 additions and 140 deletions

View File

@ -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());

View File

@ -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());
}

View File

@ -81,21 +81,6 @@ class SystemTagsByIdCollection implements ICollection {
return false;
}
/**
* 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
@ -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);
}
}

View File

@ -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
);

View File

@ -94,21 +94,6 @@ class SystemTagsObjectTypeCollection implements ICollection {
$this->fileRoot = $fileRoot;
}
/**
* 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
@ -136,7 +121,7 @@ class SystemTagsObjectTypeCollection implements ICollection {
return new SystemTagsObjectMappingCollection(
$objectId,
$this->objectType,
$this->getUserId(),
$this->userSession->getUser(),
$this->tagManager,
$this->tagMapper
);

View File

@ -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()
);

View File

@ -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;
}

View File

@ -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);
}

View File

@ -43,11 +43,6 @@ class SystemTagManagerTest extends TestCase {
*/
private $groupManager;
/**
* @var IUserManager
*/
private $userManager;
/**
* @var EventDispatcherInterface
*/
@ -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));
}
/**