If user with less/different permissions edits a page after a user with more/different permissions data lost could occur

I need to update the summary more and figure out the relationship to 📌 [PP-1] Add entity access checks to routes that deal with entities Postponed which I also need to review

I tagged this "Needs tests" because someone could actually write a test that confirms the problem even if don't know that actually solution yet.

This feels like it should be a beta blocker?

+1 for this being a beta blocker because of data loss.

wim leers → credited effulgentsia → .

wim leers → credited jessebaker → .

Just discussed this issue for ~20 mins with @jessebaker and @effulgentsia.

We propose that:

1. if an auto-save entry exists for an entity with a field that has granular access control (which is not the case by default) and that field has been changed in the auto-save entry compared to the saved entity (this should NOT be blocked on #2862574: Add ability to track an entity object's dirty fields (and see if it has changed) → because \Drupal\experience_builder\ClientDataToEntityConverter::convert() takes everything in the entity form and upcasts it to the full entity, but … inaccessible fields would not be shown in the entity form, so shouldn't be in the auto-save entry!)
2. then the entity_form_fields blob returned by the experience_builder.api.layout.get route from auto-saveshould be able to check whether the auto-save entries are modifying fields the current user cannot access.
3. Then we tweak the response returned by that controller (kinda similar to how we did in 📌 Omit `PageRegion` representation from `ApiLayoutController::get()`, 403 if sending it in `::patch()` or `::post()` Active ), but since the content entity is the primary purpose of this very route, we can't omit entity_form_fields — but what we can do is something like not returning entity_form_fields.
   Note: the XB component tree can still be edited, so this user can still make meaningful changes! They just won't be able to edit anything under the tab.
   (They are guaranteed to have field-level edit access to the component tree thanks to \Drupal\experience_builder\Access\ComponentTreeEditAccessCheck aka the _xb_component_tree_edit_access: TRUE route requirement added last week in 📌 [PP-1] Update `experience_builder.(experience_builder|api.layout.get) routes` to respect content entity update/field edit access of edited XB field Active having verified that first.)

(Which AFAICT means 🐛 ApiLayoutController::buildPreviewRenderable unnecessarily call \ClientDataToEntityConverter::converts causing errors Active had some oversights 😅)

Implementation outline of this proposal attached as a patch.

I haven't looked at the XB codebase related to this deep enough to comment on the code-level implementation suggestions of #8, but I'm +1 to the high level premise if I understand it correctly, which is:

If an auto-saved entry exists, and there is 1 or more fields for which:

• The value is different than what the published entity has, and
• The current user doesn't have edit permission for it

Then the user should not be allowed to make any changes at all on the Page Data tab. I think ideally the Page Data tab would still be visible, but be read-only, with a message at the top saying that it's read-only because there exist unpublished changes to it that were made by someone with different permissions and must be published or reverted by someone with those permissions before it can be editable by others.

Then we can punt any further scope (i.e., allowing edits of the permissible fields and merging those into the auto-save) to a post-beta, and maybe even post-stable, follow-up.

I agree that #8 🐛 If user with less/different permissions edits a page after a user with more/different permissions data lost could occur Active would be an acceptable way to get us to stable. This could be then improved post-1.0.0.

Thanks!

I think 🐛 Page status changes from "Published" to "Changed" even when no actual changes are made Active will resolve this, postponing