bug #33799 [Security]: Don't let falsy usernames slip through impersonation (j4nr6n)
This PR was merged into the 3.4 branch.
Discussion
----------
[Security]: Don't let falsy usernames slip through impersonation
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR |
When you try to impersonate users with a falsy username, `SwitchUserListener::handle()` would `return;` and impersonation would fail.
I'm using a third party OAuth provider that allows users to change their usernames with no guaranteed protection against re-use. To overcome that issue, I implemented `UserLoaderInterface::loadUserByUsername()` and query by a `providerId`.
After loading development fixtures, One user has `0` as it's `providerId`.
Commits
-------
64aecab0a7
Don't let falsey usernames slip through
This commit is contained in:
commit
8622c8c95e
@ -77,9 +77,16 @@ class SwitchUserListener implements ListenerInterface
|
|||||||
public function handle(GetResponseEvent $event)
|
public function handle(GetResponseEvent $event)
|
||||||
{
|
{
|
||||||
$request = $event->getRequest();
|
$request = $event->getRequest();
|
||||||
$username = $request->get($this->usernameParameter) ?: $request->headers->get($this->usernameParameter);
|
|
||||||
|
|
||||||
if (!$username) {
|
// usernames can be falsy
|
||||||
|
$username = $request->get($this->usernameParameter);
|
||||||
|
|
||||||
|
if (null === $username || '' === $username) {
|
||||||
|
$username = $request->headers->get($this->usernameParameter);
|
||||||
|
}
|
||||||
|
|
||||||
|
// if it's still "empty", nothing to do.
|
||||||
|
if (null === $username || '' === $username) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -191,6 +191,32 @@ class SwitchUserListenerTest extends TestCase
|
|||||||
$this->assertInstanceOf('Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken', $this->tokenStorage->getToken());
|
$this->assertInstanceOf('Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken', $this->tokenStorage->getToken());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testSwitchUserWorksWithFalsyUsernames()
|
||||||
|
{
|
||||||
|
$token = new UsernamePasswordToken('username', '', 'key', ['ROLE_FOO']);
|
||||||
|
$user = new User('username', 'password', []);
|
||||||
|
|
||||||
|
$this->tokenStorage->setToken($token);
|
||||||
|
$this->request->query->set('_switch_user', '0');
|
||||||
|
|
||||||
|
$this->accessDecisionManager->expects($this->once())
|
||||||
|
->method('decide')->with($token, ['ROLE_ALLOWED_TO_SWITCH'])
|
||||||
|
->willReturn(true);
|
||||||
|
|
||||||
|
$this->userProvider->expects($this->once())
|
||||||
|
->method('loadUserByUsername')->with('0')
|
||||||
|
->willReturn($user);
|
||||||
|
$this->userChecker->expects($this->once())
|
||||||
|
->method('checkPostAuth')->with($user);
|
||||||
|
|
||||||
|
$listener = new SwitchUserListener($this->tokenStorage, $this->userProvider, $this->userChecker, 'provider123', $this->accessDecisionManager);
|
||||||
|
$listener->handle($this->event);
|
||||||
|
|
||||||
|
$this->assertSame([], $this->request->query->all());
|
||||||
|
$this->assertSame('', $this->request->server->get('QUERY_STRING'));
|
||||||
|
$this->assertInstanceOf('Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken', $this->tokenStorage->getToken());
|
||||||
|
}
|
||||||
|
|
||||||
public function testSwitchUserKeepsOtherQueryStringParameters()
|
public function testSwitchUserKeepsOtherQueryStringParameters()
|
||||||
{
|
{
|
||||||
$token = new UsernamePasswordToken('username', '', 'key', ['ROLE_FOO']);
|
$token = new UsernamePasswordToken('username', '', 'key', ['ROLE_FOO']);
|
||||||
|
Reference in New Issue
Block a user