Skip to content

Commit d561ba1

Browse files
AltahrimAltahrim
committed
feat(systemtags): add setting to block non admin to create system tags
Signed-off-by: Benjamin Gaussorgues <benjamin.gaussorgues@nextcloud.com>
1 parent 21666e0 commit d561ba1

File tree

8 files changed

+163
-6
lines changed

8 files changed

+163
-6
lines changed

‎apps/dav/lib/SystemTag/SystemTagPlugin.php‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use OCP\SystemTag\ISystemTagManager;
1818
use OCP\SystemTag\ISystemTagObjectMapper;
1919
use OCP\SystemTag\TagAlreadyExistsException;
20+
use OCP\SystemTag\TagCreationForbiddenException;
2021
use OCP\Util;
2122
use Sabre\DAV\Exception\BadRequest;
2223
use Sabre\DAV\Exception\Conflict;
@@ -187,6 +188,8 @@ private function createTag($data, $contentType = 'application/json') {
187188
return $tag;
188189
} catch (TagAlreadyExistsException $e) {
189190
throw new Conflict('Tag already exists', 0, $e);
191+
} catch (TagCreationForbiddenException $e) {
192+
throw new Forbidden('You don’t have right to create tags', 0, $e);
190193
}
191194
}
192195

@@ -370,7 +373,7 @@ public function handleUpdateProperties($path, PropPatch $propPatch) {
370373
if (!($node instanceof SystemTagNode) && !($node instanceof SystemTagObjectType)) {
371374
return;
372375
}
373-
376+
374377
$propPatch->handle([self::OBJECTIDS_PROPERTYNAME], function ($props) use ($node) {
375378
if (!($node instanceof SystemTagObjectType)) {
376379
return false;
@@ -388,7 +391,7 @@ public function handleUpdateProperties($path, PropPatch $propPatch) {
388391
if (count($objectTypes) !== 1 || $objectTypes[0] !== $node->getName()) {
389392
throw new BadRequest('Invalid object-ids property. All object types must be of the same type: ' . $node->getName());
390393
}
391-
394+
392395
$this->tagMapper->setObjectIdsForTag($node->getSystemTag()->getId(), $node->getName(), array_keys($objects));
393396
}
394397

‎lib/composer/composer/autoload_classmap.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -765,6 +765,7 @@
765765
'OCP\\SystemTag\\MapperEvent' => $baseDir . '/lib/public/SystemTag/MapperEvent.php',
766766
'OCP\\SystemTag\\SystemTagsEntityEvent' => $baseDir . '/lib/public/SystemTag/SystemTagsEntityEvent.php',
767767
'OCP\\SystemTag\\TagAlreadyExistsException' => $baseDir . '/lib/public/SystemTag/TagAlreadyExistsException.php',
768+
'OCP\\SystemTag\\TagCreationForbiddenException' => $baseDir . '/lib/public/SystemTag/TagCreationForbiddenException.php',
768769
'OCP\\SystemTag\\TagNotFoundException' => $baseDir . '/lib/public/SystemTag/TagNotFoundException.php',
769770
'OCP\\Talk\\Exceptions\\NoBackendException' => $baseDir . '/lib/public/Talk/Exceptions/NoBackendException.php',
770771
'OCP\\Talk\\IBroker' => $baseDir . '/lib/public/Talk/IBroker.php',

‎lib/composer/composer/autoload_static.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -806,6 +806,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
806806
'OCP\\SystemTag\\MapperEvent' => __DIR__ . '/../../..' . '/lib/public/SystemTag/MapperEvent.php',
807807
'OCP\\SystemTag\\SystemTagsEntityEvent' => __DIR__ . '/../../..' . '/lib/public/SystemTag/SystemTagsEntityEvent.php',
808808
'OCP\\SystemTag\\TagAlreadyExistsException' => __DIR__ . '/../../..' . '/lib/public/SystemTag/TagAlreadyExistsException.php',
809+
'OCP\\SystemTag\\TagCreationForbiddenException' => __DIR__ . '/../../..' . '/lib/public/SystemTag/TagCreationForbiddenException.php',
809810
'OCP\\SystemTag\\TagNotFoundException' => __DIR__ . '/../../..' . '/lib/public/SystemTag/TagNotFoundException.php',
810811
'OCP\\Talk\\Exceptions\\NoBackendException' => __DIR__ . '/../../..' . '/lib/public/Talk/Exceptions/NoBackendException.php',
811812
'OCP\\Talk\\IBroker' => __DIR__ . '/../../..' . '/lib/public/Talk/IBroker.php',

‎lib/private/SystemTag/ManagerFactory.php‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,11 @@
99
namespace OC\SystemTag;
1010

1111
use OCP\EventDispatcher\IEventDispatcher;
12+
use OCP\IAppConfig;
13+
use OCP\IDBConnection;
14+
use OCP\IGroupManager;
1215
use OCP\IServerContainer;
16+
use OCP\IUserSession;
1317
use OCP\SystemTag\ISystemTagManager;
1418
use OCP\SystemTag\ISystemTagManagerFactory;
1519
use OCP\SystemTag\ISystemTagObjectMapper;
@@ -36,9 +40,11 @@ public function __construct(
3640
*/
3741
public function getManager(): ISystemTagManager {
3842
return new SystemTagManager(
39-
$this->serverContainer->getDatabaseConnection(),
40-
$this->serverContainer->getGroupManager(),
43+
$this->serverContainer->get(IDBConnection::class),
44+
$this->serverContainer->get(IGroupManager::class),
4145
$this->serverContainer->get(IEventDispatcher::class),
46+
$this->serverContainer->get(IUserSession::class),
47+
$this->serverContainer->get(IAppConfig::class),
4248
);
4349
}
4450

@@ -50,7 +56,7 @@ public function getManager(): ISystemTagManager {
5056
*/
5157
public function getObjectMapper(): ISystemTagObjectMapper {
5258
return new SystemTagObjectMapper(
53-
$this->serverContainer->getDatabaseConnection(),
59+
$this->serverContainer->get(IDBConnection::class),
5460
$this->getManager(),
5561
$this->serverContainer->get(IEventDispatcher::class),
5662
);

‎lib/private/SystemTag/SystemTagManager.php‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,16 @@
1111
use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
1212
use OCP\DB\QueryBuilder\IQueryBuilder;
1313
use OCP\EventDispatcher\IEventDispatcher;
14+
use OCP\IAppConfig;
1415
use OCP\IDBConnection;
1516
use OCP\IGroupManager;
1617
use OCP\IUser;
18+
use OCP\IUserSession;
1719
use OCP\SystemTag\ISystemTag;
1820
use OCP\SystemTag\ISystemTagManager;
1921
use OCP\SystemTag\ManagerEvent;
2022
use OCP\SystemTag\TagAlreadyExistsException;
23+
use OCP\SystemTag\TagCreationForbiddenException;
2124
use OCP\SystemTag\TagNotFoundException;
2225

2326
/**
@@ -36,6 +39,8 @@ public function __construct(
3639
protected IDBConnection $connection,
3740
protected IGroupManager $groupManager,
3841
protected IEventDispatcher $dispatcher,
42+
private IUserSession $userSession,
43+
private IAppConfig $appConfig,
3944
) {
4045
$query = $this->connection->getQueryBuilder();
4146
$this->selectTagQuery = $query->select('*')
@@ -157,6 +162,10 @@ public function getTag(string $tagName, bool $userVisible, bool $userAssignable)
157162
* {@inheritdoc}
158163
*/
159164
public function createTag(string $tagName, bool $userVisible, bool $userAssignable): ISystemTag {
165+
$user = $this->userSession->getUser();
166+
if (!$this->canUserCreateTag($user)) {
167+
throw new TagCreationForbiddenException('Tag creation forbidden');
168+
}
160169
// Length of name column is 64
161170
$truncatedTagName = substr($tagName, 0, 64);
162171
$query = $this->connection->getQueryBuilder();
@@ -335,6 +344,19 @@ public function canUserAssignTag(ISystemTag $tag, ?IUser $user): bool {
335344
return false;
336345
}
337346

347+
public function canUserCreateTag(?IUser $user): bool {
348+
if ($user === null) {
349+
// If no user given, allows only calls from CLI
350+
return \OC::$CLI;
351+
}
352+
353+
if ($this->appConfig->getValueBool('systemtags', 'only_admins_can_create', false) === false) {
354+
return true;
355+
}
356+
357+
return $this->groupManager->isAdmin($user->getUID());
358+
}
359+
338360
/**
339361
* {@inheritdoc}
340362
*/

‎lib/public/SystemTag/ISystemTagManager.php‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,10 @@ public function getTag(string $tagName, bool $userVisible, bool $userAssignable)
5757
* @return ISystemTag system tag
5858
*
5959
* @throws TagAlreadyExistsException if tag already exists
60+
* @throws TagCreationForbiddenException if user doesn’t have the right to create a new tag
6061
*
6162
* @since 9.0.0
63+
* @since 31.0.0 Can throw TagCreationForbiddenExceptionif user doesn’t have the right to create a new tag
6264
*/
6365
public function createTag(string $tagName, bool $userVisible, bool $userAssignable): ISystemTag;
6466

@@ -115,6 +117,16 @@ public function deleteTags($tagIds);
115117
*/
116118
public function canUserAssignTag(ISystemTag $tag, ?IUser $user): bool;
117119

120+
/**
121+
* Checks whether the given user is allowed to create new tags
122+
*
123+
* @param IUser|null $user user to check permission for
124+
* @return bool true if the user is allowed to create a new tag, false otherwise
125+
*
126+
* @since 31.0.0
127+
*/
128+
public function canUserCreateTag(?IUser $user): bool;
129+
118130
/**
119131
* Checks whether the given user is allowed to see the tag with the given id.
120132
*

‎lib/public/SystemTag/TagCreationForbiddenException.php‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-only
8+
*/
9+
10+
namespace OCP\SystemTag;
11+
12+
/**
13+
* Exception when a user don't have the rigth to create a tag
14+
*
15+
* @since 31.0.0
16+
*/
17+
class TagCreationForbiddenException extends \RuntimeException {
18+
}

‎tests/lib/SystemTag/SystemTagManagerTest.php‎

Lines changed: 95 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,11 @@
1111
use OC\SystemTag\SystemTagManager;
1212
use OC\SystemTag\SystemTagObjectMapper;
1313
use OCP\EventDispatcher\IEventDispatcher;
14+
use OCP\IAppConfig;
1415
use OCP\IDBConnection;
1516
use OCP\IGroupManager;
1617
use OCP\IUser;
18+
use OCP\IUserSession;
1719
use OCP\SystemTag\ISystemTag;
1820
use OCP\SystemTag\ISystemTagManager;
1921
use Test\TestCase;
@@ -40,6 +42,16 @@ class SystemTagManagerTest extends TestCase {
4042
*/
4143
private $groupManager;
4244

45+
/**
46+
* @var IUserSession
47+
*/
48+
private $userSession;
49+
50+
/**
51+
* @var IAppConfig
52+
*/
53+
private $appConfig;
54+
4355
/**
4456
* @var IEventDispatcher
4557
*/
@@ -52,11 +64,15 @@ protected function setUp(): void {
5264

5365
$this->dispatcher = $this->createMock(IEventDispatcher::class);
5466
$this->groupManager = $this->createMock(IGroupManager::class);
67+
$this->userSession = $this->createMock(IUserSession::class);
68+
$this->appConfig = $this->createMock(IAppConfig::class);
5569

5670
$this->tagManager = new SystemTagManager(
5771
$this->connection,
5872
$this->groupManager,
59-
$this->dispatcher
73+
$this->dispatcher,
74+
$this->userSession,
75+
$this->appConfig,
6076
);
6177
$this->pruneTagsTables();
6278
}
@@ -527,6 +543,84 @@ public function testEmptyTagGroup(): void {
527543
$this->assertEquals([], $this->tagManager->getTagGroups($tag1));
528544
}
529545

546+
private function allowedToCreateProvider(): array {
547+
return [
548+
[true, null, true],
549+
[true, null, false],
550+
[false, true, true],
551+
[false, true, false],
552+
[false, false, false],
553+
];
554+
}
555+
556+
/**
557+
* @dataProvider allowedToCreateProvider
558+
*/
559+
public function testAllowedToCreateTag(bool $isCli, ?bool $isAdmin, bool $isRestricted): void {
560+
$oldCli = \OC::$CLI;
561+
\OC::$CLI = $isCli;
562+
563+
$user = $this->getMockBuilder(IUser::class)->getMock();
564+
$user->expects($this->any())
565+
->method('getUID')
566+
->willReturn('test');
567+
$this->userSession->expects($this->any())
568+
->method('getUser')
569+
->willReturn($isAdmin === null ? null : $user);
570+
$this->groupManager->expects($this->any())
571+
->method('isAdmin')
572+
->with('test')
573+
->willReturn($isAdmin);
574+
$this->appConfig->expects($this->any())
575+
->method('getValueBool')
576+
->with('systemtags', 'only_admins_can_create')
577+
->willReturn($isRestricted);
578+
579+
$name = uniqid('tag_', true);
580+
$tag = $this->tagManager->createTag($name, true, true);
581+
$this->assertEquals($tag->getName(), $name);
582+
$this->tagManager->deleteTags($tag->getId());
583+
584+
\OC::$CLI = $oldCli;
585+
}
586+
587+
private function disallowedToCreateProvider(): array {
588+
return [
589+
[false],
590+
[null],
591+
];
592+
}
593+
594+
/**
595+
* @dataProvider disallowedToCreateProvider
596+
*/
597+
public function testDisallowedToCreateTag(?bool $isAdmin): void {
598+
$oldCli = \OC::$CLI;
599+
\OC::$CLI = false;
600+
601+
$user = $this->getMockBuilder(IUser::class)->getMock();
602+
$user->expects($this->any())
603+
->method('getUID')
604+
->willReturn('test');
605+
$this->userSession->expects($this->any())
606+
->method('getUser')
607+
->willReturn($isAdmin === null ? null : $user);
608+
$this->groupManager->expects($this->any())
609+
->method('isAdmin')
610+
->with('test')
611+
->willReturn($isAdmin);
612+
$this->appConfig->expects($this->any())
613+
->method('getValueBool')
614+
->with('systemtags', 'only_admins_can_create')
615+
->willReturn(true);
616+
617+
$this->expectException(\Exception::class);
618+
$tag = $this->tagManager->createTag(uniqid('tag_', true), true, true);
619+
620+
\OC::$CLI = $oldCli;
621+
}
622+
623+
530624
/**
531625
* @param ISystemTag $tag1
532626
* @param ISystemTag $tag2

0 commit comments

Comments
 (0)