Fix api-platform/api-platform#132: disallow update for non-PUT methods (#776)

Author: meyerbaptiste

features/hydra/error.feature

@@ -84,3 +84,42 @@ Feature: Error handling
     And the JSON node "hydra:title" should be equal to "An error occurred"
     And the JSON node "hydra:description" should exist
     And the JSON node "trace" should exist
+
+    Scenario: Get an error during update of an existing resource with a non-allowed update operation
+      When I add "Content-Type" header equal to "application/ld+json"
+      And I send a "POST" request to "/dummies" with body:
+      """
+      {
+        "@id": "/dummies/1",
+        "name": "Foo"
+      }
+      """
+      Then the response status code should be 400
+      And the response should be in JSON
+      And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
+      And the JSON node "@context" should be equal to "/contexts/Error"
+      And the JSON node "@type" should be equal to "Error"
+      And the JSON node "hydra:title" should be equal to "An error occurred"
+      And the JSON node "hydra:description" should be equal to "Update is not allowed for this operation."
+      And the JSON node "trace" should exist
+
+    Scenario: Get an error during update of an existing relation with a non-allowed update operation
+      When I add "Content-Type" header equal to "application/ld+json"
+      And I send a "POST" request to "/relation_embedders" with body:
+      """
+      {
+        "anotherRelated": {
+          "@id": "/related_dummies/2",
+          "@type": "https://schema.org/Product",
+          "symfony": "phalcon"
+        }
+      }
+      """
+      Then the response status code should be 400
+      And the response should be in JSON
+      And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
+      And the JSON node "@context" should be equal to "/contexts/Error"
+      And the JSON node "@type" should be equal to "Error"
+      And the JSON node "hydra:title" should be equal to "An error occurred"
+      And the JSON node "hydra:description" should be equal to "Update is not allowed for this operation."
+      And the JSON node "trace" should exist

features/relation.feature

@@ -303,6 +303,7 @@ Feature: Relations support
     And the response should be in JSON
     And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
 
+  @dropSchema
   Scenario: Update an embedded relation
     When I add "Content-Type" header equal to "application/ld+json"
     And I send a "PUT" request to "/relation_embedders/2" with body:
