Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[Security] Improve DX of recent additions#59805
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Changes fromall commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
- Loading branch information
Uh oh!
There was an error while loading.Please reload this page.
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -31,7 +31,6 @@ final class SecurityExtension extends AbstractExtension | ||
public function __construct( | ||
private ?AuthorizationCheckerInterface $securityChecker = null, | ||
private ?ImpersonateUrlGenerator $impersonateUrlGenerator = null, | ||
) { | ||
} | ||
@@ -58,8 +57,12 @@ public function isGranted(mixed $role, mixed $object = null, ?string $field = nu | ||
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".%s', __METHOD__, UserAuthorizationCheckerInterface::class, interface_exists(UserAuthorizationCheckerInterface::class) ? ' Try upgrading the "symfony/security-core" package to v7.3 minimum.' : '')); | ||
} | ||
if (null !== $field) { | ||
@@ -71,7 +74,7 @@ public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $s | ||
} | ||
try { | ||
return $this->securityChecker->isGrantedForUser($user, $attribute, $subject, $accessDecision); | ||
} catch (AuthenticationCredentialsNotFoundException) { | ||
return false; | ||
} | ||
@@ -123,7 +126,7 @@ public function getFunctions(): array | ||
new TwigFunction('impersonation_path', $this->getImpersonatePath(...)), | ||
]; | ||
nicolas-grekas marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
if ($this->securityChecker instanceof UserAuthorizationCheckerInterface) { | ||
$functions[] = new TwigFunction('is_granted_for_user', $this->isGrantedForUser(...)); | ||
} | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -15,12 +15,23 @@ | ||
use Symfony\Bridge\PhpUnit\ClassExistsMock; | ||
use Symfony\Bridge\Twig\Extension\SecurityExtension; | ||
use Symfony\Component\Security\Acl\Voter\FieldVote; | ||
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\User\UserInterface; | ||
class SecurityExtensionTest extends TestCase | ||
{ | ||
public static function setUpBeforeClass(): void | ||
{ | ||
ClassExistsMock::register(SecurityExtension::class); | ||
} | ||
protected function tearDown(): void | ||
{ | ||
ClassExistsMock::withMockedClasses([FieldVote::class => true]); | ||
} | ||
/** | ||
* @dataProvider provideObjectFieldAclCases | ||
*/ | ||
@@ -39,17 +50,16 @@ public function testIsGrantedCreatesFieldVoteObjectWhenFieldNotNull($object, $fi | ||
public function testIsGrantedThrowsWhenFieldNotNullAndFieldVoteClassDoesNotExist() | ||
{ | ||
if (!interface_exists(UserAuthorizationCheckerInterface::class)) { | ||
$this->markTestSkipped('This test requires symfony/security-core 7.3 or superior.'); | ||
} | ||
$securityChecker = $this->createMock(AuthorizationCheckerInterface::class); | ||
ClassExistsMock::withMockedClasses([FieldVote::class => false]); | ||
$this->expectException(\LogicException::class); | ||
$this->expectExceptionMessage('Passing a $field to the "is_granted()" function requires symfony/acl.'); | ||
$securityExtension = new SecurityExtension($securityChecker); | ||
$securityExtension->isGranted('ROLE', 'object', 'bar'); | ||
@@ -60,49 +70,74 @@ public function testIsGrantedThrowsWhenFieldNotNullAndFieldVoteClassDoesNotExist | ||
*/ | ||
public function testIsGrantedForUserCreatesFieldVoteObjectWhenFieldNotNull($object, $field, $expectedSubject) | ||
{ | ||
if (!interface_exists(UserAuthorizationCheckerInterface::class)) { | ||
$this->markTestSkipped('This test requires symfony/security-core 7.3 or superior.'); | ||
} | ||
$user = $this->createMock(UserInterface::class); | ||
$securityChecker = $this->createMockAuthorizationChecker(); | ||
$securityExtension = new SecurityExtension($securityChecker); | ||
$this->assertTrue($securityExtension->isGrantedForUser($user, 'ROLE', $object, $field)); | ||
$this->assertSame($user, $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 [ | ||
[null, null, null], | ||
['object', null, 'object'], | ||
['object', false, new FieldVote('object', false)], | ||
['object', 0, new FieldVote('object', 0)], | ||
['object', '', new FieldVote('object', '')], | ||
['object', 'field', new FieldVote('object', 'field')], | ||
]; | ||
} | ||
public function testIsGrantedForUserThrowsWhenFieldNotNullAndFieldVoteClassDoesNotExist() | ||
{ | ||
if (!interface_exists(UserAuthorizationCheckerInterface::class)) { | ||
$this->markTestSkipped('This test requires symfony/security-core 7.3 or superior.'); | ||
} | ||
$securityChecker = $this->createMockAuthorizationChecker(); | ||
ClassExistsMock::withMockedClasses([FieldVote::class => false]); | ||
$this->expectException(\LogicException::class); | ||
$this->expectExceptionMessage('Passing a $field to the "is_granted_for_user()" function requires symfony/acl.'); | ||
$securityExtension = new SecurityExtension($securityChecker); | ||
$securityExtension->isGrantedForUser($this->createMock(UserInterface::class), 'ROLE', 'object', 'bar'); | ||
} | ||
privatefunctioncreateMockAuthorizationChecker(): AuthorizationCheckerInterface&UserAuthorizationCheckerInterface | ||
nicolas-grekas marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
{ | ||
return new class implements AuthorizationCheckerInterface, UserAuthorizationCheckerInterface { | ||
public UserInterface $user; | ||
public mixed $attribute; | ||
public mixed $subject; | ||
public function isGranted(mixed $attribute, mixed $subject = null, ?AccessDecision $accessDecision = null): bool | ||
{ | ||
throw new \BadMethodCallException('This method should not be called.'); | ||
} | ||
public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $subject = null, ?AccessDecision $accessDecision = null): bool | ||
{ | ||
$this->user = $user; | ||
$this->attribute = $attribute; | ||
$this->subject = $subject; | ||
return true; | ||
} | ||
}; | ||
} | ||
} |
This file was deleted.
Uh oh!
There was an error while loading.Please reload this page.
This file was deleted.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.