8000 [Security] Fix fatal error on non string username/password · symfony/symfony@a3e17da · GitHub
[go: up one dir, main page]
Skip to content

Commit a3e17da

Browse files
Robin Chalaschalasr
Robin Chalas
authored and
chalasr
committed
[Security] Fix fatal error on non string username/password
1 parent 28485af commit a3e17da

File tree

3 files changed

+88
-13
lines changed

3 files changed

+88
-13
lines changed

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderAdapter;
1616
use Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface;
1717
use Symfony\Component\HttpFoundation\Request;
18+
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
1819
use Symfony\Component\Security\Core\Exception\InvalidArgumentException;
1920
use Symfony\Component\Security\Core\Exception\InvalidCsrfTokenException;
2021
use Symfony\Component\Security\Csrf\CsrfToken;
@@ -107,18 +108,22 @@ protected function attemptAuthentication(Request $request)
107108
}
108109
}
109110

110-
if ($this->options['post_only']) {
111-
$username = trim($request->request->get($this->options['username_parameter'], null, true));
112-
$password = $request->request->get($this->options['password_parameter'], null, true);
113-
} else {
114-
$username = trim($request->get($this->options['username_parameter'], null, true));
115-
$password = $request->get($this->options['password_parameter'], null, true);
111+
$requestBag = $this->options['post_only'] ? $request->request : $request;
112+
113+
if (!is_string($username = $requestBag->get($this->options['username_parameter'], null, true))) {
114+
throw new BadRequestHttpException(sprintf('The key "%s" must be a string, "%s" given.', $this->options['username_parameter'], \gettype($username)));
116115
}
117116

117+
$username = trim($username);
118+
118119
if (strlen($username) > Security::MAX_USERNAME_LENGTH) {
119120
throw new BadCredentialsException('Invalid username.');
120121
}
121122

123+
if (!is_string($password = $requestBag->get($this->options['password_parameter'], null, true))) {
124+
throw new BadRequestHttpException(sprintf('The key "%s" must be a string, "%s" given.', $this->options['password_parameter'], \gettype($password)));
125+
}
126+
122127
$request->getSession()->set(Security::LAST_USERNAME, $username);
123128

124129
$token = $this->simpleAuthenticator->createToken($request, $username, $password, $this->providerKey);

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface;
1616
use Symfony\Component\HttpFoundation\Request;
1717
use Psr\Log\LoggerInterface;
18+
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
1819
use Symfony\Component\Security\Csrf\CsrfToken;
1920
use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
2021
use Symfony\Component\Security\Http\Authentication\AuthenticationFailureHandlerInterface;
@@ -84,18 +85,22 @@ protected function attemptAuthentication(Request $request)
8485
}
8586
}
8687

