When creating a config entity (eg. webform) via JSONAPI POST, I got the following error:

Error: Call to a member function access() on string in Drupal\jsonapi\Controller\EntityResource->createIndividual() (line 205 of modules/contrib/jsonapi/src/Controller/EntityResource.php)

Currently EntityResource::createIndividual() checks for edit permission on all fields of the new entity. Only ContentEntity (FieldableEntityInterface) implements such method. ConfigEntity does not provide real fields. createIndividual() should check for the type of the new entity before checking field access permissions.

CommentFileSizeAuthor
#11 2984494-11.patch3.55 KBwim leers
#2 jsonapi-create_individual_field_access_content_entity-2984494-2.patch3.07 KBsonnykt

Comments

sonnykt created an issue. See original summary.

sonnykt’s picture

sonnykt
Location Melbourne, Australia
commented
Status: Active » Needs review
StatusFileSize
new jsonapi-create_individual_field_access_content_entity-2984494-2.patch3.07 KB
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Postponed (maintainer needs more info)
Related issues: +#2957474: Move the write functionality of config entities to a sub-module in preparation for removal, +#2887313: JSON API indicates it supports POST/PATCH/DELETE of config entity types, but that's impossible

Thanks for reporting this and providing a patch!

Config entity mutation support was supported on a "if it works, it works" basis, it was never thoroughly tested. That's why the 2.x branch chose to do https://www.drupal.org/node/2982668 — it's now up to JSON API Extras to provide support for this.

The code you cited was added for a JSON API security release: https://www.drupal.org/project/jsonapi/releases/8.x-1.9.

I'm not quite sure what to do here — making this proposed change actually signals it's safe to modify config entities, whereas it's not. Could you explain what your use case is? That'd help us understand the relative priority better too!

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +API-First Initiative, +Configuration system, +Entity validation
sonnykt’s picture

sonnykt
Location Melbourne, Australia
commented

Sorry I picked the wrong version. The patch was for JSONAPI 1.x.

I was trying to create a webform via JSONAPI POST command.

POST /api/webform--webform

{
    "data": {
        "type": "webform",
        "attributes": {
            "id": "new_webform",
            "status": "open",
            "title": "New form",
            "description": "New webform",
            "elements": "was_this_page_helpful:\n  '#type': radios\n  '#title': 'Was this page helpful?'\n  '#options': yes_no\n  '#required': true"
        }
    }
}

Webform is a config entity, and the POST command failed at the same line in EntityResource::createIndividual(). Basically $entity->get('id') returns a string instead of a field item, hence ->access() fails.

I haven't tested with 8.x-2.x and JSONAPI Extras 2.x.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: EntityResource::createIndividual() should only check for field access on Content Entity » EntityResource::createIndividual() does not work for config entities because it always checks field access
Category: Bug report » Feature request
Status: Postponed (maintainer needs more info) » Active

Only JSON API Extras 2.x is supported, but I'm 99% certain that you'll run into the same problem there.

Did this work in the past?

sonnykt’s picture

sonnykt
Location Melbourne, Australia
commented

I'm testing on both 1.16 and 1.22, not sure if it ever worked earlier. The same error occurs with creating menu via JSONAPI POST command.

POST /api/menu--menu

{
    "data": {
        "type": "menu",
        "attributes": {
            "id": "new-menu",
            "label": "New menu",
            "description": "New menu"
        }
    }
}
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: Unassigned » gabesullice

This code was introduced in JSON API 1.9 for a security release. Did it work in JSON API 1.8? In any case, it's very telling that nobody reported this being broken for nearly 4 months…

On the one hand, I don't want to fix this because https://www.drupal.org/node/2982668. On the other hand, it means preventing you from solving your need, even if it's insecure.

I'm torn… assigning to @gabesullice to get his thoughts.

gabesullice’s picture

gabesullice
Primary language English
commented
Title: EntityResource::createIndividual() does not work for config entities because it always checks field access » EntityResource calls `$entity->get('field_name')` without first ensuring the entity is a fieldable entity
Assigned: gabesullice » Unassigned
Category: Feature request » Bug report

Perhaps this is too clever by half...

What if we add a check for FieldableEntityInterface not ContentEntityInterface... not because of config, but because it's the right thing to do before calling $entity->get('anything).

gabesullice’s picture

gabesullice
Primary language English
commented

FWIW, I only think that this fix should be in the 2.x version.

@sonnykt, since 1.22 was the final release of the 1.x branch you can probably just add your own patch to your composer file without worrying that the patch might not apply some day.

Even with this fix committed to 2.x, it won't solve your problem because the 2.x version of JSON API explicitly removes support for config entities (support has been moved to JSON API Extras).

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Active » Needs review
StatusFileSize
new 2984494-11.patch3.55 KB

#9++

[…] because it's the right thing to do before calling […]

Why didn't I think of that? 🙈

wim leers’s picture

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

I just took @sonnykt's patch, modified it per @gabesullice's reasoning. I think it's RTBC. I'll let another JSON API maintainer commit/do final review.

gabesullice’s picture

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

Nothin' like a good commit to start the mornin'

gabesullice’s picture

gabesullice
Primary language English
commented

Looks like d.o didn't pick up the commit, it's here: https://cgit.drupalcode.org/jsonapi/commit/?id=2e63677

  • gabesullice committed 2e63677 on 8.x-2.x 
    Issue #2984494 by sonnykt, Wim Leers, gabesullice: EntityResource calls...
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

There it is :P

Status: Fixed » Closed (fixed)

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