Due to the security warning, I'm testing an upgrade from WB-Mod 1.4 to 3.0 but I'm observing very different behaviour post-upgrade.

Situation sketch: very basic configuration, there are only two states, "Unpublished" and "Published" corresponding to "Draft" and "Published" and two transitions configured (obviously)

Old situation:

  1. I create a new page, save as published. I get a single revision (1) in moderation history.
  2. I create a new draft, save as unpublished. I get two revisions (1,2) in moderation history.
  3. I set the draft to published, still two revisions (1,2) but now (2) is published.

New situation:

  1. I create a new page, save as published. I get a single revision (1) in moderation history.
  2. I create a new draft, save as unpublished. I get three revisions (1,2,3) in moderation history. Revision (2) is the draft, revision (3) is an identical copy of (1) and has state published
  3. I set the draft to published, I still have three revisions, but revision (2) is now the published revision, revision (3) exists, is still an identical copy of (1) but appears in the moderation history above (2), without any clear hint that it's actually identical to (1) and content-wise older than (2).

This is plain weird from a user experience POV, it also appears that for every draft I create, two new revisions are being created, rather than the one previously - considering that on large, frequently changed sites, the number of revisions can cause performance issues, that seems like a very undesirable result.

Is this an inevitable consequence of the switchover to drafty or a bug - I've tested this on an upgraded site and one a clean installation (of the "openfed" distribution, 2.7-rc2 of that includes the new workbench-moderation-3.0 release)

It seems a bit awful that a security issue is fixed by a new release that fundamentally changes the architecture? It puts me in the awful position of having to choose between security and end user confusion :/

CommentFileSizeAuthor
#2 moderation-3.x-publish-draft.png160.99 KBbdeclerc

Comments

bdeclerc created an issue. See original summary.

bdeclerc’s picture

bdeclerc commented
StatusFileSize
new moderation-3.x-publish-draft.png160.99 KB

I've attached a screenshot which shows the situation after point (3) in the above description for wb-moderation 3.0

kcolwell’s picture

kcolwell commented

My situation seems slightly different but still just as totally confusing.

swhitters’s picture

swhitters commented

I have a similar problem. I am using 7.x-1.3+10-dev and when I upgrade to 3.x it seems the "needs review" table populates with a bunch of old revisions that have already been published? As noted, these revisions seem to behave as separate copies from the published version.

rauch’s picture

rauch commented

Same problem at my site. When I create a new version of a content in state "draft" I get two new versions. One in state draft (Published --> Draft ) and one in state published (Published --> Published).

nelsonre’s picture

nelsonre commented

Same problem here. Changing the state of a revision, creates an extra revision that always goes from Published --> Published.

morenstrat’s picture

morenstrat
he/him
Primary language German
Location Bochum, Germany
commented

Same problem after an update from 1.4 to 3.0 with the extra revision + an additional problem with language negotiation for anonymous users. I have four languages enabled. For logged in users, language negotiation works like before the update. For anonymous users, some nodes only show the English version when they shouldn't (using Entity Translation). After reverting to 1.4 everyting is fine again.

It would be really nice to have a security patch for 1.4.

Ben Buske’s picture

Ben Buske commented

same problem here on Drupal 7.44.

morenstrat’s picture

morenstrat
he/him
Primary language German
Location Bochum, Germany
commented

Regarding the language problem in #7 : error only shows with alpha8 of the title module (which is required by drafty)

baronmunchowsen’s picture

baronmunchowsen commented

I am also experiencing this issue and other potentially related issues which I have detailed in this report: https://www.drupal.org/node/2824725

das-peter’s picture

das-peter commented

Unfortunately it was imperative to change the behaviour since the old behaviour was the actual reason for the information disclosure issue.
The D7 API doesn't allow to easily change the revision id in the entity base table - so as a workaround the revision id was set in the shut-down handler. But this left a time window in which the draft was the active revision = potential Information disclosure.
Drafty now saves the draft and when doing so saves, within the same transaction, a forward copy of the active revision. This ensures data consistency in the DB.
This also means that the highest revision id is usually the active one.

Currently the deprecated copy of the active revision is left behind - but there's a patch for drafty we've been working on: #2579205: Delete redundant revisions
Reviews would be more than welcome and with a bit attention I'm quite sure we can get this in soon :)

bdeclerc’s picture

bdeclerc commented

Okay, I'll see about organising some help for that issue, but there's something wrong with the communications here - this was presented as a security fix without any clarification of the behavioural changes, which are rather significant, and on the user side very far from logical - I'd also argue that it would make more sense to save the draft *after* the forward copy, rather than the other way around - that way you could delete the old revision but keep a sensible order in the visible revisions.

Right now, the latest revision isn't actually the latest revision at all, which makes no sense :(

