Skip to content

Commit

Permalink
bug #25657 [Security] Fix fatal error on non string username (chalasr)
Browse files Browse the repository at this point in the history
This PR was merged into the 2.7 branch.

Discussion
----------

[Security] Fix fatal error on non string username

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #25612
| License       | MIT
| Doc PR        | n/a

That's consistent with what #22569 did for the `json_login` listener.

Commits
-------

8f09568 [Security] Fix fatal error on non string username
  • Loading branch information
fabpot committed Jan 16, 2018
2 parents 06ef68f + 8f09568 commit 6c16252
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 13 deletions.
Expand Up @@ -15,6 +15,7 @@
use Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderAdapter;
use Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
use Symfony\Component\Security\Core\Exception\InvalidArgumentException;
use Symfony\Component\Security\Core\Exception\InvalidCsrfTokenException;
use Symfony\Component\Security\Csrf\CsrfToken;
Expand Down Expand Up @@ -107,15 +108,17 @@ protected function attemptAuthentication(Request $request)
}
}

if ($this->options['post_only']) {
$username = trim($request->request->get($this->options['username_parameter'], null, true));
$password = $request->request->get($this->options['password_parameter'], null, true);
} else {
$username = trim($request->get($this->options['username_parameter'], null, true));
$password = $request->get($this->options['password_parameter'], null, true);
$requestBag = $this->options['post_only'] ? $request->request : $request;
$username = $requestBag->get($this->options['username_parameter'], null, true);
$password = $requestBag->get($this->options['password_parameter'], null, true);

if (!\is_string($username) || (\is_object($username) && !\method_exists($username, '__toString'))) {
throw new BadRequestHttpException(sprintf('The key "%s" must be a string, "%s" given.', $this->options['username_parameter'], \gettype($username)));
}

if (strlen($username) > Security::MAX_USERNAME_LENGTH) {
$username = trim($username);

if (\strlen($username) > Security::MAX_USERNAME_LENGTH) {
throw new BadCredentialsException('Invalid username.');
}

Expand Down
Expand Up @@ -15,6 +15,7 @@
use Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface;
use Symfony\Component\HttpFoundation\Request;
use Psr\Log\LoggerInterface;
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
use Symfony\Component\Security\Csrf\CsrfToken;
use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
use Symfony\Component\Security\Http\Authentication\AuthenticationFailureHandlerInterface;
Expand Down Expand Up @@ -84,14 +85,16 @@ protected function attemptAuthentication(Request $request)
}
}

if ($this->options['post_only']) {
$username = trim($request->request->get($this->options['username_parameter'], null, true));
$password = $request->request->get($this->options['password_parameter'], null, true);
} else {
$username = trim($request->get($this->options['username_parameter'], null, true));
$password = $request->get($this->options['password_parameter'], null, true);
$requestBag = $this->options['post_only'] ? $request->request : $request;
$username = $requestBag->get($this->options['username_parameter'], null, true);
$password = $requestBag->get($this->options['password_parameter'], null, true);

if (!\is_string($username) || (\is_object($username) && !\method_exists($username, '__toString'))) {
throw new BadRequestHttpException(sprintf('The key "%s" must be a string, "%s" given.', $this->options['username_parameter'], \gettype($username)));
}

$username = trim($username);

if (strlen($username) > Security::MAX_USERNAME_LENGTH) {
throw new BadCredentialsException('Invalid username.');
}
Expand Down
Expand Up @@ -14,8 +14,15 @@
use PHPUnit\Framework\TestCase;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
use Symfony\Component\Security\Http\Authentication\DefaultAuthenticationFailureHandler;
use Symfony\Component\Security\Http\Authentication\DefaultAuthenticationSuccessHandler;
use Symfony\Component\Security\Http\Firewall\UsernamePasswordFormAuthenticationListener;
use Symfony\Component\Security\Core\SecurityContextInterface;
use Symfony\Component\Security\Http\HttpUtils;
use Symfony\Component\Security\Http\Session\SessionAuthenticationStrategy;

class UsernamePasswordFormAuthenticationListenerTest extends TestCase
{
Expand Down Expand Up @@ -69,6 +76,31 @@ public function testHandleWhenUsernameLength($username, $ok)
$listener->handle($event);
}

/**
* @expectedException \Symfony\Component\HttpKernel\Exception\BadRequestHttpException
* @expectedExceptionMessage The key "_username" must be a string, "array" given.
*/
public function testHandleNonStringUsername()
{
$request = Request::create('/login_check', 'POST', array('_username' => array()));
$request->setSession($this->getMockBuilder('Symfony\Component\HttpFoundation\Session\SessionInterface')->getMock());

$listener = new UsernamePasswordFormAuthenticationListener(
new TokenStorage(),
$this->getMockBuilder('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface')->getMock(),
new SessionAuthenticationStrategy(SessionAuthenticationStrategy::NONE),
$httpUtils = new HttpUtils(),
'foo',
new DefaultAuthenticationSuccessHandler($httpUtils),
new DefaultAuthenticationFailureHandler($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $httpUtils),
array('require_previous_session' => false)
);

$event = new GetResponseEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $request, HttpKernelInterface::MASTER_REQUEST);

$listener->handle($event);
}

public function getUsernameForLength()
{
return array(
Expand Down

0 comments on commit 6c16252

Please sign in to comment.