From 04761414b21670a198f2aec66cd31ad76f2a2d62 Mon Sep 17 00:00:00 2001 From: blanchonvincent Date: Wed, 12 Nov 2014 17:47:22 +0100 Subject: [PATCH 1/4] Add fail test case --- .../Tests/Firewall/ContextListenerTest.php | 50 +++++++++++++++++++ .../Fixtures/Core/SimpleSecurityContext.php | 35 +++++++++++++ 2 files changed, 85 insertions(+) create mode 100644 src/Symfony/Component/Security/Tests/Fixtures/Core/SimpleSecurityContext.php diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php index 90af07e86a62..3996f9492ff7 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php @@ -21,7 +21,9 @@ use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; use Symfony\Component\Security\Core\SecurityContext; +use Symfony\Component\Security\Core\User\InMemoryUserProvider; use Symfony\Component\Security\Http\Firewall\ContextListener; +use Symfony\Component\Security\Tests\Fixtures\Core\SimpleSecurityContext; class ContextListenerTest extends \PHPUnit_Framework_TestCase { @@ -220,6 +222,54 @@ public function testHandleRemovesTokenIfNoPreviousSessionWasFound() $listener->handle($event); } + public function testCanRefreshUserWithIdenticalProviders() + { + $providers = array( + $provider1 = new InMemoryUserProvider(array( + 'foo' => array(), + )), + $provider2 = new InMemoryUserProvider(array( + 'bar' => array(), + )), + ); + + $session = new Session(new MockArraySessionStorage()); + + /** @var \Symfony\Component\HttpFoundation\Request $request */ + $request = $this->getMock('Symfony\Component\HttpFoundation\Request'); + $request->expects($this->any())->method('hasPreviousSession')->will($this->returnValue(true)); + $request->expects($this->any())->method('getSession')->will($this->returnValue($session)); + + $event = $this->getMockBuilder('Symfony\Component\HttpKernel\Event\GetResponseEvent') + ->disableOriginalConstructor() + ->getMock(); + $event->expects($this->any())->method('getRequest')->will($this->returnValue($request)); + + $context = new SimpleSecurityContext(); + + /** + * We are trying to refresh the "foo" user + */ + $user = new UsernamePasswordToken($provider1->loadUserByUsername('foo'), '123456', 'memory'); + $session->set('_security_'. $key = 'key123', serialize($user)); + + $listener = new ContextListener($context, $providers, $key); + $listener->handle($event); + + $this->assertNotNull($context->getToken()); + + /** + * We are trying to refresh the "bar" user + */ + $user = new UsernamePasswordToken($provider2->loadUserByUsername('bar'), '123456', 'memory'); + $session->set('_security_'. $key = 'key123', serialize($user)); + + $listener = new ContextListener($context, $providers, $key); + $listener->handle($event); + + $this->assertNotNull($context->getToken()); + } + protected function runSessionOnKernelResponse($newToken, $original = null) { $session = new Session(new MockArraySessionStorage()); diff --git a/src/Symfony/Component/Security/Tests/Fixtures/Core/SimpleSecurityContext.php b/src/Symfony/Component/Security/Tests/Fixtures/Core/SimpleSecurityContext.php new file mode 100644 index 000000000000..803c100e1f79 --- /dev/null +++ b/src/Symfony/Component/Security/Tests/Fixtures/Core/SimpleSecurityContext.php @@ -0,0 +1,35 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Tests\Fixtures\Core; + +use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\SecurityContextInterface; + +class SimpleSecurityContext implements SecurityContextInterface +{ + protected $token; + + public function getToken() + { + return $this->token; + } + + public function setToken(TokenInterface $token = null) + { + $this->token = $token; + } + + public function isGranted($attributes, $object = null) + { + return true; + } +} \ No newline at end of file From c0b27af2881bc7c28a47dbf90a22c0cf49337e39 Mon Sep 17 00:00:00 2001 From: blanchonvincent Date: Wed, 12 Nov 2014 17:59:03 +0100 Subject: [PATCH 2/4] Fix fail test case --- .../Security/Http/Firewall/ContextListener.php | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Component/Security/Http/Firewall/ContextListener.php b/src/Symfony/Component/Security/Http/Firewall/ContextListener.php index e61907eff302..6fba1de025f1 100644 --- a/src/Symfony/Component/Security/Http/Firewall/ContextListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/ContextListener.php @@ -167,13 +167,19 @@ protected function refreshUser(TokenInterface $token) // let's try the next user provider } catch (UsernameNotFoundException $notFound) { if (null !== $this->logger) { - $this->logger->warning(sprintf('Username "%s" could not be found.', $notFound->getUsername())); + $this->logger->warning(sprintf('Username "%s" could not be found from "%s" provider.', $notFound->getUsername(), get_class($provider))); } - - return; + // let's try the next user provider } } + // if a UsernameNotFoundException has been thrown, there are providers + // associated with the token, but none able to refresh it + if (isset($notFound)) { + + return; + } + throw new \RuntimeException(sprintf('There is no user provider for user "%s".', get_class($user))); } } From 37e29508f0454d3101eb63d5a6aeeaee9c765df8 Mon Sep 17 00:00:00 2001 From: blanchonvincent Date: Sat, 15 Nov 2014 21:21:49 +0100 Subject: [PATCH 3/4] Fix coding style --- .../Component/Security/Http/Firewall/ContextListener.php | 7 ++++--- .../Security/Http/Tests/Firewall/ContextListenerTest.php | 6 +++--- .../Security/Tests/Fixtures/Core/SimpleSecurityContext.php | 2 +- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/Symfony/Component/Security/Http/Firewall/ContextListener.php b/src/Symfony/Component/Security/Http/Firewall/ContextListener.php index 6fba1de025f1..58d2a710775f 100644 --- a/src/Symfony/Component/Security/Http/Firewall/ContextListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/ContextListener.php @@ -153,6 +153,7 @@ protected function refreshUser(TokenInterface $token) $this->logger->debug(sprintf('Reloading user from user provider.')); } + $lastException = null; foreach ($this->userProviders as $provider) { try { $refreshedUser = $provider->refreshUser($user); @@ -170,13 +171,13 @@ protected function refreshUser(TokenInterface $token) $this->logger->warning(sprintf('Username "%s" could not be found from "%s" provider.', $notFound->getUsername(), get_class($provider))); } // let's try the next user provider + $lastException = $notFound; } } - // if a UsernameNotFoundException has been thrown, there are providers + // if a UsernameNotFoundException has been thrown, there are providers // associated with the token, but none able to refresh it - if (isset($notFound)) { - + if (null !== $lastException) { return; } diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php index 3996f9492ff7..8e6ca8623c0d 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php @@ -222,7 +222,7 @@ public function testHandleRemovesTokenIfNoPreviousSessionWasFound() $listener->handle($event); } - public function testCanRefreshUserWithIdenticalProviders() + public function testCanRefreshUserWithIdenticalSupportedProviders() { $providers = array( $provider1 = new InMemoryUserProvider(array( @@ -251,7 +251,7 @@ public function testCanRefreshUserWithIdenticalProviders() * We are trying to refresh the "foo" user */ $user = new UsernamePasswordToken($provider1->loadUserByUsername('foo'), '123456', 'memory'); - $session->set('_security_'. $key = 'key123', serialize($user)); + $session->set('_security_' . $key = 'key123', serialize($user)); $listener = new ContextListener($context, $providers, $key); $listener->handle($event); @@ -262,7 +262,7 @@ public function testCanRefreshUserWithIdenticalProviders() * We are trying to refresh the "bar" user */ $user = new UsernamePasswordToken($provider2->loadUserByUsername('bar'), '123456', 'memory'); - $session->set('_security_'. $key = 'key123', serialize($user)); + $session->set('_security_' . $key = 'key123', serialize($user)); $listener = new ContextListener($context, $providers, $key); $listener->handle($event); diff --git a/src/Symfony/Component/Security/Tests/Fixtures/Core/SimpleSecurityContext.php b/src/Symfony/Component/Security/Tests/Fixtures/Core/SimpleSecurityContext.php index 803c100e1f79..53104b76d361 100644 --- a/src/Symfony/Component/Security/Tests/Fixtures/Core/SimpleSecurityContext.php +++ b/src/Symfony/Component/Security/Tests/Fixtures/Core/SimpleSecurityContext.php @@ -32,4 +32,4 @@ public function isGranted($attributes, $object = null) { return true; } -} \ No newline at end of file +} From 36c07a2cd57032e6ea27d3307005714290d45867 Mon Sep 17 00:00:00 2001 From: blanchonvincent Date: Thu, 20 Nov 2014 09:10:20 +0100 Subject: [PATCH 4/4] Improve error handling from piotrpasich's idea --- .../Security/Http/Firewall/ContextListener.php | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/Symfony/Component/Security/Http/Firewall/ContextListener.php b/src/Symfony/Component/Security/Http/Firewall/ContextListener.php index 58d2a710775f..4b99fa7bdbe3 100644 --- a/src/Symfony/Component/Security/Http/Firewall/ContextListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/ContextListener.php @@ -153,7 +153,7 @@ protected function refreshUser(TokenInterface $token) $this->logger->debug(sprintf('Reloading user from user provider.')); } - $lastException = null; + $exceptions = []; foreach ($this->userProviders as $provider) { try { $refreshedUser = $provider->refreshUser($user); @@ -167,18 +167,20 @@ protected function refreshUser(TokenInterface $token) } catch (UnsupportedUserException $unsupported) { // let's try the next user provider } catch (UsernameNotFoundException $notFound) { - if (null !== $this->logger) { - $this->logger->warning(sprintf('Username "%s" could not be found from "%s" provider.', $notFound->getUsername(), get_class($provider))); - } // let's try the next user provider - $lastException = $notFound; + $exceptions[] = [$notFound, $provider]; } } // if a UsernameNotFoundException has been thrown, there are providers // associated with the token, but none able to refresh it - if (null !== $lastException) { - return; + if ($exceptions) { + if (null !== $this->logger) { + foreach ($exceptions as $exception) { + list($notFound, $provider) = $exception; + $this->logger->warning(sprintf('Username "%s" could not be found from "%s" provider.', $notFound->getUsername(), get_class($provider))); + } + } } throw new \RuntimeException(sprintf('There is no user provider for user "%s".', get_class($user)));