Skip to content

[Security] make TokenInterface::getUser() nullable to tell about unauthenticated tokens #42650

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 20, 2021
Merged

[Security] make TokenInterface::getUser() nullable to tell about unauthenticated tokens #42650

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions UPGRADE-5.4.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ Security
behavior when using `enable_authenticator_manager: true`)
* Deprecate not setting the `$exceptionOnNoToken` argument of `AccessListener` to `false`
(this is the default behavior when using `enable_authenticator_manager: true`)
* Deprecate `TokenInterface:isAuthenticated()` and `setAuthenticated()` methods without replacement.
Security tokens won't have an "authenticated" flag anymore, so they will always be considered authenticated
* Deprecate `TokenInterface:isAuthenticated()` and `setAuthenticated()` methods,
return `null` from `getUser()` instead when a token is not authenticated
* Deprecate `DeauthenticatedEvent`, use `TokenDeauthenticatedEvent` instead
* Deprecate `CookieClearingLogoutHandler`, `SessionLogoutHandler` and `CsrfTokenClearingLogoutHandler`.
Use `CookieClearingLogoutListener`, `SessionLogoutListener` and `CsrfTokenClearingLogoutListener` instead
Expand Down
4 changes: 2 additions & 2 deletions UPGRADE-6.0.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,8 +359,8 @@ Security
`UsernamePasswordFormAuthenticationListener`, `UsernamePasswordJsonAuthenticationListener` and `X509AuthenticationListener`
from security-http, use the new authenticator system instead
* Remove the Guard component, use the new authenticator system instead
* Remove `TokenInterface:isAuthenticated()` and `setAuthenticated()` methods without replacement.
Security tokens won't have an "authenticated" flag anymore, so they will always be considered authenticated
* Remove `TokenInterface:isAuthenticated()` and `setAuthenticated()`,
return `null` from `getUser()` instead when a token is not authenticated
* Remove `DeauthenticatedEvent`, use `TokenDeauthenticatedEvent` instead
* Remove `CookieClearingLogoutHandler`, `SessionLogoutHandler` and `CsrfTokenClearingLogoutHandler`.
Use `CookieClearingLogoutListener`, `SessionLogoutListener` and `CsrfTokenClearingLogoutListener` instead
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Bridge/Monolog/Processor/AbstractTokenProcessor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public function __invoke(array $record): array

