diff --git a/apps/dav/lib/SystemTag/SystemTagPlugin.php b/apps/dav/lib/SystemTag/SystemTagPlugin.php index 00585953b2917..35945037ad49b 100644 --- a/apps/dav/lib/SystemTag/SystemTagPlugin.php +++ b/apps/dav/lib/SystemTag/SystemTagPlugin.php @@ -17,6 +17,7 @@ use OCP\SystemTag\ISystemTagManager; use OCP\SystemTag\ISystemTagObjectMapper; use OCP\SystemTag\TagAlreadyExistsException; +use OCP\SystemTag\TagCreationForbiddenException; use OCP\Util; use Sabre\DAV\Exception\BadRequest; use Sabre\DAV\Exception\Conflict; @@ -187,6 +188,8 @@ private function createTag($data, $contentType = 'application/json') { return $tag; } catch (TagAlreadyExistsException $e) { throw new Conflict('Tag already exists', 0, $e); + } catch (TagCreationForbiddenException $e) { + throw new Forbidden('You don’t have right to create tags', 0, $e); } } @@ -370,7 +373,7 @@ public function handleUpdateProperties($path, PropPatch $propPatch) { if (!($node instanceof SystemTagNode) && !($node instanceof SystemTagObjectType)) { return; } - + $propPatch->handle([self::OBJECTIDS_PROPERTYNAME], function ($props) use ($node) { if (!($node instanceof SystemTagObjectType)) { return false; @@ -388,7 +391,7 @@ public function handleUpdateProperties($path, PropPatch $propPatch) { if (count($objectTypes) !== 1 || $objectTypes[0] !== $node->getName()) { throw new BadRequest('Invalid object-ids property. All object types must be of the same type: ' . $node->getName()); } - + $this->tagMapper->setObjectIdsForTag($node->getSystemTag()->getId(), $node->getName(), array_keys($objects)); } diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 5834bfd47b502..4c0b5f039e876 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -765,6 +765,7 @@ 'OCP\\SystemTag\\MapperEvent' => $baseDir . '/lib/public/SystemTag/MapperEvent.php', 'OCP\\SystemTag\\SystemTagsEntityEvent' => $baseDir . '/lib/public/SystemTag/SystemTagsEntityEvent.php', 'OCP\\SystemTag\\TagAlreadyExistsException' => $baseDir . '/lib/public/SystemTag/TagAlreadyExistsException.php', + 'OCP\\SystemTag\\TagCreationForbiddenException' => $baseDir . '/lib/public/SystemTag/TagCreationForbiddenException.php', 'OCP\\SystemTag\\TagNotFoundException' => $baseDir . '/lib/public/SystemTag/TagNotFoundException.php', 'OCP\\Talk\\Exceptions\\NoBackendException' => $baseDir . '/lib/public/Talk/Exceptions/NoBackendException.php', 'OCP\\Talk\\IBroker' => $baseDir . '/lib/public/Talk/IBroker.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index e510e80d4c6ce..c8779fe83c29f 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -806,6 +806,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OCP\\SystemTag\\MapperEvent' => __DIR__ . '/../../..' . '/lib/public/SystemTag/MapperEvent.php', 'OCP\\SystemTag\\SystemTagsEntityEvent' => __DIR__ . '/../../..' . '/lib/public/SystemTag/SystemTagsEntityEvent.php', 'OCP\\SystemTag\\TagAlreadyExistsException' => __DIR__ . '/../../..' . '/lib/public/SystemTag/TagAlreadyExistsException.php', + 'OCP\\SystemTag\\TagCreationForbiddenException' => __DIR__ . '/../../..' . '/lib/public/SystemTag/TagCreationForbiddenException.php', 'OCP\\SystemTag\\TagNotFoundException' => __DIR__ . '/../../..' . '/lib/public/SystemTag/TagNotFoundException.php', 'OCP\\Talk\\Exceptions\\NoBackendException' => __DIR__ . '/../../..' . '/lib/public/Talk/Exceptions/NoBackendException.php', 'OCP\\Talk\\IBroker' => __DIR__ . '/../../..' . '/lib/public/Talk/IBroker.php', diff --git a/lib/private/SystemTag/ManagerFactory.php b/lib/private/SystemTag/ManagerFactory.php index 0fd2ca5fd36f9..4d3a139652935 100644 --- a/lib/private/SystemTag/ManagerFactory.php +++ b/lib/private/SystemTag/ManagerFactory.php @@ -9,7 +9,11 @@ namespace OC\SystemTag; use OCP\EventDispatcher\IEventDispatcher; +use OCP\IAppConfig; +use OCP\IDBConnection; +use OCP\IGroupManager; use OCP\IServerContainer; +use OCP\IUserSession; use OCP\SystemTag\ISystemTagManager; use OCP\SystemTag\ISystemTagManagerFactory; use OCP\SystemTag\ISystemTagObjectMapper; @@ -36,9 +40,11 @@ public function __construct( */ public function getManager(): ISystemTagManager { return new SystemTagManager( - $this->serverContainer->getDatabaseConnection(), - $this->serverContainer->getGroupManager(), + $this->serverContainer->get(IDBConnection::class), + $this->serverContainer->get(IGroupManager::class), $this->serverContainer->get(IEventDispatcher::class), + $this->serverContainer->get(IUserSession::class), + $this->serverContainer->get(IAppConfig::class), ); } @@ -50,7 +56,7 @@ public function getManager(): ISystemTagManager { */ public function getObjectMapper(): ISystemTagObjectMapper { return new SystemTagObjectMapper( - $this->serverContainer->getDatabaseConnection(), + $this->serverContainer->get(IDBConnection::class), $this->getManager(), $this->serverContainer->get(IEventDispatcher::class), ); diff --git a/lib/private/SystemTag/SystemTagManager.php b/lib/private/SystemTag/SystemTagManager.php index 70bb8e6e70b25..efd5024ffc39b 100644 --- a/lib/private/SystemTag/SystemTagManager.php +++ b/lib/private/SystemTag/SystemTagManager.php @@ -11,13 +11,16 @@ use Doctrine\DBAL\Exception\UniqueConstraintViolationException; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\EventDispatcher\IEventDispatcher; +use OCP\IAppConfig; use OCP\IDBConnection; use OCP\IGroupManager; use OCP\IUser; +use OCP\IUserSession; use OCP\SystemTag\ISystemTag; use OCP\SystemTag\ISystemTagManager; use OCP\SystemTag\ManagerEvent; use OCP\SystemTag\TagAlreadyExistsException; +use OCP\SystemTag\TagCreationForbiddenException; use OCP\SystemTag\TagNotFoundException; /** @@ -36,6 +39,8 @@ public function __construct( protected IDBConnection $connection, protected IGroupManager $groupManager, protected IEventDispatcher $dispatcher, + private IUserSession $userSession, + private IAppConfig $appConfig, ) { $query = $this->connection->getQueryBuilder(); $this->selectTagQuery = $query->select('*') @@ -157,6 +162,10 @@ public function getTag(string $tagName, bool $userVisible, bool $userAssignable) * {@inheritdoc} */ public function createTag(string $tagName, bool $userVisible, bool $userAssignable): ISystemTag { + $user = $this->userSession->getUser(); + if (!$this->canUserCreateTag($user)) { + throw new TagCreationForbiddenException('Tag creation forbidden'); + } // Length of name column is 64 $truncatedTagName = substr($tagName, 0, 64); $query = $this->connection->getQueryBuilder(); @@ -335,6 +344,19 @@ public function canUserAssignTag(ISystemTag $tag, ?IUser $user): bool { return false; } + public function canUserCreateTag(?IUser $user): bool { + if ($user === null) { + // If no user given, allows only calls from CLI + return \OC::$CLI; + } + + if ($this->appConfig->getValueBool('systemtags', 'only_admins_can_create', false) === false) { + return true; + } + + return $this->groupManager->isAdmin($user->getUID()); + } + /** * {@inheritdoc} */ diff --git a/lib/public/SystemTag/ISystemTagManager.php b/lib/public/SystemTag/ISystemTagManager.php index 1c08d3b22e144..5212e264cd823 100644 --- a/lib/public/SystemTag/ISystemTagManager.php +++ b/lib/public/SystemTag/ISystemTagManager.php @@ -57,8 +57,10 @@ public function getTag(string $tagName, bool $userVisible, bool $userAssignable) * @return ISystemTag system tag * * @throws TagAlreadyExistsException if tag already exists + * @throws TagCreationForbiddenException if user doesn’t have the right to create a new tag * * @since 9.0.0 + * @since 31.0.0 Can throw TagCreationForbiddenExceptionif user doesn’t have the right to create a new tag */ public function createTag(string $tagName, bool $userVisible, bool $userAssignable): ISystemTag; @@ -115,6 +117,16 @@ public function deleteTags($tagIds); */ public function canUserAssignTag(ISystemTag $tag, ?IUser $user): bool; + /** + * Checks whether the given user is allowed to create new tags + * + * @param IUser|null $user user to check permission for + * @return bool true if the user is allowed to create a new tag, false otherwise + * + * @since 31.0.0 + */ + public function canUserCreateTag(?IUser $user): bool; + /** * Checks whether the given user is allowed to see the tag with the given id. * diff --git a/lib/public/SystemTag/TagCreationForbiddenException.php b/lib/public/SystemTag/TagCreationForbiddenException.php new file mode 100644 index 0000000000000..f7c70de9f75dd --- /dev/null +++ b/lib/public/SystemTag/TagCreationForbiddenException.php @@ -0,0 +1,18 @@ +dispatcher = $this->createMock(IEventDispatcher::class); $this->groupManager = $this->createMock(IGroupManager::class); + $this->userSession = $this->createMock(IUserSession::class); + $this->appConfig = $this->createMock(IAppConfig::class); $this->tagManager = new SystemTagManager( $this->connection, $this->groupManager, - $this->dispatcher + $this->dispatcher, + $this->userSession, + $this->appConfig, ); $this->pruneTagsTables(); } @@ -527,6 +543,84 @@ public function testEmptyTagGroup(): void { $this->assertEquals([], $this->tagManager->getTagGroups($tag1)); } + private function allowedToCreateProvider(): array { + return [ + [true, null, true], + [true, null, false], + [false, true, true], + [false, true, false], + [false, false, false], + ]; + } + + /** + * @dataProvider allowedToCreateProvider + */ + public function testAllowedToCreateTag(bool $isCli, ?bool $isAdmin, bool $isRestricted): void { + $oldCli = \OC::$CLI; + \OC::$CLI = $isCli; + + $user = $this->getMockBuilder(IUser::class)->getMock(); + $user->expects($this->any()) + ->method('getUID') + ->willReturn('test'); + $this->userSession->expects($this->any()) + ->method('getUser') + ->willReturn($isAdmin === null ? null : $user); + $this->groupManager->expects($this->any()) + ->method('isAdmin') + ->with('test') + ->willReturn($isAdmin); + $this->appConfig->expects($this->any()) + ->method('getValueBool') + ->with('systemtags', 'only_admins_can_create') + ->willReturn($isRestricted); + + $name = uniqid('tag_', true); + $tag = $this->tagManager->createTag($name, true, true); + $this->assertEquals($tag->getName(), $name); + $this->tagManager->deleteTags($tag->getId()); + + \OC::$CLI = $oldCli; + } + + private function disallowedToCreateProvider(): array { + return [ + [false], + [null], + ]; + } + + /** + * @dataProvider disallowedToCreateProvider + */ + public function testDisallowedToCreateTag(?bool $isAdmin): void { + $oldCli = \OC::$CLI; + \OC::$CLI = false; + + $user = $this->getMockBuilder(IUser::class)->getMock(); + $user->expects($this->any()) + ->method('getUID') + ->willReturn('test'); + $this->userSession->expects($this->any()) + ->method('getUser') + ->willReturn($isAdmin === null ? null : $user); + $this->groupManager->expects($this->any()) + ->method('isAdmin') + ->with('test') + ->willReturn($isAdmin); + $this->appConfig->expects($this->any()) + ->method('getValueBool') + ->with('systemtags', 'only_admins_can_create') + ->willReturn(true); + + $this->expectException(\Exception::class); + $tag = $this->tagManager->createTag(uniqid('tag_', true), true, true); + + \OC::$CLI = $oldCli; + } + + /** * @param ISystemTag $tag1 * @param ISystemTag $tag2