Skip to content

Commit 148e7ef

Browse files
committed
bug symfony#27297 Triggering RememberMe's loginFail() when token cannot be created (weaverryan)
This PR was submitted for the 2.7 branch but it was merged into the 2.8 branch instead (closes symfony#27297). Discussion ---------- Triggering RememberMe's loginFail() when token cannot be created | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no (but minor behavior change) | Deprecations? | no-> | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | not needed This is an edge-case bug fix. If, for example, someone tampers with the remember me cookie, and so it is invalid, this causes the `->autoLogin()` call to throw an `AuthenticationException`. But, this did not call the `loginFail()` method. Honestly, I'm not sure if the old or new behavior is correct. But, we should discuss and merge or close. Commits ------- e3412e6 Triggering RememberMe's loginFail() when token cannot be created
2 parents 4279f53 + e3412e6 commit 148e7ef

File tree

2 files changed

+56
-1
lines changed

2 files changed

+56
-1
lines changed

src/Symfony/Component/Security/Http/Firewall/RememberMeListener.php

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,25 @@ public function handle(GetResponseEvent $event)
6868
}
6969

7070
$request = $event->getRequest();
71-
if (null === $token = $this->rememberMeServices->autoLogin($request)) {
71+
try {
72+
if (null === $token = $this->rememberMeServices->autoLogin($request)) {
73+
return;
74+
}
75+
} catch (AuthenticationException $e) {
76+
if (null !== $this->logger) {
77+
$this->logger->warning(
78+
'The token storage was not populated with remember-me token as the'
79+
.' RememberMeServices was not able to create a token from the remember'
80+
.' me information.', array('exception' => $e)
81+
);
82+
}
83+
84+
$this->rememberMeServices->loginFail($request);
85+
86+
if (!$this->catchExceptions) {
87+
throw $e;
88+
}
89+
7290
return;
7391
}
7492

src/Symfony/Component/Security/Http/Tests/Firewall/RememberMeListenerTest.php

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,43 @@ public function testOnCoreSecurityIgnoresAuthenticationOptionallyRethrowsExcepti
143143
$listener->handle($event);
144144
}
145145

146+
public function testOnCoreSecurityAuthenticationExceptionDuringAutoLoginTriggersLoginFail()
147+
{
148+
list($listener, $tokenStorage, $service, $manager) = $this->getListener();
149+
150+
$tokenStorage
151+
->expects($this->once())
152+
->method('getToken')
153+
->will($this->returnValue(null))
154+
;
155+
156+
$exception = new AuthenticationException('Authentication failed.');
157+
$service
158+
->expects($this->once())
159+
->method('autoLogin')
160+
->will($this->throwException($exception))
161+
;
162+
163+
$service
164+
->expects($this->once())
165+
->method('loginFail')
166+
;
167+
168+
$manager
169+
->expects($this->never())
170+
->method('authenticate')
171+
;
172+
173+
$event = $this->getGetResponseEvent();
174+
$event
175+
->expects($this->once())
176+
->method('getRequest')
177+
->will($this->returnValue(new Request()))
178+
;
179+
180+
$listener->handle($event);
181+
}
182+
146183
public function testOnCoreSecurity()
147184
{
148185
list($listener, $tokenStorage, $service, $manager) = $this->getListener();

0 commit comments

Comments
 (0)