if (null !== $token = $this->getToken()) {
$record['extra'][$this->getKey()] = [
'authenticated' => method_exists($token, 'isAuthenticated') ? $token->isAuthenticated(false) : true, // @deprecated since Symfony 5.4, always true in 6.0
'authenticated' => method_exists($token, 'isAuthenticated') ? $token->isAuthenticated(false) : (bool) $token->getUser(),
'roles' => $token->getRoleNames(),
];

Expand Down
5 changes: 3 additions & 2 deletions src/Symfony/Bridge/Twig/AppVariable.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use Symfony\Component\HttpFoundation\Session\Session;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\User\UserInterface;

/**
* Exposes some Symfony parameters and services as an "app" global variable.
Expand Down Expand Up @@ -68,7 +69,7 @@ public function getToken()
/**
* Returns the current user.
*
* @return object|null
* @return UserInterface|null
*
* @see TokenInterface::getUser()
*/
Expand All @@ -84,7 +85,7 @@ public function getUser()

$user = $token->getUser();

// @deprecated since 5.4, $user will always be a UserInterface instance
// @deprecated since Symfony 5.4, $user will always be a UserInterface instance
return \is_object($user) ? $user : null;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Bundle/SecurityBundle/DataCollector/SecurityDataCollector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ public function collect(Request $request, Response $response, \Throwable $except

$this->data = [
'enabled' => true,
'authenticated' => method_exists($token, 'isAuthenticated') ? $token->isAuthenticated(false) : true,
'authenticated' => method_exists($token, 'isAuthenticated') ? $token->isAuthenticated(false) : (bool) $token->getUser(),
'impersonated' => null !== $impersonatorUser,
'impersonator_user' => $impersonatorUser,
'impersonation_exit_path' => null,
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Security/Core/Authentication/AuthenticationProviderManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public function authenticate(TokenInterface $token)
}

// @deprecated since Symfony 5.3
if ($user = $result->getUser() instanceof UserInterface && !method_exists($result->getUser(), 'getUserIdentifier')) {
if ($result->getUser() instanceof UserInterface && !method_exists($result->getUser(), 'getUserIdentifier')) {
trigger_deprecation('symfony/security-core', '5.3', 'Not implementing method "getUserIdentifier(): string" in user class "%s" is deprecated. This method will replace "getUsername()" in Symfony 6.0.', get_debug_type($result->getUser()));
}

Expand Down
23 changes: 5 additions & 18 deletions src/Symfony/Component/Security/Core/Authentication/AuthenticationTrustResolver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
namespace Symfony\Component\Security\Core\Authentication;

use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken;
use Symfony\Component\Security\Core\Authentication\Token\NullToken;
use Symfony\Component\Security\Core\Authentication\Token\RememberMeToken;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;

Expand All @@ -25,9 +24,9 @@ class AuthenticationTrustResolver implements AuthenticationTrustResolverInterfac
{
public function isAuthenticated(TokenInterface $token = null): bool
{
return null !== $token && !$token instanceof NullToken
return $token && $token->getUser()
// @deprecated since Symfony 5.4, TokenInterface::isAuthenticated() and AnonymousToken no longer exists in 6.0
&& !$token instanceof AnonymousToken && $token->isAuthenticated(false);
&& !$token instanceof AnonymousToken && (!method_exists($token, 'isAuthenticated') || $token->isAuthenticated(false));
}

/**
Expand All @@ -39,34 +38,22 @@ public function isAnonymous(TokenInterface $token = null/*, $deprecation = true*
trigger_deprecation('symfony/security-core', '5.4', 'The "%s()" method is deprecated, use "isAuthenticated()" or "isFullFledged()" if you want to check if the request is (fully) authenticated.', __METHOD__);
}

if (null === $token) {
return false;
}

return $token instanceof AnonymousToken || $token instanceof NullToken;
return $token && !$this->isAuthenticated($token);
}

/**
* {@inheritdoc}
*/
public function isRememberMe(TokenInterface $token = null)
{
if (null === $token) {
return false;
}

return $token instanceof RememberMeToken;
return $token && $token instanceof RememberMeToken;
}

/**
* {@inheritdoc}
*/
public function isFullFledged(TokenInterface $token = null)
{
if (null === $token) {
return false;
}

return !$this->isAnonymous($token, false) && !$this->isRememberMe($token);
return $token && !$this->isAnonymous($token, false) && !$this->isRememberMe($token);
}
}
4 changes: 2 additions & 2 deletions src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ public function setUser($user)
public function isAuthenticated()
{
if (1 > \func_num_args() || func_get_arg(0)) {
trigger_deprecation('symfony/security-core', '5.4', 'Method "%s()" is deprecated. In version 6.0, security tokens won\'t have an "authenticated" flag anymore and will always be considered authenticated.', __METHOD__);
trigger_deprecation('symfony/security-core', '5.4', 'Method "%s()" is deprecated, return null from "getUser()" instead when a token is not authenticated.', __METHOD__);
}

return $this->authenticated;
Expand All @@ -153,7 +153,7 @@ public function isAuthenticated()
public function setAuthenticated(bool $authenticated)
{
if (2 > \func_num_args() || func_get_arg(1)) {
trigger_deprecation('symfony/security-core', '5.4', 'Method "%s()" is deprecated. In version 6.0, security tokens won\'t have an "authenticated" state anymore and will always be considered as authenticated.', __METHOD__);
trigger_deprecation('symfony/security-core', '5.4', 'Method "%s()" is deprecated', __METHOD__);
}

$this->authenticated = $authenticated;
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Security/Core/Authentication/Token/NullToken.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public function getUserIdentifier(): string
public function isAuthenticated()
{
if (0 === \func_num_args() || func_get_arg(0)) {
trigger_deprecation('symfony/security-core', '5.4', 'Method "%s()" is deprecated. In version 6.0, security tokens won\'t have an "authenticated" flag anymore and will always be considered authenticated.', __METHOD__);
trigger_deprecation('symfony/security-core', '5.4', 'Method "%s()" is deprecated, return null from "getUser()" instead when a token is not authenticated.', __METHOD__);
}

return true;
Expand Down
6 changes: 3 additions & 3 deletions src/Symfony/Component/Security/Core/Authentication/Token/TokenInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public function getCredentials();
/**
* Returns a user representation.
*
* @return UserInterface
* @return UserInterface|null
*
* @see AbstractToken::setUser()
*/
Expand All @@ -71,14 +71,14 @@ public function setUser($user);
*
* @return bool true if the token has been authenticated, false otherwise
*
* @deprecated since Symfony 5.4. In 6.0, security tokens will always be considered authenticated
* @deprecated since Symfony 5.4, return null from "getUser()" instead when a token is not authenticated
*/
public function isAuthenticated();

/**
* Sets the authenticated flag.
*
* @deprecated since Symfony 5.4. In 6.0, security tokens will always be considered authenticated
* @deprecated since Symfony 5.4
*/
public function setAuthenticated(bool $isAuthenticated);

Expand Down
6 changes: 4 additions & 2 deletions src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@ public function __construct(TokenStorageInterface $tokenStorage, /*AccessDecisio
*/
final public function isGranted($attribute, $subject = null): bool
{
if (null === ($token = $this->tokenStorage->getToken())) {
$token = $this->tokenStorage->getToken();

if (!$token || !$token->getUser()) {
if ($this->exceptionOnNoToken) {
throw new AuthenticationCredentialsNotFoundException('The token storage contains no authentication token. One possible reason may be that there is no firewall configured for this URL.');
}
Expand All @@ -78,7 +80,7 @@ final public function isGranted($attribute, $subject = null): bool
// @deprecated since Symfony 5.4
if ($this->alwaysAuthenticate || !$authenticated = $token->isAuthenticated(false)) {
if (!($authenticated ?? true)) {
trigger_deprecation('symfony/core', '5.4', 'Returning false from "%s()" is deprecated and won\'t have any effect in Symfony 6.0 as security tokens will always be considered authenticated.');
trigger_deprecation('symfony/core', '5.4', 'Returning false from "%s()" is deprecated, return null from "getUser()" instead.');
}
$this->tokenStorage->setToken($token = $this->authenticationManager->authenticate($token));
}
Expand Down
3 changes: 1 addition & 2 deletions src/Symfony/Component/Security/Core/Authorization/Voter/AuthenticatedVoter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
namespace Symfony\Component\Security\Core\Authorization\Voter;

use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverInterface;
use Symfony\Component\Security\Core\Authentication\Token\NullToken;
use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;

Expand Down Expand Up @@ -96,7 +95,7 @@ public function vote(TokenInterface $token, $subject, array $attributes)
if (self::IS_AUTHENTICATED === $attribute
&& (method_exists($this->authenticationTrustResolver, 'isAuthenticated')
? $this->authenticationTrustResolver->isAuthenticated($token)
: (null !== $token && !$token instanceof NullToken))) {
: ($token && $token->getUser()))) {
return VoterInterface::ACCESS_GRANTED;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Security/Core/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ CHANGELOG
* Deprecate setting the `$alwaysAuthenticate` argument to `true` and not setting the
`$exceptionOnNoToken` argument to `false` of `AuthorizationChecker`
* Deprecate methods `TokenInterface::isAuthenticated()` and `setAuthenticated`,
tokens will always be considered authenticated in 6.0
return null from "getUser()" instead when a token is not authenticated

5.3
---
Expand Down
6 changes: 1 addition & 5 deletions src/Symfony/Component/Security/Core/Security.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,8 @@ public function getUser(): ?UserInterface
}

$user = $token->getUser();
// @deprecated since 5.4, $user will always be a UserInterface instance
if (!\is_object($user)) {
return null;
}

// @deprecated since 5.4, $user will always be a UserInterface instance
// @deprecated since Symfony 5.4, $user will always be a UserInterface instance
if (!$user instanceof UserInterface) {
return null;
}
Expand Down
18 changes: 5 additions & 13 deletions src/Symfony/Component/Security/Core/Tests/Authentication/AuthenticationTrustResolverTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
use Symfony\Component\Security\Core\Authentication\Token\RememberMeToken;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\User\InMemoryUser;
use Symfony\Component\Security\Core\User\User;
use Symfony\Component\Security\Core\User\UserInterface;

class AuthenticationTrustResolverTest extends TestCase
Expand All @@ -29,7 +28,6 @@ public function testIsAnonymous()
{
$resolver = new AuthenticationTrustResolver();
$this->assertFalse($resolver->isAnonymous(null));
$this->assertFalse($resolver->isAnonymous($this->getToken()));
$this->assertFalse($resolver->isAnonymous($this->getRememberMeToken()));
$this->assertFalse($resolver->isAnonymous(new FakeCustomToken()));
}
Expand All @@ -39,7 +37,6 @@ public function testIsRememberMe()
$resolver = new AuthenticationTrustResolver();

$this->assertFalse($resolver->isRememberMe(null));
$this->assertFalse($resolver->isRememberMe($this->getToken()));
$this->assertFalse($resolver->isRememberMe(new FakeCustomToken()));
$this->assertTrue($resolver->isRememberMe(new RealCustomRememberMeToken()));
$this->assertTrue($resolver->isRememberMe($this->getRememberMeToken()));
Expand All @@ -52,7 +49,6 @@ public function testisFullFledged()
$this->assertFalse($resolver->isFullFledged(null));
$this->assertFalse($resolver->isFullFledged($this->getRememberMeToken()));
$this->assertFalse($resolver->isFullFledged(new RealCustomRememberMeToken()));
$this->assertTrue($resolver->isFullFledged($this->getToken()));
$this->assertTrue($resolver->isFullFledged(new FakeCustomToken()));
}

Expand All @@ -72,7 +68,7 @@ public function testIsAnonymousWithClassAsConstructorButStillExtending()
$resolver = $this->getResolver();

$this->assertFalse($resolver->isAnonymous(null));
$this->assertFalse($resolver->isAnonymous($this->getToken()));
$this->assertFalse($resolver->isAnonymous(new FakeCustomToken()));
$this->assertFalse($resolver->isAnonymous($this->getRememberMeToken()));
}

Expand All @@ -81,7 +77,7 @@ public function testIsRememberMeWithClassAsConstructorButStillExtending()
$resolver = $this->getResolver();

$this->assertFalse($resolver->isRememberMe(null));
$this->assertFalse($resolver->isRememberMe($this->getToken()));
$this->assertFalse($resolver->isRememberMe(new FakeCustomToken()));
$this->assertTrue($resolver->isRememberMe($this->getRememberMeToken()));
$this->assertTrue($resolver->isRememberMe(new RealCustomRememberMeToken()));
}
Expand All @@ -93,7 +89,7 @@ public function testisFullFledgedWithClassAsConstructorButStillExtending()
$this->assertFalse($resolver->isFullFledged(null));
$this->assertFalse($resolver->isFullFledged($this->getRememberMeToken()));
$this->assertFalse($resolver->isFullFledged(new RealCustomRememberMeToken()));
$this->assertTrue($resolver->isFullFledged($this->getToken()));
$this->assertTrue($resolver->isFullFledged(new FakeCustomToken()));
}

/**
Expand All @@ -112,11 +108,6 @@ public function testLegacy()
$this->assertFalse($resolver->isFullFledged($this->getRealCustomAnonymousToken()));
}

protected function getToken()
{
return $this->createMock(TokenInterface::class);
}

protected function getAnonymousToken()
{
return new AnonymousToken('secret', 'anon.');
Expand All @@ -133,7 +124,7 @@ public function __construct()

protected function getRememberMeToken()
{
$user = class_exists(InMemoryUser::class) ? new InMemoryUser('wouter', '', ['ROLE_USER']) : new User('wouter', '', ['ROLE_USER']);
$user = new InMemoryUser('wouter', '', ['ROLE_USER']);

return new RememberMeToken($user, 'main', 'secret');
}
Expand Down Expand Up @@ -179,6 +170,7 @@ public function getCredentials()

public function getUser(): UserInterface
{
return new InMemoryUser('wouter', '', ['ROLE_USER']);
}

public function setUser($user)
Expand Down
31 changes: 23 additions & 8 deletions src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AuthenticatedVoterTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,13 @@

use PHPUnit\Framework\TestCase;
use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolver;
use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken;
use Symfony\Component\Security\Core\Authentication\Token\AbstractToken;
use Symfony\Component\Security\Core\Authentication\Token\NullToken;
use Symfony\Component\Security\Core\Authentication\Token\RememberMeToken;
use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter;
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
use Symfony\Component\Security\Core\User\InMemoryUser;

class AuthenticatedVoterTest extends TestCase
{
Expand Down Expand Up @@ -84,14 +85,28 @@ public function getLegacyVoteTests()

protected function getToken($authenticated)
{
$user = new InMemoryUser('wouter', '', ['ROLE_USER']);

if ('fully' === $authenticated) {
return $this->createMock(TokenInterface::class);
} elseif ('remembered' === $authenticated) {
return $this->getMockBuilder(RememberMeToken::class)->setMethods(['setPersistent'])->disableOriginalConstructor()->getMock();
} elseif ('impersonated' === $authenticated) {
$token = new class() extends AbstractToken {
public function getCredentials()
{
}
};
$token->setUser($user);
$token->setAuthenticated(true, false);

return $token;
}

if ('remembered' === $authenticated) {
return new RememberMeToken($user, 'foo', 'bar');
}

if ('impersonated' === $authenticated) {
return $this->getMockBuilder(SwitchUserToken::class)->disableOriginalConstructor()->getMock();
} else {
return $this->getMockBuilder(AnonymousToken::class)->setConstructorArgs(['', ''])->getMock();
}

return new NullToken();
}
}
1 change: 0 additions & 1 deletion src/Symfony/Component/Security/Http/EventListener/UserCheckerListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ public function preCheckCredentials(CheckPassportEvent $event): void
public function postCheckCredentials(AuthenticationSuccessEvent $event): void
{
$user = $event->getAuthenticationToken()->getUser();
// @deprecated since 5.4, $user will always be an UserInterface instance
if (!$user instanceof UserInterface) {
return;
}
Expand Down
Loading