diff --git a/apps/dav/lib/SystemTag/SystemTagNode.php b/apps/dav/lib/SystemTag/SystemTagNode.php index da51279a9d250..b48d61f3058a1 100644 --- a/apps/dav/lib/SystemTag/SystemTagNode.php +++ b/apps/dav/lib/SystemTag/SystemTagNode.php @@ -98,18 +98,10 @@ public function update($name, $userVisible, $userAssignable, $color): 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()); } // Make sure color is a proper hex diff --git a/apps/dav/tests/unit/SystemTag/SystemTagNodeTest.php b/apps/dav/tests/unit/SystemTag/SystemTagNodeTest.php index 32ee733dce8c5..3d0e3dd5366ae 100644 --- a/apps/dav/tests/unit/SystemTag/SystemTagNodeTest.php +++ b/apps/dav/tests/unit/SystemTag/SystemTagNodeTest.php @@ -85,19 +85,22 @@ public function tagNodeProvider() { [ true, new SystemTag(1, 'Original', true, true), - ['Renamed', true, true, null] + ['Renamed', true, true, null], + true, ], [ true, new SystemTag(1, 'Original', true, true), - ['Original', false, false, null] + ['Original', false, false, null], + true, ], // non-admin [ - // renaming allowed + // renaming not allowed false, new SystemTag(1, 'Original', true, true), - ['Rename', true, true, '0082c9'] + ['Renamed', true, true, null], + false, ], ]; } @@ -105,18 +108,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], $changedArgs[3]); + if ($allowed) { + $this->tagManager->expects($this->once()) + ->method('updateTag') + ->with(1, $changedArgs[0], $changedArgs[1], $changedArgs[2], $changedArgs[3]); + } 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], $changedArgs[3]); } @@ -206,7 +213,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, null); + $this->getTagNode(true, $tag)->update('Renamed', true, true, null); } @@ -226,7 +233,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, null); + $this->getTagNode(true, $tag)->update('Renamed', true, true, null); } /** 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 diff --git a/tests/lib/Net/IpAddressClassifierTest.php b/tests/lib/Net/IpAddressClassifierTest.php index 91ca11d6e8ee8..199d67904d152 100644 --- a/tests/lib/Net/IpAddressClassifierTest.php +++ b/tests/lib/Net/IpAddressClassifierTest.php @@ -81,7 +81,9 @@ public static function mappedAddresses(): array { ]; } - #[\PHPUnit\Framework\Attributes\DataProvider('mappedAddresses')] + /** + * @dataProvider mappedAddresses + */ public function testMappedAddresses(string $ipv6, ?string $ipv4): void { $mapped = $this->classifier->getMappedIpv4(IPv6::parseString($ipv6));