Merge pull request #707 from nextcloud/backport/705/stable25

[stable25] fix: Check tag attribute

CHANGELOG.md

@@ -1,6 +1,10 @@
 # Changelog
 All notable changes to this project will be documented in this file.
 
+## 1.15.3 – 2023-03-23
+### Fixed
+* Check tag attribute
+
 ## 1.15.2 – 2023-02-15
 ### Fixed
 * [Allow selecting any tag when many exist (#684)](https://github.com/nextcloud/files_automatedtagging/pull/684)

appinfo/info.xml

@@ -12,7 +12,7 @@ The tags can later be used to control retention, file access, automatic script e
 To define tags, administrators can create and manage a set of rule groups. Each rule group consists of one or more rules combined through operators. Rules can include criteria like file type, size, time and more. A request matches a group if all rules evaluate to true. On uploading a file all defined groups are evaluated and when matching, the given tags are assigned to the file.
 	</description>
 
-	<version>1.15.2</version>
+	<version>1.15.3</version>
 	<licence>agpl</licence>
 	<author>Joas Schilling</author>
 	<namespace>FilesAutomatedTagging</namespace>

lib/Operation.php

@@ -32,8 +32,11 @@
 use OCP\Files\Node;
 use OCP\Files\Storage\IStorage;
 use OCP\IConfig;
+use OCP\IGroupManager;
 use OCP\IL10N;
 use OCP\IURLGenerator;
+use OCP\IUser;
+use OCP\IUserSession;
 use OCP\SystemTag\ISystemTagManager;
 use OCP\SystemTag\ISystemTagObjectMapper;
 use OCP\SystemTag\TagNotFoundException;
@@ -54,6 +57,10 @@ class Operation implements ISpecificOperation, IComplexOperation {
 	private IMountManager $mountManager;
 	private IRootFolder $rootFolder;
 	private File $fileEntity;
+	/** @var IUserSession */
+	protected $userSession;
+	/** @var IGroupManager */
+	protected $groupManager;
 
 	public function __construct(
 		ISystemTagObjectMapper $objectMapper,
@@ -64,7 +71,9 @@ public function __construct(
 		IURLGenerator $urlGenerator,
 		IMountManager $mountManager,
 		IRootFolder $rootFolder,
-		File $fileEntity
+		File $fileEntity,
+		IUserSession $userSession,
+		IGroupManager $groupManager
 	) {
 		$this->objectMapper = $objectMapper;
 		$this->tagManager = $tagManager;
@@ -75,6 +84,8 @@ public function __construct(
 		$this->mountManager = $mountManager;
 		$this->rootFolder = $rootFolder;
 		$this->fileEntity = $fileEntity;
+		$this->userSession = $userSession;
+		$this->groupManager = $groupManager;
 	}
 
 	public function checkOperations(IStorage $storage, int $fileId, string $file): void {
@@ -100,16 +111,27 @@ public function checkOperations(IStorage $storage, int $fileId, string $file): v
 	 */
 	public function validateOperation(string $name, array $checks, string $operation): void {
 		if ($operation === '') {
-			throw new UnexpectedValueException($this->l->t('No tags given'));
+			throw new UnexpectedValueException($this->l->t('No tags given'), 1);
 		}
 
 		$systemTagIds = explode(',', $operation);
 		try {
-			$this->tagManager->getTagsByIds($systemTagIds);
+			$tags = $this->tagManager->getTagsByIds($systemTagIds);
+
+			$user = $this->userSession->getUser();
+			$isAdmin = $user instanceof IUser && $this->groupManager->isAdmin($user->getUID());
+
+			if (!$isAdmin) {
+				foreach ($tags as $tag) {
+					if (!$tag->isUserAssignable() || !$tag->isUserVisible()) {
+						throw new UnexpectedValueException($this->l->t('At least one of the given tags is invalid'), 4);
+					}
+				}
+			}
 		} catch (TagNotFoundException $e) {
-			throw new UnexpectedValueException($this->l->t('Tag(s) could not be found: %s', implode(', ', $e->getMissingTags())));
+			throw new UnexpectedValueException($this->l->t('At least one of the given tags is invalid'), 2);
 		} catch (InvalidArgumentException $e) {
-			throw new UnexpectedValueException($this->l->t('At least one of the given tags is invalid'));
+			throw new UnexpectedValueException($this->l->t('At least one of the given tags is invalid'), 3);
 		}
 	}
 
@@ -181,7 +203,10 @@ public function getIcon(): string {
 	}
 
 	public function isAvailableForScope(int $scope): bool {
-		return true;
+		return in_array($scope, [
+			IManager::SCOPE_ADMIN,
+			IManager::SCOPE_USER,
+		], true);
 	}
 
 	public function onEvent(string $eventName, Event $event, IRuleMatcher $ruleMatcher): void {

tests/OperationTest.php

@@ -31,10 +31,15 @@
 use OCP\Files\Mount\IMountPoint;
 use OCP\Files\Storage\IStorage;
 use OCP\IConfig;
+use OCP\IGroupManager;
 use OCP\IL10N;
 use OCP\IURLGenerator;
+use OCP\IUser;
+use OCP\IUserSession;
+use OCP\SystemTag\ISystemTag;
 use OCP\SystemTag\ISystemTagManager;
 use OCP\SystemTag\ISystemTagObjectMapper;
+use OCP\SystemTag\TagNotFoundException;
 use OCP\WorkflowEngine\IManager;
 use OCP\WorkflowEngine\IRuleMatcher;
 use PHPUnit\Framework\MockObject\MockObject;
@@ -59,6 +64,10 @@ class OperationTest extends TestCase {
 	protected $ruleMatcher;
 	/** @var IMountManager|MockObject */
 	protected $mountManager;
+	/** @var IUserSession|MockObject */
+	protected $userSession;
+	/** @var IGroupManager|MockObject */
+	protected $groupManager;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -85,6 +94,8 @@ protected function setUp(): void {
 		$this->mountManager = $this->createMock(IMountManager::class);
 		$this->rootFolder = $this->createMock(IRootFolder::class);
 		$this->fileEntity = $this->createMock(\OCA\WorkflowEngine\Entity\File::class);
+		$this->userSession = $this->createMock(IUserSession::class);
+		$this->groupManager = $this->createMock(IGroupManager::class);
 
 		$this->operation = new Operation(
 			$this->objectMapper,
@@ -95,7 +106,9 @@ protected function setUp(): void {
 			$this->urlGenerator,
 			$this->mountManager,
 			$this->rootFolder,
-			$this->fileEntity
+			$this->fileEntity,
+			$this->userSession,
+			$this->groupManager
 		);
 	}
 
@@ -161,6 +174,77 @@ public function testCheckOperations(IStorage $storage, $fileId, $file, array $ma
 		$this->operation->checkOperations($storage, $fileId, $file);
 	}
 
+	public function dataValidateOperation() {
+		$public = $this->createMock(ISystemTag::class);
+		$public->method('isUserVisible')
+			->willReturn(true);
+		$public->method('isUserAssignable')
+			->willReturn(true);
+		$restricted = $this->createMock(ISystemTag::class);
+		$restricted->method('isUserVisible')
+			->willReturn(true);
+		$restricted->method('isUserAssignable')
+			->willReturn(false);
+		$invisible = $this->createMock(ISystemTag::class);
+		$invisible->method('isUserVisible')
+			->willReturn(false);
+		$invisible->method('isUserAssignable')
+			->willReturn(false);
+
+		return [
+			['', null, false, 1],
+			['1', [$public], false, null],
+			['2,1', [$restricted, $public], false, 4],
+			['1,3', [$public, $invisible], false, 4],
+			['1', [$public], true, null],
+			['2,1', [$restricted, $public], true, null],
+			['1,3', [$public, $invisible], true, null],
+			['4', new TagNotFoundException(), false, 2],
+			['5', new \InvalidArgumentException(), false, 3],
+		];
+	}
+
+	/**
+	 * @dataProvider dataValidateOperation
+	 *
+	 * @param string $operation
+	 * @param ISystemTag[]|\Exception|null $tags
+	 * @param bool $isAdmin
+	 * @param int|null $exceptionCode
+	 */
+	public function testValidateOperation(string $operation, $tags, bool $isAdmin, ?int $exceptionCode) {
+		if ($tags === null) {
+			$this->tagManager->expects($this->never())
+				->method('getTagsByIds');
+		} elseif (is_array($tags)) {
+			$this->tagManager->expects($this->once())
+				->method('getTagsByIds')
+				->willReturn($tags);
+		} else {
+			$this->tagManager->expects($this->once())
+				->method('getTagsByIds')
+				->willThrowException($tags);
+		}
+
+		if ($isAdmin) {
+			$userId = 'admin';
+			$user = $this->createMock(IUser::class);
+			$user->method('getUID')
+				->willReturn($userId);
+			$this->userSession->method('getUser')
+				->willReturn($user);
+			$this->groupManager->method('isAdmin')
+				->with($userId)
+				->willReturn(true);
+		}
+
+		if ($exceptionCode !== null) {
+			$this->expectExceptionCode($exceptionCode);
+		}
+
+		$this->operation->validateOperation('', [], $operation);
+	}
+
 	public function taggingPathDataProvider() {
 		$homeId = 'home::alice';
 		$localId = 'local::/mnt/users/alice';