Refactor SavedExport permissions and voter logic

Revised SavedExportVoter to improve consistency and streamline permission checks. Updated tests, controller logic, and templates to align with new voter structure and attributes. Fixed typos in permission constants and added checks for delete/edit actions in the UI.
This commit is contained in:
Julien Fastré 2025-04-18 14:09:13 +02:00
parent edeb8edbea
commit 0f6b10aa0a
Signed by: julienfastre
GPG Key ID: BDE2190974723FCB
4 changed files with 40 additions and 22 deletions

View File

@ -509,8 +509,8 @@ class ExportController extends AbstractController
default => $this->savedExportOrExportGenerationRepository->findById($savedExportId),
};
if (null !== $savedExport && !$this->security->isGranted(SavedExportVoter::EDIT, $savedExport)) {
throw new AccessDeniedHttpException('saved export edition not allowed');
if (null !== $savedExport && !$this->security->isGranted(SavedExportVoter::GENERATE, $savedExport)) {
throw new AccessDeniedHttpException('saved export generation not allowed');
}
return $savedExport;

View File

@ -26,9 +26,9 @@
</div>
<div class="card-body">
<h2 class="card-title">{{ saved.title }}</h2>
{% if app.user is same as saved.createdBy %}
{% if app.user is same as saved.user %}
<p class="card-text tags">
{% if app.user is same as saved.createdBy %}<span class="badge bg-primary">{{ 'saved_export.Owner'|trans }}</span>{% endif %}
{% if app.user is same as saved.user %}<span class="badge bg-primary">{{ 'saved_export.Owner'|trans }}</span>{% endif %}
</p>
{% endif %}
<p class="card-text my-3">{{ saved.description|chill_markdown_to_html }}</p>
@ -58,9 +58,14 @@
{{ 'Actions'|trans }}
</button>
<ul class="dropdown-menu">
<li><a href="{{ chill_path_add_return_path('chill_main_export_saved_edit', {'id': saved.id }) }}" class="dropdown-item"><i class="fa fa-pencil"></i> {{ 'saved_export.update_title_and_description'|trans }}</a></li>
{% if is_granted('CHILL_MAIN_EXPORT_SAVED_EDIT', saved) %}
<li><a href="{{ chill_path_add_return_path('chill_main_export_saved_edit', {'id': saved.id }) }}" class="dropdown-item"><i class="fa fa-pencil"></i> {{ 'saved_export.update_title_and_description'|trans }}</a></li>
{% endif %}
{# reminder: the controller already checked that the user can generate saved exports #}
<li><a href="{{ chill_path_add_return_path('chill_main_export_new', {'alias': saved.exportAlias,'from_saved': saved.id }) }}" class="dropdown-item"><i class="fa fa-pencil"></i> {{ 'saved_export.update_filters_aggregators_and_execute'|trans }}</a></li>
<li><a href="{{ chill_path_add_return_path('chill_main_export_saved_delete', {'id': saved.id }) }}" class="dropdown-item"><i class="fa fa-trash"></i> {{ 'Delete'|trans }}</a></li>
{% if is_granted('CHILL_MAIN_EXPORT_SAVED_DELETE', saved) %}
<li><a href="{{ chill_path_add_return_path('chill_main_export_saved_delete', {'id': saved.id }) }}" class="dropdown-item"><i class="fa fa-trash"></i> {{ 'Delete'|trans }}</a></li>
{% endif %}
</ul>
</div>
</li>

View File

@ -16,17 +16,16 @@ use Chill\MainBundle\Entity\User;
use Chill\MainBundle\Export\ExportManager;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Authorization\Voter\Voter;
use Symfony\Component\Security\Core\Security;
class SavedExportVoter extends Voter
final class SavedExportVoter extends Voter
{
final public const DELETE = 'CHLL_MAIN_EXPORT_SAVED_DELETE';
final public const DELETE = 'CHILL_MAIN_EXPORT_SAVED_DELETE';
final public const EDIT = 'CHLL_MAIN_EXPORT_SAVED_EDIT';
final public const EDIT = 'CHILL_MAIN_EXPORT_SAVED_EDIT';
final public const GENERATE = 'CHLL_MAIN_EXPORT_SAVED_GENERATE';
final public const GENERATE = 'CHILL_MAIN_EXPORT_SAVED_GENERATE';
final public const SHARE = 'CHLL_MAIN_EXPORT_SAVED_SHARE';
final public const SHARE = 'CHILL_MAIN_EXPORT_SAVED_SHARE';
private const ALL = [
self::DELETE,
@ -35,7 +34,7 @@ class SavedExportVoter extends Voter
self::SHARE,
];
public function __construct(private ExportManager $exportManager) {}
public function __construct(private readonly ExportManager $exportManager) {}
protected function supports($attribute, $subject): bool
{

View File

@ -22,7 +22,6 @@ use Prophecy\Argument;
use Prophecy\PhpUnit\ProphecyTrait;
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
use Symfony\Component\Security\Core\Security;
/**
* @internal
@ -36,18 +35,14 @@ class SavedExportVoterTest extends TestCase
/**
* @dataProvider voteProvider
*/
public function testVote(string $attribute, SavedExport $savedExport, User $user, $expectedResult, ?bool $isGranted = null): void
public function testVote(string $attribute, mixed $savedExport, User $user, $expectedResult, ?bool $isGranted = null): void
{
$security = $this->prophesize(Security::class);
if (null !== $isGranted) {
$security->isGranted(Argument::any(), Argument::any())->willReturn($isGranted);
}
$export = $this->prophesize(ExportInterface::class);
$exportManager = $this->prophesize(ExportManager::class);
$exportManager->getExport($savedExport->getExportAlias())->willReturn($export->reveal());
$exportManager->getExport('dummy_export')->willReturn($export->reveal());
$exportManager->isGrantedForElement(Argument::any())->willReturn($isGranted);
$voter = new SavedExportVoter($exportManager->reveal(), $security->reveal());
$voter = new SavedExportVoter($exportManager->reveal());
$token = new UsernamePasswordToken($user, 'default', ['ROLE_USER']);
self::assertEquals($expectedResult, $voter->vote($token, $savedExport, [$attribute]));
@ -66,6 +61,25 @@ class SavedExportVoterTest extends TestCase
$savedExport->setExportAlias('dummy_export');
$savedExport->setUser($userA);
// abstain
foreach ($alls as $attribute) {
yield [
$attribute,
new \stdClass(),
$userA,
VoterInterface::ACCESS_ABSTAIN,
true,
];
}
yield [
'dummy',
$savedExport,
$userA,
VoterInterface::ACCESS_ABSTAIN,
false,
];
foreach ($alls as $attribute) {
yield [
$attribute,