The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.Overview
Forked from #3494915-45: Support entity-level + field-level access checking in auto-save — i.e. in `experience_builder.api.api.(layout.post|auto-save.post)` and #3529426: Add entity access check on `ApiAutoSaveController::post()` later.
Add field access check on `ApiAutoSaveController::post()`
Proposed resolution
- Update
`ApiAutoSaveController::post()`, which must:- if a content entity, iterate over all changed fields, throw a
CacheableAccessDeniedHttpExceptionif any of them fail on$entity->get($field_name)->access(operation: 'edit', return_as_object: TRUE).
- if a content entity, iterate over all changed fields, throw a
- Update
`\Drupal\Tests\experience_builder\Kernel\ApiAutoSaveControllerTest::testApiAutoSaveControllerPost`
User interface changes
None.
Issue fork experience_builder-3532454
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #3
wim leersI think this is the intent?
Comment #4
wim leersTest coverage looks great already 👍
Comment #5
tedbowMarking as critical see https://git.drupalcode.org/project/experience_builder/-/merge_requests/1... in #3527156: Add test coverage for access exception in \Drupal\experience_builder\ClientDataToEntityConverter::checkPatchFieldAccess()
Basically
\Drupal\experience_builder\Controller\ApiAutoSaveController::postuse to call\Drupal\experience_builder\ClientDataToEntityConverter::convertwhich did field access checks but it no longer does since #3529622: Make auto-save manager only support passing and returning entity objects, remove support for arbitrary data, use deterministic hashing to ensure entries are actually updatesso basically 1 user with edit permission could get a field update into the auto-save and then another user without field update permission could publish the field update. I think the product goal is that user publishing should have all the permissions to do all the updates that happen on publish
We should move the test coverage from #3527156 https://git.drupalcode.org/project/experience_builder/-/blob/fe773efe8f3... to this issue, or make sure we have the same coverage
Comment #7
penyaskitoComment #8
penyaskitoMarking for preliminary review. We might want to do the trait for checking the field as Lee suggested, but could perfectly be part of the follow-up we wanted to open anyway and clear another critical beta blocker.
Leaving for Wim to evaluate.
Comment #9
wim leersI think @tedbow's Q should be answered by test coverage. 😇 This is a security matter, so here it makes sense to err on the side of caution.
Comment #10
wim leersComment #11
penyaskitoTagging needs review. We need to verify #note_543156 and #note_545354 is the expected outcome.
Comment #12
wim leersMerged in upstream after #3492722: Update XB to require Drupal 11.2 landed; resolved several conflicts.
Comment #13
wim leersVery nice end result, and @penyaskito showing off with code elegance rarely seen! 🤓🤩🙇
Comment #15
wim leers