premium_access should not be cached in $node. Global $user can change behind the scenes [#700084]

The attached patch resolves an issue where the global $user is changing behind the scenes. In that case, the storage of premium_access inside the node array is inappropriate, as the value stored may not be correct at the time nodeapi:load is called.

What is the problem: Premium stores away the access value, premium_access, inside the node variable, granting access or non-access for the life of the node. But it's not valid for the life of the node. It should only be valid at the time nodeapi op==load is called.

How to demonstrate: A node is created and accessed. Sometime later in the execution $user is changed. $node->premium_access is no longer correct.

The solution: Don't cache the value in the node object, just check at the nodeapi time whether access should be granted.

Comment	File	Size	Author
#2	premium.dont_cache_access_status.patch	1.32 KB	rfay

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

hefox CreditAttribution: hefox commented 8 February 2010 at 21:22

(The patch is not there D:!)

possible could fix this also with the patch

function _premium_access($node, $teaser) {
  global $user;  // this is missing
    .... 
    if (is_bool($access = $function($user, $node))) { // cough

Comment #2

rfay

English

Palisade, CO, USA

CreditAttribution: rfay commented 9 February 2010 at 03:11

File	Size
premium.dont_cache_access_status.patch	1.32 KB

Sorry! Here's the patch. That happened to me more than once that night. I don't think it was me, either.

Comment #3

mikl CreditAttribution: mikl commented 16 April 2011 at 21:49

Status:

Needs review

» Needs work

Hi Randy,

This is a very sensible change. I'd very much like to merge it into my Premium-fork, but it'll probably need a reroll :)

http://drupal.org/sandbox/mikl/1078824

Comment #4

mikl CreditAttribution: mikl commented 19 April 2011 at 16:47

Project:	Premium	» Premium content
Version:	6.x-1.0-alpha1	» 6.x-1.0-alpha2

I plan to fix this soon :)

Comment #5

mikl CreditAttribution: mikl commented 12 May 2011 at 19:03

Version:

6.x-1.0-alpha2

» 6.x-1.x-dev

I was pondering this, and I can't see a good way to implement this without breaking backwards compatibility. It's common to rely on $node->premium_access in templates, etc., so removing it would be troublesome.

I can't really think of a scenario where the uid changes from when the node is loaded to when it's displayed. Is this a common issue?

Comment #6

BenK CreditAttribution: BenK commented 24 May 2011 at 18:47

Subscribing

Comment #7

Taxoman CreditAttribution: Taxoman commented 21 September 2011 at 22:20

@rfay; which situations would cause a change of $user after a node is created and accessed, without this being a legitime logout/login? Can that happen within the same login session, without resetting the session cookie?

Comment #8

tanc

he/him

English

Italy

CreditAttribution: tanc commented 2 November 2011 at 06:02

Hi Mikl, just in my basic testing I came across this issue immediately. I've opened an issue without realising it was a duplicate of this one #1328964: Permissions issues on cached nodes

The problem is that Drupal heavily caches the node objects, especially in D7 so there is no guarantee the cached object will apply to different users/roles. I think the only way around this is to call the access check on hook_node_view as well. I'm also having to add an access check when rendering custom lists using the Bean module. I'm not sure if there is a central way of doing this without custom calls to premium_user_content_access().