Merge pull request #2992 from vincentchalamon/issue/1646

Deprecate access_control attribute, add security + security_post_denormalize attributes
api-platform · Aug 20, 2019 · 23d0f8f · 23d0f8f
2 parents 3c98fdb + 1f06b4e
commit 23d0f8f
Show file tree

Hide file tree

Showing 27 changed files with 814 additions and 117 deletions.
diff --git a/features/authorization/legacy_deny.feature b/features/authorization/legacy_deny.feature
@@ -0,0 +1,96 @@
+Feature: Authorization checking
+  In order to use the API
+  As a client software user
+  I need to be authorized to access a given resource using legacy access_control attribute.
+
+  @createSchema
+  Scenario: An anonymous user retrieves a secured resource
+    When I add "Accept" header equal to "application/ld+json"
+    And I send a "GET" request to "/legacy_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 "/legacy_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 "/legacy_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 "/legacy_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 "/legacy_secured_dummies" with body:
+    """
+    {
+        "title": "Special Title",
+        "description": "Description",
+        "owner": "dunglas"
+    }
+    """
+    Then the response status code should be 201
+
+  Scenario: A user cannot retrieve an item they 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 "/legacy_secured_dummies/1"
+    Then the response status code should be 403
+    And the response should be in JSON
+
+  Scenario: A user can retrieve an item they 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 "/legacy_secured_dummies/2"
+    Then the response status code should be 200
+
+  Scenario: A user can't assign to themself an item they doesn't own
+    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 "PUT" request to "/legacy_secured_dummies/2" with body:
+    """
+    {
+        "owner": "kitten"
+    }
+    """
+    Then the response status code should be 403
+
+  Scenario: A user can update an item they owns and transfer it
+    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 "PUT" request to "/legacy_secured_dummies/2" with body:
+    """
+    {
+        "owner": "vincent"
+    }
+    """
+    Then the response status code should be 200
diff --git a/src/Annotation/ApiResource.php b/src/Annotation/ApiResource.php
@@ -25,6 +25,10 @@
  * @Attributes(
  *     @Attribute("accessControl", type="string"),
  *     @Attribute("accessControlMessage", type="string"),
+ *     @Attribute("security", type="string"),
+ *     @Attribute("securityMessage", type="string"),
+ *     @Attribute("securityPostDenormalize", type="string"),
+ *     @Attribute("securityPostDenormalizeMessage", type="string"),
  *     @Attribute("attributes", type="array"),
  *     @Attribute("cacheHeaders", type="array"),
  *     @Attribute("collectionOperations", type="array"),
@@ -108,14 +112,28 @@ final class ApiResource
      *
      * @var string
      */
-    private $accessControl;
+    private $security;
 
     /**
      * @see https://github.com/Haehnchen/idea-php-annotation-plugin/issues/112
      *
      * @var string
      */
-    private $accessControlMessage;
+    private $securityMessage;
+
+    /**
+     * @see https://github.com/Haehnchen/idea-php-annotation-plugin/issues/112
+     *
+     * @var string
+     */
+    private $securityPostDenormalize;
+
+    /**
+     * @see https://github.com/Haehnchen/idea-php-annotation-plugin/issues/112
+     *
+     * @var string
+     */
+    private $securityPostDenormalizeMessage;
 
     /**
      * @see https://github.com/Haehnchen/idea-php-annotation-plugin/issues/112

diff --git a/src/Bridge/Symfony/Bundle/Resources/config/graphql.xml b/src/Bridge/Symfony/Bundle/Resources/config/graphql.xml
@@ -11,15 +11,17 @@
 
         <service id="api_platform.graphql.resolver.factory.item" class="ApiPlatform\Core\GraphQl\Resolver\Factory\ItemResolverFactory" public="false">
             <argument type="service" id="api_platform.graphql.resolver.stage.read" />
-            <argument type="service" id="api_platform.graphql.resolver.stage.deny_access" />
+            <argument type="service" id="api_platform.graphql.resolver.stage.security" />
+            <argument type="service" id="api_platform.graphql.resolver.stage.security_post_denormalize" />
             <argument type="service" id="api_platform.graphql.resolver.stage.serialize" />
             <argument type="service" id="api_platform.graphql.query_resolver_locator" />
             <argument type="service" id="api_platform.metadata.resource.metadata_factory" />
         </service>
 
         <service id="api_platform.graphql.resolver.factory.collection" class="ApiPlatform\Core\GraphQl\Resolver\Factory\CollectionResolverFactory" public="false">
             <argument type="service" id="api_platform.graphql.resolver.stage.read" />
-            <argument type="service" id="api_platform.graphql.resolver.stage.deny_access" />
+            <argument type="service" id="api_platform.graphql.resolver.stage.security" />
+            <argument type="service" id="api_platform.graphql.resolver.stage.security_post_denormalize" />
             <argument type="service" id="api_platform.graphql.resolver.stage.serialize" />
             <argument type="service" id="api_platform.graphql.query_resolver_locator" />
             <argument type="service" id="api_platform.metadata.resource.metadata_factory" />
@@ -28,7 +30,8 @@
 
         <service id="api_platform.graphql.resolver.factory.item_mutation" class="ApiPlatform\Core\GraphQl\Resolver\Factory\ItemMutationResolverFactory" public="false">
             <argument type="service" id="api_platform.graphql.resolver.stage.read" />
-            <argument type="service" id="api_platform.graphql.resolver.stage.deny_access" />
+            <argument type="service" id="api_platform.graphql.resolver.stage.security" />
+            <argument type="service" id="api_platform.graphql.resolver.stage.security_post_denormalize" />
             <argument type="service" id="api_platform.graphql.resolver.stage.serialize" />
             <argument type="service" id="api_platform.graphql.resolver.stage.deserialize" />
             <argument type="service" id="api_platform.graphql.resolver.stage.write" />
@@ -47,7 +50,12 @@
             <argument type="service" id="api_platform.graphql.serializer.context_builder" />
         </service>
 
-        <service id="api_platform.graphql.resolver.stage.deny_access" class="ApiPlatform\Core\GraphQl\Resolver\Stage\DenyAccessStage" public="false">
+        <service id="api_platform.graphql.resolver.stage.security" class="ApiPlatform\Core\GraphQl\Resolver\Stage\SecurityStage" public="false">
+            <argument type="service" id="api_platform.metadata.resource.metadata_factory" />
+            <argument type="service" id="api_platform.security.resource_access_checker" />
+        </service>
+
+        <service id="api_platform.graphql.resolver.stage.security_post_denormalize" class="ApiPlatform\Core\GraphQl\Resolver\Stage\SecurityPostDenormalizeStage" public="false">
             <argument type="service" id="api_platform.metadata.resource.metadata_factory" />
             <argument type="service" id="api_platform.security.resource_access_checker" />
         </service>

diff --git a/src/Bridge/Symfony/Bundle/Resources/config/security.xml b/src/Bridge/Symfony/Bundle/Resources/config/security.xml
@@ -20,8 +20,10 @@
             <argument type="service" id="api_platform.metadata.resource.metadata_factory" />
             <argument type="service" id="api_platform.security.resource_access_checker" />
 
-            <!-- This listener must be executed only when the current object is available -->
-            <tag name="kernel.event_listener" event="kernel.request" method="onKernelRequest" priority="1" />
+            <!-- This method must be executed only when the current object is available, before deserialization -->
+            <tag name="kernel.event_listener" event="kernel.request" method="onSecurity" priority="3" />
+            <!-- This method must be executed only when the current object is available, after deserialization -->
+            <tag name="kernel.event_listener" event="kernel.request" method="onSecurityPostDenormalize" priority="1" />
         </service>
 
         <service id="api_platform.security.expression_language_provider" class="ApiPlatform\Core\Security\Core\Authorization\ExpressionLanguageProvider" public="false">

diff --git a/src/GraphQl/Resolver/Factory/CollectionResolverFactory.php b/src/GraphQl/Resolver/Factory/CollectionResolverFactory.php
@@ -14,8 +14,9 @@
 namespace ApiPlatform\Core\GraphQl\Resolver\Factory;
 
 use ApiPlatform\Core\GraphQl\Resolver\QueryCollectionResolverInterface;
-use ApiPlatform\Core\GraphQl\Resolver\Stage\DenyAccessStageInterface;
 use ApiPlatform\Core\GraphQl\Resolver\Stage\ReadStageInterface;
+use ApiPlatform\Core\GraphQl\Resolver\Stage\SecurityPostDenormalizeStageInterface;
+use ApiPlatform\Core\GraphQl\Resolver\Stage\SecurityStageInterface;
 use ApiPlatform\Core\GraphQl\Resolver\Stage\SerializeStageInterface;
 use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
 use ApiPlatform\Core\Util\CloneTrait;
@@ -30,22 +31,25 @@
  *
  * @author Alan Poulain <contact@alanpoulain.eu>
  * @author Kévin Dunglas <dunglas@gmail.com>
+ * @author Vincent Chalamon <vincentchalamon@gmail.com>
  */
 final class CollectionResolverFactory implements ResolverFactoryInterface
 {
     use CloneTrait;
 
     private $readStage;
-    private $denyAccessStage;
+    private $securityStage;
+    private $securityPostDenormalizeStage;
     private $serializeStage;
     private $queryResolverLocator;
     private $requestStack;
     private $resourceMetadataFactory;
 
-    public function __construct(ReadStageInterface $readStage, DenyAccessStageInterface $denyAccessStage, SerializeStageInterface $serializeStage, ContainerInterface $queryResolverLocator, ResourceMetadataFactoryInterface $resourceMetadataFactory, RequestStack $requestStack = null)
+    public function __construct(ReadStageInterface $readStage, SecurityStageInterface $securityStage, SecurityPostDenormalizeStageInterface $securityPostDenormalizeStage, SerializeStageInterface $serializeStage, ContainerInterface $queryResolverLocator, ResourceMetadataFactoryInterface $resourceMetadataFactory, RequestStack $requestStack = null)
     {
         $this->readStage = $readStage;
-        $this->denyAccessStage = $denyAccessStage;
+        $this->securityStage = $securityStage;
+        $this->securityPostDenormalizeStage = $securityPostDenormalizeStage;
         $this->serializeStage = $serializeStage;
         $this->queryResolverLocator = $queryResolverLocator;
         $this->requestStack = $requestStack;
@@ -83,7 +87,12 @@ public function __invoke(?string $resourceClass = null, ?string $rootClass = nul
                 $collection = $queryResolver($collection, $resolverContext);
             }
 
-            ($this->denyAccessStage)($resourceClass, $operationName, $resolverContext + [
+            ($this->securityStage)($resourceClass, $operationName, $resolverContext + [
+                'extra_variables' => [
+                    'object' => $collection,
+                ],
+            ]);
+            ($this->securityPostDenormalizeStage)($resourceClass, $operationName, $resolverContext + [
                 'extra_variables' => [
                     'object' => $collection,
                     'previous_object' => $this->clone($collection),

diff --git a/src/GraphQl/Resolver/Factory/ItemMutationResolverFactory.php b/src/GraphQl/Resolver/Factory/ItemMutationResolverFactory.php
@@ -14,9 +14,10 @@
 namespace ApiPlatform\Core\GraphQl\Resolver\Factory;
 
 use ApiPlatform\Core\GraphQl\Resolver\MutationResolverInterface;
-use ApiPlatform\Core\GraphQl\Resolver\Stage\DenyAccessStageInterface;
 use ApiPlatform\Core\GraphQl\Resolver\Stage\DeserializeStageInterface;
 use ApiPlatform\Core\GraphQl\Resolver\Stage\ReadStageInterface;
+use ApiPlatform\Core\GraphQl\Resolver\Stage\SecurityPostDenormalizeStageInterface;
+use ApiPlatform\Core\GraphQl\Resolver\Stage\SecurityStageInterface;
 use ApiPlatform\Core\GraphQl\Resolver\Stage\SerializeStageInterface;
 use ApiPlatform\Core\GraphQl\Resolver\Stage\ValidateStageInterface;
 use ApiPlatform\Core\GraphQl\Resolver\Stage\WriteStageInterface;
@@ -33,25 +34,28 @@
  * @experimental
  *
  * @author Alan Poulain <contact@alanpoulain.eu>
+ * @author Vincent Chalamon <vincentchalamon@gmail.com>
  */
 final class ItemMutationResolverFactory implements ResolverFactoryInterface
 {
     use ClassInfoTrait;
     use CloneTrait;
 
     private $readStage;
-    private $denyAccessStage;
+    private $securityStage;
+    private $securityPostDenormalizeStage;
     private $serializeStage;
     private $deserializeStage;
     private $writeStage;
     private $validateStage;
     private $mutationResolverLocator;
     private $resourceMetadataFactory;
 
-    public function __construct(ReadStageInterface $readStage, DenyAccessStageInterface $denyAccessStage, SerializeStageInterface $serializeStage, DeserializeStageInterface $deserializeStage, WriteStageInterface $writeStage, ValidateStageInterface $validateStage, ContainerInterface $mutationResolverLocator, ResourceMetadataFactoryInterface $resourceMetadataFactory)
+    public function __construct(ReadStageInterface $readStage, SecurityStageInterface $securityStage, SecurityPostDenormalizeStageInterface $securityPostDenormalizeStage, SerializeStageInterface $serializeStage, DeserializeStageInterface $deserializeStage, WriteStageInterface $writeStage, ValidateStageInterface $validateStage, ContainerInterface $mutationResolverLocator, ResourceMetadataFactoryInterface $resourceMetadataFactory)
     {
         $this->readStage = $readStage;
-        $this->denyAccessStage = $denyAccessStage;
+        $this->securityStage = $securityStage;
+        $this->securityPostDenormalizeStage = $securityPostDenormalizeStage;
         $this->serializeStage = $serializeStage;
         $this->deserializeStage = $deserializeStage;
         $this->writeStage = $writeStage;
@@ -73,16 +77,20 @@ public function __invoke(?string $resourceClass = null, ?string $rootClass = nul
             if (null !== $item && !\is_object($item)) {
                 throw new \LogicException('Item from read stage should be a nullable object.');
             }
+            ($this->securityStage)($resourceClass, $operationName, $resolverContext + [
+                'extra_variables' => [
+                    'object' => $item,
+                ],
+            ]);
             $previousItem = $this->clone($item);
 
             if ('delete' === $operationName) {
-                ($this->denyAccessStage)($resourceClass, $operationName, $resolverContext + [
+                ($this->securityPostDenormalizeStage)($resourceClass, $operationName, $resolverContext + [
                     'extra_variables' => [
                         'object' => $item,
                         'previous_object' => $previousItem,
                     ],
                 ]);
-
                 $item = ($this->writeStage)($item, $resourceClass, $operationName, $resolverContext);
 
                 return ($this->serializeStage)($item, $resourceClass, $operationName, $resolverContext);
@@ -102,7 +110,7 @@ public function __invoke(?string $resourceClass = null, ?string $rootClass = nul
                 }
             }
 
-            ($this->denyAccessStage)($resourceClass, $operationName, $resolverContext + [
+            ($this->securityPostDenormalizeStage)($resourceClass, $operationName, $resolverContext + [
                 'extra_variables' => [
                     'object' => $item,
                     'previous_object' => $previousItem,

diff --git a/src/GraphQl/Resolver/Factory/ItemResolverFactory.php b/src/GraphQl/Resolver/Factory/ItemResolverFactory.php
@@ -14,8 +14,9 @@
 namespace ApiPlatform\Core\GraphQl\Resolver\Factory;
 
 use ApiPlatform\Core\GraphQl\Resolver\QueryItemResolverInterface;
-use ApiPlatform\Core\GraphQl\Resolver\Stage\DenyAccessStageInterface;
 use ApiPlatform\Core\GraphQl\Resolver\Stage\ReadStageInterface;
+use ApiPlatform\Core\GraphQl\Resolver\Stage\SecurityPostDenormalizeStageInterface;
+use ApiPlatform\Core\GraphQl\Resolver\Stage\SecurityStageInterface;
 use ApiPlatform\Core\GraphQl\Resolver\Stage\SerializeStageInterface;
 use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
 use ApiPlatform\Core\Util\ClassInfoTrait;
@@ -31,22 +32,25 @@
  *
  * @author Alan Poulain <contact@alanpoulain.eu>
  * @author Kévin Dunglas <dunglas@gmail.com>
+ * @author Vincent Chalamon <vincentchalamon@gmail.com>
  */
 final class ItemResolverFactory implements ResolverFactoryInterface
 {
     use CloneTrait;
     use ClassInfoTrait;
 
     private $readStage;
-    private $denyAccessStage;
+    private $securityStage;
+    private $securityPostDenormalizeStage;
     private $serializeStage;
     private $queryResolverLocator;
     private $resourceMetadataFactory;
 
-    public function __construct(ReadStageInterface $readStage, DenyAccessStageInterface $denyAccessStage, SerializeStageInterface $serializeStage, ContainerInterface $queryResolverLocator, ResourceMetadataFactoryInterface $resourceMetadataFactory)
+    public function __construct(ReadStageInterface $readStage, SecurityStageInterface $securityStage, SecurityPostDenormalizeStageInterface $securityPostDenormalizeStage, SerializeStageInterface $serializeStage, ContainerInterface $queryResolverLocator, ResourceMetadataFactoryInterface $resourceMetadataFactory)
     {
         $this->readStage = $readStage;
-        $this->denyAccessStage = $denyAccessStage;
+        $this->securityStage = $securityStage;
+        $this->securityPostDenormalizeStage = $securityPostDenormalizeStage;
         $this->serializeStage = $serializeStage;
         $this->queryResolverLocator = $queryResolverLocator;
         $this->resourceMetadataFactory = $resourceMetadataFactory;
@@ -79,7 +83,12 @@ public function __invoke(?string $resourceClass = null, ?string $rootClass = nul
                 $resourceClass = $this->getResourceClass($item, $resourceClass, $info, sprintf('Custom query resolver "%s"', $queryResolverId).' has to return an item of class %s but returned an item of class %s.');
             }
 
-            ($this->denyAccessStage)($resourceClass, $operationName, $resolverContext + [
+            ($this->securityStage)($resourceClass, $operationName, $resolverContext + [
+                'extra_variables' => [
+                    'object' => $item,
+                ],
+            ]);
+            ($this->securityPostDenormalizeStage)($resourceClass, $operationName, $resolverContext + [
                 'extra_variables' => [
                     'object' => $item,
                     'previous_object' => $this->clone($item),