@@ -333,36 +334,3 @@ Feature: Relations support
       "related": null
     }
     """
-
-  @dropSchema
-  Scenario: Update an existing relation
-    When I add "Content-Type" header equal to "application/ld+json"
-    And I send a "POST" request to "/relation_embedders" with body:
-    """
-    {
-      "anotherRelated": {
-        "@id": "/related_dummies/2",
-        "@type": "https://schema.org/Product",
-        "symfony": "phalcon"
-      }
-    }
-    """
-    Then the response status code should be 201
-    And the response should be in JSON
-    And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
-    And the JSON should be equal to:
-    """
-    {
-      "@context": "/contexts/RelationEmbedder",
-      "@id": "/relation_embedders/3",
-      "@type": "RelationEmbedder",
-      "krondstadt": "Krondstadt",
-      "anotherRelated": {
-        "@id": "/related_dummies/2",
-        "@type": "https://schema.org/Product",
-        "symfony": "phalcon",
-        "thirdLevel": null
-      },
-      "related": null
-    }
-    """

src/JsonLd/Serializer/ItemNormalizer.php

@@ -13,6 +13,7 @@
 
 use ApiPlatform\Core\Api\IriConverterInterface;
 use ApiPlatform\Core\Api\ResourceClassResolverInterface;
+use ApiPlatform\Core\Exception\InvalidArgumentException;
 use ApiPlatform\Core\JsonLd\ContextBuilderInterface;
 use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
 use ApiPlatform\Core\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface;
@@ -83,11 +84,17 @@ public function supportsDenormalization($data, $type, $format = null)
 
     /**
      * {@inheritdoc}
+     *
+     * @throws InvalidArgumentException
      */
     public function denormalize($data, $class, $format = null, array $context = [])
     {
         // Avoid issues with proxies if we populated the object
         if (isset($data['@id']) && !isset($context['object_to_populate'])) {
+            if (isset($context['api_allow_update']) && true !== $context['api_allow_update']) {
+                throw new InvalidArgumentException('Update is not allowed for this operation.');
+            }
+
             $context['object_to_populate'] = $this->iriConverter->getItemFromIri($data['@id'], true);
         }
 

src/Serializer/ItemNormalizer.php

@@ -11,6 +11,8 @@
 
 namespace ApiPlatform\Core\Serializer;
 
+use ApiPlatform\Core\Exception\InvalidArgumentException;
+
 /**
  * Generic item normalizer.
  *
@@ -20,11 +22,17 @@ final class ItemNormalizer extends AbstractItemNormalizer
 {
     /**
      * {@inheritdoc}
+     *
+     * @throws InvalidArgumentException
      */
     public function denormalize($data, $class, $format = null, array $context = [])
     {
         // Avoid issues with proxies if we populated the object
         if (isset($data['id']) && !isset($context['object_to_populate'])) {
+            if (isset($context['api_allow_update']) && true !== $context['api_allow_update']) {
+                throw new InvalidArgumentException('Update is not allowed for this operation.');
+            }
+
             $context['object_to_populate'] = $this->iriConverter->getItemFromIri($data['id'], true);
         }
 

src/Serializer/SerializerContextBuilder.php

@@ -49,6 +49,10 @@ public function createFromRequest(Request $request, bool $normalization, array $
             $context['item_operation_name'] = $attributes['item_operation_name'];
         }
 
+        if (!$normalization && !isset($context['api_allow_update'])) {
+            $context['api_allow_update'] = Request::METHOD_PUT === $request->getMethod();
+        }
+
         $context['resource_class'] = $attributes['resource_class'];
         $context['request_uri'] = $request->getRequestUri();
 

tests/Serializer/ItemNormalizerTest.php

@@ -97,7 +97,7 @@ public function testNormalize()
 
     public function testDenormalize()
     {
-        $context = ['resource_class' => Dummy::class];
+        $context = ['resource_class' => Dummy::class, 'api_allow_update' => true];
 
         $propertyNameCollection = new PropertyNameCollection(['name']);
         $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class);
@@ -124,4 +124,33 @@ public function testDenormalize()
 
         $this->assertInstanceOf(Dummy::class, $normalizer->denormalize(['name' => 'hello'], Dummy::class, null, $context));
     }
+
+    /**
+     * @expectedException \ApiPlatform\Core\Exception\InvalidArgumentException
+     * @expectedExceptionMessage Update is not allowed for this operation.
+     */
+    public function testDenormalizeWithIdAndUpdateNotAllowed()
+    {
+        $context = ['resource_class' => Dummy::class, 'api_allow_update' => false];
+
+        $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class);
+
+        $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class);
+
+        $iriConverterProphecy = $this->prophesize(IriConverterInterface::class);
+
+        $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class);
+
+        $serializerProphecy = $this->prophesize(SerializerInterface::class);
+        $serializerProphecy->willImplement(DenormalizerInterface::class);
+
+        $normalizer = new ItemNormalizer(
+            $propertyNameCollectionFactoryProphecy->reveal(),
+            $propertyMetadataFactoryProphecy->reveal(),
+            $iriConverterProphecy->reveal(),
+            $resourceClassResolverProphecy->reveal()
+        );
+        $normalizer->setSerializer($serializerProphecy->reveal());
+        $normalizer->denormalize(['id' => '/dummies/12', 'name' => 'hello'], Dummy::class, null, $context);
+    }
 }

tests/Serializer/SerializerContextBuilderTest.php

@@ -54,11 +54,15 @@ public function testCreateFromRequest()
         $this->assertEquals($expected, $this->builder->createFromRequest($request, true));
 
         $request = new Request([], [], ['_api_resource_class' => 'Foo', '_api_item_operation_name' => 'get', '_api_format' => 'xml', '_api_mime_type' => 'text/xml']);
-        $expected = ['bar' => 'baz', 'item_operation_name' => 'get',  'resource_class' => 'Foo', 'request_uri' => ''];
+        $expected = ['bar' => 'baz', 'item_operation_name' => 'get',  'resource_class' => 'Foo', 'request_uri' => '', 'api_allow_update' => false];
         $this->assertEquals($expected, $this->builder->createFromRequest($request, false));
 
-        $request = new Request([], [], ['_api_resource_class' => 'Foo', '_api_collection_operation_name' => 'post', '_api_format' => 'xml', '_api_mime_type' => 'text/xml']);
-        $expected = ['bar' => 'baz', 'collection_operation_name' => 'post',  'resource_class' => 'Foo', 'request_uri' => ''];
+        $request = new Request([], [], ['_api_resource_class' => 'Foo', '_api_collection_operation_name' => 'post', '_api_format' => 'xml', '_api_mime_type' => 'text/xml'], [], [], ['REQUEST_METHOD' => 'POST']);
+        $expected = ['bar' => 'baz', 'collection_operation_name' => 'post',  'resource_class' => 'Foo', 'request_uri' => '', 'api_allow_update' => false];
+        $this->assertEquals($expected, $this->builder->createFromRequest($request, false));
+
+        $request = new Request([], [], ['_api_resource_class' => 'Foo', '_api_collection_operation_name' => 'put', '_api_format' => 'xml', '_api_mime_type' => 'text/xml'], [], [], ['REQUEST_METHOD' => 'PUT']);
+        $expected = ['bar' => 'baz', 'collection_operation_name' => 'put', 'resource_class' => 'Foo', 'request_uri' => '', 'api_allow_update' => true];
         $this->assertEquals($expected, $this->builder->createFromRequest($request, false));
     }
 
@@ -72,7 +76,7 @@ public function testThrowExceptionOnInvalidRequest()
 
     public function testReuseExistingAttributes()
     {
-        $expected = ['bar' => 'baz', 'item_operation_name' => 'get', 'resource_class' => 'Foo', 'request_uri' => ''];
+        $expected = ['bar' => 'baz', 'item_operation_name' => 'get', 'resource_class' => 'Foo', 'request_uri' => '', 'api_allow_update' => false];
         $this->assertEquals($expected, $this->builder->createFromRequest(new Request(), false, ['resource_class' => 'Foo', 'item_operation_name' => 'get']));
     }
 }