diff --git a/apps/dav/lib/SystemTag/SystemTagNode.php b/apps/dav/lib/SystemTag/SystemTagNode.php index 06eead814b27b..130ce2f5d9604 100644 --- a/apps/dav/lib/SystemTag/SystemTagNode.php +++ b/apps/dav/lib/SystemTag/SystemTagNode.php @@ -112,18 +112,10 @@ public function update($name, $userVisible, $userAssignable): void { if (!$this->tagManager->canUserSeeTag($this->tag, $this->user)) { throw new NotFound('Tag with id ' . $this->tag->getId() . ' does not exist'); } - if (!$this->tagManager->canUserAssignTag($this->tag, $this->user)) { - throw new Forbidden('No permission to update tag ' . $this->tag->getId()); - } - // only admin is able to change permissions, regular users can only rename + // only admin is able to update system tags if (!$this->isAdmin) { - // only renaming is allowed for regular users - if ($userVisible !== $this->tag->isUserVisible() - || $userAssignable !== $this->tag->isUserAssignable() - ) { - throw new Forbidden('No permission to update permissions for tag ' . $this->tag->getId()); - } + throw new Forbidden('No permission to update tag ' . $this->tag->getId()); } $this->tagManager->updateTag($this->tag->getId(), $name, $userVisible, $userAssignable); diff --git a/apps/dav/tests/unit/SystemTag/SystemTagNodeTest.php b/apps/dav/tests/unit/SystemTag/SystemTagNodeTest.php index 82aa81674dfa2..720dbcd73aacb 100644 --- a/apps/dav/tests/unit/SystemTag/SystemTagNodeTest.php +++ b/apps/dav/tests/unit/SystemTag/SystemTagNodeTest.php @@ -75,19 +75,22 @@ public function tagNodeProvider() { [ true, new SystemTag(1, 'Original', true, true), - ['Renamed', true, true] + ['Renamed', true, true], + true, ], [ true, new SystemTag(1, 'Original', true, true), - ['Original', false, false] + ['Original', false, false], + true, ], // non-admin [ - // renaming allowed + // renaming not allowed false, new SystemTag(1, 'Original', true, true), - ['Rename', true, true] + ['Renamed', true, true], + false, ], ]; } @@ -95,18 +98,22 @@ public function tagNodeProvider() { /** * @dataProvider tagNodeProvider */ - public function testUpdateTag($isAdmin, ISystemTag $originalTag, $changedArgs): void { - $this->tagManager->expects($this->once()) - ->method('canUserSeeTag') + public function testUpdateTag($isAdmin, ISystemTag $originalTag, $changedArgs, $allowed): void { + $this->tagManager->method('canUserSeeTag') ->with($originalTag) ->willReturn($originalTag->isUserVisible() || $isAdmin); - $this->tagManager->expects($this->once()) - ->method('canUserAssignTag') + $this->tagManager->method('canUserAssignTag') ->with($originalTag) ->willReturn($originalTag->isUserAssignable() || $isAdmin); - $this->tagManager->expects($this->once()) - ->method('updateTag') - ->with(1, $changedArgs[0], $changedArgs[1], $changedArgs[2]); + if ($allowed) { + $this->tagManager->expects($this->once()) + ->method('updateTag') + ->with(1, $changedArgs[0], $changedArgs[1], $changedArgs[2]); + } else { + $this->expectException(\Sabre\DAV\Exception\Forbidden::class); + $this->tagManager->expects($this->never()) + ->method('updateTag'); + } $this->getTagNode($isAdmin, $originalTag) ->update($changedArgs[0], $changedArgs[1], $changedArgs[2]); } @@ -196,7 +203,7 @@ public function testUpdateTagAlreadyExists(): void { ->method('updateTag') ->with(1, 'Renamed', true, true) ->will($this->throwException(new TagAlreadyExistsException())); - $this->getTagNode(false, $tag)->update('Renamed', true, true); + $this->getTagNode(true, $tag)->update('Renamed', true, true); } @@ -216,7 +223,7 @@ public function testUpdateTagNotFound(): void { ->method('updateTag') ->with(1, 'Renamed', true, true) ->will($this->throwException(new TagNotFoundException())); - $this->getTagNode(false, $tag)->update('Renamed', true, true); + $this->getTagNode(true, $tag)->update('Renamed', true, true); } /** diff --git a/build/integration/files_features/tags.feature b/build/integration/files_features/tags.feature index fef8068cbc809..f7da05edfcc67 100644 --- a/build/integration/files_features/tags.feature +++ b/build/integration/files_features/tags.feature @@ -36,13 +36,13 @@ Feature: tags Then The response should have a status code "400" And "0" tags should exist for "user0" - Scenario: Renaming a normal tag as regular user should work + Scenario: Renaming a normal tag as regular user should fail Given user "user0" exists Given "admin" creates a "normal" tag with name "MySuperAwesomeTagName" When "user0" edits the tag with name "MySuperAwesomeTagName" and sets its name to "AnotherTagName" - Then The response should have a status code "207" + Then The response should have a status code "403" And The following tags should exist for "admin" - |AnotherTagName|true|true| + |MySuperAwesomeTagName|true|true| Scenario: Renaming a not user-assignable tag as regular user should fail Given user "user0" exists