fix(systemtags): prevent tag edition if restricted

Signed-off-by: skjnldsv <[email protected]>

apps/dav/lib/SystemTag/SystemTagPlugin.php

@@ -19,6 +19,7 @@
 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 @@ private function createTag($data, $contentType = 'application/json') {
 		} 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 @@ public function handleUpdateProperties($path, PropPatch $propPatch) {
 			}
 
 			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 create tags', 0, $e);
+				}
 			}
 
 			return true;

apps/settings/tests/Settings/Admin/ServerTest.php

@@ -85,6 +85,10 @@ public function testGetForm(): void {
 			->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')

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 v-if="canEditOrCreateTag && input.trim() !== ''">
+					<NcButton v-if="canEditOrCreateTag"
 						: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'
@@ -150,11 +153,9 @@ 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 { getNodeSystemTags, setNodeSystemTags } from '../utils'
 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<false|true>('systemtags', 'restrictSystemTagsCreationToAdmin', false) === true
 
 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,
 		}
 	},
 
@@ -247,7 +248,7 @@ export default defineComponent({
 			return this.toAdd.length > 0 || this.toRemove.length > 0
 		},
 
-		canCreateTag(): boolean {
+		canEditOrCreateTag(): boolean {
 			return this.input.trim() !== ''
 				&& !this.tags.some(tag => tag.displayName.trim().toLocaleLowerCase() === this.input.trim().toLocaleLowerCase())
 		},
@@ -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

apps/systemtags/src/components/SystemTagsCreationControl.vue

@@ -10,13 +10,13 @@
 		</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, regular accounts will be restricted from editing or creating new tags but will still be able to assign and remove them from their files.') }}
 		</p>
 
 		<NcCheckboxRadioSwitch type="switch"
 			:checked.sync="systemTagsCreationRestrictedToAdmin"
 			@update:checked="updateSystemTagsDefault">
-			{{ t('settings', 'Restrict tag creation to admins only') }}
+			{{ t('settings', 'Restrict tag edition and creation to admins only') }}
 		</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,
 		}
 	},
 	methods: {
-		t,
 		async updateSystemTagsDefault(isRestricted: boolean) {
 			try {
 				const responseData = await updateSystemTagsAdminRestriction(isRestricted)

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')
+		})
+	})
 })

lib/private/SystemTag/SystemTagManager.php

@@ -22,6 +22,7 @@
 use OCP\SystemTag\TagAlreadyExistsException;
 use OCP\SystemTag\TagCreationForbiddenException;
 use OCP\SystemTag\TagNotFoundException;
+use OCP\SystemTag\TagUpdateForbiddenException;
 
 /**
  * Manager class for system tags
@@ -151,9 +152,10 @@ public function getTag(string $tagName, bool $userVisible, bool $userAssignable)
 
 	public function createTag(string $tagName, bool $userVisible, bool $userAssignable): ISystemTag {
 		$user = $this->userSession->getUser();
-		if (!$this->canUserCreateTag($user)) {
+		if (!$this->canUserEditOrCreateTag($user)) {
 			throw new TagCreationForbiddenException('Tag creation forbidden');
 		}
+
 		// Length of name column is 64
 		$truncatedTagName = substr($tagName, 0, 64);
 		$query = $this->connection->getQueryBuilder();
@@ -206,6 +208,11 @@ public function updateTag(
 			);
 		}
 
+		$user = $this->userSession->getUser();
+		if (!$this->canUserEditOrCreateTag($user)) {
+			throw new TagUpdateForbiddenException('Tag update forbidden');
+		}
+
 		$beforeUpdate = array_shift($tags);
 		// Length of name column is 64
 		$newName = trim($newName);
@@ -329,7 +336,7 @@ public function canUserAssignTag(ISystemTag $tag, ?IUser $user): bool {
 		return false;
 	}
 
-	public function canUserCreateTag(?IUser $user): bool {
+	public function canUserEditOrCreateTag(?IUser $user): bool {
 		if ($user === null) {
 			// If no user given, allows only calls from CLI
 			return \OC::$CLI;

lib/public/SystemTag/TagUpdateForbiddenException.php

@@ -0,0 +1,18 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 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.0
+ */
+class TagUpdateForbiddenException extends \RuntimeException {
+}