bug #37943 [Security] Fixed RememberMeAuthenticator::autoLogin() logic in the authenticator (wouterj)

This PR was merged into the 5.1 branch.

Discussion
----------

[Security] Fixed RememberMeAuthenticator::autoLogin() logic in the authenticator

| Q             | A
| ------------- | ---
| Branch?       | 5.1
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #37534
| License       | MIT
| Doc PR        | n/a

The `RememberMeAuthenticator` wrongly assumed the implementation details of `AbstractRememberMeServices::autoLogin()`. This means that (a) the authenticator did not work with other - custom - implementations of `RememberMeServicesInterface` and (b) there was a potentional to get an "Call to a member function getUser() on null" error.

This code removes all assumptions of the `autoLogin()` logic, other than that stated in the PHPdoc: 32ca714e93/src/Symfony/Component/Security/Http/RememberMe/RememberMeServicesInterface.php (L43-L53)

Commits
-------

93aea910d9 Fixed autoLogin() returning null
This commit is contained in:
Fabien Potencier 2020-08-25 21:08:41 +02:00
commit df7950d0e3
2 changed files with 21 additions and 29 deletions

View File

@ -19,7 +19,6 @@ use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Exception\AuthenticationException;
use Symfony\Component\Security\Http\Authenticator\Passport\PassportInterface;
use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPassport;
use Symfony\Component\Security\Http\RememberMe\AbstractRememberMeServices;
use Symfony\Component\Security\Http\RememberMe\RememberMeServicesInterface;
/**
@ -57,13 +56,12 @@ class RememberMeAuthenticator implements InteractiveAuthenticatorInterface
return false;
}
if (($cookie = $request->attributes->get(AbstractRememberMeServices::COOKIE_ATTR_NAME)) && null === $cookie->getValue()) {
$token = $this->rememberMeServices->autoLogin($request);
if (null === $token) {
return false;
}
if (isset($this->options['name']) && !$request->cookies->has($this->options['name'])) {
return false;
}
$request->attributes->set('_remember_me_token', $token);
// the `null` return value indicates that this authenticator supports lazy firewalls
return null;
@ -71,7 +69,10 @@ class RememberMeAuthenticator implements InteractiveAuthenticatorInterface
public function authenticate(Request $request): PassportInterface
{
$token = $this->rememberMeServices->autoLogin($request);
$token = $request->attributes->get('_remember_me_token');
if (null === $token) {
throw new \LogicException('No remember me token is set.');
}
return new SelfValidatingPassport($token->getUser());
}

View File

@ -12,14 +12,12 @@
namespace Symfony\Component\Security\Http\Tests\Authenticator;
use PHPUnit\Framework\TestCase;
use Symfony\Component\HttpFoundation\Cookie;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Security\Core\Authentication\Token\RememberMeToken;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\User\User;
use Symfony\Component\Security\Http\Authenticator\RememberMeAuthenticator;
use Symfony\Component\Security\Http\RememberMe\AbstractRememberMeServices;
use Symfony\Component\Security\Http\RememberMe\RememberMeServicesInterface;
class RememberMeAuthenticatorTest extends TestCase
@ -37,8 +35,6 @@ class RememberMeAuthenticatorTest extends TestCase
'name' => '_remember_me_cookie',
]);
$this->request = new Request();
$this->request->cookies->set('_remember_me_cookie', $val = $this->generateCookieValue());
$this->request->attributes->set(AbstractRememberMeServices::COOKIE_ATTR_NAME, new Cookie('_remember_me_cookie', $val));
}
public function testSupportsTokenStorageWithToken()
@ -48,39 +44,34 @@ class RememberMeAuthenticatorTest extends TestCase
$this->assertFalse($this->authenticator->supports($this->request));
}
public function testSupportsRequestWithoutAttribute()
/**
* @dataProvider provideSupportsData
*/
public function testSupports($autoLoginResult, $support)
{
$this->request->attributes->remove(AbstractRememberMeServices::COOKIE_ATTR_NAME);
$this->rememberMeServices->expects($this->once())->method('autoLogin')->with($this->request)->willReturn($autoLoginResult);
$this->assertNull($this->authenticator->supports($this->request));
$this->assertSame($support, $this->authenticator->supports($this->request));
}
public function testSupportsRequestWithoutCookie()
public function provideSupportsData()
{
$this->request->cookies->remove('_remember_me_cookie');
$this->assertFalse($this->authenticator->supports($this->request));
}
public function testSupports()
{
$this->assertNull($this->authenticator->supports($this->request));
yield [null, false];
yield [$this->createMock(TokenInterface::class), null];
}
public function testAuthenticate()
{
$this->rememberMeServices->expects($this->once())
->method('autoLogin')
->with($this->request)
->willReturn(new RememberMeToken($user = new User('wouter', 'test'), 'main', 'secret'));
$this->request->attributes->set('_remember_me_token', new RememberMeToken($user = new User('wouter', 'test'), 'main', 'secret'));
$passport = $this->authenticator->authenticate($this->request);
$this->assertSame($user, $passport->getUser());
}
private function generateCookieValue()
public function testAuthenticateWithoutToken()
{
return base64_encode(implode(AbstractRememberMeServices::COOKIE_DELIMITER, ['part1', 'part2']));
$this->expectException(\LogicException::class);
$this->authenticator->authenticate($this->request);
}
}