Skip to content

Commit

Permalink
Fixing a bug where having an authentication failure would log you out.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
weaverryan committed Sep 21, 2015
1 parent 396a162 commit 302235e
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 8 deletions.
Expand Up @@ -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);
Expand Down
Expand Up @@ -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;

Expand Down Expand Up @@ -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) {
Expand Down
Expand Up @@ -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,
Expand Down
Expand Up @@ -18,9 +18,6 @@
use Symfony\Component\Security\Http\Event\InteractiveLoginEvent;
use Symfony\Component\Security\Http\SecurityEvents;

/**
* @author Ryan Weaver <weaverryan@gmail.com>
*/
class GuardAuthenticatorHandlerTest extends \PHPUnit_Framework_TestCase
{
private $tokenStorage;
Expand Down Expand Up @@ -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!');
Expand All @@ -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');
Expand Down

0 comments on commit 302235e

Please sign in to comment.