Problem/Motivation

When attempting to POST an entity without the necessary authorization, one receives a 403 response. This is correct.

  1. What's incorrect/confusing, is that the server-side generated id (a UUID) is exposed as if the client could possibly know what the UUID is, or as if it's remotely helpful.
  2. Related, but the inverse problem: when POSTing with an invalid id, thanks to #2934386: Accept client-generated IDs (UUIDs), we now get a 403 (saying IDs should be properly generated and formatted UUIDs as described in RFC 4122) … but there we want a pointer, and we want the id.

Proposed resolution

Allow the first parameter to \Drupal\jsonapi\Exception\EntityAccessDeniedHttpException::__construct() is now allowed to be NULL. An assertion can still ensure it's either an entity or NULL.

Remaining tasks

None.

User interface changes

None.

API changes

The first parameter to \Drupal\jsonapi\Exception\EntityAccessDeniedHttpException::__construct() is now allowed to be NULL. (An assertion ensures it's still either an entity or NULL.)

Data model changes

None.

CommentFileSizeAuthor
#11 2942243-followup-11.patch845 byteswim leers
#6 2942243-6.patch6.37 KBwim leers
#6 interdiff-5-6.txt830 byteswim leers
#5 2942243-5.patch6.04 KBwim leers
#5 interdiff-2-5.txt1.95 KBwim leers
#2 2942243-2.patch4.89 KBwim leers

Comments

Wim Leers created an issue. See original summary.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Active » Needs review
StatusFileSize
new 2942243-2.patch4.89 KB
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +blocker

This blocks a green re-roll of the last green patch at #2934386: Accept client-generated IDs (UUIDs).

gabesullice’s picture

gabesullice
Primary language English
commented
Status: Needs review » Needs work 
  1. +++ b/src/Exception/EntityAccessDeniedHttpException.php
@@ -33,8 +33,8 @@ class EntityAccessDeniedHttpException extends HttpException {
+   * @param \Drupal\Core\Entity\EntityInterface|NULL $entity

    null 

  2. +++ b/src/Exception/EntityAccessDeniedHttpException.php
@@ -48,7 +48,8 @@ class EntityAccessDeniedHttpException extends HttpException {
+    assert(is_null($entity) || $entity instanceof EntityInterface);

    nice.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-2-5.txt1.95 KB
new 2942243-5.patch6.04 KB

#4: done, good catch!

Also updated the existing test coverage.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
StatusFileSize
new interdiff-5-6.txt830 bytes
new 2942243-6.patch6.37 KB

Removed unused class.

gabesullice’s picture

gabesullice
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

RTBC if tests pass.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Considering this hard-blocks #2930028-48: Comprehensive JSON API integration test coverage phase 1: for every entity type, individual resources only, I'm going to go ahead and commit this.

  • Wim Leers committed b9909c9 on 8.x-1.x 
    Issue #2942243 by Wim Leers, gabesullice: Follow-up for #2934386: server...
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Reviewed & tested by the community » Fixed

Test coverage for this can be found in #2930028-48: Comprehensive JSON API integration test coverage phase 1: for every entity type, individual resources only.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
StatusFileSize
new 2942243-followup-11.patch845 bytes

This added a coding standards violation. Fixing.

  • Wim Leers committed 6ac575a on 8.x-1.x 
    Issue #2942243 by Wim Leers, gabesullice: Follow-up for #2934386: server...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.