Skip to content

Commit

Permalink
feature #21792 [Security] deprecate multiple providers in context lis…
Browse files Browse the repository at this point in the history
…tener (xabbuh)

This PR was squashed before being merged into the 3.3-dev branch (closes #21792).

Discussion
----------

[Security] deprecate multiple providers in context listener

 Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

Passing multiple user providers to the context listener does not make
much sense. The listener is only responsible to refresh users for a
particular firewall. Thus, it must only be aware of the user provider
for this particular firewall.

Commits
-------

53df0de [Security] deprecate multiple providers in context listener
fbd9f88 [SecurityBundle] only pass relevant user provider
  • Loading branch information
fabpot committed Feb 28, 2017
2 parents afff0ce + 53df0de commit 924c1f0
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 10 deletions.
3 changes: 3 additions & 0 deletions UPGRADE-3.3.md
Expand Up @@ -105,6 +105,9 @@ Process
Security
--------

* Deprecated the ability to pass multiple user providers to the `ContextListener`. Pass only the user provider responsible
for the active firewall instead.

* The `RoleInterface` has been deprecated. Extend the `Symfony\Component\Security\Core\Role\Role`
class in your custom role implementations instead.

Expand Down
3 changes: 3 additions & 0 deletions UPGRADE-4.0.md
Expand Up @@ -261,6 +261,9 @@ Process
Security
--------

* Dropped support for passing multiple user providers to the `ContextListener`. Pass only the user provider responsible
for the active firewall instead.

* The `RoleInterface` has been removed. Extend the `Symfony\Component\Security\Core\Role\Role`
class instead.

Expand Down
Expand Up @@ -434,7 +434,7 @@ private function createContextListener($container, $contextKey, $providerId)

$listenerId = 'security.context_listener.'.count($this->contextListeners);
$listener = $container->setDefinition($listenerId, new ChildDefinition('security.context_listener'));
$listener->replaceArgument(1, array(new Reference($providerId)));
$listener->replaceArgument(1, new Reference($providerId));
$listener->replaceArgument(2, $contextKey);

return $this->contextListeners[$contextKey] = $listenerId;
Expand Down
6 changes: 6 additions & 0 deletions src/Symfony/Component/Security/CHANGELOG.md
@@ -1,6 +1,12 @@
CHANGELOG
=========

3.3.0
-----

* Deprecated the ability to pass multiple user providers to the `ContextListener`. Pass only the user provider responsible
for the active firewall instead.

3.2.0
-----

Expand Down
16 changes: 15 additions & 1 deletion src/Symfony/Component/Security/Http/Firewall/ContextListener.php
Expand Up @@ -44,12 +44,26 @@ class ContextListener implements ListenerInterface
private $registered;
private $trustResolver;

public function __construct(TokenStorageInterface $tokenStorage, array $userProviders, $contextKey, LoggerInterface $logger = null, EventDispatcherInterface $dispatcher = null, AuthenticationTrustResolverInterface $trustResolver = null)
/**
* @param TokenStorageInterface $tokenStorage
* @param UserProviderInterface|UserProviderInterface[] $userProviders
* @param string $contextKey
* @param LoggerInterface|null $logger
* @param EventDispatcherInterface|null $dispatcher
* @param AuthenticationTrustResolverInterface|null $trustResolver
*/
public function __construct(TokenStorageInterface $tokenStorage, $userProviders, $contextKey, LoggerInterface $logger = null, EventDispatcherInterface $dispatcher = null, AuthenticationTrustResolverInterface $trustResolver = null)
{
if (empty($contextKey)) {
throw new \InvalidArgumentException('$contextKey must not be empty.');
}

if (is_array($userProviders)) {
@trigger_error(sprintf('Being able to pass multiple user providers to the constructor of %s is deprecated since version 3.3 and will not be supported anymore in 4.0. Only pass the user provider for the current firewall context instead.', __CLASS__), E_USER_DEPRECATED);
} else {
$userProviders = array($userProviders);
}

foreach ($userProviders as $userProvider) {
if (!$userProvider instanceof UserProviderInterface) {
throw new \InvalidArgumentException(sprintf('User provider "%s" must implement "Symfony\Component\Security\Core\User\UserProviderInterface".', get_class($userProvider)));
Expand Down
Expand Up @@ -22,6 +22,7 @@
use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
use Symfony\Component\Security\Core\User\UserProviderInterface;
use Symfony\Component\Security\Http\Firewall\ContextListener;
use Symfony\Component\EventDispatcher\EventDispatcher;

Expand All @@ -35,12 +36,13 @@ public function testItRequiresContextKey()
{
new ContextListener(
$this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface')->getMock(),
array(),
$this->getMockBuilder(UserProviderInterface::class)->getMock(),
''
);
}

/**
* @group legacy
* @expectedException \InvalidArgumentException
* @expectedExceptionMessage User provider "stdClass" must implement "Symfony\Component\Security\Core\User\UserProviderInterface
*/
Expand Down Expand Up @@ -109,7 +111,7 @@ public function testOnKernelResponseWithoutSession()
new Response()
);

$listener = new ContextListener($tokenStorage, array(), 'session', null, new EventDispatcher());
$listener = new ContextListener($tokenStorage, $this->getMockBuilder(UserProviderInterface::class)->getMock(), 'session', null, new EventDispatcher());
$listener->onKernelResponse($event);

$this->assertTrue($session->isStarted());
Expand All @@ -128,7 +130,7 @@ public function testOnKernelResponseWithoutSessionNorToken()
new Response()
);

$listener = new ContextListener(new TokenStorage(), array(), 'session', null, new EventDispatcher());
$listener = new ContextListener(new TokenStorage(), $this->getMockBuilder(UserProviderInterface::class)->getMock(), 'session', null, new EventDispatcher());
$listener->onKernelResponse($event);

$this->assertFalse($session->isStarted());
Expand Down Expand Up @@ -163,7 +165,7 @@ public function testInvalidTokenInSession($token)
->method('setToken')
->with(null);

$listener = new ContextListener($tokenStorage, array(), 'key123');
$listener = new ContextListener($tokenStorage, $this->getMockBuilder(UserProviderInterface::class)->getMock(), 'key123');
$listener->handle($event);
}

Expand All @@ -184,7 +186,7 @@ public function testHandleAddsKernelResponseListener()
->disableOriginalConstructor()
->getMock();

$listener = new ContextListener($tokenStorage, array(), 'key123', null, $dispatcher);
$listener = new ContextListener($tokenStorage, $this->getMockBuilder(UserProviderInterface::class)->getMock(), 'key123', null, $dispatcher);

$event->expects($this->any())
->method('isMasterRequest')
Expand All @@ -208,7 +210,7 @@ public function testOnKernelResponseListenerRemovesItself()
->disableOriginalConstructor()
->getMock();

$listener = new ContextListener($tokenStorage, array(), 'key123', null, $dispatcher);
$listener = new ContextListener($tokenStorage, $this->getMockBuilder(UserProviderInterface::class)->getMock(), 'key123', null, $dispatcher);

$request = $this->getMockBuilder('Symfony\Component\HttpFoundation\Request')->getMock();
$request->expects($this->any())
Expand Down Expand Up @@ -242,7 +244,7 @@ public function testHandleRemovesTokenIfNoPreviousSessionWasFound()
$tokenStorage = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface')->getMock();
$tokenStorage->expects($this->once())->method('setToken')->with(null);

$listener = new ContextListener($tokenStorage, array(), 'key123');
$listener = new ContextListener($tokenStorage, $this->getMockBuilder(UserProviderInterface::class)->getMock(), 'key123');
$listener->handle($event);
}

Expand All @@ -268,7 +270,7 @@ protected function runSessionOnKernelResponse($newToken, $original = null)
new Response()
);

$listener = new ContextListener($tokenStorage, array(), 'session', null, new EventDispatcher());
$listener = new ContextListener($tokenStorage, $this->getMockBuilder(UserProviderInterface::class)->getMock(), 'session', null, new EventDispatcher());
$listener->onKernelResponse($event);

return $session;
Expand Down

0 comments on commit 924c1f0

Please sign in to comment.