From 302235e6e59319c14ce0f066398218f5d5f8accd Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Sun, 20 Sep 2015 20:11:34 -0400 Subject: [PATCH] Fixing a bug where having an authentication failure would log you out. This solution is a copy of what AbstractAuthenticationListener does. Scenario: 1) Login 2) Go back to the log in page 3) Put in a bad user/pass You *should* still be logged in after a failed attempt. This commit gives that behavior. --- .../Firewall/GuardAuthenticationListener.php | 2 +- .../Guard/GuardAuthenticatorHandler.php | 9 +++- .../GuardAuthenticationListenerTest.php | 2 +- .../Tests/GuardAuthenticatorHandlerTest.php | 50 +++++++++++++++++-- 4 files changed, 55 insertions(+), 8 deletions(-) diff --git a/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php b/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php index 26d5852be49a..6140be0980f0 100644 --- a/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php +++ b/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php @@ -117,7 +117,7 @@ private function executeGuardAuthenticator($uniqueGuardKey, GuardAuthenticatorIn $this->logger->info('Guard authentication failed.', array('exception' => $e, 'authenticator' => get_class($guardAuthenticator))); } - $response = $this->guardHandler->handleAuthenticationFailure($e, $request, $guardAuthenticator); + $response = $this->guardHandler->handleAuthenticationFailure($e, $request, $guardAuthenticator, $this->providerKey); if ($response instanceof Response) { $event->setResponse($response); diff --git a/src/Symfony/Component/Security/Guard/GuardAuthenticatorHandler.php b/src/Symfony/Component/Security/Guard/GuardAuthenticatorHandler.php index c588d688b7fb..5c6451ea7c7c 100644 --- a/src/Symfony/Component/Security/Guard/GuardAuthenticatorHandler.php +++ b/src/Symfony/Component/Security/Guard/GuardAuthenticatorHandler.php @@ -18,6 +18,7 @@ use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Exception\AuthenticationException; use Symfony\Component\Security\Core\User\UserInterface; +use Symfony\Component\Security\Guard\Token\PostAuthenticationGuardToken; use Symfony\Component\Security\Http\Event\InteractiveLoginEvent; use Symfony\Component\Security\Http\SecurityEvents; @@ -112,12 +113,16 @@ public function authenticateUserAndHandleSuccess(UserInterface $user, Request $r * @param AuthenticationException $authenticationException * @param Request $request * @param GuardAuthenticatorInterface $guardAuthenticator + * @param string $providerKey The key of the firewall * * @return null|Response */ - public function handleAuthenticationFailure(AuthenticationException $authenticationException, Request $request, GuardAuthenticatorInterface $guardAuthenticator) + public function handleAuthenticationFailure(AuthenticationException $authenticationException, Request $request, GuardAuthenticatorInterface $guardAuthenticator, $providerKey) { - $this->tokenStorage->setToken(null); + $token = $this->tokenStorage->getToken(); + if ($token instanceof PostAuthenticationGuardToken && $providerKey === $token->getProviderKey()) { + $this->tokenStorage->setToken(null); + } $response = $guardAuthenticator->onAuthenticationFailure($request, $authenticationException); if ($response instanceof Response || null === $response) { diff --git a/src/Symfony/Component/Security/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php b/src/Symfony/Component/Security/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php index ab7829223edf..8fab3991f98a 100644 --- a/src/Symfony/Component/Security/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php +++ b/src/Symfony/Component/Security/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php @@ -141,7 +141,7 @@ public function testHandleCatchesAuthenticationException() $this->guardAuthenticatorHandler ->expects($this->once()) ->method('handleAuthenticationFailure') - ->with($authException, $this->request, $authenticator); + ->with($authException, $this->request, $authenticator, $providerKey); $listener = new GuardAuthenticationListener( $this->guardAuthenticatorHandler, diff --git a/src/Symfony/Component/Security/Guard/Tests/GuardAuthenticatorHandlerTest.php b/src/Symfony/Component/Security/Guard/Tests/GuardAuthenticatorHandlerTest.php index 6b27e209bb0b..6f36702df1a3 100644 --- a/src/Symfony/Component/Security/Guard/Tests/GuardAuthenticatorHandlerTest.php +++ b/src/Symfony/Component/Security/Guard/Tests/GuardAuthenticatorHandlerTest.php @@ -18,9 +18,6 @@ use Symfony\Component\Security\Http\Event\InteractiveLoginEvent; use Symfony\Component\Security\Http\SecurityEvents; -/** - * @author Ryan Weaver - */ class GuardAuthenticatorHandlerTest extends \PHPUnit_Framework_TestCase { private $tokenStorage; @@ -63,7 +60,41 @@ public function testHandleAuthenticationSuccess() public function testHandleAuthenticationFailure() { + // setToken() not called - getToken() will return null, so there's nothing to clear + $this->tokenStorage->expects($this->never()) + ->method('setToken') + ->with(null); + $authException = new AuthenticationException('Bad password!'); + + $response = new Response('Try again, but with the right password!'); + $this->guardAuthenticator->expects($this->once()) + ->method('onAuthenticationFailure') + ->with($this->request, $authException) + ->will($this->returnValue($response)); + + $handler = new GuardAuthenticatorHandler($this->tokenStorage, $this->dispatcher); + $actualResponse = $handler->handleAuthenticationFailure($authException, $this->request, $this->guardAuthenticator, 'firewall_provider_key'); + $this->assertSame($response, $actualResponse); + } + + /** + * @dataProvider getTokenClearingTests + */ + public function testHandleAuthenticationClearsToken($tokenClass, $tokenProviderKey, $actualProviderKey, $shouldTokenBeCleared) + { + $token = $this->getMockBuilder($tokenClass) + ->disableOriginalConstructor() + ->getMock(); + $token->expects($this->any()) + ->method('getProviderKey') + ->will($this->returnValue($tokenProviderKey)); + + // make the $token be the current token $this->tokenStorage->expects($this->once()) + ->method('getToken') + ->will($this->returnValue($token)); + + $this->tokenStorage->expects($shouldTokenBeCleared ? $this->once() : $this->never()) ->method('setToken') ->with(null); $authException = new AuthenticationException('Bad password!'); @@ -75,10 +106,21 @@ public function testHandleAuthenticationFailure() ->will($this->returnValue($response)); $handler = new GuardAuthenticatorHandler($this->tokenStorage, $this->dispatcher); - $actualResponse = $handler->handleAuthenticationFailure($authException, $this->request, $this->guardAuthenticator); + $actualResponse = $handler->handleAuthenticationFailure($authException, $this->request, $this->guardAuthenticator, $actualProviderKey); $this->assertSame($response, $actualResponse); } + public function getTokenClearingTests() + { + $tests = array(); + // correct token class and matching firewall => clear the token + $tests[] = array('Symfony\Component\Security\Guard\Token\PostAuthenticationGuardToken', 'the_firewall_key', 'the_firewall_key', true); + $tests[] = array('Symfony\Component\Security\Guard\Token\PostAuthenticationGuardToken', 'the_firewall_key', 'different_key', false); + $tests[] = array('Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken', 'the_firewall_key', 'the_firewall_key', false); + + return $tests; + } + protected function setUp() { $this->tokenStorage = $this->getMock('Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface');