Skip to content

[Security] added voter report behavior #21094

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.

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ public function getConfigTreeBuilder()
->end()
->booleanNode('allow_if_all_abstain')->defaultFalse()->end()
->booleanNode('allow_if_equal_granted_denied')->defaultTrue()->end()
->scalarNode('translation_domain')->defaultValue('reports')->end()
->end()
->end()
->end()
Expand Down Expand Up @@ -369,11 +370,11 @@ private function addProvidersSection(ArrayNodeDefinition $rootNode)

$providerNodeBuilder
->validate()
->ifTrue(function ($v) {return count($v) > 1;})
->ifTrue(function ($v) {return count($v) > 1; })
->thenInvalid('You cannot set multiple provider types for the same provider')
->end()
->validate()
->ifTrue(function ($v) {return count($v) === 0;})
->ifTrue(function ($v) {return count($v) === 0; })
->thenInvalid('You must set a provider definition for the provider.')
->end()
;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ public function load(array $configs, ContainerBuilder $container)
->addArgument($config['access_decision_manager']['allow_if_all_abstain'])
->addArgument($config['access_decision_manager']['allow_if_equal_granted_denied'])
;
$container->setParameter('security.access.translation_domain', $config['access_decision_manager']['translation_domain']);
$container->setParameter('security.access.always_authenticate_before_granting', $config['always_authenticate_before_granting']);
$container->setParameter('security.authentication.hide_user_not_found', $config['hide_user_not_found']);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,20 @@
<!-- Authorization related services -->
<service id="security.access.decision_manager" class="Symfony\Component\Security\Core\Authorization\AccessDecisionManager" public="false">
<argument type="collection" />
<argument type="service" id="security.access.vote_report_builder" />
</service>

<service id="security.role_hierarchy" class="Symfony\Component\Security\Core\Role\RoleHierarchy" public="false">
<argument>%security.role_hierarchy.roles%</argument>
</service>

<service id="security.access.vote_report_builder" class="Symfony\Component\Security\Core\Authorization\Voter\Report\VoteReportBuilder" public="false">
<argument type="service" id="security.access.vote_report_collector" />
<argument type="service" id="translator" />
<argument>%security.access.translation_domain%</argument>
</service>

<service id="security.access.vote_report_collector" class="Symfony\Component\Security\Core\Authorization\Voter\Report\VoteReportCollector" public="false" />

<!-- Security Voters -->
<service id="security.access.simple_role_voter" class="Symfony\Component\Security\Core\Authorization\Voter\RoleVoter" public="false">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

namespace Symfony\Component\Security\Core\Authorization;

use Symfony\Component\Security\Core\Authorization\Voter\Report\VoteReportBuilderInterface;
use Symfony\Component\Security\Core\Authorization\Voter\Report\VoteReportInterface;
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;

Expand All @@ -26,22 +28,43 @@ class AccessDecisionManager implements AccessDecisionManagerInterface
const STRATEGY_CONSENSUS = 'consensus';
const STRATEGY_UNANIMOUS = 'unanimous';

/**
* @var VoterInterface[]
*/
private $voters;

/**
* @var string
*/
private $strategy;

/**
* @var bool
*/
private $allowIfAllAbstainDecisions;

/**
* @var bool
*/
private $allowIfEqualGrantedDeniedDecisions;

/**
* @var VoteReportBuilderInterface
*/
private $reportBuilder;

/**
* Constructor.
*
* @param VoterInterface[] $voters An array of VoterInterface instances
* @param string $strategy The vote strategy
* @param bool $allowIfAllAbstainDecisions Whether to grant access if all voters abstained or not
* @param bool $allowIfEqualGrantedDeniedDecisions Whether to grant access if result are equals
* @param VoterInterface[] $voters An array of VoterInterface instances
* @param VoteReportBuilderInterface $reportBuilder A vote report builder instance
* @param string $strategy The vote strategy
* @param bool $allowIfAllAbstainDecisions Whether to grant access if all voters abstained or not
* @param bool $allowIfEqualGrantedDeniedDecisions Whether to grant access if result are equals
*
* @throws \InvalidArgumentException
*/
public function __construct(array $voters = array(), $strategy = self::STRATEGY_AFFIRMATIVE, $allowIfAllAbstainDecisions = false, $allowIfEqualGrantedDeniedDecisions = true)
public function __construct(array $voters = array(), VoteReportBuilderInterface $reportBuilder, $strategy = self::STRATEGY_AFFIRMATIVE, $allowIfAllAbstainDecisions = false, $allowIfEqualGrantedDeniedDecisions = true)
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that this is still a BC break

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe I should set it and the behavior as optional to avoid the BC Break ?

Copy link
Member

Choose a reason for hiding this comment

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

it must be optional, and it must be at the end. Shifting all arguments is a BC break