87-
if ($this->options['post_only']) {
88-
$username = trim($request->request->get($this->options['username_parameter'], null, true));
89-
$password = $request->request->get($this->options['password_parameter'], null, true);
90-
} else {
91-
$username = trim($request->get($this->options['username_parameter'], null, true));
92-
$password = $request->get($this->options['password_parameter'], null, true);
88+
$requestBag = $this->options['post_only'] ? $request->request : $request;
89+
90+
if (!is_string($username = $requestBag->get($this->options['username_parameter'], null, true))) {
91+
throw new BadRequestHttpException(sprintf('The key "%s" must be a string, "%s" given.', $this->options['username_parameter'], \gettype($username)));
9392
}
9493

94+
$username = trim($username);
95+
9596
if (strlen($username) > Security::MAX_USERNAME_LENGTH) {
9697
throw new BadCredentialsException('Invalid username.');
9798
}
9899

100+
if (!is_string($password = $requestBag->get($this->options['password_parameter'], null, true))) {
101+
throw new BadRequestHttpException(sprintf('The key "%s" must be a string, "%s" given.', $this->options['password_parameter'], \gettype($password)));
102+
}
103+
99104
$request->getSession()->set(Security::LAST_USERNAME, $username);
100105

101106
return $this->authenticationManager->authenticate(new UsernamePasswordToken($username, $password, $this->providerKey));

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

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,13 @@
1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Component\HttpFoundation\Request;
1616
use Symfony\Component\HttpFoundation\Response;
17+
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
18+
use Symfony\Component\Security\Http\Authentication\DefaultAuthenticationFailureHandler;
19+
use Symfony\Component\Security\Http\Authentication\DefaultAuthenticationSuccessHandler;
1720
use Symfony\Component\Security\Http\Firewall\UsernamePasswordFormAuthenticationListener;
1821
use Symfony\Component\Security\Core\SecurityContextInterface;
22+
use Symfony\Component\Security\Http\HttpUtils;
23+
use Symfony\Component\Security\Http\Session\SessionAuthenticationStrategy;
1924

2025
class UsernamePasswordFormAuthenticationListenerTest extends TestCase
2126
{
@@ -24,7 +29,7 @@ class UsernamePasswordFormAuthenticationListenerTest extends TestCase
2429
*/
2530
public function testHandleWhenUsernameLength($username, $ok)
2631
{
27-
$request = Request::create('/login_check', 'POST', array('_username' => $username));
32+
$request = Request::create('/login_check', 'POST', array('_username' => $username, ''));
2833
$request->setSession($this->getMockBuilder('Symfony\Component\HttpFoundation\Session\SessionInterface')->getMock());
2934

3035
$httpUtils = $this->getMockBuilder('Symfony\Component\Security\Http\HttpUtils')->getMock();
@@ -69,6 +74,66 @@ public function testHandleWhenUsernameLength($username, $ok)
6974
$listener->handle($event);
7075
}
7176

77+
/**
78+
* @expectedException \Symfony\Component\HttpKernel\Exception\BadRequestHttpException
79+
* @expectedExceptionMessage The key "_username" must be a string, "array" given.
80+
*/
81+
public function testHandleNonStringUsername()
82+
{
83+
$request = Request::create('/login_check', 'POST', array('_username' => array()));
84+
$request->setSession($this->getMockBuilder('Symfony\Component\HttpFoundation\Session\SessionInterface')->getMock());
85+
86+
$listener = new UsernamePasswordFormAuthenticationListener(
87+
new TokenStorage(),
88+
$this->getMockBuilder('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface')->getMock(),
89+
new SessionAuthenticationStrategy(SessionAuthenticationStrategy::NONE),
90+
$httpUtils = new HttpUtils(),
91+
'foo',
92+
new DefaultAuthenticationSuccessHandler($httpUtils),
93+
new DefaultAuthenticationFailureHandler($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $httpUtils),
94+
array('require_previous_session' => false)
95+
);
96+
97+
$event = $this->getMockBuilder('Symfony\Component\HttpKernel\Event\GetResponseEvent')->disableOriginalConstructor()->getMock();
98+
$event
99+
->expects($this->any())
100+
->method('getRequest')
101+
->will($this->returnValue($request))
102+
;
103+
104+
$listener->handle($event);
105+
}
106+
107+
/**
108+
* @expectedException \Symfony\Component\HttpKernel\Exception\BadRequestHttpException
109+
* @expectedExceptionMessage The key "_password" must be a string, "array" given.
110+
*/
111+
public function testHandleNonStringPassword()
112+
{
113+
$request = Request::create('/login_check', 'POST', array('_username' => 'foo', '_password' => array()));
114+
$request->setSession($this->getMockBuilder('Symfony\Component\HttpFoundation\Session\SessionInterface')->getMock());
115+
116+
$listener = new UsernamePasswordFormAuthenticationListener(
117+
new TokenStorage(),
118+
$this->getMockBuilder('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface')->getMock(),
119+
new SessionAuthenticationStrategy(SessionAuthenticationStrategy::NONE),
120+
$httpUtils = new HttpUtils(),
121+
'foo',
122+
new DefaultAuthenticationSuccessHandler($httpUtils),
123+
new DefaultAuthenticationFailureHandler($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $httpUtils),
124+
array('require_previous_session' => false)
125+
);
126+
127+
$event = $this->getMockBuilder('Symfony\Component\HttpKernel\Event\GetResponseEvent')->disableOriginalConstructor()->getMock();
128+
$event
129+
->expects($this->any())
130+
->method('getRequest')
131+
->will($this->returnValue($request))
132+
;
133+
134+
$listener->handle($event);
135+
}
136+
72137
public function getUsernameForLength()
73138
{
74139
return array(

0 commit comments

Comments
 (0)
0