Skip to content

Commit cb3d53b

Browse files
derrabusderrabus
committed
Merge branch '4.4' into 5.3
* 4.4: Don't produce TypeErrors for non-string CSRF tokens [HttpKernel] Fix SplFileInfo mock in HttpKernelBrowserTest Update NativeSessionStorage docblock to match defaults
2 parents 03219b9 + ebbf7f1 commit cb3d53b

File tree

4 files changed

+104
-18
lines changed

4 files changed

+104
-18
lines changed

‎Firewall/LogoutListener.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ public function authenticate(RequestEvent $event)
113113
if (null !== $this->csrfTokenManager) {
114114
$csrfToken = ParameterBagUtils::getRequestParameterValue($request, $this->options['csrf_parameter']);
115115

116-
if (false === $this->csrfTokenManager->isTokenValid(new CsrfToken($this->options['csrf_token_id'], $csrfToken))) {
116+
if (!\is_string($csrfToken) || false === $this->csrfTokenManager->isTokenValid(new CsrfToken($this->options['csrf_token_id'], $csrfToken))) {
117117
throw new LogoutException('Invalid CSRF token.');
118118
}
119119
}

‎Firewall/UsernamePasswordFormAuthenticationListener.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ protected function attemptAuthentication(Request $request)
7676
if (null !== $this->csrfTokenManager) {
7777
$csrfToken = ParameterBagUtils::getRequestParameterValue($request, $this->options['csrf_parameter']);
7878

79-
if (false === $this->csrfTokenManager->isTokenValid(new CsrfToken($this->options['csrf_token_id'], $csrfToken))) {
79+
if (!\is_string($csrfToken) || false === $this->csrfTokenManager->isTokenValid(new CsrfToken($this->options['csrf_token_id'], $csrfToken))) {
8080
throw new InvalidCsrfTokenException('Invalid CSRF token.');
8181
}
8282
}

‎Tests/Firewall/LogoutListenerTest.php

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,22 +141,27 @@ public function testNoResponseSet()
141141
$listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST));
142142
}
143143

144-
public function testCsrfValidationFails()
144+
/**
145+
* @dataProvider provideInvalidCsrfTokens
146+
*/
147+
public function testCsrfValidationFails($invalidToken)
145148
{
146149
$this->expectException(LogoutException::class);
147150
$tokenManager = $this->getTokenManager();
148151

149152
[$listener, , $httpUtils, $options] = $this->getListener(null, $tokenManager);
150153

151154
$request = new Request();
152-
$request->query->set('_csrf_token', 'token');
155+
if (null !== $invalidToken) {
156+
$request->query->set('_csrf_token', $invalidToken);
157+
}
153158

154159
$httpUtils->expects($this->once())
155160
->method('checkRequestPath')
156161
->with($request, $options['logout_path'])
157162
->willReturn(true);
158163

159-
$tokenManager->expects($this->once())
164+
$tokenManager
160165
->method('isTokenValid')
161166
->willReturn(false);
162167

@@ -199,6 +204,15 @@ public function testLegacyLogoutHandlers()
199204
$this->assertSame($response, $event->getResponse());
200205
}
201206

207+
public function provideInvalidCsrfTokens(): array
208+
{
209+
return [
210+
['invalid'],
211+
[['in' => 'valid']],
212+
[null],
213+
];
214+
}
215+
202216
private function getTokenManager()
203217
{
204218
return $this->createMock(CsrfTokenManagerInterface::class);

‎Tests/Firewall/UsernamePasswordFormAuthenticationListenerTest.php

Lines changed: 85 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
2525
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
2626
use Symfony\Component\Security\Core\Security;
27+
use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
2728
use Symfony\Component\Security\Http\Authentication\AuthenticationFailureHandlerInterface;
2829
use Symfony\Component\Security\Http\Authentication\DefaultAuthenticationFailureHandler;
2930
use Symfony\Component\Security\Http\Authentication\DefaultAuthenticationSuccessHandler;
@@ -40,7 +41,7 @@ class UsernamePasswordFormAuthenticationListenerTest extends TestCase
4041
/**
4142
* @dataProvider getUsernameForLength
4243
*/
43-
public function testHandleWhenUsernameLength($username, $ok)
44+
public function testHandleWhenUsernameLength(string $username, bool $ok)
4445
{
4546
$request = Request::create('/login_check', 'POST', ['_username' => $username, '_password' => 'symfony']);
4647
$request->setSession($this->createMock(SessionInterface::class));
@@ -87,10 +88,8 @@ public function testHandleWhenUsernameLength($username, $ok)
8788
/**
8889
* @dataProvider postOnlyDataProvider
8990
*/
90-
public function testHandleNonStringUsernameWithArray($postOnly)
91+
public function testHandleNonStringUsernameWithArray(bool $postOnly)
9192
{
92-
$this->expectException(BadRequestHttpException::class);
93-
$this->expectExceptionMessage('The key "_username" must be a string, "array" given.');
9493
$request = Request::create('/login_check', 'POST', ['_username' => []]);
9594
$request->setSession($this->createMock(SessionInterface::class));
9695
$listener = new UsernamePasswordFormAuthenticationListener(
@@ -104,16 +103,18 @@ public function testHandleNonStringUsernameWithArray($postOnly)
104103
['require_previous_session' => false, 'post_only' => $postOnly]
105104
);
106105
$event = new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST);
106+
107+
$this->expectException(BadRequestHttpException::class);
108+
$this->expectExceptionMessage('The key "_username" must be a string, "array" given.');
109+
107110
$listener($event);
108111
}
109112

110113
/**
111114
* @dataProvider postOnlyDataProvider
112115
*/
113-
public function testHandleNonStringUsernameWithInt($postOnly)
116+
public function testHandleNonStringUsernameWithInt(bool $postOnly)
114117
{
115-
$this->expectException(BadRequestHttpException::class);
116-
$this->expectExceptionMessage('The key "_username" must be a string, "int" given.');
117118
$request = Request::create('/login_check', 'POST', ['_username' => 42]);
118119
$request->setSession($this->createMock(SessionInterface::class));
119120
$listener = new UsernamePasswordFormAuthenticationListener(
@@ -127,16 +128,18 @@ public function testHandleNonStringUsernameWithInt($postOnly)
127128
['require_previous_session' => false, 'post_only' => $postOnly]
128129
);
129130
$event = new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST);
131+
132+
$this->expectException(BadRequestHttpException::class);
133+
$this->expectExceptionMessage('The key "_username" must be a string, "int" given.');
134+
130135
$listener($event);
131136
}
132137

