From e3847c167c0616b4f98e3f00befdbbfcf409216c Mon Sep 17 00:00:00 2001 From: webchick <webchick@24967.no-reply.drupal.org> Date: Thu, 1 Mar 2012 20:28:28 -0800 Subject: [PATCH] Issue #1064954 by xjm, Dave Reid, becw: Fixed _node_revision_access() static usage. --- core/modules/node/node.module | 49 ++++++++++++----- core/modules/node/node.test | 99 +++++++++++++++++++++++++++++++++++ 2 files changed, 134 insertions(+), 14 deletions(-) diff --git a/core/modules/node/node.module b/core/modules/node/node.module index 80d0897d193a..399f543443c8 100644 --- a/core/modules/node/node.module +++ b/core/modules/node/node.module @@ -1912,21 +1912,41 @@ function theme_node_search_admin($variables) { * The node to check. * @param $op * (optional) The specific operation being checked. Defaults to 'view.' + * @param object $account + * (optional) A user object representing the user for whom the operation is + * to be performed. Determines access for a user other than the current user. * * @return - * Access status for the revision. + * TRUE if the operation may be performed, FALSE otherwise. * * @see node_menu() */ -function _node_revision_access($node, $op = 'view') { +function _node_revision_access($node, $op = 'view', $account = NULL) { $access = &drupal_static(__FUNCTION__, array()); - if (!isset($access[$node->vid])) { - // To save additional calls to the database, return early if the user - // doesn't have the required permissions. - $map = array('view' => 'view revisions', 'update' => 'revert revisions', 'delete' => 'delete revisions'); - if (isset($map[$op]) && (!user_access($map[$op]) && !user_access('administer nodes'))) { - $access[$node->vid] = FALSE; - return FALSE; + + $map = array( + 'view' => 'view revisions', + 'update' => 'revert revisions', + 'delete' => 'delete revisions', + ); + + if (!$node || !isset($map[$op])) { + // If there was no node to check against, or the $op was not one of the + // supported ones, we return access denied. + return FALSE; + } + + if (!isset($account)) { + $account = $GLOBALS['user']; + } + + // Statically cache access by revision ID, user account ID, and operation. + $cid = $node->vid . ':' . $account->uid . ':' . $op; + + if (!isset($access[$cid])) { + // Perform basic permission checks first. + if (!user_access($map[$op], $account) && !user_access('administer nodes', $account)) { + return $access[$cid] = FALSE; } $node_current_revision = node_load($node->nid); @@ -1938,18 +1958,19 @@ function _node_revision_access($node, $op = 'view') { // Also, if you try to revert to or delete the current revision, that's // not good. if ($is_current_revision && (db_query('SELECT COUNT(vid) FROM {node_revision} WHERE nid = :nid', array(':nid' => $node->nid))->fetchField() == 1 || $op == 'update' || $op == 'delete')) { - $access[$node->vid] = FALSE; + $access[$cid] = FALSE; } - elseif (user_access('administer nodes')) { - $access[$node->vid] = TRUE; + elseif (user_access('administer nodes', $account)) { + $access[$cid] = TRUE; } else { // First check the access to the current revision and finally, if the // node passed in is not the current revision then access to that, too. - $access[$node->vid] = node_access($op, $node_current_revision) && ($is_current_revision || node_access($op, $node)); + $access[$cid] = node_access($op, $node_current_revision, $account) && ($is_current_revision || node_access($op, $node, $account)); } } - return $access[$node->vid]; + + return $access[$cid]; } /** diff --git a/core/modules/node/node.test b/core/modules/node/node.test index 96d4e01e340c..b9cae4c605de 100644 --- a/core/modules/node/node.test +++ b/core/modules/node/node.test @@ -2302,3 +2302,102 @@ class NodeTokenReplaceTestCase extends DrupalWebTestCase { } } } + +/** + * Tests user permissions for node revisions. + */ +class NodeRevisionPermissionsTestCase extends DrupalWebTestCase { + protected $node_revisions = array(); + protected $accounts = array(); + + // Map revision permission names to node revision access ops. + protected $map = array( + 'view' => 'view revisions', + 'update' => 'revert revisions', + 'delete' => 'delete revisions', + ); + + public static function getInfo() { + return array( + 'name' => 'Node revision permissions', + 'description' => 'Tests user permissions for node revision operations.', + 'group' => 'Node', + ); + } + + function setUp() { + parent::setUp(); + + // Create a node with several revisions. + $node = $this->drupalCreateNode(); + $this->node_revisions[] = $node; + + for ($i = 0; $i < 3; $i++) { + // Create a revision for the same nid and settings with a random log. + $revision = clone $node; + $revision->revision = 1; + $revision->log = $this->randomName(32); + node_save($revision); + $this->node_revisions[] = $revision; + } + + // Create three users, one with each revision permission. + foreach ($this->map as $op => $permission) { + // Create the user. + $account = $this->drupalCreateUser( + array( + 'access content', + 'edit any page content', + 'delete any page content', + $permission, + ) + ); + $account->op = $op; + $this->accounts[] = $account; + } + + // Create an admin account (returns TRUE for all revision permissions). + $admin_account = $this->drupalCreateUser(array('access content', 'administer nodes')); + $admin_account->is_admin = TRUE; + $this->accounts['admin'] = $admin_account; + + // Create a normal account (returns FALSE for all revision permissions). + $normal_account = $this->drupalCreateUser(); + $normal_account->op = FALSE; + $this->accounts[] = $normal_account; + } + + /** + * Tests the _node_revision_access() function. + */ + function testNodeRevisionAccess() { + $revision = $this->node_revisions[1]; + + $parameters = array( + 'op' => array_keys($this->map), + 'account' => $this->accounts, + ); + + $permutations = $this->generatePermutations($parameters); + foreach ($permutations as $case) { + if (!empty($case['account']->is_admin) || $case['op'] == $case['account']->op) { + $this->assertTrue(_node_revision_access($revision, $case['op'], $case['account']), "{$this->map[$case['op']]} granted."); + } + else { + $this->assertFalse(_node_revision_access($revision, $case['op'], $case['account']), "{$this->map[$case['op']]} not granted."); + } + } + + // Test that access is FALSE for a node administrator with an invalid $node + // or $op parameters. + $admin_account = $this->accounts['admin']; + $this->assertFalse(_node_revision_access(FALSE, 'view', $admin_account), '_node_revision_access() returns FALSE with an invalid node.'); + $this->assertFalse(_node_revision_access($revision, 'invalid-op', $admin_account), '_node_revision_access() returns FALSE with an invalid op.'); + + // Test that the $account parameter defaults to the "logged in" user. + $original_user = $GLOBALS['user']; + $GLOBALS['user'] = $admin_account; + $this->assertTrue(_node_revision_access($revision, 'view'), '_node_revision_access() returns TRUE when used with global user.'); + $GLOBALS['user'] = $original_user; + } +} -- GitLab