Skip to content

Allow (Domain-) Admins to delete aliases #873

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

Open
t2d wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Allow (Domain-) Admins to delete aliases #873

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
42 changes: 29 additions & 13 deletions src/Voter/AliasVoter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,19 @@

use App\Entity\Alias;
use App\Entity\User;
use App\Enum\Roles;
use Symfony\Component\Security\Core\Security;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Authorization\Voter\Voter;

class AliasVoter extends Voter
{
public const DELETE = 'delete';

public function __construct(private readonly Security $security)
{
}

protected function supports(string $attribute, mixed $subject): bool
{
if ($attribute !== self::DELETE) {
Expand All @@ -27,18 +33,28 @@ protected function supports(string $attribute, mixed $subject): bool
protected function voteOnAttribute(string $attribute, mixed $subject, TokenInterface $token): bool
{
$user = $token->getUser();
$alias = $subject;

$isUserValid = $user instanceof User;
$isAliasValid = $alias instanceof Alias;
$isNotDeleted = $isAliasValid && !$alias->isDeleted();
$isRandom = $isAliasValid && $alias->isRandom();
$isOwner = $isAliasValid && $alias->getUser() === $user;

return $isUserValid
&& $isAliasValid
&& $isNotDeleted
&& $isRandom
&& $isOwner;
$alias = $subject; // already ensured to be Alias by supports()
Copy link
Member

@0x46616c6b 0x46616c6b Sep 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SonarCloud complains when a method has more than three return statements...

Copy link
Contributor Author

@t2d t2d Sep 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I try to refactor it with 3 or less, it only get's worse. Do you have a good idea?


if (!$user instanceof User || !$alias instanceof Alias) {
return false; // sanity check
}

if ($alias->isDeleted()) {
return false; // cannot delete already deleted
}

$isOwner = $alias->getUser() === $user;

// owner can delete own random alias
if ($alias->isRandom() && $isOwner) {
return true;
}

// ADMIN or DOMAIN_ADMIN can delete their own custom aliases
if ($isOwner && ($this->security->isGranted(Roles::ADMIN) || $this->security->isGranted(Roles::DOMAIN_ADMIN))) {
return true;
}

return false;
}
}
31 changes: 21 additions & 10 deletions templates/Alias/show.html.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,16 +116,27 @@
{% for alias in aliases_custom %}
<div class="flex items-center justify-between p-3 bg-gray-50 border border-gray-200 rounded-md gap-3">
<span class="font-mono text-sm text-gray-900 truncate min-w-0 flex-1 font-stretch-semi-condensed" title="{{ alias.source }}">{{ alias.source }}</span>
<button type="button"
class="inline-flex items-center p-2 text-gray-700 bg-gray-100 rounded hover:bg-gray-200 focus:outline-none focus:ring-2 focus:ring-gray-500 focus:ring-offset-1 transition-colors flex-shrink-0"
data-button="copy-to-clipboard"
data-value="{{ alias.source }}"
title="{{ "copy-to-clipboard"|trans }}"
aria-label="{{ "copy-to-clipboard"|trans }}"
data-toggle="tooltip"
data-placement="top">
{{ ux_icon('heroicons:clipboard', {class: 'w-4 h-4'}) }}
</button>
<div class="flex items-center space-x-2 flex-shrink-0">
<button type="button"
class="inline-flex items-center p-2 text-gray-700 bg-gray-100 rounded hover:bg-gray-200 focus:outline-none focus:ring-2 focus:ring-gray-500 focus:ring-offset-1 transition-colors"
data-button="copy-to-clipboard"
data-value="{{ alias.source }}"
title="{{ "copy-to-clipboard"|trans }}"
aria-label="{{ "copy-to-clipboard"|trans }}"
data-toggle="tooltip"
data-placement="top">
{{ ux_icon('heroicons:clipboard', {class: 'w-4 h-4'}) }}
</button>
{% if is_granted('delete', alias) %}
<a href="{{ url('alias_delete', {'id': alias.id}) }}"
class="inline-flex items-center p-2 text-red-700 bg-red-100 rounded hover:bg-red-200 focus:outline-none focus:ring-2 focus:ring-red-500 focus:ring-offset-1 transition-colors"
title="{{ "alias.delete"|trans }}"
data-toggle="tooltip"
data-placement="top">
{{ ux_icon('heroicons:x-mark', {class: 'w-4 h-4'}) }}
</a>
{% endif %}
</div>
</div>
{% endfor %}
{% endif %}
Expand Down
36 changes: 30 additions & 6 deletions tests/Voter/AliasVoterTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,67 +4,79 @@

use App\Entity\Alias;
use App\Entity\User;
use App\Enum\Roles;
use App\Voter\AliasVoter;
use Symfony\Component\Security\Core\Security;
use PHPUnit\Framework\TestCase;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;

class AliasVoterTest extends TestCase
{
public function testSupportsReturnsTrueForDeleteAndAlias(): void
{
$voter = new AliasVoter();
$alias = new Alias();
$voter = new AliasVoter($this->createSecurity(false));
$this->assertTrue($this->invokeSupports($voter, AliasVoter::DELETE, $alias));
}

public function testSupportsReturnsFalseForOtherAttribute(): void
{
$voter = new AliasVoter();
$alias = new Alias();
$voter = new AliasVoter($this->createSecurity(false));
$this->assertFalse($this->invokeSupports($voter, 'OTHER_ATTRIBUTE', $alias));
}

public function testSupportsReturnsFalseForNonAliasSubject(): void
{
$voter = new AliasVoter();
$user = new User();
$voter = new AliasVoter($this->createSecurity(false));
$this->assertFalse($this->invokeSupports($voter, AliasVoter::DELETE, $user));
}

public function testVoteOnAttributeReturnsTrueIfUserOwnsAlias(): void
{
$voter = new AliasVoter();
$user = new User();
$alias = new Alias();
$alias->setRandom(true);
$alias->setUser($user);
$voter = new AliasVoter($this->createSecurity(false));
$token = $this->createMock(TokenInterface::class);
$token->method('getUser')->willReturn($user);
$this->assertTrue($this->invokeVoteOnAttribute($voter, AliasVoter::DELETE, $alias, $token));
}

public function testVoteOnAttributeReturnsFalseIfUserDoesNotOwnAlias(): void
{
$voter = new AliasVoter();
$user = new User();
$otherUser = new User();
$alias = new Alias();
$alias->setRandom(true);
$alias->setUser($otherUser);
$voter = new AliasVoter($this->createSecurity(false));
$token = $this->createMock(TokenInterface::class);
$token->method('getUser')->willReturn($user);
$this->assertFalse($this->invokeVoteOnAttribute($voter, AliasVoter::DELETE, $alias, $token));
}

public function testVoteOnAttributeReturnsFalseIfTokenUserIsNotUser(): void
{
$voter = new AliasVoter();
$alias = new Alias();
$voter = new AliasVoter($this->createSecurity(false));
$token = $this->createMock(TokenInterface::class);
$token->method('getUser')->willReturn(null);
$this->assertFalse($this->invokeVoteOnAttribute($voter, AliasVoter::DELETE, $alias, $token));
}

public function testVoteOnAttributeAllowsAdminCustomAlias(): void
{
$user = new User();
$alias = new Alias();
$alias->setUser($user); // custom (not random)
$voter = new AliasVoter($this->createSecurity(true));
$token = $this->createMock(TokenInterface::class);
$token->method('getUser')->willReturn($user);
$this->assertTrue($this->invokeVoteOnAttribute($voter, AliasVoter::DELETE, $alias, $token));
}
private function invokeSupports(AliasVoter $voter, string $attribute, $subject): bool
{
$ref = new \ReflectionClass($voter);
Expand All @@ -73,6 +85,18 @@ private function invokeSupports(AliasVoter $voter, string $attribute, $subject):
return $method->invoke($voter, $attribute, $subject);
}

private function createSecurity(bool $isAdmin): Security
{
$security = $this->createMock(Security::class);
$security->method('isGranted')->willReturnCallback(function (string $role) use ($isAdmin) {
if (!$isAdmin) {
return false;
}
return in_array($role, [Roles::ADMIN, Roles::DOMAIN_ADMIN], true);
});
return $security;
}

private function invokeVoteOnAttribute(AliasVoter $voter, string $attribute, $subject, TokenInterface $token): bool
{
$ref = new \ReflectionClass($voter);
Expand Down