Skip to content

Consistent error handling in remember me services #23042

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 14, 2017
Merged

Consistent error handling in remember me services #23042

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
2 changes: 1 addition & 1 deletion src/Symfony/Component/Security/Http/Firewall/RememberMeListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public function handle(GetResponseEvent $event)
);
}

$this->rememberMeServices->loginFail($request);
$this->rememberMeServices->loginFail($request, $e);

if (!$this->catchExceptions) {
throw $e;
Expand Down
32 changes: 21 additions & 11 deletions src/Symfony/Component/Security/Http/RememberMe/AbstractRememberMeServices.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public function getSecret()
final public function autoLogin(Request $request)
{
if (null === $cookie = $request->cookies->get($this->options['name'])) {
return;
return null;
}

if (null !== $this->logger) {
Expand All @@ -128,24 +128,32 @@ final public function autoLogin(Request $request)

return new RememberMeToken($user, $this->providerKey, $this->secret);
} catch (CookieTheftException $e) {
$this->cancelCookie($request);
$this->loginFail($request, $e);

throw $e;
} catch (UsernameNotFoundException $e) {
if (null !== $this->logger) {
$this->logger->info('User for remember-me cookie not found.');
$this->logger->info('User for remember-me cookie not found.', array('exception' => $e));
}

$this->loginFail($request, $e);
} catch (UnsupportedUserException $e) {
if (null !== $this->logger) {
$this->logger->warning('User class for remember-me cookie not supported.');
$this->logger->warning('User class for remember-me cookie not supported.', array('exception' => $e));
}

$this->loginFail($request, $e);
} catch (AuthenticationException $e) {
if (null !== $this->logger) {
$this->logger->debug('Remember-Me authentication failed.', array('exception' => $e));
}
}

$this->cancelCookie($request);
$this->loginFail($request, $e);
} catch (\Exception $e) {
$this->loginFail($request, $e);

throw $e;
}
}

/**
Expand All @@ -164,12 +172,13 @@ public function logout(Request $request, Response $response, TokenInterface $tok
* Implementation for RememberMeServicesInterface. Deletes the cookie when
* an attempted authentication fails.
*
* @param Request $request
* @param Request $request
* @param \Exception|null $exception
*/
final public function loginFail(Request $request)
final public function loginFail(Request $request, \Exception $exception = null)
{
$this->cancelCookie($request);
$this->onLoginFail($request);
$this->onLoginFail($request, $exception);
}

/**
Expand Down Expand Up @@ -226,9 +235,10 @@ final public function loginSuccess(Request $request, Response $response, TokenIn
abstract protected function processAutoLoginCookie(array $cookieParts, Request $request);

/**
* @param Request $request
* @param Request $request
* @param \Exception|null $exception
*/
protected function onLoginFail(Request $request)
protected function onLoginFail(Request $request, \Exception $exception = null)
{
}

Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Component/Security/Http/RememberMe/PersistentTokenBasedRememberMeServices.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
*/
class PersistentTokenBasedRememberMeServices extends AbstractRememberMeServices
{
/** @var TokenProviderInterface */
private $tokenProvider;

/**
Expand Down
5 changes: 3 additions & 2 deletions src/Symfony/Component/Security/Http/RememberMe/RememberMeServicesInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,10 @@ public function autoLogin(Request $request);
*
* This method needs to take care of invalidating the cookie.
*
* @param Request $request
* @param Request $request
* @param \Exception|null $exception
*/
public function loginFail(Request $request);
public function loginFail(Request $request, \Exception $exception = null);

/**
* Called whenever an interactive authentication attempt is successful
Expand Down
6 changes: 4 additions & 2 deletions src/Symfony/Component/Security/Http/Tests/Firewall/RememberMeListenerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ public function testOnCoreSecurityDoesNothingWhenNoCookieIsSet()
public function testOnCoreSecurityIgnoresAuthenticationExceptionThrownByAuthenticationManagerImplementation()
{
list($listener, $tokenStorage, $service, $manager) = $this->getListener();
$request = new Request();
$exception = new AuthenticationException('Authentication failed.');

$tokenStorage
->expects($this->once())
Expand All @@ -82,9 +84,9 @@ public function testOnCoreSecurityIgnoresAuthenticationExceptionThrownByAuthenti
$service
->expects($this->once())
->method('loginFail')
->with($request, $exception)
;

$exception = new AuthenticationException('Authentication failed.');
$manager
->expects($this->once())
->method('authenticate')
Expand All @@ -95,7 +97,7 @@ public function testOnCoreSecurityIgnoresAuthenticationExceptionThrownByAuthenti
$event
->expects($this->once())
->method('getRequest')
->will($this->returnValue(new Request()))
->will($this->returnValue($request))
;

$listener->handle($event);
Expand Down