{
$strategyMethod = 'decide'.ucfirst($strategy);
if (!is_callable(array($this, $strategyMethod))) {
Expand All @@ -52,6 +75,7 @@ public function __construct(array $voters = array(), $strategy = self::STRATEGY_
$this->strategy = $strategyMethod;
$this->allowIfAllAbstainDecisions = (bool) $allowIfAllAbstainDecisions;
$this->allowIfEqualGrantedDeniedDecisions = (bool) $allowIfEqualGrantedDeniedDecisions;
$this->reportBuilder = $reportBuilder;
}

/**
Expand Down Expand Up @@ -82,7 +106,8 @@ private function decideAffirmative(TokenInterface $token, array $attributes, $ob
{
$deny = 0;
foreach ($this->voters as $voter) {
$result = $voter->vote($token, $object, $attributes);
$result = $this->voteWith($voter, $token, $object, $attributes);

switch ($result) {
case VoterInterface::ACCESS_GRANTED:
return true;
Expand Down Expand Up @@ -123,7 +148,7 @@ private function decideConsensus(TokenInterface $token, array $attributes, $obje
$grant = 0;
$deny = 0;
foreach ($this->voters as $voter) {
$result = $voter->vote($token, $object, $attributes);
$result = $this->voteWith($voter, $token, $object, $attributes);

switch ($result) {
case VoterInterface::ACCESS_GRANTED:
Expand Down Expand Up @@ -164,7 +189,7 @@ private function decideUnanimous(TokenInterface $token, array $attributes, $obje
$grant = 0;
foreach ($attributes as $attribute) {
foreach ($this->voters as $voter) {
$result = $voter->vote($token, $object, array($attribute));
$result = $this->voteWith($voter, $token, $object, array($attribute));

switch ($result) {
case VoterInterface::ACCESS_GRANTED:
Expand All @@ -188,4 +213,32 @@ private function decideUnanimous(TokenInterface $token, array $attributes, $obje

return $this->allowIfAllAbstainDecisions;
}

/**
* @param VoterInterface $voter
* @param TokenInterface $token
* @param mixed $subject
* @param array $attributes
*
* @return int
*/
private function voteWith(VoterInterface $voter, TokenInterface $token, $subject, array $attributes)
{
$result = $voter->vote($token, $subject, $attributes);
if (!$result instanceof VoteReportInterface) {
return $result;
}

$finalResult = $result->getResult();

while ($result !== null) {
if ($result->getMessage() !== null) {
$this->reportBuilder->addReport($voter, $result, $subject, $token);
}

$result = $result->getPrevious();
}

return $finalResult;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Security\Core\Authorization\Voter\Report;

use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;

/**
* @author Maxime Perrimond <max.perrimond@gmail.com>
*/
final class VoteReport implements VoteReportInterface
{
/**
* @var string
*/
private $message;

/**
* @var string
*/
private $attribute;

/**
* @var mixed
*/
private $subject;

/**
* @var TokenInterface
*/
private $token;

/**
* @var VoterInterface
*/
private $voter;

/**
* @param string $message
* @param VoterInterface $voter
* @param string $attribute
* @param mixed $subject
* @param TokenInterface $token
*/
public function __construct($message, VoterInterface $voter, $attribute, $subject, TokenInterface $token)
{
$this->message = $message;
$this->attribute = $attribute;
$this->subject = $subject;
$this->voter = $voter;
$this->token = $token;
}

/**
* {@inheritdoc}
*/
public function getMessage()
{
return $this->message;
}

/**
* {@inheritdoc}
*/
public function getAttribute()
{
return $this->attribute;
}

/**
* {@inheritdoc}
*/
public function getSubject()
{
return $this->subject;
}

/**
* {@inheritdoc}
*/
public function getToken()
{
return $this->token;
}

/**
* {@inheritdoc}
*/
public function getVoter()
{
return $this->voter;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Security\Core\Authorization\Voter\Report;

use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Authorization\Voter\Result\VoterResultInterface;
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
use Symfony\Component\Translation\TranslatorInterface;

/**
* @author Maxime Perrimond <max.perrimond@gmail.com>
*/
final class VoteReportBuilder implements VoteReportBuilderInterface
{
/**
* @var VoteReportCollectorInterface
*/
private $collector;

/**
* @var TranslatorInterface
*/
private $translator;

/**
* @var null|string
*/
private $translationDomain;

/**
* @param VoteReportCollectorInterface $collector
* @param TranslatorInterface $translator
* @param string|null $translationDomain
*/
public function __construct(VoteReportCollectorInterface $collector, TranslatorInterface $translator, $translationDomain = null)
{
$this->collector = $collector;
$this->translator = $translator;
$this->translationDomain = $translationDomain;
}

/**
* {@inheritdoc}
*/
public function addReport(VoterInterface $voter, VoterResultInterface $voterResult, $subject, TokenInterface $token)
{
if (null === $voterResult->getPlural()) {
$translatedMessage = $this->translator->trans(
$voterResult->getMessage(),
$voterResult->getParameters(),
$this->translationDomain
);
} else {
try {
$translatedMessage = $this->translator->transChoice(
$voterResult->getMessage(),
$voterResult->getPlural(),
$voterResult->getParameters(),
$this->translationDomain
);
} catch (\InvalidArgumentException $e) {
$translatedMessage = $this->translator->trans(
$voterResult->getMessage(),
$voterResult->getParameters(),
$this->translationDomain
);
}
}

$this->collector->add(new VoteReport(
$translatedMessage,
$voter,
$voterResult->getAttribute(),
$subject,
$token
));
}
}
Loading