From bf8d75ed869ef3fa537fbcb1420563e2f86300fa Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Mon, 11 Jul 2022 16:42:27 +0200 Subject: [PATCH] [Security] Add `#[IsGranted()]` --- .../Resources/config/security.php | 5 + .../Bundle/SecurityBundle/composer.json | 6 +- .../Security/Http/Attribute/IsGranted.php | 43 +++ .../Component/Security/Http/CHANGELOG.md | 1 + .../IsGrantedAttributeListener.php | 106 +++++++ .../IsGrantedAttributeListenerTest.php | 276 ++++++++++++++++++ .../Fixtures/IsGrantedAttributeController.php | 27 ++ .../IsGrantedAttributeMethodsController.php | 61 ++++ .../Component/Security/Http/composer.json | 2 +- 9 files changed, 523 insertions(+), 4 deletions(-) create mode 100644 src/Symfony/Component/Security/Http/Attribute/IsGranted.php create mode 100644 src/Symfony/Component/Security/Http/EventListener/IsGrantedAttributeListener.php create mode 100644 src/Symfony/Component/Security/Http/Tests/EventListener/IsGrantedAttributeListenerTest.php create mode 100644 src/Symfony/Component/Security/Http/Tests/Fixtures/IsGrantedAttributeController.php create mode 100644 src/Symfony/Component/Security/Http/Tests/Fixtures/IsGrantedAttributeMethodsController.php diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/security.php b/src/Symfony/Bundle/SecurityBundle/Resources/config/security.php index d4b8835194455..f584dae33b2eb 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/security.php +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/security.php @@ -42,6 +42,7 @@ use Symfony\Component\Security\Core\Validator\Constraints\UserPasswordValidator; use Symfony\Component\Security\Http\Authentication\AuthenticationUtils; use Symfony\Component\Security\Http\Controller\UserValueResolver; +use Symfony\Component\Security\Http\EventListener\IsGrantedAttributeListener; use Symfony\Component\Security\Http\Firewall; use Symfony\Component\Security\Http\FirewallMapInterface; use Symfony\Component\Security\Http\HttpUtils; @@ -269,5 +270,9 @@ service('security.expression_language'), ]) ->tag('kernel.cache_warmer') + + ->set('controller.is_granted_attribute_listener', IsGrantedAttributeListener::class) + ->args([service('security.authorization_checker')]) + ->tag('kernel.event_subscriber') ; }; diff --git a/src/Symfony/Bundle/SecurityBundle/composer.json b/src/Symfony/Bundle/SecurityBundle/composer.json index 8302390ad5b1f..fb8519ab9a04a 100644 --- a/src/Symfony/Bundle/SecurityBundle/composer.json +++ b/src/Symfony/Bundle/SecurityBundle/composer.json @@ -19,10 +19,10 @@ "php": ">=8.1", "composer-runtime-api": ">=2.1", "ext-xml": "*", - "symfony/config": "^5.4|^6.0", - "symfony/dependency-injection": "^5.4|^6.0", + "symfony/config": "^6.1", + "symfony/dependency-injection": "^6.1", "symfony/event-dispatcher": "^5.4|^6.0", - "symfony/http-kernel": "^5.4|^6.0", + "symfony/http-kernel": "^6.2", "symfony/http-foundation": "^5.4|^6.0", "symfony/password-hasher": "^5.4|^6.0", "symfony/security-core": "^5.4|^6.0", diff --git a/src/Symfony/Component/Security/Http/Attribute/IsGranted.php b/src/Symfony/Component/Security/Http/Attribute/IsGranted.php new file mode 100644 index 0000000000000..928babe261f24 --- /dev/null +++ b/src/Symfony/Component/Security/Http/Attribute/IsGranted.php @@ -0,0 +1,43 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Http\Attribute; + +/** + * @author Ryan Weaver + */ +#[\Attribute(\Attribute::IS_REPEATABLE | \Attribute::TARGET_CLASS | \Attribute::TARGET_METHOD | \Attribute::TARGET_FUNCTION)] +class IsGranted +{ + public function __construct( + /** + * Sets the first argument that will be passed to isGranted(). + */ + public array|string|null $attributes = null, + + /** + * Sets the second argument passed to isGranted(). + */ + public array|string|null $subject = null, + + /** + * The message of the exception - has a nice default if not set. + */ + public ?string $message = null, + + /** + * If set, will throw HttpKernel's HttpException with the given $statusCode. + * If null, Security\Core's AccessDeniedException will be used. + */ + public ?int $statusCode = null, + ) { + } +} diff --git a/src/Symfony/Component/Security/Http/CHANGELOG.md b/src/Symfony/Component/Security/Http/CHANGELOG.md index 536a16dc6a7cd..147eadefda891 100644 --- a/src/Symfony/Component/Security/Http/CHANGELOG.md +++ b/src/Symfony/Component/Security/Http/CHANGELOG.md @@ -5,6 +5,7 @@ CHANGELOG --- * Add maximum username length enforcement of 4096 characters in `UserBadge` + * Add `#[IsGranted()]` 6.0 --- diff --git a/src/Symfony/Component/Security/Http/EventListener/IsGrantedAttributeListener.php b/src/Symfony/Component/Security/Http/EventListener/IsGrantedAttributeListener.php new file mode 100644 index 0000000000000..d9c6830604024 --- /dev/null +++ b/src/Symfony/Component/Security/Http/EventListener/IsGrantedAttributeListener.php @@ -0,0 +1,106 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Http\EventListener; + +use Symfony\Component\EventDispatcher\EventSubscriberInterface; +use Symfony\Component\HttpKernel\Event\ControllerArgumentsEvent; +use Symfony\Component\HttpKernel\Exception\HttpException; +use Symfony\Component\HttpKernel\KernelEvents; +use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; +use Symfony\Component\Security\Core\Exception\AccessDeniedException; +use Symfony\Component\Security\Http\Attribute\IsGranted; + +/** + * Handles the IsGranted attribute on controllers. + * + * @author Ryan Weaver + */ +class IsGrantedAttributeListener implements EventSubscriberInterface +{ + public function __construct( + private AuthorizationCheckerInterface $authChecker, + ) { + } + + public function onKernelControllerArguments(ControllerArgumentsEvent $event) + { + /** @var IsGranted[] $attributes */ + if (!\is_array($attributes = $event->getAttributes()[IsGranted::class] ?? null)) { + return; + } + + $namedArguments = []; + $arguments = $event->getArguments(); + $r = $event->getRequest()->attributes->get('_controller_reflectors')[1] ?? new \ReflectionFunction($event->getController()); + + foreach ($r->getParameters() as $i => $param) { + if ($param->isVariadic()) { + $namedArguments[$param->name] = \array_slice($arguments, $i); + break; + } + if (\array_key_exists($i, $arguments)) { + $namedArguments[$param->name] = $arguments[$i]; + } + } + + foreach ($attributes as $attribute) { + $subjectRef = $attribute->subject; + $subject = null; + + if ($subjectRef) { + if (\is_array($subjectRef)) { + foreach ($subjectRef as $ref) { + if (!\array_key_exists($ref, $namedArguments)) { + throw new \RuntimeException(sprintf('Could not find the subject "%s" for the #[IsGranted] attribute. Try adding a "$%s" argument to your controller method.', $ref, $ref)); + } + $subject[$ref] = $namedArguments[$ref]; + } + } elseif (!\array_key_exists($subjectRef, $namedArguments)) { + throw new \RuntimeException(sprintf('Could not find the subject "%s" for the #[IsGranted] attribute. Try adding a "$%s" argument to your controller method.', $subjectRef, $subjectRef)); + } else { + $subject = $namedArguments[$subjectRef]; + } + } + + if (!$this->authChecker->isGranted($attribute->attributes, $subject)) { + $message = $attribute->message ?: sprintf('Access Denied by #[IsGranted(%s)] on controller', $this->getIsGrantedString($attribute)); + + if ($statusCode = $attribute->statusCode) { + throw new HttpException($statusCode, $message); + } + + $accessDeniedException = new AccessDeniedException($message); + $accessDeniedException->setAttributes($attribute->attributes); + $accessDeniedException->setSubject($subject); + + throw $accessDeniedException; + } + } + } + + public static function getSubscribedEvents(): array + { + return [KernelEvents::CONTROLLER_ARGUMENTS => ['onKernelControllerArguments', 10]]; + } + + private function getIsGrantedString(IsGranted $isGranted): string + { + $attributes = array_map(fn ($attribute) => '"'.$attribute.'"', (array) $isGranted->attributes); + $argsString = 1 === \count($attributes) ? reset($attributes) : '['.implode(', ', $attributes).']'; + + if (null !== $isGranted->subject) { + $argsString .= ', "'.implode('", "', (array) $isGranted->subject).'"'; + } + + return $argsString; + } +} diff --git a/src/Symfony/Component/Security/Http/Tests/EventListener/IsGrantedAttributeListenerTest.php b/src/Symfony/Component/Security/Http/Tests/EventListener/IsGrantedAttributeListenerTest.php new file mode 100644 index 0000000000000..6f6fd4d3c72df --- /dev/null +++ b/src/Symfony/Component/Security/Http/Tests/EventListener/IsGrantedAttributeListenerTest.php @@ -0,0 +1,276 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Http\Tests\EventListener; + +use PHPUnit\Framework\TestCase; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpKernel\Event\ControllerArgumentsEvent; +use Symfony\Component\HttpKernel\Exception\HttpException; +use Symfony\Component\HttpKernel\HttpKernelInterface; +use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; +use Symfony\Component\Security\Core\Exception\AccessDeniedException; +use Symfony\Component\Security\Http\EventListener\IsGrantedAttributeListener; +use Symfony\Component\Security\Http\Tests\Fixtures\IsGrantedAttributeController; +use Symfony\Component\Security\Http\Tests\Fixtures\IsGrantedAttributeMethodsController; + +class IsGrantedAttributeListenerTest extends TestCase +{ + public function testAttribute() + { + $authChecker = $this->createMock(AuthorizationCheckerInterface::class); + $authChecker->expects($this->exactly(2)) + ->method('isGranted') + ->willReturn(true); + + $event = new ControllerArgumentsEvent( + $this->createMock(HttpKernelInterface::class), + [new IsGrantedAttributeController(), 'foo'], + [], + new Request(), + null + ); + + $listener = new IsGrantedAttributeListener($authChecker); + $listener->onKernelControllerArguments($event); + + $authChecker = $this->getMockBuilder(AuthorizationCheckerInterface::class)->getMock(); + $authChecker->expects($this->once()) + ->method('isGranted') + ->willReturn(true); + + $event = new ControllerArgumentsEvent( + $this->createMock(HttpKernelInterface::class), + [new IsGrantedAttributeController(), 'bar'], + [], + new Request(), + null + ); + + $listener = new IsGrantedAttributeListener($authChecker); + $listener->onKernelControllerArguments($event); + } + + public function testNothingHappensWithNoConfig() + { + $authChecker = $this->getMockBuilder(AuthorizationCheckerInterface::class)->getMock(); + $authChecker->expects($this->never()) + ->method('isGranted'); + + $event = new ControllerArgumentsEvent( + $this->createMock(HttpKernelInterface::class), + [new IsGrantedAttributeMethodsController(), 'noAttribute'], + [], + new Request(), + null + ); + + $listener = new IsGrantedAttributeListener($authChecker); + $listener->onKernelControllerArguments($event); + } + + public function testIsGrantedCalledCorrectly() + { + $authChecker = $this->getMockBuilder(AuthorizationCheckerInterface::class)->getMock(); + $authChecker->expects($this->once()) + ->method('isGranted') + ->with('ROLE_ADMIN') + ->willReturn(true); + + $event = new ControllerArgumentsEvent( + $this->createMock(HttpKernelInterface::class), + [new IsGrantedAttributeMethodsController(), 'admin'], + [], + new Request(), + null + ); + + $listener = new IsGrantedAttributeListener($authChecker); + $listener->onKernelControllerArguments($event); + } + + public function testIsGrantedSubjectFromArguments() + { + $authChecker = $this->getMockBuilder(AuthorizationCheckerInterface::class)->getMock(); + $authChecker->expects($this->once()) + ->method('isGranted') + // the subject => arg2name will eventually resolve to the 2nd argument, which has this value + ->with('ROLE_ADMIN', 'arg2Value') + ->willReturn(true); + + $event = new ControllerArgumentsEvent( + $this->createMock(HttpKernelInterface::class), + [new IsGrantedAttributeMethodsController(), 'withSubject'], + ['arg1Value', 'arg2Value'], + new Request(), + null + ); + + // create metadata for 2 named args for the controller + $listener = new IsGrantedAttributeListener($authChecker); + $listener->onKernelControllerArguments($event); + } + + public function testIsGrantedSubjectFromArgumentsWithArray() + { + $authChecker = $this->createMock(AuthorizationCheckerInterface::class); + $authChecker->expects($this->once()) + ->method('isGranted') + // the subject => arg2name will eventually resolve to the 2nd argument, which has this value + ->with('ROLE_ADMIN', [ + 'arg1Name' => 'arg1Value', + 'arg2Name' => 'arg2Value', + ]) + ->willReturn(true); + + $event = new ControllerArgumentsEvent( + $this->createMock(HttpKernelInterface::class), + [new IsGrantedAttributeMethodsController(), 'withSubjectArray'], + ['arg1Value', 'arg2Value'], + new Request(), + null + ); + + // create metadata for 2 named args for the controller + $listener = new IsGrantedAttributeListener($authChecker); + $listener->onKernelControllerArguments($event); + } + + public function testIsGrantedNullSubjectFromArguments() + { + $authChecker = $this->getMockBuilder(AuthorizationCheckerInterface::class)->getMock(); + $authChecker->expects($this->once()) + ->method('isGranted') + ->with('ROLE_ADMIN', null) + ->willReturn(true); + + $event = new ControllerArgumentsEvent( + $this->createMock(HttpKernelInterface::class), + [new IsGrantedAttributeMethodsController(), 'withSubject'], + ['arg1Value', null], + new Request(), + null + ); + + $listener = new IsGrantedAttributeListener($authChecker); + $listener->onKernelControllerArguments($event); + } + + public function testIsGrantedArrayWithNullValueSubjectFromArguments() + { + $authChecker = $this->getMockBuilder(AuthorizationCheckerInterface::class)->getMock(); + $authChecker->expects($this->once()) + ->method('isGranted') + ->with('ROLE_ADMIN', [ + 'arg1Name' => 'arg1Value', + 'arg2Name' => null, + ]) + ->willReturn(true); + + $event = new ControllerArgumentsEvent( + $this->createMock(HttpKernelInterface::class), + [new IsGrantedAttributeMethodsController(), 'withSubjectArray'], + ['arg1Value', null], + new Request(), + null + ); + + $listener = new IsGrantedAttributeListener($authChecker); + $listener->onKernelControllerArguments($event); + } + + public function testExceptionWhenMissingSubjectAttribute() + { + $this->expectException(\RuntimeException::class); + + $authChecker = $this->getMockBuilder(AuthorizationCheckerInterface::class)->getMock(); + + $event = new ControllerArgumentsEvent( + $this->createMock(HttpKernelInterface::class), + [new IsGrantedAttributeMethodsController(), 'withMissingSubject'], + [], + new Request(), + null + ); + + $listener = new IsGrantedAttributeListener($authChecker); + $listener->onKernelControllerArguments($event); + } + + /** + * @dataProvider getAccessDeniedMessageTests + */ + public function testAccessDeniedMessages(array $attributes, ?string $subject, string $method, string $expectedMessage) + { + $authChecker = $this->getMockBuilder(AuthorizationCheckerInterface::class)->getMock(); + $authChecker->expects($this->any()) + ->method('isGranted') + ->willReturn(false); + + // avoid the error of the subject not being found in the request attributes + $arguments = []; + if (null !== $subject) { + $arguments[] = 'bar'; + } + + $listener = new IsGrantedAttributeListener($authChecker); + + $event = new ControllerArgumentsEvent( + $this->createMock(HttpKernelInterface::class), + [new IsGrantedAttributeMethodsController(), $method], + $arguments, + new Request(), + null + ); + + try { + $listener->onKernelControllerArguments($event); + $this->fail(); + } catch (AccessDeniedException $e) { + $this->assertSame($expectedMessage, $e->getMessage()); + $this->assertSame($attributes, $e->getAttributes()); + if (null !== $subject) { + $this->assertSame('bar', $e->getSubject()); + } else { + $this->assertNull($e->getSubject()); + } + } + } + + public function getAccessDeniedMessageTests() + { + yield [['ROLE_ADMIN'], null, 'admin', 'Access Denied by #[IsGranted("ROLE_ADMIN")] on controller']; + yield [['ROLE_ADMIN', 'ROLE_USER'], null, 'adminOrUser', 'Access Denied by #[IsGranted(["ROLE_ADMIN", "ROLE_USER"])] on controller']; + yield [['ROLE_ADMIN', 'ROLE_USER'], 'product', 'adminOrUserWithSubject', 'Access Denied by #[IsGranted(["ROLE_ADMIN", "ROLE_USER"], "product")] on controller']; + } + + public function testNotFoundHttpException() + { + $this->expectException(HttpException::class); + $this->expectExceptionMessage('Not found'); + + $authChecker = $this->createMock(AuthorizationCheckerInterface::class); + $authChecker->expects($this->any()) + ->method('isGranted') + ->willReturn(false); + + $event = new ControllerArgumentsEvent( + $this->createMock(HttpKernelInterface::class), + [new IsGrantedAttributeMethodsController(), 'notFound'], + [], + new Request(), + null + ); + + $listener = new IsGrantedAttributeListener($authChecker); + $listener->onKernelControllerArguments($event); + } +} diff --git a/src/Symfony/Component/Security/Http/Tests/Fixtures/IsGrantedAttributeController.php b/src/Symfony/Component/Security/Http/Tests/Fixtures/IsGrantedAttributeController.php new file mode 100644 index 0000000000000..72ea29eae9274 --- /dev/null +++ b/src/Symfony/Component/Security/Http/Tests/Fixtures/IsGrantedAttributeController.php @@ -0,0 +1,27 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Http\Tests\Fixtures; + +use Symfony\Component\Security\Http\Attribute\IsGranted; + +#[IsGranted(attributes: ['ROLE_ADMIN', 'ROLE_USER'])] +class IsGrantedAttributeController +{ + #[IsGranted(attributes: ['ROLE_ADMIN'])] + public function foo() + { + } + + public function bar() + { + } +} diff --git a/src/Symfony/Component/Security/Http/Tests/Fixtures/IsGrantedAttributeMethodsController.php b/src/Symfony/Component/Security/Http/Tests/Fixtures/IsGrantedAttributeMethodsController.php new file mode 100644 index 0000000000000..b69ab2659dd3a --- /dev/null +++ b/src/Symfony/Component/Security/Http/Tests/Fixtures/IsGrantedAttributeMethodsController.php @@ -0,0 +1,61 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Http\Tests\Fixtures; + +use Symfony\Component\Security\Http\Attribute\IsGranted; + +class IsGrantedAttributeMethodsController +{ + public function noAttribute() + { + } + + #[IsGranted()] + public function emptyAttribute() + { + } + + #[IsGranted(attributes: 'ROLE_ADMIN')] + public function admin() + { + } + + #[IsGranted(attributes: ['ROLE_ADMIN', 'ROLE_USER'])] + public function adminOrUser() + { + } + + #[IsGranted(attributes: ['ROLE_ADMIN', 'ROLE_USER'], subject: 'product')] + public function adminOrUserWithSubject($product) + { + } + + #[IsGranted(attributes: 'ROLE_ADMIN', subject: 'arg2Name')] + public function withSubject($arg1Name, $arg2Name) + { + } + + #[IsGranted(attributes: 'ROLE_ADMIN', subject: ['arg1Name', 'arg2Name'])] + public function withSubjectArray($arg1Name, $arg2Name) + { + } + + #[IsGranted(attributes: 'ROLE_ADMIN', subject: 'non_existent')] + public function withMissingSubject() + { + } + + #[IsGranted(attributes: 'ROLE_ADMIN', statusCode: 404, message: 'Not found')] + public function notFound() + { + } +} diff --git a/src/Symfony/Component/Security/Http/composer.json b/src/Symfony/Component/Security/Http/composer.json index 13bb3a203ffb2..907cfbaa5b942 100644 --- a/src/Symfony/Component/Security/Http/composer.json +++ b/src/Symfony/Component/Security/Http/composer.json @@ -19,7 +19,7 @@ "php": ">=8.1", "symfony/security-core": "^5.4.7|^6.0", "symfony/http-foundation": "^5.4|^6.0", - "symfony/http-kernel": "^6.1", + "symfony/http-kernel": "^6.2", "symfony/polyfill-mbstring": "~1.0", "symfony/property-access": "^5.4|^6.0" },