Skip to content

Commit

Permalink
bug #54248 [Security] Correctly initialize the voter property (aschempp)
Browse files Browse the repository at this point in the history
This PR was squashed before being merged into the 5.4 branch.

Discussion
----------

[Security] Correctly initialize the voter property

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | #54225
| License       | MIT

This fixes the basic issue that causes #54225. If `getVoters()` returns an empty array, the `$this->data['voters']` property is never initialized, and therefore returns an invalid value.

`@fritzmg` or I will follow up if necessary with a fix about decorated access decision managers that cannot "provide" their voters for tracing.

Commits
-------

b184401 [Security] Correctly initialize the voter property
  • Loading branch information
nicolas-grekas committed Mar 14, 2024
2 parents c9bce39 + b184401 commit 3fbca7b
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ public function collect(Request $request, Response $response, ?\Throwable $excep
// collect voters and access decision manager information
if ($this->accessDecisionManager instanceof TraceableAccessDecisionManager) {
$this->data['voter_strategy'] = $this->accessDecisionManager->getStrategy();
$this->data['voters'] = [];

foreach ($this->accessDecisionManager->getVoters() as $voter) {
if ($voter instanceof TraceableVoter) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,36 @@ public function dispatch(object $event, ?string $eventName = null): object
$this->assertSame($dataCollector->getVoterStrategy(), $strategy, 'Wrong value returned by getVoterStrategy');
}

public function testGetVotersIfAccessDecisionManagerHasNoVoters()
{
$strategy = MainConfiguration::STRATEGY_AFFIRMATIVE;

$accessDecisionManager = $this->createMock(TraceableAccessDecisionManager::class);

$accessDecisionManager
->method('getStrategy')
->willReturn($strategy);

$accessDecisionManager
->method('getVoters')
->willReturn([]);

$accessDecisionManager
->method('getDecisionLog')
->willReturn([[
'attributes' => ['view'],
'object' => new \stdClass(),
'result' => true,
'voterDetails' => [],
]]);

$dataCollector = new SecurityDataCollector(null, null, null, $accessDecisionManager, null, null, true);

$dataCollector->collect(new Request(), new Response());

$this->assertEmpty($dataCollector->getVoters());
}

public static function provideRoles(): array
{
return [
Expand Down

0 comments on commit 3fbca7b

Please sign in to comment.