Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions UPGRADE-8.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,14 @@ DependencyInjection

* Deprecate configuring options `alias`, `parent`, `synthetic`, `file`, `arguments`, `properties`, `configurator` or `calls` when using `from_callable`


FrameworkBundle
---------------

* Deprecate setting the `framework.profiler.collect_serializer_data` config option


Security
--------

* Deprecated `UserAuthorizationCheckerInterface`, implement `UserGuestAuthorizationCheckerInterface` instead
13 changes: 9 additions & 4 deletions src/Symfony/Bridge/Twig/Extension/SecurityExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Symfony\Component\Security\Core\Authorization\AccessDecision;
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
use Symfony\Component\Security\Core\Authorization\UserAuthorizationCheckerInterface;
use Symfony\Component\Security\Core\Authorization\UserGuestAuthorizationCheckerInterface;
use Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException;
use Symfony\Component\Security\Core\User\UserInterface;
use Symfony\Component\Security\Http\Impersonate\ImpersonateUrlGenerator;
Expand Down Expand Up @@ -67,14 +68,18 @@ public function getAccessDecision(mixed $role, mixed $object = null, ?string $fi
return $accessDecision;
}

public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $subject = null, ?string $field = null, ?AccessDecision $accessDecision = null): bool
public function isGrantedForUser(?UserInterface $user, mixed $attribute, mixed $subject = null, ?string $field = null, ?AccessDecision $accessDecision = null): bool
{
if (null === $this->securityChecker) {
return false;
}

if (!$this->securityChecker instanceof UserAuthorizationCheckerInterface) {
throw new \LogicException(\sprintf('You cannot use "%s()" if the authorization checker doesn\'t implement "%s".', __METHOD__, UserAuthorizationCheckerInterface::class));
if (null === $user && !$this->securityChecker instanceof UserGuestAuthorizationCheckerInterface) {
throw new \LogicException(\sprintf('You cannot use "%s()" with a NULL-user if the authorization checker doesn\'t implement "%s".%s', __METHOD__, UserGuestAuthorizationCheckerInterface::class, interface_exists(UserGuestAuthorizationCheckerInterface::class) ? ' Try upgrading the "symfony/security-core" package to v7.4 minimum.' : ''));
}

if (!$this->securityChecker instanceof UserGuestAuthorizationCheckerInterface && !$this->securityChecker instanceof UserAuthorizationCheckerInterface) {
throw new \LogicException(\sprintf('You cannot use "%s()" if the authorization checker doesn\'t implement "%s" or "%s".', __METHOD__, UserGuestAuthorizationCheckerInterface::class, UserAuthorizationCheckerInterface::class));
}

if (null !== $field) {
Expand All @@ -92,7 +97,7 @@ public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $s
}
}

public function getAccessDecisionForUser(UserInterface $user, mixed $attribute, mixed $subject = null, ?string $field = null): AccessDecision
public function getAccessDecisionForUser(?UserInterface $user, mixed $attribute, mixed $subject = null, ?string $field = null): AccessDecision
{
if (!class_exists(AccessDecision::class)) {
throw new \LogicException(\sprintf('Using the "access_decision_for_user()" function requires symfony/security-core >= 7.3. Try running "composer %s symfony/security-core".', $this->securityChecker ? 'update' : 'require'));
Expand Down
35 changes: 28 additions & 7 deletions src/Symfony/Bridge/Twig/Tests/Extension/SecurityExtensionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use Symfony\Component\Security\Core\Authorization\AccessDecision;
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
use Symfony\Component\Security\Core\Authorization\UserAuthorizationCheckerInterface;
use Symfony\Component\Security\Core\Authorization\UserGuestAuthorizationCheckerInterface;
use Symfony\Component\Security\Core\User\UserInterface;

class SecurityExtensionTest extends TestCase
Expand Down Expand Up @@ -86,6 +87,27 @@ public function testIsGrantedForUserCreatesFieldVoteObjectWhenFieldNotNull($obje
}
}

#[DataProvider('provideObjectFieldAclCases')]
public function testIsGrantedForGuestCreatesFieldVoteObjectWhenFieldNotNull($object, $field, $expectedSubject)
{
if (!interface_exists(UserGuestAuthorizationCheckerInterface::class)) {
$this->markTestSkipped('This test requires symfony/security-core 7.4 or superior.');
}

$securityChecker = $this->createMockAuthorizationChecker();

$securityExtension = new SecurityExtension($securityChecker);
$this->assertTrue($securityExtension->isGrantedForUser(null, 'ROLE', $object, $field));
$this->assertNull($securityChecker->user);
$this->assertSame('ROLE', $securityChecker->attribute);

if (null === $field) {
$this->assertSame($object, $securityChecker->subject);
} else {
$this->assertEquals($expectedSubject, $securityChecker->subject);
}
}

public static function provideObjectFieldAclCases()
{
return [
Expand Down Expand Up @@ -201,15 +223,14 @@ public function testAccessDecisionForUserWithField()
$this->markTestSkipped('This test requires symfony/security-core 7.3 or superior.');
}

$user = $this->createMock(UserInterface::class);
$securityChecker = $this->createMockAuthorizationChecker();

$securityExtension = new SecurityExtension($securityChecker);
$accessDecision = $securityExtension->getAccessDecisionForUser($user, 'ROLE', 'object', 'field');
$accessDecision = $securityExtension->getAccessDecisionForUser(null, 'ROLE', 'object', 'field');

$this->assertInstanceOf(AccessDecision::class, $accessDecision);
$this->assertTrue($accessDecision->isGranted);
$this->assertSame($user, $securityChecker->user);
$this->assertNull($securityChecker->user);
$this->assertSame('ROLE', $securityChecker->attribute);
$this->assertEquals(new FieldVote('object', 'field'), $securityChecker->subject);
}
Expand All @@ -231,10 +252,10 @@ public function testAccessDecisionForUserThrowsWhenAccessDecisionClassDoesNotExi
$securityExtension->getAccessDecisionForUser($this->createMock(UserInterface::class), 'ROLE', 'object');
}

private function createMockAuthorizationChecker(): AuthorizationCheckerInterface&UserAuthorizationCheckerInterface
private function createMockAuthorizationChecker(): AuthorizationCheckerInterface&UserAuthorizationCheckerInterface&UserGuestAuthorizationCheckerInterface
{
return new class implements AuthorizationCheckerInterface, UserAuthorizationCheckerInterface {
public UserInterface $user;
return new class implements AuthorizationCheckerInterface, UserAuthorizationCheckerInterface, UserGuestAuthorizationCheckerInterface {
public ?UserInterface $user;
public mixed $attribute;
public mixed $subject;

Expand All @@ -243,7 +264,7 @@ public function isGranted(mixed $attribute, mixed $subject = null, ?AccessDecisi
throw new \BadMethodCallException('This method should not be called.');
}

public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $subject = null, ?AccessDecision $accessDecision = null): bool
public function isGrantedForUser(?UserInterface $user, mixed $attribute, mixed $subject = null, ?AccessDecision $accessDecision = null): bool
{
$this->user = $user;
$this->attribute = $attribute;
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Bundle/SecurityBundle/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ CHANGELOG
}
```
* Deprecate `LazyFirewallContext::__invoke()`
* `Security::isGrantedForUser()` now allows to check for guests (NULL-user) as well.

7.3
---
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
use Symfony\Component\Security\Core\Authorization\ExpressionLanguage;
use Symfony\Component\Security\Core\Authorization\UserAuthorizationCheckerInterface;
use Symfony\Component\Security\Core\Authorization\UserGuestAuthorizationCheckerInterface;
use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter;
use Symfony\Component\Security\Core\Authorization\Voter\ClosureVoter;
use Symfony\Component\Security\Core\Authorization\Voter\ExpressionVoter;
Expand Down Expand Up @@ -69,6 +70,7 @@
])
->alias(AuthorizationCheckerInterface::class, 'security.authorization_checker')
->alias(UserAuthorizationCheckerInterface::class, 'security.authorization_checker')
->alias(UserGuestAuthorizationCheckerInterface::class, 'security.authorization_checker')

->set('security.token_storage', UsageTrackingTokenStorage::class)
->args([
Expand Down
7 changes: 4 additions & 3 deletions src/Symfony/Bundle/SecurityBundle/Security.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use Symfony\Component\Security\Core\Authorization\AccessDecision;
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
use Symfony\Component\Security\Core\Authorization\UserAuthorizationCheckerInterface;
use Symfony\Component\Security\Core\Authorization\UserGuestAuthorizationCheckerInterface;
use Symfony\Component\Security\Core\Exception\LogicException;
use Symfony\Component\Security\Core\Exception\LogoutException;
use Symfony\Component\Security\Core\User\UserInterface;
Expand All @@ -39,7 +40,7 @@
*
* @final
*/
class Security implements AuthorizationCheckerInterface, UserAuthorizationCheckerInterface
class Security implements AuthorizationCheckerInterface, UserAuthorizationCheckerInterface, UserGuestAuthorizationCheckerInterface
{
public function __construct(
private readonly ContainerInterface $container,
Expand Down Expand Up @@ -78,13 +79,13 @@ public function getAccessDecision(mixed $attributes, mixed $subject = null): Acc
*
* This should be used over isGranted() when checking permissions against a user that is not currently logged in or while in a CLI context.
*/
public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $subject = null, ?AccessDecision $accessDecision = null): bool
public function isGrantedForUser(?UserInterface $user, mixed $attribute, mixed $subject = null, ?AccessDecision $accessDecision = null): bool
{
return $this->container->get('security.user_authorization_checker')
->isGrantedForUser($user, $attribute, $subject, $accessDecision);
}

public function getAccessDecisionForUser(UserInterface $user, mixed $attributes, mixed $subject = null): AccessDecision
public function getAccessDecisionForUser(?UserInterface $user, mixed $attributes, mixed $subject = null): AccessDecision
{
$accessDecision = new AccessDecision();
$this->isGrantedForUser($user, $attributes, $subject, $accessDecision);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ public function testUserAuthorizationChecker()
$this->assertFalse($security->isGranted('ROLE_BAR'));
$this->assertTrue($security->isGrantedForUser($offlineUser, 'ROLE_BAR'));
$this->assertFalse($security->isGrantedForUser($offlineUser, 'ROLE_FOO'));
$this->assertFalse($security->isGrantedForUser(null, 'ROLE_BAR'));
}

#[DataProvider('userWillBeMarkedAsChangedIfRolesHasChangedProvider')]
Expand Down
23 changes: 23 additions & 0 deletions src/Symfony/Bundle/SecurityBundle/Tests/SecurityTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
use Symfony\Component\Security\Core\Authorization\AccessDecision;
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
use Symfony\Component\Security\Core\Authorization\UserAuthorizationCheckerInterface;
use Symfony\Component\Security\Core\Authorization\UserGuestAuthorizationCheckerInterface;
use Symfony\Component\Security\Core\Exception\LogicException;
use Symfony\Component\Security\Core\User\InMemoryUser;
use Symfony\Component\Security\Core\User\UserCheckerInterface;
Expand Down Expand Up @@ -145,6 +146,28 @@ public function testAccessDecisionForUser()
$this->assertFalse($accessDecision->isGranted);
}

public function testAccessDecisionForGuest()
{
$userAuthorizationChecker = $this->createMock(UserGuestAuthorizationCheckerInterface::class);

$userAuthorizationChecker->expects($this->once())
->method('isGrantedForUser')
->with(null, 'SOME_ATTRIBUTE', 'SOME_SUBJECT', $this->isInstanceOf(AccessDecision::class))
->willReturnCallback(function ($user, $attribute, $subject, $accessDecision) {
$accessDecision->isGranted = false;

return false;
});

$container = $this->createContainer('security.user_authorization_checker', $userAuthorizationChecker);

$security = new Security($container);
$accessDecision = $security->getAccessDecisionForUser(null, 'SOME_ATTRIBUTE', 'SOME_SUBJECT');

$this->assertInstanceOf(AccessDecision::class, $accessDecision);
$this->assertFalse($accessDecision->isGranted);
}

#[DataProvider('getFirewallConfigTests')]
public function testGetFirewallConfig(Request $request, ?FirewallConfig $expectedFirewallConfig)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class_exists(OfflineTokenInterface::class);
* @author Fabien Potencier <fabien@symfony.com>
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
*/
class AuthorizationChecker implements AuthorizationCheckerInterface, UserAuthorizationCheckerInterface
class AuthorizationChecker implements AuthorizationCheckerInterface, UserAuthorizationCheckerInterface, UserGuestAuthorizationCheckerInterface
{
private array $tokenStack = [];
private array $accessDecisionStack = [];
Expand All @@ -44,7 +44,7 @@ final public function isGranted(mixed $attribute, mixed $subject = null, ?Access
{
$token = end($this->tokenStack) ?: $this->tokenStorage->getToken();

if (!$token || !$token->getUser()) {
if (!$token || (!$token->getUser() && !$token instanceof OfflineTokenInterface)) {
$token = new NullToken();
}
$accessDecision ??= end($this->accessDecisionStack) ?: new AccessDecision();
Expand All @@ -57,10 +57,15 @@ final public function isGranted(mixed $attribute, mixed $subject = null, ?Access
}
}

final public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $subject = null, ?AccessDecision $accessDecision = null): bool
final public function isGrantedForUser(?UserInterface $user, mixed $attribute, mixed $subject = null, ?AccessDecision $accessDecision = null): bool
{
$token = new class($user->getRoles()) extends AbstractToken implements OfflineTokenInterface {};
$token->setUser($user);
if ($user) {
$token = new class($user->getRoles()) extends AbstractToken implements OfflineTokenInterface {};
$token->setUser($user);
} else {
$token = new class extends AbstractToken implements OfflineTokenInterface {};
}

$this->tokenStack[] = $token;

try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
* Interface is used to check user authorization without a session.
*
* @author Nate Wiebe <nate@northern.co>
*
* @deprecated since Symfony 7.4, implement the UserGuestAuthorizationCheckerInterface instead.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we have to deprecate? it might be better not to?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you mean someone in Symfony 8.x might want to implement only checking for users but not for guests? I would think of the new interface as replacing the current one (because it allows both), but I'm fine with either way. Maybe we could also come up with something more generic than UserGuest... that would work as a better replacement in the future 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Autowiring alias added.

What do you think of OfflineAuthorizationCheckerInterface since we have an OfflineToken as well?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dunno, others might have an opinion :) (please mind fabbot, it still has spotted a few things)

*/
interface UserAuthorizationCheckerInterface
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Security\Core\Authorization;

use Symfony\Component\Security\Core\User\UserInterface;

/**
* Interface is used to check user authorization without a session.
*
* @author Nate Wiebe <nate@northern.co>
* @author Andreas Schempp <andreas.schempp@terminal42.ch>
*/
interface UserGuestAuthorizationCheckerInterface
{
/**
* Checks if the attribute is granted against the user and optionally supplied subject.
*
* @param mixed $attribute A single attribute to vote on (can be of any type, string and instance of Expression are supported by the core)
* @param AccessDecision|null $accessDecision Should be used to explain the decision
*/
public function isGrantedForUser(?UserInterface $user, mixed $attribute, mixed $subject = null, ?AccessDecision $accessDecision = null): bool;
}
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,18 @@ public function testIsGrantedForUser(bool $decide, array $roles)
$this->assertSame($decide, $this->authorizationChecker->isGrantedForUser($user, 'ROLE_FOO'));
}

#[DataProvider('isGrantedForUserProvider')]
public function testIsGrantedForGuest(bool $decide, array $roles)
{
$this->accessDecisionManager
->expects($this->once())
->method('decide')
->with($this->callback(static fn (OfflineTokenInterface $token) => null === $token->getUser()), ['ROLE_FOO'])
->willReturn($decide);

$this->assertSame($decide, $this->authorizationChecker->isGrantedForUser(null, 'ROLE_FOO'));
}

public static function isGrantedForUserProvider(): array
{
return [
Expand Down
Loading