Skip to content

Commit 374d705

Browse files
fabpotfabpot
committed
feature #37620 [Security] Use NullToken while checking authorization (wouterj)
This PR was merged into the 5.2-dev branch. Discussion ---------- [Security] Use NullToken while checking authorization | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | Fix #37523 | License | MIT | Doc PR | tbd This allows voters to grant access to unauthenticated users. E.g. some objects can be viewed by anyone, in this case the voter has to be able to grant access to unauthenticated users. This *does break* the interface PHPdoc of `TokenInterface`: `getUser()` returns `null` instead of `string|UserInterface`. This is only true when using the new system, so not a real BC break. I think the only thing we can do to "guide" users is to add some custom handling for type errors related to `null` and `UserInterface` methods ("Did you forgot to check for `null` in the Voter?"). Is this something I should add to this PR? Commits ------- e370915 Use NullToken while checking authorization
2 parents bd59105 + e370915 commit 374d705

File tree

9 files changed

+154
-29
lines changed

9 files changed

+154
-29
lines changed

‎UPGRADE-5.2.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,3 +48,9 @@ Validator
4848
* })
4949
*/
5050
```
51+
52+
Security
53+
--------
54+
55+
* [BC break] In the experimental authenticator-based system, * `TokenInterface::getUser()`
56+
returns `null` in case of unauthenticated session.

‎src/Symfony/Component/Security/CHANGELOG.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ CHANGELOG
44
5.2.0
55
-----
66

7-
* Added attributes on ``Passport``
7+
* Added attributes on `Passport`
8+
* Changed `AuthorizationChecker` to call the access decision manager in unauthenticated sessions with a `NullToken`
89

910
5.1.0
1011
-----

‎src/Symfony/Component/Security/Core/Authentication/AuthenticationTrustResolver.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\Security\Core\Authentication;
1313

1414
use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken;
15+
use Symfony\Component\Security\Core\Authentication\Token\NullToken;
1516
use Symfony\Component\Security\Core\Authentication\Token\RememberMeToken;
1617
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
1718

@@ -31,7 +32,7 @@ public function isAnonymous(TokenInterface $token = null)
3132
return false;
3233
}
3334

34-
return $token instanceof AnonymousToken;
35+
return $token instanceof AnonymousToken || $token instanceof NullToken;
3536
}
3637

3738
/**

‎src/Symfony/Component/Security/Core/Authentication/Token/NullToken.php‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Security\Core\Authentication\Token;
13+
14+
/**
15+
* @author Wouter de Jong <wouter@wouterj.nl>
16+
*/
17+
class NullToken implements TokenInterface
18+
{
19+
public function __toString(): string
20+
{
21+
return '';
22+
}
23+
24+
public function getRoleNames(): array
25+
{
26+
return [];
27+
}
28+
29+
public function getCredentials()
30+
{
31+
return '';
32+
}
33+
34+
public function getUser()
35+
{
36+
return null;
37+
}
38+
39+
public function setUser($user)
40+
{
41+
throw new \BadMethodCallException('Cannot set user on a NullToken.');
42+
}
43+
44+
public function getUsername()
45+
{
46+
return '';
47+
}
48+
49+
public function isAuthenticated()
50+
{
51+
return true;
52+
}
53+
54+
public function setAuthenticated(bool $isAuthenticated)
55+
{
56+
throw new \BadMethodCallException('Cannot change authentication state of NullToken.');
57+
}
58+
59+
public function eraseCredentials()
60+
{
61+
}
62+
63+
public function getAttributes()
64+
{
65+
return [];
66+
}
67+
68+
public function setAttributes(array $attributes)
69+
{
70+
throw new \BadMethodCallException('Cannot set attributes of NullToken.');
71+
}
72+
73+
public function hasAttribute(string $name)
74+
{
75+
return false;
76+
}
77+
78+
public function getAttribute(string $name)
79+
{
80+
return null;
81+
}
82+
83+
public function setAttribute(string $name, $value)
84+
{
85+
throw new \BadMethodCallException('Cannot add attribute to NullToken.');
86+
}
87+
88+
public function __serialize(): array
89+
{
90+
return [];
91+
}
92+
93+
public function __unserialize(array $data): void
94+
{
95+
}
96+
97+
public function serialize()
98+
{
99+
return '';
100+
}
101+
102+
public function unserialize($serialized)
103+
{
104+
}
105+
}

‎src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\Security\Core\Authorization;
1313

1414
use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
15+
use Symfony\Component\Security\Core\Authentication\Token\NullToken;
1516
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
1617
use Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException;
1718

@@ -52,11 +53,11 @@ final public function isGranted($attribute, $subject = null): bool
5253
throw new AuthenticationCredentialsNotFoundException('The token storage contains no authentication token. One possible reason may be that there is no firewall configured for this URL.');
5354
}
5455

55-
return false;
56-
}
57-
58-
if ($this->alwaysAuthenticate || !$token->isAuthenticated()) {
59-
$this->tokenStorage->setToken($token = $this->authenticationManager->authenticate($token));
56+
$token = new NullToken();
57+
} else {
58+
if ($this->alwaysAuthenticate || !$token->isAuthenticated()) {
59+
$this->tokenStorage->setToken($token = $this->authenticationManager->authenticate($token));
60+
}
6061
}
6162

6263
return $this->accessDecisionManager->decide($token, [$attribute], $subject);

‎src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\Security\Core\Tests\Authorization;
1313

1414
use PHPUnit\Framework\TestCase;
15+
use Symfony\Component\Security\Core\Authentication\Token\NullToken;
1516
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
1617
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
1718
use Symfony\Component\Security\Core\Authorization\AuthorizationChecker;
@@ -77,7 +78,13 @@ public function testVoteWithoutAuthenticationTokenAndExceptionOnNoTokenIsFalse()
7778
{
7879
$authorizationChecker = new AuthorizationChecker($this->tokenStorage, $this->authenticationManager, $this->accessDecisionManager, false, false);
7980

80-
$this->assertFalse($authorizationChecker->isGranted('ROLE_FOO'));
81+
$this->accessDecisionManager
82+
->expects($this->once())
83+
->method('decide')
84+
->with($this->isInstanceOf(NullToken::class))
85+
->willReturn(true);
86+
87+
$this->assertTrue($authorizationChecker->isGranted('ANONYMOUS'));
8188
}
8289

8390
/**

‎src/Symfony/Component/Security/Http/Firewall/AccessListener.php‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use Symfony\Component\HttpFoundation\Request;
1515
use Symfony\Component\HttpKernel\Event\RequestEvent;
1616
use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
17+
use Symfony\Component\Security\Core\Authentication\Token\NullToken;
1718
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
1819
use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface;
1920
use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter;
@@ -89,19 +90,7 @@ public function authenticate(RequestEvent $event)
8990
throw new AuthenticationCredentialsNotFoundException('A Token was not found in the TokenStorage.');
9091
}
9192

92-
if ([AuthenticatedVoter::IS_AUTHENTICATED_ANONYMOUSLY] === $attributes) {
93-
trigger_deprecation('symfony/security-http', '5.1', 'Using "IS_AUTHENTICATED_ANONYMOUSLY" in your access_control rules when using the authenticator Security system is deprecated, use "PUBLIC_ACCESS" instead.');
94-
95-
return;
96-
}
97-
98-
if ([self::PUBLIC_ACCESS] !== $attributes) {
99-
throw $this->createAccessDeniedException($request, $attributes);
100-
}
101-
}
102-
103-
if ([self::PUBLIC_ACCESS] === $attributes) {
104-
return;
93+
$token = new NullToken();
10594
}
10695

10796
if (!$token->isAuthenticated()) {

‎src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php‎

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Symfony\Component\HttpKernel\Event\RequestEvent;
1717
use Symfony\Component\HttpKernel\HttpKernelInterface;
1818
use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
19+
use Symfony\Component\Security\Core\Authentication\Token\NullToken;
1920
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
2021
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
2122
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
@@ -245,9 +246,15 @@ public function testHandleWhenTheSecurityTokenStorageHasNoTokenAndExceptionOnTok
245246
->willReturn([['foo' => 'bar'], null])
246247
;
247248

249+
$accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class);
250+
$accessDecisionManager->expects($this->once())
251+
->method('decide')
252+
->with($this->isInstanceOf(NullToken::class))
253+
->willReturn(false);
254+
248255
$listener = new AccessListener(
249256
$tokenStorage,
250-
$this->getMockBuilder('Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface')->getMock(),
257+
$accessDecisionManager,
251258
$accessMap,
252259
$this->getMockBuilder('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface')->getMock(),
253260
false
@@ -268,17 +275,21 @@ public function testHandleWhenPublicAccessIsAllowedAndExceptionOnTokenIsFalse()
268275
->willReturn([[AccessListener::PUBLIC_ACCESS], null])
269276
;
270277

278+
$accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class);
279+
$accessDecisionManager->expects($this->once())
280+
->method('decide')
281+
->with($this->isInstanceOf(NullToken::class), [AccessListener::PUBLIC_ACCESS])
282+
->willReturn(true);
283+
271284
$listener = new AccessListener(
272285
$tokenStorage,
273-
$this->getMockBuilder('Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface')->getMock(),
286+
$accessDecisionManager,
274287
$accessMap,
275288
$this->getMockBuilder('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface')->getMock(),
276289
false
277290
);
278291

279292
$listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MASTER_REQUEST));
280-
281-
$this->expectNotToPerformAssertions();
282293
}
283294

284295
public function testHandleWhenPublicAccessWhileAuthenticated()
@@ -295,17 +306,21 @@ public function testHandleWhenPublicAccessWhileAuthenticated()
295306
->willReturn([[AccessListener::PUBLIC_ACCESS], null])
296307
;
297308

309+
$accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class);
310+
$accessDecisionManager->expects($this->once())
311+
->method('decide')
312+
->with($this->equalTo($token), [AccessListener::PUBLIC_ACCESS])
313+
->willReturn(true);
314+
298315
$listener = new AccessListener(
299316
$tokenStorage,
300-
$this->getMockBuilder('Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface')->getMock(),
317+
$accessDecisionManager,
301318
$accessMap,
302319
$this->getMockBuilder('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface')->getMock(),
303320
false
304321
);
305322

306323
$listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MASTER_REQUEST));
307-
308-
$this->expectNotToPerformAssertions();
309324
}
310325

311326
public function testHandleMWithultipleAttributesShouldBeHandledAsAnd()

‎src/Symfony/Component/Security/Http/composer.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
"require": {
1919
"php": ">=7.2.5",
2020
"symfony/deprecation-contracts": "^2.1",
21-
"symfony/security-core": "^5.1",
21+
"symfony/security-core": "^5.2",
2222
"symfony/http-foundation": "^4.4.7|^5.0.7",
2323
"symfony/http-kernel": "^4.4|^5.0",
2424
"symfony/polyfill-php80": "^1.15",

0 commit comments

Comments
 (0)