das-peter’s picture

das-peter commented

I'd also argue that it would make more sense to save the draft *after* the forward copy,

That's the point, as far as I'm aware it is not possible to do this with D7, at least not without introducing the information disclosure issue.
This missing API functionality was added in D8 but a backport doesn't seem to be feasible. I don't have the related issues at hand atm. but it was discussed.

bdeclerc’s picture

bdeclerc commented

Okay, I see the root problem - indeed not an easy one, but the current behaviour is really really hard to explain to regular users, though deleting the previous revision would help.

For consistency's sake, shouldn't publishing the draft also create a new revision and delete the old one? At least then we wouldn't get an "old" depublished revision being "newer" than the published revision...

bsains’s picture

bsains commented

@das-peter & @bdeclerc

I would also suggest that the idea of deleting the 'old' revision has to be handled with care.

Two issues I see at the moment.

  1. the timestamp of the originally published version is NOT set as same on the forward revision
  2. published by user of originally published version is NOT set as same on the forward version.

I'm seeing this on a 7.51 vanilla install with both the dev & recommended versions of workbench_moderation and drafty.

The environments I work within require, from a legal and legislative viewpoint, a proper audit trail of the who, what and when of published content.

I'd suggest that the deletion of revisions cannot be implemented until this issue can be rectified.

I've created a new issue in regards to this problem.

https://www.drupal.org/node/2824798

yonailo’s picture

yonailo commented

@bdeclerc, I like your idea but better than creating a new revision when publishing, it would be enough to delete the forward-copy to get the proper history, is that possible ? We can delete de forward copy as it is just a copy and is no longer published.

sri@re’s picture

sri@re commented

i just tried to delete forward copy of published revision when save state from publish -> draft and draft ->publish or publsih-> needs_review and needs_review->publish by using hook workbench_moderation_moderate_submit.

function workbench_moderation_moderate_form_submit($form, &$form_state) {
//add db_delete to delete extra revision of published.
db_delete('node_revision')->condition('vid',$form_state['values']['node']->vid+1)->execute();
         db_delete('workbench_moderation_node_history')->condition('vid',$form_state['values']['node']->vid+1)->execute();
.....
}
jyraya’s picture

jyraya commented

@sri@re,

In a context of concurrent accesses to content editing, are you 100% sure that "vid+1" is really the forward copy revision of the published revision?

sri@re’s picture

sri@re commented

@jyraya,
sure. can anyone please test and tell the feedback.?Thanks.

joseph.olstad’s picture

joseph.olstad
Primary language French
commented

#2900747-13: SA-CONTRIB-2016-060 upgrade workbench_moderation to 7.x-3.x with patches AND entity_translation to latest with patches

joseph.olstad’s picture

joseph.olstad
Primary language French
commented

annoying, seemed to work simpler in 1.x however I'm working on a 3.x patch to allow publishing new draft of an already published node from /admin/content patch will check path, make assumption that you want to publish most recent draft. I'll follow up soon.

jstoller’s picture

jstoller
he/him
commented

This is just a shot in the dark, but what if it worked something like this...

I have a published node (vid=1). I edit the node and save a draft. This creates two new revisions—the new draft (vid=2) and a copy of the old published revision (vid=3), which is active—just as Workbench 3.x is doing now. But then we update the node table making vid 1 the active revision again, like what Workbench 1.x does, and we delete vid 3. This adds a couple more steps, but if I understand things correctly it should prevent the data leak issue without creating unnecessary revisions, or reordering revisions in a funky way.

Thoughts?

joseph.olstad’s picture

joseph.olstad
Primary language French
commented

I wrote a patch to allow publishing from the /admin/content page , we're now adopting the new drafty behavior but applying patches that are currently being reviewed by the drafty maintainer and community.

see our patch
#2910310-10: drafty allow publishing current draft from /admin/content page when node already published when Workbench Moderation 7.x-3.x is enabled

as of today, we're actually using these three patches for drafty

drafty version = "7.x-1.0-beta4"
 - https://www.drupal.org/files/issues/7x1x_drafty_publish_from_admin_content_page-2910310-10.patch
 - https://www.drupal.org/files/issues/fix_head_tests_of_drafty7x1x-11.patch
 - https://www.drupal.org/files/issues/drafty-n2579205-116.patch

and these workbench_moderation 3.x patches.

workbench_moderation version = "7.x-3.0+5-dev"
- https://www.drupal.org/files/issues/upgrade_from_1x3_to_3x_fails-2428371-51.patch
- https://www.drupal.org/files/issues/workbench_moderation-playnicewithpanels-40.patch
- https://www.drupal.org/files/issues/workbench_moderation-preserve-history-2824798-14.patch