Skip to content

[stable32] fix: require admin permissions for all systemtag updates #62922

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sorbaugh merged 2 commits into from
Aug 5, 2026
+26 −27
Merged

[stable32] fix: require admin permissions for all systemtag updates #62922

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6b2bd29
fix: require admin permissions for all systemtag updates
icewind1991 Jul 27, 2026
96ff624
test: adjust tests to new tag rename rule
icewind1991 Jul 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 2 additions & 10 deletions apps/dav/lib/SystemTag/SystemTagNode.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
35 changes: 21 additions & 14 deletions apps/dav/tests/unit/SystemTag/SystemTagNodeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,36 +70,43 @@ public static function tagNodeProvider(): array {
[
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,
],
];
}

#[\PHPUnit\Framework\Attributes\DataProvider('tagNodeProvider')]
public function testUpdateTag(bool $isAdmin, ISystemTag $originalTag, array $changedArgs): void {
$this->tagManager->expects($this->once())
->method('canUserSeeTag')
public function testUpdateTag(bool $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]);
}
Expand Down Expand Up @@ -187,7 +194,7 @@ public function testUpdateTagAlreadyExists(): void {
->method('updateTag')
->with(1, 'Renamed', true, true)
->willThrowException(new TagAlreadyExistsException());
$this->getTagNode(false, $tag)->update('Renamed', true, true, null);
$this->getTagNode(true, $tag)->update('Renamed', true, true, null);
}


Expand All @@ -207,7 +214,7 @@ public function testUpdateTagNotFound(): void {
->method('updateTag')
->with(1, 'Renamed', true, true)
->willThrowException(new TagNotFoundException());
$this->getTagNode(false, $tag)->update('Renamed', true, true, null);
$this->getTagNode(true, $tag)->update('Renamed', true, true, null);
}

#[\PHPUnit\Framework\Attributes\DataProvider('adminFlagProvider')]
Expand Down
6 changes: 3 additions & 3 deletions build/integration/files_features/tags.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading