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

Could we solve this via config?

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

Or maybe just load the collector in debug?

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

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?

👍 Let's try that! Thanks.

You should keep the check with hasParameter() since some tests are still failing

@javiereguiluz I tried this PR and the issue still exists :(

https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/SecurityBundle/Resources/config/collectors.xml#L13: The data_collector.security service ("which is always loaded" L66) require debug.security.access.decision_manager, so when $container->getParameter('kernel.debug') is false this service isn't loaded:

php bin/console --env=dev --no-debug

  [Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException]
  The service "profiler" has a dependency on a non-existent service "debug.security.access.decision_manager".

The road should be here https://github.com/symfony/symfony-standard/issues/968#issuecomment-223022665:

@stof: ... It should be based on whether the profiler is enabled instead of being based on the debug mode ...

Is it OK to load the collectors.xml file unconditionally? Is it really used when debug = false ? If this can be changed, the solution would be trivial: move $loader->load('collectors.xml'); under the if ($container->getParameter('kernel.debug')) condition.

well, if you do it, the security panel would not appear when enabling the profiler in no-debug mode, which might be confusing

Could we check web_profiler.controller.profiler service definition ? it's defined only if WebProfileBundle is enabled. BUT, this would imply that all panels profile should be loaded only if this "WebProfileBundle" is enabled ? Currenly this is not taken into consideration in others collectors.

@yceruto WebProfilerBundle is not related to enabling the profiler (the bundle does not provide the profiler. It is only about providing a visualization UI).
The proper service would be detecting profiler (coming from FrameworkBundle), but this cannot be done in a DI extension, because DI extension cannot see services defined in other extensions (see the previous discussion)

@stof you're right, then

#18934 (comment)

... Only the parameters are copied from actual to tmp container ...

We can create a new parameter something like profiler.collect into FrameworkExtension::registerProfilerConfiguration ?:

$container->setParameter('profiler.collect', $config['collect']);

and use it here ?

if ($container->hasParameter('profiler.collect') && $container->getParameter('profiler.collect')) {
    $loader->load('collectors.xml');
    $loader->load('security_debug.xml');
}

@javiereguiluz WDYT?

@yceruto my guess is that they won't accept to introduce a parameter for this. Let's try to do the compilar pass that @stof suggested in the first place.

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

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

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)

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

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