From 3d12f34c97babe26f023abacc57bce03ec5e1fcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Wed, 8 Feb 2017 15:37:04 +0100 Subject: [PATCH] Allow to configure auth access from the resource class --- composer.json | 3 + features/authorization/deny.feature | 74 ++++++++++ .../Doctrine/EventListener/WriteListener.php | 1 - .../Symfony/Bundle/Resources/config/api.xml | 10 +- src/EventListener/DenyAccessListener.php | 79 +++++++++++ .../ApiPlatformExtensionTest.php | 1 + .../EventListener/DenyAccessListenerTest.php | 131 ++++++++++++++++++ .../TestBundle/Entity/SecuredDummy.php | 106 ++++++++++++++ tests/Fixtures/app/config/security.yml | 29 ++-- 9 files changed, 423 insertions(+), 11 deletions(-) create mode 100644 features/authorization/deny.feature create mode 100644 src/EventListener/DenyAccessListener.php create mode 100644 tests/EventListener/DenyAccessListenerTest.php create mode 100644 tests/Fixtures/TestBundle/Entity/SecuredDummy.php diff --git a/composer.json b/composer.json index 872e809a0de..a8a0778e75e 100644 --- a/composer.json +++ b/composer.json @@ -46,6 +46,7 @@ "symfony/config": "^3.2", "symfony/dependency-injection": "^2.7 || ^3.0", "symfony/doctrine-bridge": "^2.8 || ^3.0", + "symfony/expression-language": "^2.8 || ^3.0", "symfony/phpunit-bridge": "^2.7 || ^3.0", "symfony/security": "^2.7 || ^3.0", "symfony/templating": "^2.7 || ^3.0", @@ -59,7 +60,9 @@ "phpdocumentor/reflection-docblock": "To support extracting metadata from PHPDoc.", "psr/cache-implementation": "To use metadata caching.", "symfony/cache": "To have metadata caching when using Symfony integration.", + "symfony/expression-language": "To use authorization features.", "symfony/config": "To load XML configuration files.", + "symfony/security": "To use authorization features.", "symfony/twig-bundle": "To use the Swagger UI integration." }, "autoload": { diff --git a/features/authorization/deny.feature b/features/authorization/deny.feature new file mode 100644 index 00000000000..ecae4050e97 --- /dev/null +++ b/features/authorization/deny.feature @@ -0,0 +1,74 @@ +Feature: Authorization checking + In order to use the API + As a client software developer + I need to be authorized to access a given resource. + + @createSchema + Scenario: An anonymous user retrieve a secured resource + When I add "Accept" header equal to "application/ld+json" + And I send a "GET" request to "/secured_dummies" + Then the response status code should be 401 + + Scenario: An authenticated user retrieve a secured resource + When I add "Accept" header equal to "application/ld+json" + And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg==" + And I send a "GET" request to "/secured_dummies" + Then the response status code should be 200 + And the response should be in JSON + + + Scenario: A standard user cannot create a secured resource + When I add "Accept" header equal to "application/ld+json" + And I add "Content-Type" header equal to "application/ld+json" + And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg==" + And I send a "POST" request to "/secured_dummies" with body: + """ + { + "title": "Title", + "description": "Description", + "owner": "foo" + } + """ + Then the response status code should be 403 + + Scenario: An admin can create a secured resource + When I add "Accept" header equal to "application/ld+json" + And I add "Content-Type" header equal to "application/ld+json" + And I add "Authorization" header equal to "Basic YWRtaW46a2l0dGVu" + And I send a "POST" request to "/secured_dummies" with body: + """ + { + "title": "Title", + "description": "Description", + "owner": "someone" + } + """ + Then the response status code should be 201 + + Scenario: An admin can create another secured resource + When I add "Accept" header equal to "application/ld+json" + And I add "Content-Type" header equal to "application/ld+json" + And I add "Authorization" header equal to "Basic YWRtaW46a2l0dGVu" + And I send a "POST" request to "/secured_dummies" with body: + """ + { + "title": "Special Title", + "description": "Description", + "owner": "dunglas" + } + """ + Then the response status code should be 201 + + Scenario: An user retrieve cannot retrieve an item he doesn't own + When I add "Accept" header equal to "application/ld+json" + And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg==" + And I send a "GET" request to "/secured_dummies/1" + Then the response status code should be 403 + And the response should be in JSON + + @dropSchema + Scenario: An user can retrieve an item he owns + When I add "Accept" header equal to "application/ld+json" + And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg==" + And I send a "GET" request to "/secured_dummies/2" + Then the response status code should be 200 diff --git a/src/Bridge/Doctrine/EventListener/WriteListener.php b/src/Bridge/Doctrine/EventListener/WriteListener.php index d11c9b8f788..e093438cd61 100644 --- a/src/Bridge/Doctrine/EventListener/WriteListener.php +++ b/src/Bridge/Doctrine/EventListener/WriteListener.php @@ -79,7 +79,6 @@ public function onKernelView(GetResponseForControllerResultEvent $event) private function getManager(string $resourceClass, $data) { $objectManager = $this->managerRegistry->getManagerForClass($resourceClass); - if (null === $objectManager || !is_object($data)) { return; } diff --git a/src/Bridge/Symfony/Bundle/Resources/config/api.xml b/src/Bridge/Symfony/Bundle/Resources/config/api.xml index df83c7742ff..71725256a64 100644 --- a/src/Bridge/Symfony/Bundle/Resources/config/api.xml +++ b/src/Bridge/Symfony/Bundle/Resources/config/api.xml @@ -88,6 +88,7 @@ + %api_platform.formats% @@ -95,7 +96,6 @@ - @@ -111,6 +111,14 @@ + + + + + + + + diff --git a/src/EventListener/DenyAccessListener.php b/src/EventListener/DenyAccessListener.php new file mode 100644 index 00000000000..4357e803228 --- /dev/null +++ b/src/EventListener/DenyAccessListener.php @@ -0,0 +1,79 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace ApiPlatform\Core\EventListener; + +use ApiPlatform\Core\Exception\RuntimeException; +use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; +use ApiPlatform\Core\Util\RequestAttributesExtractor; +use Symfony\Component\ExpressionLanguage\Expression; +use Symfony\Component\HttpKernel\Event\GetResponseEvent; +use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; +use Symfony\Component\Security\Core\Exception\AccessDeniedException; + +/** + * Denies access to the current resource if the logged user doesn't have sufficient permissions. + * + * @author Kévin Dunglas + */ +final class DenyAccessListener +{ + private $resourceMetadataFactory; + private $authorizationChecker; + + public function __construct(ResourceMetadataFactoryInterface $resourceMetadataFactory, AuthorizationCheckerInterface $authorizationChecker = null) + { + $this->resourceMetadataFactory = $resourceMetadataFactory; + $this->authorizationChecker = $authorizationChecker; + } + + /** + * Sets the applicable format to the HttpFoundation Request. + * + * @param GetResponseEvent $event + * + * @throws AccessDeniedException + */ + public function onKernelRequest(GetResponseEvent $event) + { + $request = $event->getRequest(); + + try { + $attributes = RequestAttributesExtractor::extractAttributes($request); + } catch (RuntimeException $e) { + return; + } + + $resourceMetadata = $this->resourceMetadataFactory->create($attributes['resource_class']); + + if (isset($attributes['collection_operation_name'])) { + $isGranted = $resourceMetadata->getCollectionOperationAttribute($attributes['collection_operation_name'], 'is_granted', null, true); + } else { + $isGranted = $resourceMetadata->getItemOperationAttribute($attributes['item_operation_name'], 'is_granted', null, true); + } + + if (null === $isGranted) { + return; + } + + if (null === $this->authorizationChecker) { + throw new \LogicException(sprintf('The "symfony/security" library must be installed to use the "is_granted" attribute on class "%s".', $attributes['resource_class'])); + } + + if (!class_exists(Expression::class)) { + throw new \LogicException(sprintf('The "symfony/expression-language" library must be installed to use the "is_granted" attribute on class "%s".', $attributes['resource_class'])); + } + + if (!$this->authorizationChecker->isGranted(new Expression($isGranted), $request->attributes->get('data'))) { + throw new AccessDeniedException(); + } + } +} diff --git a/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php b/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php index 25c55e165a1..0e7c8b4e312 100644 --- a/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php +++ b/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php @@ -290,6 +290,7 @@ private function getContainerBuilderProphecy() 'api_platform.listener.view.respond', 'api_platform.listener.view.serialize', 'api_platform.listener.view.validate', + 'api_platform.listener.request.deny_access', 'api_platform.metadata.extractor.yaml', 'api_platform.metadata.extractor.xml', 'api_platform.metadata.property.metadata_factory.annotation', diff --git a/tests/EventListener/DenyAccessListenerTest.php b/tests/EventListener/DenyAccessListenerTest.php new file mode 100644 index 00000000000..7fb6cd13872 --- /dev/null +++ b/tests/EventListener/DenyAccessListenerTest.php @@ -0,0 +1,131 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace ApiPlatform\Core\Tests\EventListener; + +use ApiPlatform\Core\EventListener\DenyAccessListener; +use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; +use ApiPlatform\Core\Metadata\Resource\ResourceMetadata; +use Prophecy\Argument; +use Symfony\Component\ExpressionLanguage\Expression; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpKernel\Event\GetResponseEvent; +use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; + +/** + * @author Kévin Dunglas + */ +class DenyAccessListenerTest extends \PHPUnit_Framework_TestCase +{ + public function testNoResourceClass() + { + $request = new Request(); + + $eventProphecy = $this->prophesize(GetResponseEvent::class); + $eventProphecy->getRequest()->willReturn($request)->shouldBeCalled(); + $event = $eventProphecy->reveal(); + + $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class); + $resourceMetadataFactoryProphecy->create()->shouldNotBeCalled(); + $resourceMetadataFactory = $resourceMetadataFactoryProphecy->reveal(); + + $authorizationCheckerProphecy = $this->prophesize(AuthorizationCheckerInterface::class); + $authorizationCheckerProphecy->isGranted()->shouldNotBeCalled(); + $authorizationChecker = $authorizationCheckerProphecy->reveal(); + + $listener = new DenyAccessListener($resourceMetadataFactory, $authorizationChecker); + $listener->onKernelRequest($event); + } + + public function testNoIsGrantedAttribute() + { + $request = new Request([], [], ['_api_resource_class' => 'Foo', '_api_collection_operation_name' => 'get']); + + $eventProphecy = $this->prophesize(GetResponseEvent::class); + $eventProphecy->getRequest()->willReturn($request)->shouldBeCalled(); + $event = $eventProphecy->reveal(); + + $resourceMetadata = new ResourceMetadata(); + + $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class); + $resourceMetadataFactoryProphecy->create('Foo')->willReturn($resourceMetadata)->shouldBeCalled(); + + $authorizationCheckerProphecy = $this->prophesize(AuthorizationCheckerInterface::class); + $authorizationCheckerProphecy->isGranted()->shouldNotBeCalled(); + + $listener = new DenyAccessListener($resourceMetadataFactoryProphecy->reveal(), $authorizationCheckerProphecy->reveal()); + $listener->onKernelRequest($event); + } + + public function testIsGranted() + { + $data = new \stdClass(); + $request = new Request([], [], ['_api_resource_class' => 'Foo', '_api_collection_operation_name' => 'get', 'data' => $data]); + + $eventProphecy = $this->prophesize(GetResponseEvent::class); + $eventProphecy->getRequest()->willReturn($request)->shouldBeCalled(); + $event = $eventProphecy->reveal(); + + $resourceMetadata = new ResourceMetadata(null, null, null, null, null, ['is_granted' => 'has_role("ROLE_ADMIN")']); + + $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class); + $resourceMetadataFactoryProphecy->create('Foo')->willReturn($resourceMetadata)->shouldBeCalled(); + + $authorizationCheckerProphecy = $this->prophesize(AuthorizationCheckerInterface::class); + $authorizationCheckerProphecy->isGranted(Argument::type(Expression::class), $data)->willReturn(true)->shouldBeCalled(); + + $listener = new DenyAccessListener($resourceMetadataFactoryProphecy->reveal(), $authorizationCheckerProphecy->reveal()); + $listener->onKernelRequest($event); + } + + /** + * @expectedException \Symfony\Component\Security\Core\Exception\AccessDeniedException + */ + public function testIsNotGranted() + { + $request = new Request([], [], ['_api_resource_class' => 'Foo', '_api_collection_operation_name' => 'get']); + + $eventProphecy = $this->prophesize(GetResponseEvent::class); + $eventProphecy->getRequest()->willReturn($request)->shouldBeCalled(); + $event = $eventProphecy->reveal(); + + $resourceMetadata = new ResourceMetadata(null, null, null, null, null, ['is_granted' => 'has_role("ROLE_ADMIN")']); + + $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class); + $resourceMetadataFactoryProphecy->create('Foo')->willReturn($resourceMetadata)->shouldBeCalled(); + + $authorizationCheckerProphecy = $this->prophesize(AuthorizationCheckerInterface::class); + $authorizationCheckerProphecy->isGranted(Argument::type(Expression::class), null)->willReturn(false)->shouldBeCalled(); + + $listener = new DenyAccessListener($resourceMetadataFactoryProphecy->reveal(), $authorizationCheckerProphecy->reveal()); + $listener->onKernelRequest($event); + } + + /** + * @expectedException \LogicException + */ + public function testAuthorizationCheckerNotAvailable() + { + $request = new Request([], [], ['_api_resource_class' => 'Foo', '_api_collection_operation_name' => 'get']); + + $eventProphecy = $this->prophesize(GetResponseEvent::class); + $eventProphecy->getRequest()->willReturn($request)->shouldBeCalled(); + $event = $eventProphecy->reveal(); + + $resourceMetadata = new ResourceMetadata(null, null, null, null, null, ['is_granted' => 'has_role("ROLE_ADMIN")']); + + $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class); + $resourceMetadataFactoryProphecy->create('Foo')->willReturn($resourceMetadata)->shouldBeCalled(); + + $listener = new DenyAccessListener($resourceMetadataFactoryProphecy->reveal(), null); + $listener->onKernelRequest($event); + } +} diff --git a/tests/Fixtures/TestBundle/Entity/SecuredDummy.php b/tests/Fixtures/TestBundle/Entity/SecuredDummy.php new file mode 100644 index 00000000000..c734aef84bd --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/SecuredDummy.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 ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity; + +use ApiPlatform\Core\Annotation\ApiResource; +use Doctrine\ORM\Mapping as ORM; +use Symfony\Component\Validator\Constraints as Assert; + +/** + * Secured resource. + * + * @author Kévin Dunglas + * + * @ApiResource( + * attributes={"is_granted"="has_role('ROLE_USER')"}, + * collectionOperations={ + * "get"={"method"="GET"}, + * "post"={"method"="POST", "is_granted"="has_role('ROLE_ADMIN')"} + * }, + * itemOperations={ + * "get"={"method"="GET", "is_granted"="has_role('ROLE_USER') and object.getOwner() == user"} + * } + * ) + * @ORM\Entity + */ +class SecuredDummy +{ + /** + * @var int + * + * @ORM\Column(type="integer") + * @ORM\Id + * @ORM\GeneratedValue(strategy="AUTO") + */ + private $id; + + /** + * @var string The title + * + * @ORM\Column + * @Assert\NotBlank + */ + private $title; + + /** + * @var string The description + * + * @ORM\Column + */ + private $description = ''; + + /** + * @var string The owner + * + * @ORM\Column + * @Assert\NotBlank + */ + private $owner; + + /** + * @return int + */ + public function getId(): int + { + return $this->id; + } + + public function getTitle(): string + { + return $this->title; + } + + public function setTitle(string $title) + { + $this->title = $title; + } + + public function getDescription() + { + return $this->description; + } + + public function setDescription(string $description) + { + $this->description = $description; + } + + public function getOwner(): string + { + return $this->owner; + } + + public function setOwner(string $owner) + { + $this->owner = $owner; + } +} diff --git a/tests/Fixtures/app/config/security.yml b/tests/Fixtures/app/config/security.yml index 49c39eeef9e..0d982d58a01 100644 --- a/tests/Fixtures/app/config/security.yml +++ b/tests/Fixtures/app/config/security.yml @@ -1,23 +1,34 @@ security: encoders: - FOS\UserBundle\Model\UserInterface: plaintext + # Don't use plaintext in production! + Symfony\Component\Security\Core\User\UserInterface: plaintext providers: + chain_provider: + chain: + providers: [in_memory, fos_userbundle] + + in_memory: + memory: + users: + dunglas: + password: kevin + roles: 'ROLE_USER' + admin: + password: kitten + roles: 'ROLE_ADMIN' + fos_userbundle: id: fos_user.user_provider.username_email firewalls: dev: - pattern: ^/(_(profiler|wdt|error)|css|images|js)/ - security: false - - api: - pattern: ^/ + pattern: ^/(_(profiler|wdt|error)|css|images|js)/ security: false - stateless: true - anonymous: true - anonymous: + default: + provider: chain_provider + http_basic: ~ anonymous: ~ access_control: