Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit e8075f0

Browse filesBrowse files
derrabusderrabus
committed
Don't produce TypeErrors for non-string CSRF tokens
Signed-off-by: Alexander M. Turek <me@derrabus.de>
1 parent 66817c2 commit e8075f0
Copy full SHA for e8075f0

File tree

5 files changed

+106
-20
lines changed
Filter options

5 files changed

+106
-20
lines changed

‎src/Symfony/Component/Security/Http/Firewall/LogoutListener.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/Firewall/LogoutListener.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ public function authenticate(RequestEvent $event)
8787
if (null !== $this->csrfTokenManager) {
8888
$csrfToken = ParameterBagUtils::getRequestParameterValue($request, $this->options['csrf_parameter']);
8989

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

‎src/Symfony/Component/Security/Http/Firewall/SimpleFormAuthenticationListener.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/Firewall/SimpleFormAuthenticationListener.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ protected function attemptAuthentication(Request $request)
8484
if (null !== $this->csrfTokenManager) {
8585
$csrfToken = ParameterBagUtils::getRequestParameterValue($request, $this->options['csrf_parameter']);
8686

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

‎src/Symfony/Component/Security/Http/Firewall/UsernamePasswordFormAuthenticationListener.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/Firewall/UsernamePasswordFormAuthenticationListener.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ protected function attemptAuthentication(Request $request)
7272
if (null !== $this->csrfTokenManager) {
7373
$csrfToken = ParameterBagUtils::getRequestParameterValue($request, $this->options['csrf_parameter']);
7474

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

‎src/Symfony/Component/Security/Http/Tests/Firewall/LogoutListenerTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/Tests/Firewall/LogoutListenerTest.php
+17-3Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -152,28 +152,42 @@ public function testSuccessHandlerReturnsNonResponse()
152152
$listener($event);
153153
}
154154

155-
public function testCsrfValidationFails()
155+
/**
156+
* @dataProvider provideInvalidCsrfTokens
157+
*/
158+
public function testCsrfValidationFails($invalidToken)
156159
{
157160
$this->expectException(LogoutException::class);
158161
$tokenManager = $this->getTokenManager();
159162

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

162165
$request = new Request();
163-
$request->query->set('_csrf_token', 'token');
166+
if (null !== $invalidToken) {
167+
$request->query->set('_csrf_token', $invalidToken);
168+
}
164169

165170
$httpUtils->expects($this->once())
166171
->method('checkRequestPath')
167172
->with($request, $options['logout_path'])
168173
->willReturn(true);
169174

170-
$tokenManager->expects($this->once())
175+
$tokenManager
171176
->method('isTokenValid')
172177
->willReturn(false);
173178

174179
$listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MASTER_REQUEST));
175180
}
176181

182+
public function provideInvalidCsrfTokens(): array
183+
{
184+
return [
185+
['invalid'],
186+
[['in' => 'valid']],
187+
[null],
188+
];
189+
}
190+
177191
private function getTokenManager()
178192
{
179193
return $this->createMock(CsrfTokenManagerInterface::class);

‎src/Symfony/Component/Security/Http/Tests/Firewall/UsernamePasswordFormAuthenticationListenerTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/Tests/Firewall/UsernamePasswordFormAuthenticationListenerTest.php
+86-14Lines changed: 86 additions & 14 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;
@@ -37,7 +38,7 @@ class UsernamePasswordFormAuthenticationListenerTest extends TestCase
3738
/**
3839
* @dataProvider getUsernameForLength
3940
*/
40-
public function testHandleWhenUsernameLength($username, $ok)
41+
public function testHandleWhenUsernameLength(string $username, bool $ok)
4142
{
4243
$request = Request::create('/login_check', 'POST', ['_username' => $username]);
4344
$request->setSession($this->createMock(SessionInterface::class));
@@ -75,7 +76,7 @@ public function testHandleWhenUsernameLength($username, $ok)
7576
'TheProviderKey',
7677
new DefaultAuthenticationSuccessHandler($httpUtils),
7778
$failureHandler,
78-
['require_previous_session' => false]
79+
['require_previous_session' => false],
7980
);
8081

8182
$listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MASTER_REQUEST));
@@ -84,10 +85,8 @@ public function testHandleWhenUsernameLength($username, $ok)
8485
/**
8586
* @dataProvider postOnlyDataProvider
8687
*/
87-
public function testHandleNonStringUsernameWithArray($postOnly)
88+
public function testHandleNonStringUsernameWithArray(bool $postOnly)
8889
{
89-
$this->expectException(BadRequestHttpException::class);
90-
$this->expectExceptionMessage('The key "_username" must be a string, "array" given.');
9190
$request = Request::create('/login_check', 'POST', ['_username' => []]);
9291
$request->setSession($this->createMock(SessionInterface::class));
9392
$listener = new UsernamePasswordFormAuthenticationListener(
@@ -101,16 +100,18 @@ public function testHandleNonStringUsernameWithArray($postOnly)
101100
['require_previous_session' => false, 'post_only' => $postOnly]
102101
);
103102
$event = new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MASTER_REQUEST);
103+
104+
$this->expectException(BadRequestHttpException::class);
105+
$this->expectExceptionMessage('The key "_username" must be a string, "array" given.');
106+
104107
$listener($event);
105108
}
106109

