Skip to content

[FrameworkBundle][SecurityBundle] Moved security expression providers pass logic to SecurityBundle #27611

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

Merged
merged 1 commit into from
Jun 19, 2018
Merged

[FrameworkBundle][SecurityBundle] Moved security expression providers pass logic to SecurityBundle #27611

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ CHANGELOG

* Allowed configuring taggable cache pools via a new `framework.cache.pools.tags` option (bool|service-id)
* Deprecated auto-injection of the container in AbstractController instances, register them as service subscribers instead
* Deprecated processing of services tagged `security.expression_language_provider` in favor of a new `AddExpressionLanguageProvidersPass` in SecurityBundle.

4.1.0
-----
Expand Down
14 changes: 13 additions & 1 deletion ...undle/FrameworkBundle/DependencyInjection/Compiler/AddExpressionLanguageProvidersPass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler;

use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddExpressionLanguageProvidersPass as SecurityExpressionLanguageProvidersPass;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\Reference;
Expand All @@ -22,6 +23,17 @@
*/
class AddExpressionLanguageProvidersPass implements CompilerPassInterface
{
private $handleSecurityLanguageProviders;

public function __construct(bool $handleSecurityLanguageProviders = true)
{
if ($handleSecurityLanguageProviders) {
@trigger_error(sprintf('Registering services tagged "security.expression_language_provider" with "%s" is deprecated since Symfony 4.2, use the "%s" instead.', __CLASS__, SecurityExpressionLanguageProvidersPass::class), E_USER_DEPRECATED);
}

$this->handleSecurityLanguageProviders = $handleSecurityLanguageProviders;
}

/**
* {@inheritdoc}
*/
Expand All @@ -36,7 +48,7 @@ public function process(ContainerBuilder $container)
}

