Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.The primary test for whether a user has permission to create a node in og_node_access() effectively ignores the specified $account, and always tests access against the current global $user instead of $account.
The result is that calls to node_access('create', $node_type, $account) will return invalid results if OG is the module granting create access. However, when logged in as $account, calls to node_access('create', $node_type) return the correct result.
The "test" patch contains a simple addition to og.test that demonstrates the problem -- and OG currently fails the added test. The problem is also apparent when using Devel Node Access, which notices the access inconsistencies and triggers a warning about "Core seems to disagree on this item. This is a bug in either DNA or Core and should be fixed!"
The source of the problem is that OgSelectionHandler always uses the global $user when building the list of referencable entities. So when og_node_access() checks "if there are any groups the user will be able to select" using entityreference_get_selection_handler($field, $instance)->countReferencableEntities() the check is done as $user not as $account.
The "fix" patch fixes the problem by adding a setAccount() function to OgSelectionHandler, and changing all access checks in the class to respect the specified value of $account. The resulting code is able to pass the newly-added test.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | og-node_access_create_fix-2245235-8.patch | 6.31 KB | mdolnik |
| |||
| #5 | og-node_access_create_fix-2245235-4.patch | 5.97 KB | brad.bulger |
| |||
| #3 | og-node_access_create_fix-2245235-3.patch | 6.05 KB | brad.bulger |
| |||
| og-node_access_create_test.patch | 687 bytes | Nephele | |
| og-node_access_create_fix.patch | 5.68 KB | Nephele | |
Comments
Comment #2
Nephele CreditAttribution: Nephele commentedJust to followup given the lack of feedback:
"og-node_access_create_test.patch" fails the test intentionally, as the easiest objective way of demonstrating that there is a bug in the code. The only change in that patch is adding one simple test to the og test suite -- a test that calls node_access() and checks whether the returned value is correct. The test fails because og contains a bug when node_access is called.
The second patch ("og-node_access_create_fix.patch") -- the patch that passed the tests -- is the one that needs to be applied to the code. It contains everything in the first patch (i.e., the new node_access() test), as well as the code modifications necessary to fix the problem.
Therefore, the correct status is needs review, not needs work.
Comment #3
brad.bulger CreditAttribution: brad.bulger commentedreroll against current 2.x branch.
Comment #5
brad.bulger CreditAttribution: brad.bulger commentedreroll against 2.10
Comment #6
brad.bulger CreditAttribution: brad.bulger commentedComment #7
joseph.olstadSeems to make sense to me, and has tests.
Comment #8
mdolnik CreditAttribution: mdolnik as a volunteer and commentedThere is one issue with the latest patch above...
When we are checking the access of nodes, the query built by
OgSelectionHandler::buildEntityFieldQuery()ends up having thenode_query_entity_field_access_alter()query-alter called which in-turn calls_node_query_node_access_alter()In
_node_query_node_access_alter()it checks the access for the currently logged in user, not the specific user we are checking access for.We can get around this by adding the following to
OgSelectionHandler::buildEntityFieldQuery():I have updated the patch accordingly (re-rolled against the latest 7.x-2.x)...