133138
/**
134139
* @dataProvider postOnlyDataProvider
135140
*/
136-
public function testHandleNonStringUsernameWithObject($postOnly)
141+
public function testHandleNonStringUsernameWithObject(bool $postOnly)
137142
{
138-
$this->expectException(BadRequestHttpException::class);
139-
$this->expectExceptionMessage('The key "_username" must be a string, "stdClass" given.');
140143
$request = Request::create('/login_check', 'POST', ['_username' => new \stdClass()]);
141144
$request->setSession($this->createMock(SessionInterface::class));
142145
$listener = new UsernamePasswordFormAuthenticationListener(
@@ -150,13 +153,17 @@ public function testHandleNonStringUsernameWithObject($postOnly)
150153
['require_previous_session' => false, 'post_only' => $postOnly]
151154
);
152155
$event = new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST);
156+
157+
$this->expectException(BadRequestHttpException::class);
158+
$this->expectExceptionMessage('The key "_username" must be a string, "stdClass" given.');
159+
153160
$listener($event);
154161
}
155162

156163
/**
157164
* @dataProvider postOnlyDataProvider
158165
*/
159-
public function testHandleNonStringUsernameWith__toString($postOnly)
166+
public function testHandleNonStringUsernameWith__toString(bool $postOnly)
160167
{
161168
$usernameClass = $this->createMock(DummyUserClass::class);
162169
$usernameClass
@@ -204,21 +211,86 @@ public function testHandleWhenPasswordAreNull($postOnly)
204211
$listener($event);
205212
}
206213

207-
public function postOnlyDataProvider()
214+
/**
215+
* @dataProvider provideInvalidCsrfTokens
216+
*/
217+
public function testInvalidCsrfToken($invalidToken)
218+
{
219+
$formBody = ['_username' => 'fabien', '_password' => 'symfony'];
220+
if (null !== $invalidToken) {
221+
$formBody['_csrf_token'] = $invalidToken;
222+
}
223+
224+
$request = Request::create('/login_check', 'POST', $formBody);
225+
$request->setSession($this->createMock(SessionInterface::class));
226+
227+
$httpUtils = $this->createMock(HttpUtils::class);
228+
$httpUtils
229+
->method('checkRequestPath')
230+
->willReturn(true)
231+
;
232+
$httpUtils
233+
->method('createRedirectResponse')
234+
->willReturn(new RedirectResponse('/hello'))
235+
;
236+
237+
$failureHandler = $this->createMock(AuthenticationFailureHandlerInterface::class);
238+
$failureHandler
239+
->expects($this->once())
240+
->method('onAuthenticationFailure')
241+
->willReturn(new Response())
242+
;
243+
244+
$authenticationManager = $this->createMock(AuthenticationProviderManager::class);
245+
$authenticationManager
246+
->expects($this->never())
247+
->method('authenticate')
248+
;
249+
250+
$csrfTokenManager = $this->createMock(CsrfTokenManagerInterface::class);
251+
$csrfTokenManager->method('isTokenValid')->willReturn(false);
252+
253+
$listener = new UsernamePasswordFormAuthenticationListener(
254+
$this->createMock(TokenStorageInterface::class),
255+
$authenticationManager,
256+
$this->createMock(SessionAuthenticationStrategyInterface::class),
257+
$httpUtils,
258+
'TheProviderKey',
259+
new DefaultAuthenticationSuccessHandler($httpUtils),
260+
$failureHandler,
261+
['require_previous_session' => false],
262+
null,
263+
null,
264+
$csrfTokenManager
265+
);
266+
267+
$listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MASTER_REQUEST));
268+
}
269+
270+
public function postOnlyDataProvider(): array
208271
{
209272
return [
210273
[true],
211274
[false],
212275
];
213276
}
214277

215-
public function getUsernameForLength()
278+
public function getUsernameForLength(): array
216279
{
217280
return [
218281
[str_repeat('x', Security::MAX_USERNAME_LENGTH + 1), false],
219282
[str_repeat('x', Security::MAX_USERNAME_LENGTH - 1), true],
220283
];
221284
}
285+
286+
public function provideInvalidCsrfTokens(): array
287+
{
288+
return [
289+
['invalid'],
290+
[['in' => 'valid']],
291+
[null],
292+
];
293+
}
222294
}
223295

224296
class DummyUserClass

0 commit comments

Comments
 (0)