Problem/Motivation
When you have a "Needs review" published state for a content moderation workflow and decide to switch the 'published' property to FALSE, the moderated content with that state will not be updated to reflect the new publishing status.
Since the publishing status for existing moderated content is not updated, changing an already existing moderation state like mentioned above will introduce an access bypass issue for any view listing that content.
Proposed resolution
Disallow changing the 'published' property of a moderation state after it has been added to a workflow.
Remaining tasks
- code this in \Drupal\content_moderation\EventSubscriber\ConfigImportSubscriber too
- add tests
User interface changes
The 'Published' checkbox from the State edit form will now be disabled.
API changes
Nope.
Data model changes
Nope.
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | 2907453-6.patch | 2.11 KB | vijaycs85 |
| #6 | 2907453-6-test-only-fail.patch | 1.32 KB | vijaycs85 |
Comments
Comment #2
amateescu commentedThis takes care of the UI part, we still to code this into
\Drupal\content_moderation\EventSubscriber\ConfigImportSubscriberand add tests.Comment #3
xjmI think what we actually need to do is prevent it from being changed once there is content in the state? Otherwise I can see the limitation getting really annoying otherwise; I can imagine having to delete and re-create states all the time.
Comment #4
xjmOr, alternately, we need a batch update to publish or unpublish the entities as appropriate. I could see a short-term fix that disallows it when there's any content in the state and the batch operation as a longer term solution.
Comment #5
amateescu commentedEven when there is no content in the state, there might other parts of the system (let's say.. a view :P) that operate under the impression that content in this state is considered published/unpublished, so I think the current approach is much safer, as annoying as it might be.
Comment #6
vijaycs85Adding test. Just changing the status to needs to review to test.
Comment #8
timmillwoodMoving back to "Needs work" for the
ConfigImportSubscriberpart mentioned in #2.Comment #10
rakesh.gectcrAssigning myself to give it try,
Did talk with tim on slack about isuue and he had given me the directions to approach this issue.
Comment #21
smustgrave commentedThis came up as a daily BSI target
Un-assigning as it's been 8 years :)
I see there was a rough patch probably good to turn into an MR going forward.
Seems this still needs the ConfigImporter part though.