Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Fixed some issues of the AccessDecisionManager profiler #18934

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

Closed
wants to merge 11 commits into from
Closed

Fixed some issues of the AccessDecisionManager profiler #18934

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7721ba4
Fixed some issues of the AccessDecisionManager profiler
javiereguiluz Jun 1, 2016
639c8ea
Improved the code
javiereguiluz Jun 2, 2016
16dd82b
Restored the original condition (but simplify it a bit)
javiereguiluz Jun 5, 2016
ec9ffeb
Added a compiler pass to enable the debug access manager conditionally
javiereguiluz Jun 28, 2016
2ee2361
Pass the decorated access decision manager service to the security co…
javiereguiluz Jun 28, 2016
800ef55
Initialize the voters property to aovid errors in tests
javiereguiluz Jun 28, 2016
e4cbf01
Trying another way to solve this issue
javiereguiluz Jun 29, 2016
5d96ec6
Removed an unused "use" import
javiereguiluz Jun 29, 2016
804fcec
Load the debug config conditionally
javiereguiluz Jun 29, 2016
e3c9c29
Reverted some changes
javiereguiluz Jun 29, 2016
82f0541
Updated the "symfony/security" dependency
javiereguiluz Jun 29, 2016
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
2 changes: 1 addition & 1 deletion 2 src/Symfony/Bundle/SecurityBundle/Resources/config/collectors.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<argument type="service" id="security.token_storage" on-invalid="ignore" />
<argument type="service" id="security.role_hierarchy" />
<argument type="service" id="security.logout_url_generator" />
<argument type="service" id="debug.security.access.decision_manager" />
<argument type="service" id="security.access.decision_manager" />
Copy link
Member

@yceruto yceruto Jun 29, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

if the compiler pass was removed then when this argument is changed by debug.security.access.decision_manager ?
I missing something ?

Copy link
Contributor

@HeahDude HeahDude Jun 29, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

@yceruto the service is decorated when debug is on by loading security_debug.xml.

</service>
</services>
</container>
4 changes: 2 additions & 2 deletions 4 src/Symfony/Bundle/SecurityBundle/Resources/config/security_debug.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
<?xml version="1.0" ?>

<container xmlns="http://symfony.com/schema/dic/services"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">

<services>
<service id="debug.security.access.decision_manager" class="Symfony\Component\Security\Core\Authorization\DebugAccessDecisionManager" decorates="security.access.decision_manager" public="false">
Expand Down
2 changes: 1 addition & 1 deletion 2 src/Symfony/Bundle/SecurityBundle/composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
],
"require": {
"php": ">=5.5.9",
"symfony/security": "~3.1",
"symfony/security": "~3.1,>=3.1.2",
"symfony/http-kernel": "~2.8|~3.0",
"symfony/polyfill-php70": "~1.0"
},
Expand Down
20 changes: 13 additions & 7 deletions 20 src/Symfony/Component/Security/Core/Authorization/DebugAccessDecisionManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,19 @@ class DebugAccessDecisionManager implements AccessDecisionManagerInterface
{
private $manager;
private $strategy;
private $voters;
private $voters = array();
private $decisionLog = array();

public function __construct(AccessDecisionManager $manager)
public function __construct(AccessDecisionManagerInterface $manager)
Copy link
Member

Choose a reason for hiding this comment

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

the retrieval of the strategy with reflection below should check for being an instance of AccessDecisionManager though (btw, the ReflectionProperty should use AccessDecisionManager::class as first argument to be compatible with inheritance, as this is a private property which is not accessible on a child class)

Copy link
Member

Choose a reason for hiding this comment

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

and setVoters should check that the wrapped implementation has this method too

Copy link
Contributor

@GuilhemN GuilhemN Jun 1, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

👍 I didn't see this PR, so in case it helps: 3.1...Ener-Getick:SECURITY

{
$this->manager = $manager;

// The strategy is stored in a private property of the decorated service
$reflection = new \ReflectionProperty($manager, 'strategy');
$reflection->setAccessible(true);
$this->strategy = $reflection->getValue($manager);
if ($this->manager instanceof AccessDecisionManager) {
// The strategy is stored in a private property of the decorated service
$reflection = new \ReflectionProperty(AccessDecisionManager::class, 'strategy');
$reflection->setAccessible(true);
$this->strategy = $reflection->getValue($manager);
}
}

/**
Expand All @@ -60,6 +62,10 @@ public function decide(TokenInterface $token, array $attributes, $object = null)
*/
public function setVoters(array $voters)
{
if (!$this->manager instanceof AccessDecisionManager) {
return;
}

$this->voters = $voters;
$this->manager->setVoters($voters);
}
Expand All @@ -72,7 +78,7 @@ public function getStrategy()
// The $strategy property is misleading because it stores the name of its
// method (e.g. 'decideAffirmative') instead of the original strategy name
// (e.g. 'affirmative')
return strtolower(substr($this->strategy, 6));
return null === $this->strategy ? '-' : strtolower(substr($this->strategy, 6));
}

/**
Expand Down
Morty Proxy This is a proxified and sanitized view of the page, visit original site.