107110
/**
108111
* @dataProvider postOnlyDataProvider
109112
*/
110-
public function testHandleNonStringUsernameWithInt($postOnly)
113+
public function testHandleNonStringUsernameWithInt(bool $postOnly)
111114
{
112-
$this->expectException(BadRequestHttpException::class);
113-
$this->expectExceptionMessage('The key "_username" must be a string, "integer" given.');
114115
$request = Request::create('/login_check', 'POST', ['_username' => 42]);
115116
$request->setSession($this->createMock(SessionInterface::class));
116117
$listener = new UsernamePasswordFormAuthenticationListener(
@@ -124,16 +125,18 @@ public function testHandleNonStringUsernameWithInt($postOnly)
124125
['require_previous_session' => false, 'post_only' => $postOnly]
125126
);
126127
$event = new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MASTER_REQUEST);
128+
129+
$this->expectException(BadRequestHttpException::class);
130+
$this->expectExceptionMessage('The key "_username" must be a string, "integer" given.');
131+
127132
$listener($event);
128133
}
129134

130135
/**
131136
* @dataProvider postOnlyDataProvider
132137
*/
133-
public function testHandleNonStringUsernameWithObject($postOnly)
138+
public function testHandleNonStringUsernameWithObject(bool $postOnly)
134139
{
135-
$this->expectException(BadRequestHttpException::class);
136-
$this->expectExceptionMessage('The key "_username" must be a string, "object" given.');
137140
$request = Request::create('/login_check', 'POST', ['_username' => new \stdClass()]);
138141
$request->setSession($this->createMock(SessionInterface::class));
139142
$listener = new UsernamePasswordFormAuthenticationListener(
@@ -147,13 +150,17 @@ public function testHandleNonStringUsernameWithObject($postOnly)
147150
['require_previous_session' => false, 'post_only' => $postOnly]
148151
);
149152
$event = new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MASTER_REQUEST);
153+
154+
$this->expectException(BadRequestHttpException::class);
155+
$this->expectExceptionMessage('The key "_username" must be a string, "object" given.');
156+
150157
$listener($event);
151158
}
152159

153160
/**
154161
* @dataProvider postOnlyDataProvider
155162
*/
156-
public function testHandleNonStringUsernameWith__toString($postOnly)
163+
public function testHandleNonStringUsernameWith__toString(bool $postOnly)
157164
{
158165
$usernameClass = $this->createMock(DummyUserClass::class);
159166
$usernameClass
@@ -177,21 +184,86 @@ public function testHandleNonStringUsernameWith__toString($postOnly)
177184
$listener($event);
178185
}
179186

180-
public function postOnlyDataProvider()
187+
/**
188+
* @dataProvider provideInvalidCsrfTokens
189+
*/
190+
public function testInvalidCsrfToken($invalidToken)
191+
{
192+
$formBody = ['_username' => 'fabien', '_password' => 'symfony'];
193+
if (null !== $invalidToken) {
194+
$formBody['_csrf_token'] = $invalidToken;
195+
}
196+
197+
$request = Request::create('/login_check', 'POST', $formBody);
198+
$request->setSession($this->createMock(SessionInterface::class));
199+
200+
$httpUtils = $this->createMock(HttpUtils::class);
201+
$httpUtils
202+
->method('checkRequestPath')
203+
->willReturn(true)
204+
;
205+
$httpUtils
206+
->method('createRedirectResponse')
207+
->willReturn(new RedirectResponse('/hello'))
208+
;
209+
210+
$failureHandler = $this->createMock(AuthenticationFailureHandlerInterface::class);
211+
$failureHandler
212+
->expects($this->once())
213+
->method('onAuthenticationFailure')
214+
->willReturn(new Response())
215+
;
216+
217+
$authenticationManager = $this->createMock(AuthenticationProviderManager::class);
218+
$authenticationManager
219+
->expects($this->never())
220+
->method('authenticate')
221+
;
222+
223+
$csrfTokenManager = $this->createMock(CsrfTokenManagerInterface::class);
224+
$csrfTokenManager->method('isTokenValid')->willReturn(false);
225+
226+
$listener = new UsernamePasswordFormAuthenticationListener(
227+
$this->createMock(TokenStorageInterface::class),
228+
$authenticationManager,
229+
$this->createMock(SessionAuthenticationStrategyInterface::class),
230+
$httpUtils,
231+
'TheProviderKey',
232+
new DefaultAuthenticationSuccessHandler($httpUtils),
233+
$failureHandler,
234+
['require_previous_session' => false],
235+
null,
236+
null,
237+
$csrfTokenManager
238+
);
239+
240+
$listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MASTER_REQUEST));
241+
}
242+
243+
public function postOnlyDataProvider(): array
181244
{
182245
return [
183246
[true],
184247
[false],
185248
];
186249
}
187250

188-
public function getUsernameForLength()
251+
public function getUsernameForLength(): array
189252
{
190253
return [
191254
[str_repeat('x', Security::MAX_USERNAME_LENGTH + 1), false],
192255
[str_repeat('x', Security::MAX_USERNAME_LENGTH - 1), true],
193256
];
194257
}
258+
259+
public function provideInvalidCsrfTokens(): array
260+
{
261+
return [
262+
['invalid'],
263+
[['in' => 'valid']],
264+
[null],
265+
];
266+
}
195267
}
196268

197269
class DummyUserClass

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.