aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorVincent Petry <pvince81@owncloud.com>2016-05-21 19:06:54 +0200
committerVincent Petry <pvince81@owncloud.com>2016-05-21 19:06:54 +0200
commit693484008360b219848d4fbcdce9b7f26987850b (patch)
treebe128b41a2717b4fe9814c08f40d63d7913dca92
parentbca7586574dc3daf6a890688fdf7634f6fa58bab (diff)
parentb40c0bad9685873bbcd61f310717c0da0c9faf75 (diff)
downloadnextcloud-server-693484008360b219848d4fbcdce9b7f26987850b.tar.gz
nextcloud-server-693484008360b219848d4fbcdce9b7f26987850b.zip
Merge pull request #24307 from owncloud/systemtags-perminterface
Add interface methods for permission check for system tags
-rw-r--r--apps/dav/lib/SystemTag/SystemTagMappingNode.php83
-rw-r--r--apps/dav/lib/SystemTag/SystemTagNode.php43
-rw-r--r--apps/dav/lib/SystemTag/SystemTagPlugin.php68
-rw-r--r--apps/dav/lib/SystemTag/SystemTagsByIdCollection.php9
-rw-r--r--apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php73
-rw-r--r--apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php13
-rw-r--r--apps/dav/tests/unit/systemtag/systemtagmappingnode.php58
-rw-r--r--apps/dav/tests/unit/systemtag/systemtagnode.php82
-rw-r--r--apps/dav/tests/unit/systemtag/systemtagplugin.php350
-rw-r--r--apps/dav/tests/unit/systemtag/systemtagsbyidcollection.php57
-rw-r--r--apps/dav/tests/unit/systemtag/systemtagsobjectmappingcollection.php165
-rw-r--r--apps/systemtags/js/systemtagsinfoview.js2
-rw-r--r--apps/systemtags/lib/Activity/Extension.php2
-rw-r--r--apps/systemtags/tests/js/systemtagsinfoviewSpec.js43
-rw-r--r--build/integration/features/bootstrap/TagsContext.php192
-rw-r--r--build/integration/features/tags.feature57
-rw-r--r--build/jsdocs9.tar.bz2bin0 -> 575999 bytes
-rw-r--r--core/js/systemtags/systemtagmodel.js10
-rw-r--r--core/js/systemtags/systemtags.js2
-rw-r--r--core/js/systemtags/systemtagsinputfield.js8
-rw-r--r--core/js/tests/specs/systemtags/systemtagsSpec.js2
-rw-r--r--core/js/tests/specs/systemtags/systemtagsinputfieldSpec.js71
-rw-r--r--core/shipped.json1
-rw-r--r--db_structure.xml43
-rw-r--r--lib/private/SystemTag/ManagerFactory.php1
-rw-r--r--lib/private/SystemTag/SystemTagManager.php113
-rw-r--r--lib/public/SystemTag/ISystemTagManager.php48
-rw-r--r--settings/js/settings.js15
-rw-r--r--tests/lib/SystemTag/SystemTagManagerTest.php106
-rw-r--r--version.php2
30 files changed, 1305 insertions, 414 deletions
diff --git a/apps/dav/lib/SystemTag/SystemTagMappingNode.php b/apps/dav/lib/SystemTag/SystemTagMappingNode.php
index bb2936c13dc..09a3c79c757 100644
--- a/apps/dav/lib/SystemTag/SystemTagMappingNode.php
+++ b/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
@@ -51,12 +52,29 @@ 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);
}
/**
@@ -93,17 +113,52 @@ class SystemTagMappingNode extends SystemTagNode {
}
/**
+ * 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) {
diff --git a/apps/dav/lib/SystemTag/SystemTagNode.php b/apps/dav/lib/SystemTag/SystemTagNode.php
index 500e1a3adea..f139a3335c8 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,6 +50,13 @@ class SystemTagNode implements \Sabre\DAV\INode {
protected $tagManager;
/**
+ * User
+ *
+ * @var IUser
+ */
+ protected $user;
+
+ /**
* Whether to allow permissions for admins
*
* @var bool
@@ -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
diff --git a/apps/dav/lib/SystemTag/SystemTagPlugin.php b/apps/dav/lib/SystemTag/SystemTagPlugin.php
index a266b4a1214..af683954b87 100644
--- a/apps/dav/lib/SystemTag/SystemTagPlugin.php
+++ b/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;
});
+
}
}
diff --git a/apps/dav/lib/SystemTag/SystemTagsByIdCollection.php b/apps/dav/lib/SystemTag/SystemTagsByIdCollection.php
index 298902501ab..2b24bce9f35 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();
@@ -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);
}
}
diff --git a/apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php b/apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php
index eb75ed06393..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;
/**
- * 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;
- }
- // verify if user is allowed to see this tag
- $tag = $this->tagManager->getTagsByIds($tagId);
- $tag = current($tag);
- if (!$tag->isUserVisible()) {
- return false;
+ if ($result) {
+ $tags = $this->tagManager->getTagsByIds([$tagId]);
+ $tag = current($tags);
+ if (!$this->tagManager->canUserSeeTag($tag, $this->user)) {
+ 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
);
diff --git a/apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php b/apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php
index bdbc73c4e32..ae4b9d51a1b 100644
--- a/apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php
+++ b/apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php
@@ -95,17 +95,6 @@ class SystemTagsObjectTypeCollection implements ICollection {
}
/**
- * 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
* @throws Forbidden
@@ -132,7 +121,7 @@ class SystemTagsObjectTypeCollection implements ICollection {
return new SystemTagsObjectMappingCollection(
$objectId,
$this->objectType,
- $this->isAdmin(),
+ $this->userSession->getUser(),
$this->tagManager,
$this->tagMapper
);
diff --git a/apps/dav/tests/unit/systemtag/systemtagmappingnode.php b/apps/dav/tests/unit/systemtag/systemtagmappingnode.php
index 7f2ff7d6616..f0e1c3bc567 100644
--- a/apps/dav/tests/unit/systemtag/systemtagmappingnode.php
+++ b/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();
}
}
diff --git a/apps/dav/tests/unit/systemtag/systemtagnode.php b/apps/dav/tests/unit/systemtag/systemtagnode.php
index 5184b74e5c8..d9e088a7d90 100644
--- a/apps/dav/tests/unit/systemtag/systemtagnode.php
+++ b/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
);
@@ -102,6 +110,14 @@ class SystemTagNode extends \Test\TestCase {
*/
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]);
$this->getTagNode($isAdmin, $originalTag)
@@ -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();
}
}
diff --git a/apps/dav/tests/unit/systemtag/systemtagplugin.php b/apps/dav/tests/unit/systemtag/systemtagplugin.php
index 4466533f1e0..da82bc8904a 100644
--- a/apps/dav/tests/unit/systemtag/systemtagplugin.php
+++ b/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
@@ -61,6 +65,11 @@ class SystemTagPlugin extends \Test\TestCase {
private $userSession;
/**
+ * @var IUser
+ */
+ private $user;
+
+ /**
* @var \OCA\DAV\SystemTag\SystemTagPlugin
*/
private $plugin;
@@ -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,84 @@ class SystemTagPlugin extends \Test\TestCase {
$this->plugin->initialize($this->server);
}
- public function testGetProperties() {
- $systemTag = new SystemTag(1, 'Test', true, true);
- $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',
- ]
+ 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()
@@ -109,6 +189,14 @@ class SystemTagPlugin extends \Test\TestCase {
->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')
@@ -128,12 +216,62 @@ class SystemTagPlugin extends \Test\TestCase {
$result = $propFind->getResultForMultiStatus();
$this->assertEmpty($result[404]);
- unset($result[404]);
- $this->assertEquals($expectedProperties, $result);
+ $this->assertEquals($expectedProperties, $result[200]);
}
- public function testUpdateProperties() {
+ /**
+ * @expectedException \Sabre\DAV\Exception\Forbidden
+ */
+ public function testGetPropertiesForbidden() {
$systemTag = new SystemTag(1, 'Test', true, false);
+ $requestedProperties = [
+ self::ID_PROPERTYNAME,
+ 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()
+ ->getMock();
+ $node->expects($this->any())
+ ->method('getSystemTag')
+ ->will($this->returnValue($systemTag));
+
+ $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
+ );
+ }
+
+ 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')
diff --git a/apps/dav/tests/unit/systemtag/systemtagsbyidcollection.php b/apps/dav/tests/unit/systemtag/systemtagsbyidcollection.php
index a2bf571ab68..5aa28d1a254 100644
--- a/apps/dav/tests/unit/systemtag/systemtagsbyidcollection.php
+++ b/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() {
diff --git a/apps/dav/tests/unit/systemtag/systemtagsobjectmappingcollection.php b/apps/dav/tests/unit/systemtag/systemtagsobjectmappingcollection.php
index df97acd846b..9adc5b88c41 100644
--- a/apps/dav/tests/unit/systemtag/systemtagsobjectmappingcollection.php
+++ b/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->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->tagManager->expects($this->exactly(3))
+ ->method('canUserSeeTag')
+ ->will($this->returnCallback(function($tag) {
+ return $tag->isUserVisible();
+ }));
- $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())
@@ -308,14 +270,19 @@ class SystemTagsObjectMappingCollection extends \Test\TestCase {
->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() {
diff --git a/apps/systemtags/js/systemtagsinfoview.js b/apps/systemtags/js/systemtagsinfoview.js
index 2f01ca7db34..a7320a3956f 100644
--- a/apps/systemtags/js/systemtagsinfoview.js
+++ b/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;
diff --git a/apps/systemtags/lib/Activity/Extension.php b/apps/systemtags/lib/Activity/Extension.php
index 8c101a6f550..6bb83e37cfc 100644
--- a/apps/systemtags/lib/Activity/Extension.php
+++ b/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>';
}
diff --git a/apps/systemtags/tests/js/systemtagsinfoviewSpec.js b/apps/systemtags/tests/js/systemtagsinfoviewSpec.js
index 27724822c2e..449dfd859d7 100644
--- a/apps/systemtags/tests/js/systemtagsinfoviewSpec.js
+++ b/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();
diff --git a/build/integration/features/bootstrap/TagsContext.php b/build/integration/features/bootstrap/TagsContext.php
index 10d0b9ae545..882f5d9e13a 100644
--- a/build/integration/features/bootstrap/TagsContext.php
+++ b/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,15 +138,38 @@ 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
* @throws \Exception
@@ -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;
@@ -239,6 +284,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
* @param string $user
@@ -251,6 +332,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
*/
@@ -305,6 +425,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
* @param string $name
diff --git a/build/integration/features/tags.feature b/build/integration/features/tags.feature
index 286fb62bf42..d793c0d3c61 100644
--- a/build/integration/features/tags.feature
+++ b/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"
+
diff --git a/build/jsdocs9.tar.bz2 b/build/jsdocs9.tar.bz2
new file mode 100644
index 00000000000..dfab675733d
--- /dev/null
+++ b/build/jsdocs9.tar.bz2
Binary files differ
diff --git a/core/js/systemtags/systemtagmodel.js b/core/js/systemtags/systemtagmodel.js
index b41fbdde61e..89728357e25 100644
--- a/core/js/systemtags/systemtagmodel.js
+++ b/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'
};
}
});
diff --git a/core/js/systemtags/systemtags.js b/core/js/systemtags/systemtags.js
index 042f49bb8ed..05ead6f3dcd 100644
--- a/core/js/systemtags/systemtags.js
+++ b/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
diff --git a/core/js/systemtags/systemtagsinputfield.js b/core/js/systemtags/systemtagsinputfield.js
index 45dc5b7b03e..2eb8d0a44cb 100644
--- a/core/js/systemtags/systemtagsinputfield.js
+++ b/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;
}
diff --git a/core/js/tests/specs/systemtags/systemtagsSpec.js b/core/js/tests/specs/systemtags/systemtagsSpec.js
index 515b75258a0..f6d99e62a3c 100644
--- a/core/js/tests/specs/systemtags/systemtagsSpec.js
+++ b/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)');
});
});
diff --git a/core/js/tests/specs/systemtags/systemtagsinputfieldSpec.js b/core/js/tests/specs/systemtags/systemtagsinputfieldSpec.js
index 22bf0d2c82a..503bef7cf2b 100644
--- a/core/js/tests/specs/systemtags/systemtagsinputfieldSpec.js
+++ b/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
}
]);
});
diff --git a/core/shipped.json b/core/shipped.json
index e33269dad75..a3abe22d8d5 100644
--- a/core/shipped.json
+++ b/core/shipped.json
@@ -30,6 +30,7 @@
"provisioning_api",
"sharepoint",
"systemtags",
+ "systemtags_management",
"templateeditor",
"updatenotification",
"user_external",
diff --git a/db_structure.xml b/db_structure.xml
index 6e57b003fcf..640edfbfd80 100644
--- a/db_structure.xml
+++ b/db_structure.xml
@@ -1383,6 +1383,49 @@
<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>
+
+ <!--
Namespaced Key-Value Store for arbitrary data.
- Keys are namespaced per userid and appid.
- E.g. (admin, files, foo) -> bar
diff --git a/lib/private/SystemTag/ManagerFactory.php b/lib/private/SystemTag/ManagerFactory.php
index d9acf327f8a..6b238e3c428 100644
--- a/lib/private/SystemTag/ManagerFactory.php
+++ b/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()
);
}
diff --git a/lib/private/SystemTag/SystemTagManager.php b/lib/private/SystemTag/SystemTagManager.php
index 76a60a91328..2b0ef03e471 100644
--- a/lib/private/SystemTag/SystemTagManager.php
+++ b/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;
+ }
}
diff --git a/lib/public/SystemTag/ISystemTagManager.php b/lib/public/SystemTag/ISystemTagManager.php
index 983bfd636ce..35447b05fdc 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.
*
@@ -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);
}
diff --git a/settings/js/settings.js b/settings/js/settings.js
index fcbe328b76f..5a2ba4bcec7 100644
--- a/settings/js/settings.js
+++ b/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});
});
diff --git a/tests/lib/SystemTag/SystemTagManagerTest.php b/tests/lib/SystemTag/SystemTagManagerTest.php
index 1afb147f08a..e697e373465 100644
--- a/tests/lib/SystemTag/SystemTagManagerTest.php
+++ b/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
@@ -37,6 +39,11 @@ class SystemTagManagerTest extends TestCase {
private $connection;
/**
+ * @var IGroupManager
+ */
+ private $groupManager;
+
+ /**
* @var EventDispatcherInterface
*/
private $dispatcher;
@@ -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
diff --git a/version.php b/version.php
index d9e1ca1df1c..a4f1c4dbce0 100644
--- a/version.php
+++ b/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';