Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
bug #30347 [Security] Change FormAuthenticator if condition (PReimers)
This PR was squashed before being merged into the 3.4 branch (closes #30347).

Discussion
----------

[Security] Change FormAuthenticator if condition

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #30341
| License       | MIT
| Doc PR        | -

I changed the if condition in `SimpleFormAuthenticationListener` and `UsernamePasswordFormAuthenticationListener` based on the solution provided by @nikic in issue #30341

#OpenSourceFriday

Commits
-------

67ae121 [Security] Change FormAuthenticator if condition
  • Loading branch information
nicolas-grekas committed Feb 23, 2019
2 parents 173b5ea + 67ae121 commit 1aac865
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 3 deletions.
Expand Up @@ -107,7 +107,7 @@ protected function attemptAuthentication(Request $request)
$password = ParameterBagUtils::getRequestParameterValue($request, $this->options['password_parameter']);
}

if (!\is_string($username) || (\is_object($username) && !\method_exists($username, '__toString'))) {
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)));
}

Expand Down
Expand Up @@ -85,7 +85,7 @@ protected function attemptAuthentication(Request $request)
$password = ParameterBagUtils::getRequestParameterValue($request, $this->options['password_parameter']);
}

if (!\is_string($username) || (\is_object($username) && !\method_exists($username, '__toString'))) {
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)));
}

Expand Down
Expand Up @@ -81,7 +81,7 @@ public function testHandleWhenUsernameLength($username, $ok)
* @expectedException \Symfony\Component\HttpKernel\Exception\BadRequestHttpException
* @expectedExceptionMessage The key "_username" must be a string, "array" given.
*/
public function testHandleNonStringUsername($postOnly)
public function testHandleNonStringUsernameWithArray($postOnly)
{
$request = Request::create('/login_check', 'POST', ['_username' => []]);
$request->setSession($this->getMockBuilder('Symfony\Component\HttpFoundation\Session\SessionInterface')->getMock());
Expand All @@ -99,6 +99,79 @@ public function testHandleNonStringUsername($postOnly)
$listener->handle($event);
}

/**
* @dataProvider postOnlyDataProvider
* @expectedException \Symfony\Component\HttpKernel\Exception\BadRequestHttpException
* @expectedExceptionMessage The key "_username" must be a string, "integer" given.
*/
public function testHandleNonStringUsernameWithInt($postOnly)
{
$request = Request::create('/login_check', 'POST', ['_username' => 42]);
$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),
['require_previous_session' => false, 'post_only' => $postOnly]
);
$event = new GetResponseEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $request, HttpKernelInterface::MASTER_REQUEST);
$listener->handle($event);
}

/**
* @dataProvider postOnlyDataProvider
* @expectedException \Symfony\Component\HttpKernel\Exception\BadRequestHttpException
* @expectedExceptionMessage The key "_username" must be a string, "object" given.
*/
public function testHandleNonStringUsernameWithObject($postOnly)
{
$request = Request::create('/login_check', 'POST', ['_username' => new \stdClass()]);
$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),
['require_previous_session' => false, 'post_only' => $postOnly]
);
$event = new GetResponseEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $request, HttpKernelInterface::MASTER_REQUEST);
$listener->handle($event);
}

/**
* @dataProvider postOnlyDataProvider
*/
public function testHandleNonStringUsernameWith__toString($postOnly)
{
$usernameClass = $this->getMockBuilder(DummyUserClass::class)->getMock();
$usernameClass
->expects($this->atLeastOnce())
->method('__toString')
->will($this->returnValue('someUsername'));

$request = Request::create('/login_check', 'POST', ['_username' => $usernameClass]);
$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),
['require_previous_session' => false, 'post_only' => $postOnly]
);
$event = new GetResponseEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $request, HttpKernelInterface::MASTER_REQUEST);
$listener->handle($event);
}

public function postOnlyDataProvider()
{
return [
Expand All @@ -115,3 +188,11 @@ public function getUsernameForLength()
];
}
}

class DummyUserClass
{
public function __toString()
{
return '';
}
}

0 comments on commit 1aac865

Please sign in to comment.