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 1 commit
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
Prev Previous commit
Next Next commit
Trying another way to solve this issue
  • Loading branch information
@javiereguiluz
javiereguiluz committed Jun 29, 2016
commit e4cbf0122b3757d57d6293ea58d035f7af9f353f
42 changes: 0 additions & 42 deletions 42 .../Bundle/SecurityBundle/DependencyInjection/Compiler/AddDebugAccessDecisionManagerPass.php
View file
Open in desktop

This file was deleted.

1 change: 1 addition & 0 deletions 1 src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ public function load(array $configs, ContainerBuilder $container)
$loader->load('templating_twig.xml');
$loader->load('collectors.xml');
Copy link
Member

Choose a reason for hiding this comment

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

@javiereguiluz the issue still occurs if profiler service is not defined, so data_collector.security require of debug.security.access.decision_manager

Copy link
Member Author

Choose a reason for hiding this comment

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

Could we solve this via config?

<argument type="service" id="debug.security.access.decision_manager" on-invalid="ignore" />

Copy link
Contributor

Choose a reason for hiding this comment

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

Or maybe just load the collector in debug?

Copy link
Member Author

Choose a reason for hiding this comment

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

@HeahDude apparently we can't do that. See #18934 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, I see another solution, now that the security.access.decision_manager is properly decorated, just pass it to the collector instead of the debug.security.access.decision_manager, this line should ensure everything is fine, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 Let's try that! Thanks.

$loader->load('guard.xml');
$loader->load('security_debug.xml');
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you remove the debug check?

Copy link
Member Author

Choose a reason for hiding this comment

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

#18934 (comment)

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.

Yes but now the check is done in the collector, so no need to load the debug adm when not in debug mode.


if (!class_exists('Symfony\Component\ExpressionLanguage\ExpressionLanguage')) {
$container->removeDefinition('security.expression_language');
Expand Down
12 changes: 12 additions & 0 deletions 12 src/Symfony/Bundle/SecurityBundle/Resources/config/security_debug.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?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">

<services>
<service id="debug.security.access.decision_manager" class="Symfony\Component\Security\Core\Authorization\DebugAccessDecisionManager" decorates="security.access.decision_manager" public="false">
<argument type="service" id="debug.security.access.decision_manager.inner" />
</service>
</services>
</container>
1 change: 0 additions & 1 deletion 1 src/Symfony/Bundle/SecurityBundle/SecurityBundle.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,5 @@ public function build(ContainerBuilder $container)
$extension->addUserProviderFactory(new InMemoryFactory());
$extension->addUserProviderFactory(new LdapFactory());
$container->addCompilerPass(new AddSecurityVotersPass());
$container->addCompilerPass(new AddDebugAccessDecisionManagerPass());
}
}
Morty Proxy This is a proxified and sanitized view of the page, visit original site.