fix(systemtags): prevent tag edition if restricted

Signed-off-by: skjnldsv <skjnldsv@protonmail.com>

10 files changed, 142 insertions, 22 deletions

diff --git a/apps/dav/lib/SystemTag/SystemTagPlugin.php b/apps/dav/lib/SystemTag/SystemTagPlugin.php
index 428e71d91f4..d70eff17ac1 100644
--- a/apps/dav/lib/SystemTag/SystemTagPlugin.php
+++ b/apps/dav/lib/SystemTag/SystemTagPlugin.php
@@ -19,6 +19,7 @@ use OCP\SystemTag\ISystemTagManager;
 use OCP\SystemTag\ISystemTagObjectMapper;
 use OCP\SystemTag\TagAlreadyExistsException;
 use OCP\SystemTag\TagCreationForbiddenException;
+use OCP\SystemTag\TagUpdateForbiddenException;
 use OCP\Util;
 use Sabre\DAV\Exception\BadRequest;
 use Sabre\DAV\Exception\Conflict;
@@ -191,7 +192,7 @@ class SystemTagPlugin extends \Sabre\DAV\ServerPlugin {
 		} 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);
+			throw new Forbidden('You don’t have permissions to create tags', 0, $e);
 		}
 	}
 
@@ -472,7 +473,11 @@ class SystemTagPlugin extends \Sabre\DAV\ServerPlugin {
 			}
 
 			if ($updateTag) {
-				$node->update($name, $userVisible, $userAssignable, $color);
+				try {
+					$node->update($name, $userVisible, $userAssignable, $color);
+				} catch (TagUpdateForbiddenException $e) {
+					throw new Forbidden('You don’t have permissions to update tags', 0, $e);
+				}
 			}
 
 			return true;
