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 @@
@@ -49,7 +49,8 @@ - -
  • - +
    - {{ 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') }} @@ -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('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 @@ @@ -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('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 @@ -129,6 +129,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. * 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 @@ +