[Bugfix] Reject resource objects sent in relationships

The spec defines that only resource identifiers are expected,
so it is invalid to send a resource object. This is detected
based on the presence of either an attributes or a relationships
member in the object.

Author: lindyhopchris

CHANGELOG.md

@@ -5,6 +5,9 @@ All notable changes to this project will be documented in this file. This projec
 ## Unreleased
 
 ### Fixed
+- [#302](https://github.com/cloudcreativity/laravel-json-api/issues/302)
+Reject resource objects sent in relationships, as the spec defines that only resource identifiers
+are expected. This is based on the object having either an `attributes` or a `relationships` member.
 - [#295](https://github.com/cloudcreativity/laravel-json-api/issues/295)
 Use `null` for empty `from`/`to` fields in cursor page meta.
 

resources/lang/en/errors.php

@@ -46,6 +46,12 @@
         'code' => '',
     ],
 
+    'member_identifier_expected' => [
+        'title' => 'Non-Compliant JSON API Document',
+        'detail' => 'The member :member must be a resource identifier.',
+        'code' => '',
+    ],
+
     'member_string_expected' => [
         'title' => 'Non-Compliant JSON API Document',
         'detail' => "The member :member must be a string.",

src/Validation/ErrorTranslator.php

@@ -100,6 +100,26 @@ public function memberNotObject(string $path, string $member): ErrorInterface
         );
     }
 
+    /**
+     * Create an error for a member that must be a resource identifier.
+     *
+     * @param string $path
+     * @param string $member
+     * @return ErrorInterface
+     */
+    public function memberNotIdentifier(string $path, string $member): ErrorInterface
+    {
+        return new Error(
+            null,
+            null,
+            Response::HTTP_BAD_REQUEST,
+            $this->trans('member_identifier_expected', 'code'),
+            $this->trans('member_identifier_expected', 'title'),
+            $this->trans('member_identifier_expected', 'detail', compact('member')),
+            $this->pointer($path, $member)
+        );
+    }
+
     /**
      * Create an error for when a member has a field that is not allowed.
      *
@@ -475,11 +495,9 @@ protected function trans(string $key, string $member, array $replace = [], ?stri
      */
     protected function pointer(string $path, ?string $member = null): array
     {
-        if (!$member) {
-            $pointer = $path;
-        } else {
-            $pointer = $member ? sprintf('%s/%s', rtrim($path, '/'), $member) : $path;
-        }
+        /** Member can be '0' which is an empty string. */
+        $withoutMember = is_null($member) || '' === $member;
+        $pointer = !$withoutMember ? sprintf('%s/%s', rtrim($path, '/'), $member) : $path;
 
         return [Error::SOURCE_POINTER => $pointer];
     }

src/Validation/Spec/AbstractValidator.php

@@ -170,22 +170,22 @@ protected function validateIdMember($value, string $path): bool
      * Validate an identifier object.
      *
      * @param mixed $value
-     * @param string $dataPath
+     * @param string $path
      *      the path to the data member in which the identifier is contained.
      * @param int|null $index
      *      the index for the identifier, if in a collection.
      * @return bool
      */
-    protected function validateIdentifier($value, string $dataPath, ?int $index = null): bool
+    protected function validateIdentifier($value, string $path, ?int $index = null): bool
     {
         $member = is_int($index) ? (string) $index : 'data';
 
         if (!is_object($value)) {
-            $this->memberNotObject($dataPath, $member);
+            $this->memberNotObject($path, $member);
             return false;
         }
 
-        $dataPath = sprintf('%s/%s', rtrim($dataPath, '/'), $member);
+        $dataPath = sprintf('%s/%s', rtrim($path, '/'), $member);
         $valid = true;
 
         if (!property_exists($value, 'type')) {
@@ -202,6 +202,12 @@ protected function validateIdentifier($value, string $dataPath, ?int $index = nu
             $valid = false;
         }
 
+        /** If it has attributes or relationships, it is a resource object not a resource identifier */
+        if (property_exists($value, 'attributes') || property_exists($value, 'relationships')) {
+            $this->memberNotIdentifier($path, $member);
+            $valid = false;
+        }
+
         return $valid;
     }
 
@@ -363,6 +369,18 @@ protected function memberNotObject(string $path, string $member): void
         $this->errors->add($this->translator->memberNotObject($path, $member));
     }
 
+    /**
+     * Add an error for a member that must be an object.
+     *
+     * @param string $path
+     * @param string|null $member
+     * @return void
+     */
+    protected function memberNotIdentifier(string $path, string $member): void
+    {
+        $this->errors->add($this->translator->memberNotIdentifier($path, $member));
+    }
+
     /**
      * Add errors for when a member has a field that is not allowed.
      *

tests/lib/Integration/Validation/Spec/RelationshipValidationTest.php

@@ -175,6 +175,42 @@ public function toOneProvider()
                     'source' => ['pointer' => '/data'],
                 ],
             ],
+            'data:resource object with attributes' => [
+                [
+                    'data' => [
+                        'type' => 'users',
+                        'id' => '1',
+                        'attributes' => [
+                            'name' => 'John Doe',
+                        ],
+                    ],
+                ],
+                [
+                    'title' => 'Non-Compliant JSON API Document',
+                    'detail' => 'The member data must be a resource identifier.',
+                    'status' => '400',
+                    'source' => ['pointer' => '/data'],
+                ],
+            ],
+            'data:resource object with relationships' => [
+                [
+                    'data' => [
+                        'type' => 'users',
+                        'id' => '1',
+                        'relationships' => [
+                            'sites' => [
+                                'data' => [],
+                            ],
+                        ],
+                    ],
+                ],
+                [
+                    'title' => 'Non-Compliant JSON API Document',
+                    'detail' => 'The member data must be a resource identifier.',
+                    'status' => '400',
+                    'source' => ['pointer' => '/data'],
+                ],
+            ],
         ];
     }
 
@@ -310,6 +346,25 @@ public function toManyProvider()
                     'source' => ['pointer' => '/data/0'],
                 ],
             ],
+            'data:resource object with attributes' => [
+                [
+                    'data' => [
+                        [
+                            'type' => 'tags',
+                            'id' => '100',
+                            'attributes' => [
+                                'name' => 'News',
+                            ],
+                        ],
+                    ],
+                ],
+                [
+                    'title' => 'Non-Compliant JSON API Document',
+                    'detail' => 'The member 0 must be a resource identifier.',
+                    'status' => '400',
+                    'source' => ['pointer' => '/data/0'],
+                ],
+            ],
         ];
     }
 

tests/lib/Integration/Validation/Spec/ResourceValidationTest.php

@@ -338,7 +338,7 @@ public function postProvider()
                     'title' => 'Non-Compliant JSON API Document',
                     'detail' => "The member 0 must be an object.",
                     'status' => '400',
-                    'source' => ['pointer' => '/data/relationships/tags/data'],
+                    'source' => ['pointer' => '/data/relationships/tags/data/0'],
                 ],
             ],
             'data.relationships.*.data.*.type:required' => [