Hello, first of thank you for the great module.

I'm currently working on a website with security restriction: the anonymous users can't see published content, but they have the right to see the website's medias (using the media module 'View media' permission)

Looking at the line 420 of bg_image.module:

  if (is_numeric($nid)) {
    // Load the node so we can check it's status
    $node = node_load($nid);
    // Check node access permissions
    if (node_access('view', $node)) {
      // Get the path of the image on the node
      $image_path = bg_image_get_image_path_from_node($nid);
      // Add the background image
      return bg_image_add_background_image($image_path, $css_settings);
    }
  }

All my site needs is another check on the media's 'view media' permission to make this possible.

  if (is_numeric($nid)) {
    // Load the node so we can check it's status
    $node = node_load($nid);
    // Load the current user.
    global $user;
    // Check node access permissions or on the 'view media' permission @see media module
    if (node_access('view', $node)||user_access('view media',$user)) {
      // Get the path of the image on the node
      $image_path = bg_image_get_image_path_from_node($nid);
      // Add the background image
      return bg_image_add_background_image($image_path, $css_settings);
    }
  }

And since your module only gets the content's media field, it seems fair to assume the security restriction is still respected. What do you think?

Comments

Marc-Antoine’s picture

Title: Media compatibility - Look for the 'view media' permission as well. » Media module compatibility - Look for the 'view media' permission as well.

Improved the title

drclaw’s picture

Hm... That's a reasonable use case I think. I'm not sure how to handle it though. I don't think adding a user_access call for a media permission is the right solution. Maybe a new permission for bg_image? Something like "view unpublished background images"? Does that make sense?

Marc-Antoine’s picture

Yes that would work, but your permission name is incorrect, I'd want it to be simply 'see background image'

Let me try to explain the case more specifically:

Media is an advanced file management module, meaning that it gives alot more options to users and administrator to store images, give them fields etc.

Within media is a permission that gives the right for users to view medias, with no consideration for their parent content access.

This is very usefull when you want to show an image without giving more security permission (e.g. an anonymous user wouldn't have the right to see any published content, but could still see the website's medias)

So you could just totally replace the node_access check by a permission check (something like 'see background image') if you don't want to use the media's one.

drclaw’s picture

Status: Active » Fixed

Okay, I suppose "view unpublished background images" doesn't really work for the permission name since there could be other factors that restricts access to a node. I went with "view all background images" with a description that informs you that users with the permission will skip node access checks for background images.

The change is pushed to the dev branch for now, but I'll likely be rolling a new release soon.

Thanks!
drclaw

Marc-Antoine’s picture

Thank you, that's exactly what I wanted. Have a great day!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.