Download & Extend

Problem: Unauthorized Locking of Nodes that cannot be edited

Project:Checkout (Content locking)
Version:6.x-2.2
Component:User interface
Category:bug report
Priority:critical
Assigned:Unassigned
Status:needs work

Issue Summary

Take a user without permissions to view or edit some content type.

Lets say that content type nid is 45.

The said user go to the url and trigger a lock on content they are not allowed to edit or even view: http://example/node/45/edit

I can login as a user who _is_ authorized to edit and if the above user did what was mentioned, the "this document was locked by.." does in fact show up!

Additional Potentially Relevant Modules Installed: ACL, Content Access.

Comments

#1

#2

Status:active» needs review

Nice catch! I've just fixed on place: node_access() requires a node object, not a nid. Otherwise this looks good to me.

AttachmentSize
checkout.module-check_access.patch 1.19 KB

#3

Thanks for the correction, i rushed the patch out without much testing at all...

Something still went wrong during my testing, I looked at http://api.drupal.org/api/function/node_access/6

Apparently, it should not be 'edit' but instead be 'update'.

AttachmentSize
checkout.module-check_access.patch 1.2 KB

#4

Found a better solution using menu_get_item(): the hardwired node_access('update', ...) only checks for node edit permissions, which might be sufficient for node/%/edit, but not any other path that can be locked. For example, on node/%/outline we need to check for book.module's "administer outline" permission. This can be done by directly querying the menu system and letting it figure out the access permissions.

AttachmentSize
422870-checkout-access.patch 1.34 KB

#5

Subscribing.

#6

I have had the #4 patch applied for a little while now without any related troubles popup.

#7

Has anyone tested if there are problems with
· Taxonomy Access Control
· Domain Access
· Domain Access pach: multiple_node_access patch (allows you to use AND logic instead of OR logic: ALL of the access' modules must allow you to view/edit the node, not only one)

I try to reproduce the scenary and test it.

#8

I didn't check the patches yet, but I just found out that at least some of the "unauthorized" locking was caused by users opening the revisions tab of a content.

Whoa. That is an unexpected behavior. The message "...is locked" appears, but I wonder why the lock happens at all, because revisions should only be displayed at that point.

#9

Status:needs review» needs work

+1. We are also getting hit with the inadvertent locking of content when navigating to the revisions tab.

#10

We've confirmed that the patch from #4 does not work with 'Taxonomy Access Control Lite', 'Module Grants' and 'Workflow Access'. The patch from #3 does work though.

We are still interested in a fix for inadvertent locking when navigating to the 'revisions' tab.

#11

Got time to look through the code, the following line allows people to override the locking behavior using a variable:
$patterns = variable_get('checkout_edit_paths', "edit\nrevisions\nrevisions/*\noutline\nclassify");

We added only 'edit' to the settings.php file and it seems to work fine for us. Still believe the default setting shouldn't lock when navigating to the revisions tab.

#12

In settings.php? Why did you do anything in that one? *confused*

I tried using the line
   $patterns = variable_get('checkout_edit_paths', "edit");

but then nothing gets locked properly, despite the message being shown. Help would still be appreciated.

#13

perhaps something like $op should be passed to the function in question so that we know if the operation is treated as an update and can handle permissions?

#14

@itsnotme. The 'variable_get' function will use the provided variable's value ('checkout_edit_paths' in this instance) or the provided default value. As opposed to changing the source code, we choose to set the variable's value in settings.php.

$conf = array(
#   'site_name' => 'My Drupal site',
#   'theme_default' => 'minnelli',
#   'anonymous' => 'Visitor',
   'checkout_edit_paths' => 'edit',
);

Looks like it appropriately locks the node (on edit only) to me.

#15

You might want check http://drupal.org/project/content_lock . Its a fork(and partially rewrite) of checkout and activly maintained. It does not seem that checkout is maintained anymore (not officially confirmed though)

nobody click here