// security
if ($container->has('security.expression_language')) {
if ($this->handleSecurityLanguageProviders && $container->has('security.expression_language')) {
$definition = $container->findDefinition('security.expression_language');
foreach ($container->findTaggedServiceIds('security.expression_language_provider', true) as $id => $attributes) {
$definition->addMethodCall('registerProvider', array(new Reference($id)));
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public function build(ContainerBuilder $container)
$this->addCompilerPassIfExists($container, AddConsoleCommandPass::class, PassConfig::TYPE_BEFORE_REMOVING);
$this->addCompilerPassIfExists($container, TranslatorPass::class);
$container->addCompilerPass(new LoggingTranslatorPass());
$container->addCompilerPass(new AddExpressionLanguageProvidersPass());
$container->addCompilerPass(new AddExpressionLanguageProvidersPass(false));
$this->addCompilerPassIfExists($container, TranslationExtractorPass::class);
$this->addCompilerPassIfExists($container, TranslationDumperPass::class);
$container->addCompilerPass(new FragmentRendererPass());
Expand Down
51 changes: 49 additions & 2 deletions ...eworkBundle/Tests/DependencyInjection/Compiler/AddExpressionLanguageProvidersPassTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class AddExpressionLanguageProvidersPassTest extends TestCase
public function testProcessForRouter()
{
$container = new ContainerBuilder();
$container->addCompilerPass(new AddExpressionLanguageProvidersPass());
$container->addCompilerPass(new AddExpressionLanguageProvidersPass(false));

$definition = new Definition('\stdClass');
$definition->addTag('routing.expression_language_provider');
Expand All @@ -41,7 +41,7 @@ public function testProcessForRouter()
public function testProcessForRouterAlias()
{
$container = new ContainerBuilder();
$container->addCompilerPass(new AddExpressionLanguageProvidersPass());
$container->addCompilerPass(new AddExpressionLanguageProvidersPass(false));

$definition = new Definition('\stdClass');
$definition->addTag('routing.expression_language_provider');
Expand All @@ -58,6 +58,10 @@ public function testProcessForRouterAlias()
$this->assertEquals(new Reference('some_routing_provider'), $calls[0][1][0]);
}

/**
* @group legacy
* @expectedDeprecation Registering services tagged with "security.expression_language_provider" with "Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\AddExpressionLanguageProvidersPass" is deprecated since Symfony 4.2, use the "Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddExpressionLanguageProvidersPass" instead.
*/
public function testProcessForSecurity()
{
$container = new ContainerBuilder();
Expand All @@ -76,6 +80,10 @@ public function testProcessForSecurity()
$this->assertEquals(new Reference('some_security_provider'), $calls[0][1][0]);
}

/**
* @group legacy
* @expectedDeprecation Registering services tagged with "security.expression_language_provider" with "Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\AddExpressionLanguageProvidersPass" is deprecated since Symfony 4.2, use the "Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddExpressionLanguageProvidersPass" instead.
*/
public function testProcessForSecurityAlias()
{
$container = new ContainerBuilder();
Expand All @@ -94,4 +102,43 @@ public function testProcessForSecurityAlias()
$this->assertEquals('registerProvider', $calls[0][0]);
$this->assertEquals(new Reference('some_security_provider'), $calls[0][1][0]);
}

/**
* @group legacy
*/
public function testProcessIgnoreSecurity()
{
$container = new ContainerBuilder();
$container->addCompilerPass(new AddExpressionLanguageProvidersPass(false));

$definition = new Definition('\stdClass');
$definition->addTag('security.expression_language_provider');
$container->setDefinition('some_security_provider', $definition->setPublic(true));

$container->register('security.expression_language', '\stdClass')->setPublic(true);
$container->compile();

$calls = $container->getDefinition('security.expression_language')->getMethodCalls();
$this->assertCount(0, $calls);
}

/**
* @group legacy
*/
public function testProcessIgnoreSecurityAlias()
{
$container = new ContainerBuilder();
$container->addCompilerPass(new AddExpressionLanguageProvidersPass(false));

$definition = new Definition('\stdClass');
$definition->addTag('security.expression_language_provider');
$container->setDefinition('some_security_provider', $definition->setPublic(true));

$container->register('my_security.expression_language', '\stdClass')->setPublic(true);
$container->setAlias('security.expression_language', 'my_security.expression_language');
$container->compile();

$calls = $container->getDefinition('my_security.expression_language')->getMethodCalls();
$this->assertCount(0, $calls);
}
}
1 change: 1 addition & 0 deletions src/Symfony/Bundle/SecurityBundle/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ CHANGELOG
the token classes is deprecated. To use
custom tokens extend the existing `Symfony\Component\Security\Core\Authentication\Token\AnonymousToken`
or `Symfony\Component\Security\Core\Authentication\Token\RememberMeToken`.
* Added `Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddExpressionLanguageProvidersPass`

4.1.0
-----
Expand Down
37 changes: 37 additions & 0 deletions ...Bundle/SecurityBundle/DependencyInjection/Compiler/AddExpressionLanguageProvidersPass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?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\Bundle\SecurityBundle\DependencyInjection\Compiler;

use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\Reference;

/**
* Registers the expression language providers.
*
* @author Fabien Potencier <fabien@symfony.com>
*/
class AddExpressionLanguageProvidersPass implements CompilerPassInterface
{
/**
* {@inheritdoc}
*/
public function process(ContainerBuilder $container)
{
if ($container->has('security.expression_language')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be a guard clause now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the code remains cleans and small with one indent, I would keep a readable true condition here. I'll wait for other opinions before changing the style.

$definition = $container->findDefinition('security.expression_language');
foreach ($container->findTaggedServiceIds('security.expression_language_provider', true) as $id => $attributes) {
$definition->addMethodCall('registerProvider', array(new Reference($id)));
}
}
}
}
2 changes: 2 additions & 0 deletions src/Symfony/Bundle/SecurityBundle/SecurityBundle.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Bundle\SecurityBundle;

use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddExpressionLanguageProvidersPass;
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\RegisterCsrfTokenClearingLogoutHandlerPass;
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\JsonLoginFactory;
use Symfony\Component\HttpKernel\Bundle\Bundle;
Expand Down Expand Up @@ -57,6 +58,7 @@ public function build(ContainerBuilder $container)

$extension->addUserProviderFactory(new InMemoryFactory());
$extension->addUserProviderFactory(new LdapFactory());
$container->addCompilerPass(new AddExpressionLanguageProvidersPass());
$container->addCompilerPass(new AddSecurityVotersPass());
$container->addCompilerPass(new AddSessionDomainConstraintPass(), PassConfig::TYPE_BEFORE_REMOVING);
$container->addCompilerPass(new RegisterCsrfTokenClearingLogoutHandlerPass());
Expand Down
58 changes: 58 additions & 0 deletions ...urityBundle/Tests/DependencyInjection/Compiler/AddExpressionLanguageProvidersPassTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<?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\Bundle\SecurityBundle\Tests\DependencyInjection\Compiler;

use PHPUnit\Framework\TestCase;
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddExpressionLanguageProvidersPass;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Reference;

class AddExpressionLanguageProvidersPassTest extends TestCase
{
public function testProcessForSecurity()
{
$container = new ContainerBuilder();
$container->addCompilerPass(new AddExpressionLanguageProvidersPass());

$definition = new Definition('\stdClass');
$definition->addTag('security.expression_language_provider');
$container->setDefinition('some_security_provider', $definition->setPublic(true));

$container->register('security.expression_language', '\stdClass')->setPublic(true);
$container->compile();

$calls = $container->getDefinition('security.expression_language')->getMethodCalls();
$this->assertCount(1, $calls);
$this->assertEquals('registerProvider', $calls[0][0]);
$this->assertEquals(new Reference('some_security_provider'), $calls[0][1][0]);
}

public function testProcessForSecurityAlias()
{
$container = new ContainerBuilder();
$container->addCompilerPass(new AddExpressionLanguageProvidersPass());

$definition = new Definition('\stdClass');
$definition->addTag('security.expression_language_provider');
$container->setDefinition('some_security_provider', $definition->setPublic(true));

$container->register('my_security.expression_language', '\stdClass')->setPublic(true);
$container->setAlias('security.expression_language', 'my_security.expression_language');
$container->compile();

$calls = $container->getDefinition('my_security.expression_language')->getMethodCalls();
$this->assertCount(1, $calls);
$this->assertEquals('registerProvider', $calls[0][0]);
$this->assertEquals(new Reference('some_security_provider'), $calls[0][1][0]);
}
}
4 changes: 2 additions & 2 deletions src/Symfony/Bundle/SecurityBundle/composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
"symfony/dom-crawler": "~3.4|~4.0",
"symfony/event-dispatcher": "~3.4|~4.0",
"symfony/form": "~3.4|~4.0",
"symfony/framework-bundle": "~4.1",
"symfony/framework-bundle": "~4.2",
"symfony/http-foundation": "~3.4|~4.0",
"symfony/translation": "~3.4|~4.0",
"symfony/twig-bundle": "~3.4|~4.0",
Expand All @@ -46,7 +46,7 @@
"conflict": {
"symfony/var-dumper": "<3.4",
"symfony/event-dispatcher": "<3.4",
"symfony/framework-bundle": "<4.1.1",
"symfony/framework-bundle": "<4.2",
Copy link
Member

Choose a reason for hiding this comment

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

L33 should bump to 4.2 also

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

"symfony/console": "<3.4"
},
"autoload": {
Expand Down