diff --git a/apps/settings/tests/Settings/Admin/ServerTest.php b/apps/settings/tests/Settings/Admin/ServerTest.php
index f9c10d864e4..35a8a3ce7f7 100644
--- a/apps/settings/tests/Settings/Admin/ServerTest.php
+++ b/apps/settings/tests/Settings/Admin/ServerTest.php
@@ -85,6 +85,10 @@ class ServerTest extends TestCase {
 			->expects($this->any())
 			->method('getValueString')
 			->willReturnCallback(fn ($a, $b, $default) => $default);
+		$this->appConfig
+			->expects($this->any())
+			->method('getValueBool')
+			->willReturnCallback(fn ($a, $b, $default) => $default);
 		$this->profileManager
 			->expects($this->exactly(2))
 			->method('isProfileEnabled')
diff --git a/apps/systemtags/src/components/SystemTagPicker.vue b/apps/systemtags/src/components/SystemTagPicker.vue
index 26a50c82e1f..601e48ea910 100644
--- a/apps/systemtags/src/components/SystemTagPicker.vue
+++ b/apps/systemtags/src/components/SystemTagPicker.vue
@@ -25,7 +25,7 @@
 			<!-- Search or create input -->
 			<div class="systemtags-picker__input">
 				<NcTextField :value.sync="input"
-					:label="canCreateTag ? t('systemtags', 'Search or create tag') : t('systemtags', 'Search tag')"
+					:label="canEditOrCreateTag ? t('systemtags', 'Search or create tag') : t('systemtags', 'Search tag')"
 					data-cy-systemtags-picker-input>
 					<TagIcon :size="20" />
 				</NcTextField>
@@ -49,7 +49,8 @@
 					</NcCheckboxRadioSwitch>
 
 					<!-- Color picker -->
-					<NcColorPicker :data-cy-systemtags-picker-tag-color="tag.id"
+					<NcColorPicker v-if="canEditOrCreateTag"
+						:data-cy-systemtags-picker-tag-color="tag.id"
 						:value="`#${tag.color}`"
 						:shown="openedPicker === tag.id"
 						class="systemtags-picker__tag-color"
@@ -67,8 +68,8 @@
 				</li>
 
 				<!-- Create new tag -->
-				<li v-if="canCreateTag">
-					<NcButton v-if="canCreateTag"
+				<li>
+					<NcButton v-if="canEditOrCreateTag && canCreateTag"
 						:disabled="status === Status.CREATING_TAG"
 						alignment="start"
 						class="systemtags-picker__tag-create"
@@ -88,7 +89,7 @@
 			<!-- Note -->
 			<div class="systemtags-picker__note">
 				<NcNoteCard v-if="!hasChanges" type="info">
-					{{ canCreateTag ? t('systemtags', 'Select or create tags to apply to all selected files'): t('systemtags', 'Select tags to apply to all selected files') }}
+					{{ canEditOrCreateTag ? t('systemtags', 'Select or create tags to apply to all selected files'): t('systemtags', 'Select tags to apply to all selected files') }}
 				</NcNoteCard>
 				<NcNoteCard v-else type="info">
 					<span v-html="statusMessage" />
@@ -127,7 +128,9 @@ import type { Tag, TagWithId } from '../types'
 
 import { defineComponent } from 'vue'
 import { emit } from '@nextcloud/event-bus'
+import { getCurrentUser } from '@nextcloud/auth'
 import { getLanguage, n, t } from '@nextcloud/l10n'
+import { loadState } from '@nextcloud/initial-state'
 import { showError, showInfo } from '@nextcloud/dialogs'
 import debounce from 'debounce'
 import domPurify from 'dompurify'
@@ -149,12 +152,10 @@ import PencilIcon from 'vue-material-design-icons/Pencil.vue'
 import PlusIcon from 'vue-material-design-icons/Plus.vue'
 import TagIcon from 'vue-material-design-icons/Tag.vue'
 
-import { createTag, fetchTag, fetchTags, getTagObjects, setTagObjects, updateTag } from '../services/api'
-import { getNodeSystemTags, setNodeSystemTags } from '../utils'
-import { elementColor, invertTextColor, isDarkModeEnabled } from '../utils/colorUtils'
+import { createTag, fetchTag, fetchTags, getTagObjects, setTagObjects, updateTag } from '../services/api.ts'
+import { elementColor, invertTextColor, isDarkModeEnabled } from '../utils/colorUtils.ts'
+import { getNodeSystemTags, setNodeSystemTags } from '../utils.ts'
 import logger from '../logger.ts'
-import { loadState } from '@nextcloud/initial-state'
-import { getCurrentUser } from '@nextcloud/auth'
 
 const debounceUpdateTag = debounce(updateTag, 500)
 const mainBackgroundColor = getComputedStyle(document.body)
@@ -172,7 +173,7 @@ enum Status {
 	DONE = 'done',
 }
 
-const restrictSystemTagsCreationToAdmin = loadState<false|true>('settings', 'restrictSystemTagsCreationToAdmin', false) === true
+const restrictSystemTagsCreationToAdmin = loadState('systemtags', 'restrictSystemTagsCreationToAdmin', false)
 
 export default defineComponent({
 	name: 'SystemTagPicker',
@@ -209,7 +210,7 @@ export default defineComponent({
 			Status,
 			t,
 			// Either tag creation is not restricted to admins or the current user is an admin
-			canCreateTag: !restrictSystemTagsCreationToAdmin || getCurrentUser()?.isAdmin,
+			canEditOrCreateTag: !restrictSystemTagsCreationToAdmin || getCurrentUser()?.isAdmin,
 		}
 	},
 
@@ -370,6 +371,10 @@ export default defineComponent({
 			})
 			return acc
 		}, {} as TagListCount) as TagListCount
+
+		if (!this.canEditOrCreateTag) {
+			logger.debug('System tag creation is restricted to admins and the current user is not an admin')
+		}
 	},
 
 	methods: {
@@ -428,7 +433,7 @@ export default defineComponent({
 		},
 
 		async onNewTag() {
-			if (!this.canCreateTag) {
+			if (!this.canEditOrCreateTag) {
 				// Should not happen ™
 				showError(t('systemtags', 'Only admins can create new tags'))
 				return
diff --git a/apps/systemtags/src/components/SystemTagsCreationControl.vue b/apps/systemtags/src/components/SystemTagsCreationControl.vue
index e8305991983..1b69983839f 100644
--- a/apps/systemtags/src/components/SystemTagsCreationControl.vue
+++ b/apps/systemtags/src/components/SystemTagsCreationControl.vue
@@ -6,17 +6,17 @@
 <template>
 	<div id="system-tags-creation-control">
 		<h4 class="inlineblock">
-			{{ t('settings', 'System tag creation') }}
+			{{ t('settings', 'System tag management') }}
 		</h4>
 
 		<p class="settings-hint">
-			{{ t('settings', 'If enabled, regular accounts will be restricted from creating new tags but will still be able to assign and remove them from their files.') }}
+			{{ t('settings', 'If enabled, only administrators can create and edit tags. Accounts can still assign and remove them from files.') }}
 		</p>
 
 		<NcCheckboxRadioSwitch type="switch"
 			:checked.sync="systemTagsCreationRestrictedToAdmin"
 			@update:checked="updateSystemTagsDefault">
-			{{ t('settings', 'Restrict tag creation to admins only') }}
+			{{ t('settings', 'Restrict tag creation and editing to administrators') }}
 		</NcCheckboxRadioSwitch>
 	</div>
 </template>
@@ -25,8 +25,9 @@
 import { loadState } from '@nextcloud/initial-state'
 import { showError, showSuccess } from '@nextcloud/dialogs'
 import { t } from '@nextcloud/l10n'
-import logger from '../logger.ts'
+
 import { updateSystemTagsAdminRestriction } from '../services/api.js'
+import logger from '../logger.ts'
 
 import NcCheckboxRadioSwitch from '@nextcloud/vue/components/NcCheckboxRadioSwitch'
 
@@ -37,14 +38,19 @@ export default {
 		NcCheckboxRadioSwitch,
 	},
 
+	setup() {
+		return {
+			t,
+		}
+	},
+
 	data() {
 		return {
 			// By default, system tags creation is not restricted to admins
-			systemTagsCreationRestrictedToAdmin: loadState<true|false>('settings', 'restrictSystemTagsCreationToAdmin', false) === true,
+			systemTagsCreationRestrictedToAdmin: loadState('settings', 'restrictSystemTagsCreationToAdmin', false),
 		}
 	},
 	methods: {
-		t,
 		async updateSystemTagsDefault(isRestricted: boolean) {
 			try {
 				const responseData = await updateSystemTagsAdminRestriction(isRestricted)
diff --git a/cypress/e2e/systemtags/files-bulk-action.cy.ts b/cypress/e2e/systemtags/files-bulk-action.cy.ts
index 7e70e1d2836..575b4da9c38 100644
--- a/cypress/e2e/systemtags/files-bulk-action.cy.ts
+++ b/cypress/e2e/systemtags/files-bulk-action.cy.ts
@@ -75,6 +75,11 @@ describe('Systemtags: Files bulk action', { testIsolation: false }, () => {
 		resetTags()
 	})
 
+	after(() => {
+		resetTags()
+		cy.runOccCommand('config:app:set systemtags restrict_creation_to_admin --value 0')
+	})
+
 	it('Can assign tag to selection', () => {
 		cy.login(user1)
 		cy.visit('/apps/files')
@@ -87,6 +92,7 @@ describe('Systemtags: Files bulk action', { testIsolation: false }, () => {
 
 		triggerTagManagementDialogAction()
 		cy.get('[data-cy-systemtags-picker-tag]').should('have.length', 5)
+		cy.get('[data-cy-systemtags-picker-tag-color]').should('have.length', 5)
 
 		cy.intercept('PROPFIND', '/remote.php/dav/systemtags/*/files').as('getTagData')
 		cy.intercept('PROPPATCH', '/remote.php/dav/systemtags/*/files').as('assignTagData')
@@ -115,6 +121,7 @@ describe('Systemtags: Files bulk action', { testIsolation: false }, () => {
 
 		triggerTagManagementDialogAction()
 		cy.get('[data-cy-systemtags-picker-tag]').should('have.length', 5)
+		cy.get('[data-cy-systemtags-picker-tag-color]').should('have.length', 5)
 
 		cy.intercept('PROPFIND', '/remote.php/dav/systemtags/*/files').as('getTagData')
 		cy.intercept('PROPPATCH', '/remote.php/dav/systemtags/*/files').as('assignTagData')
@@ -353,4 +360,55 @@ describe('Systemtags: Files bulk action', { testIsolation: false }, () => {
 			expectInlineTagForFile('file5.txt', [newTag])
 		})
 	})
+
+	it('Cannot create tag if restriction is in place', () => {
+		let tagId: string
+
+		cy.runOccCommand('config:app:set systemtags restrict_creation_to_admin --value 1')
+		cy.runOccCommand('tag:add testTag public --output json').then(({ stdout }) => {
+			const tag = JSON.parse(stdout)
+			tagId = tag.id
+		})
+
+		cy.createRandomUser().then((user1) => {
+			files.forEach((file) => {
+				cy.uploadContent(user1, new Blob([]), 'text/plain', '/' + file)
+			})
+
+			cy.login(user1)
+			cy.visit('/apps/files')
+
+			files.forEach((file) => {
+				getRowForFile(file).should('be.visible')
+			})
+			selectAllFiles()
+
+			triggerTagManagementDialogAction()
+
+			cy.findByRole('textbox', { name: 'Search or create tag' }).should('not.exist')
+			cy.findByRole('textbox', { name: 'Search tag' }).should('be.visible')
+
+			cy.get('[data-cy-systemtags-picker-input]').type('testTag')
+
+			cy.get('[data-cy-systemtags-picker-tag]').should('have.length', 1)
+			cy.get('[data-cy-systemtags-picker-button-create]').should('not.exist')
+			cy.get('[data-cy-systemtags-picker-tag-color]').should('not.exist')
+
+			// Assign the tag
+			cy.intercept('PROPFIND', '/remote.php/dav/systemtags/*/files').as('getTagData')
+			cy.intercept('PROPPATCH', '/remote.php/dav/systemtags/*/files').as('assignTagData')
+
+			cy.get(`[data-cy-systemtags-picker-tag="${tagId}"]`).should('be.visible')
+				.findByRole('checkbox').click({ force: true })
+			cy.get('[data-cy-systemtags-picker-button-submit]').click()
+
+			cy.wait('@getTagData')
+			cy.wait('@assignTagData')
+
+			cy.get('[data-cy-systemtags-picker]').should('not.exist')
+
+			// Finally, reset the restriction
+			cy.runOccCommand('config:app:set systemtags restrict_creation_to_admin --value 0')
+		})
+	})
 })
diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php
index 26b8a7daca7..29da548198d 100644
--- a/lib/composer/composer/autoload_classmap.php
+++ b/lib/composer/composer/autoload_classmap.php
@@ -798,6 +798,7 @@ return array(
     '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\\SystemTag\\TagUpdateForbiddenException' => $baseDir . '/lib/public/SystemTag/TagUpdateForbiddenException.php',
     'OCP\\Talk\\Exceptions\\NoBackendException' => $baseDir . '/lib/public/Talk/Exceptions/NoBackendException.php',
     'OCP\\Talk\\IBroker' => $baseDir . '/lib/public/Talk/IBroker.php',
     'OCP\\Talk\\IConversation' => $baseDir . '/lib/public/Talk/IConversation.php',
diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php
index 1a5e468609b..d9451e5294f 100644
--- a/lib/composer/composer/autoload_static.php
+++ b/lib/composer/composer/autoload_static.php
@@ -847,6 +847,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         '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\\SystemTag\\TagUpdateForbiddenException' => __DIR__ . '/../../..' . '/lib/public/SystemTag/TagUpdateForbiddenException.php',
         'OCP\\Talk\\Exceptions\\NoBackendException' => __DIR__ . '/../../..' . '/lib/public/Talk/Exceptions/NoBackendException.php',
         'OCP\\Talk\\IBroker' => __DIR__ . '/../../..' . '/lib/public/Talk/IBroker.php',
         'OCP\\Talk\\IConversation' => __DIR__ . '/../../..' . '/lib/public/Talk/IConversation.php',
diff --git a/lib/private/SystemTag/SystemTagManager.php b/lib/private/SystemTag/SystemTagManager.php
index 7979b3916f1..e889ceff54e 100644
--- a/lib/private/SystemTag/SystemTagManager.php
+++ b/lib/private/SystemTag/SystemTagManager.php
@@ -22,6 +22,7 @@ use OCP\SystemTag\ManagerEvent;
 use OCP\SystemTag\TagAlreadyExistsException;
 use OCP\SystemTag\TagCreationForbiddenException;
 use OCP\SystemTag\TagNotFoundException;
+use OCP\SystemTag\TagUpdateForbiddenException;
 
 /**
  * Manager class for system tags
@@ -152,8 +153,9 @@ class SystemTagManager implements ISystemTagManager {
 	public function createTag(string $tagName, bool $userVisible, bool $userAssignable): ISystemTag {
 		$user = $this->userSession->getUser();
 		if (!$this->canUserCreateTag($user)) {
-			throw new TagCreationForbiddenException('Tag creation forbidden');
+			throw new TagCreationForbiddenException();
 		}
+
 		// Length of name column is 64
 		$truncatedTagName = substr($tagName, 0, 64);
 		$query = $this->connection->getQueryBuilder();
@@ -206,6 +208,11 @@ class SystemTagManager implements ISystemTagManager {
 			);
 		}
 
+		$user = $this->userSession->getUser();
+		if (!$this->canUserUpdateTag($user)) {
+			throw new TagUpdateForbiddenException();
+		}
+
 		$beforeUpdate = array_shift($tags);
 		// Length of name column is 64
 		$newName = trim($newName);
@@ -342,6 +349,11 @@ class SystemTagManager implements ISystemTagManager {
 		return $this->groupManager->isAdmin($user->getUID());
 	}
 
+	public function canUserUpdateTag(?IUser $user): bool {
+		// We currently have no different permissions for updating tags than for creating them
+		return $this->canUserCreateTag($user);
+	}
+
 	public function canUserSeeTag(ISystemTag $tag, ?IUser $user): bool {
 		// If no user, then we only show public tags
 		if (!$user && $tag->getAccessLevel() === ISystemTag::ACCESS_LEVEL_PUBLIC) {
diff --git a/lib/public/SystemTag/ISystemTagManager.php b/lib/public/SystemTag/ISystemTagManager.php
index 66206d677f9..96e775d6401 100644
--- a/lib/public/SystemTag/ISystemTagManager.php
+++ b/lib/public/SystemTag/ISystemTagManager.php
@@ -130,6 +130,16 @@ interface ISystemTagManager {
 	public function canUserCreateTag(?IUser $user): bool;
 
 	/**
+	 * Checks whether the given user is allowed to update tags
+	 *
+	 * @param IUser|null $user user to check permission for
+	 * @return bool true if the user is allowed to update a tag, false otherwise
+	 *
+	 * @since 31.0.0
+	 */
+	public function canUserUpdateTag(?IUser $user): bool;
+
+	/**
 	 * Checks whether the given user is allowed to see the tag with the given id.
 	 *
 	 * @param ISystemTag $tag tag to check permission for
diff --git a/lib/public/SystemTag/TagUpdateForbiddenException.php b/lib/public/SystemTag/TagUpdateForbiddenException.php
new file mode 100644
index 00000000000..e5c1e76f6fe
--- /dev/null
+++ b/lib/public/SystemTag/TagUpdateForbiddenException.php
@@ -0,0 +1,18 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+namespace OCP\SystemTag;
+
+/**
+ * Exception when a user doesn't have the right to create a tag
+ *
+ * @since 31.0.1
+ */
+class TagUpdateForbiddenException extends \RuntimeException {
+}