Fields values the user has view but not edit access can be saved to AutoSave

Leaving assigned to myself. See not in summary

NOT DONE WITH SUMMARY PLEASE LET ME FINISH BEFORE ISSUE IS WORKED ON ETC(🙏tedbow)

Will finish later today, there are few steps I want to explain. Then I will bring over changes from 📌 [PP-1] Add entity access checks to routes that deal with entities Postponed to start the MR

Update the summary, not need to start the MR from the work on 📌 [PP-1] Add entity access checks to routes that deal with entities Postponed

Ok. I am guessing the change will make entity-form-field-types-test.cy.js fail for the field field_xbt_comment see https://git.drupalcode.org/project/experience_builder/-/merge_requests/9...

Not sure it if is related but I did see problem with field_xbt_comment in 🐛 Radio button boolean fields can sometimes can be set in page data form Active

re #5 Test fail did happen

Not clear on why

What's next here? The status is not clear.

This sounds like a beta blocker given the security implications?

Basically, if I got this right, this is the same than 📌 Add field and entity access check on `ApiAutoSaveController::post()` Active with `ApiLayoutController`.

The difference is that for publishing, we want to throw an AccessDeniedException if we don't have access to the field.

For the layout, we just want to ignore the field, to prevent losing the changes other user would have made. As anyone publishing this would require to have all the needed permissions, they should be able to validate all field changes.

@Ted, can you confirm I'm right, and what's missing from the MR? (I think it is ready for review?)

Per #10, let's postpone this on 🐛 Page status changes from "Published" to "Changed" even when no actual changes are made Active . Chatted w/ @tedbow and the conclusion was to postpone since both touch buildPreviewRenderable, then reopen to test.

🐛 Page status changes from "Published" to "Changed" even when no actual changes are made Active just landed.

Too good to be true, if we can do !1086-note_540820 this is RTBC.

#10: 📌 Add field and entity access check on `ApiAutoSaveController::post()` Active landed, but had the "field access" bits descoped to 📌 Add field access check on `ApiAutoSaveController::post()` Active .

@penyaskito requested some extra test coverage.

I asked some Qs — AFAICT this entire problem is caused by #2862574: Add ability to track an entity object's dirty fields (and see if it has changed) → , would be good to see that A) confirmed/denied, B) if confirmed, a @todo pointing to it.

Maybe @penyaskito can still review this today, I can’t 😇

penyaskito → credited larowlan → .

LGTM, thanks for the extra explanations @tedbow and @larowlan!

Assigning @tedbow for commit, but needs a rebase

Re-rolled 🫰🏻

We haven't had a chance to hear back from @wim leers about https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...
but since this issue just adds test coverage I think it is practical to commit. We could still change that behavior later

I will merge

For posthumous review.

+1 for pragmatism; +1 for not waiting for my approval when there's >=2 sign-offs, and all of that doubly so when it's a test-only MR! 👍

The combination of @tedbow's and @larowlan's explanations are solid. I think that for me, the thing that really connected the dots was realizing that "the form logic will ignore" was referring to XB's call to setProgrammedBypassAccessCheck(FALSE).

I think that'd be good to do a tiny follow-up MR on this issue to update the comment? https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...

@wim leers created follow-up MR here with your suggested comment. Looks good to me

Thanks!

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