if (!image_access('view', $node)) {
  drupal_access_denied();
  return;
}

The user account is the administrative account (ID #1) and drupal_access_denied still gets called. According to the api documentation about hook_access, 'The administrative account (user ID #1) always passes any access check.' So this shouldn't happen. Also why is the administrative user allowed to view the node (ie url('node/'.$node->nid)) but this check fails? When I substitute node_access('view', $node) for image_access('view', $node), it works. But image_access is:

function image_access($op, $node) {
  global $user;

  if ($op == 'create' && user_access('create images')) {
    return TRUE;
  }

  if ($op == 'update' && $user->uid == $node->uid) {
    return TRUE;
  }

  if ($op == 'delete' && $user->uid == $node->uid) {
    return TRUE;
  }
}

So image_access('view', $node) should return what node_access('view', $node) returns because in the documentation, it says (about hook_access): 'Return value TRUE if the operation may be performed; FALSE if the operation may not be returned; NULL to not override the settings in the node_access table.' And image_access returns NULL for 'view.'

Comments

beginner’s picture

beginner commented
Version: 4.7.2 » x.y.z

How to reproduce, exactly?
If it's a bug, a fix should be provided for cvs, and backported to 4.7 soon after.

Mirrorball’s picture

Mirrorball commented

Here is a minimal way to reproduce this bug (suppose this is in a module called test.module):

<?php

function test_menu($may_cache) {
  $items = array();

  if ($may_cache) {
    $items[] = array('path' => 'test', 'callback' => 'test_test');
  }
  return $items;
}

function test_test() {
  $node = node_load(29); // Can be any node here
  if (!story_access('view', $node)) {
    drupal_access_denied();
    return;
  }
}

Login as user #1, go to q=node/29 and you can view it fine, then go to q=test and 'Access denied.' Even though you are the administrator and you can view story #29.

heine’s picture

heine commented

From the API docs: The administrative account (user ID #1) always passes any access check, so this hook is not called in that case.

Use user_access('access content') or node_access to determine access (or leave it to node_load).

For example in the menu hook:

$items[] = array(
  'path' => 'test', 
  'callback' => 'test_test', 
  'access' => user_access('access content'),
);
heine’s picture

heine commented 
If this hook is not defined for a node type, all access checks will fail, so only the administrator will be able to see content of that type.

That sentence seems to be incorrect; the grants from the node_access table will be used in that case: for a default installation view is granted, unless the user doesn't have an access content permission.

(Sorry if I'm not making much sense)

beginner’s picture

beginner commented

If I understand well, the problem seems to be with story_access() which returns nothing (==FALSE) when viewing a node:
http://api.drupal.org/api/HEAD/function/story_access
therefore, the access is denied.

It seems that the $op view has not been implemented:
http://api.drupal.org/api/HEAD/function/hook_access

Try adding a case $op == 'view' in story_access: that should solve it.

heine’s picture

heine commented

Why is that a problem? By returning nothing story_access leaves the access decision to the 'node_access' table. It is not intended to be called directly.

@OP:

if (!image_access('view', $node)) {
  drupal_access_denied();
  return;
}

Where is this code called?

beginner’s picture

beginner commented

I don't think you are supposed to call hook_access() directly, because this hook doesn't know anything about user #1.
use user_access() instead:
http://api.drupal.org/api/HEAD/function/user_access

It seems it's a bug in the image.module and not in drupal core.
In #2, the reproduce code is flawed because it should use user_access().

Where does this 'bug' appear in real life? in image.module?
Without changing any code, how can I reproduce this bug in drupal core?

beginner’s picture

beginner commented
Project: Drupal core » Image
Version: x.y.z » 6.x-1.x-dev
Component: base system » image.module
Mirrorball’s picture

Mirrorball commented
Project: Image » Drupal core
Version: 6.x-1.x-dev » 4.7.2
Component: image.module » base system

This code is not part of the image module. I was writing a module and wanted to find out if a user was allowed to view a node. I was calling image_access because the node was of the type 'image.' Isn't node_access the hook_access implementation by the node module? And isn't it supposed to be overriden by image_access for image nodes? So if I want to find out if a user is allowed to view an image, I shouldn't call node_access, because it is the default hook_access function for nodes and image_access might grant different permissions for image nodes. I should call image_access instead, because it overrides node_access for images. But calling image_access for the #1 user fails and viewing the node isn't allowed even though image_acess says nothing about viewing permission and node_access grants this permission. Therefore it's a Drupal bug. Did I get it wrong?

Mirrorball’s picture

Mirrorball commented
Status: Active » Closed (works as designed)

I was having a look at node.module and I saw that node_access calls image_access, so I don't need to call it directly. I understand that I should call node_access instead of image_access to check if a user is allowed to view a node. Correct?