iPaper incompatible with node access system

This makes sense, but I'm not sure I understand what happens when ipaper_access will return NULL. granted, other modules will be able to override, but are there any other permissions in the drupal core that will take precedence over the ipaper permissions if NULL is returned?

According to the hook_access documentation
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.

So in this case, for instance, can we expect that by returning NULL for op=='view' a user with 'access content' will be able to view that particular ipaper?
I agree that the module should be able to work well with other access control module, but we need to be cautious about blocking access where necessary, or else we might have to release a security update.

I looked at node_access and it does exactly what you said. I don't see any solution to this other than to add some kind of option to the administration page where people to choose whether to bypass permissions (return NULL in to ipaper_access). Or we could add some kind of if (module_existss('nodeaccess')) return NULL;
Does anyone know how other modules defining content types deal with this?

On the other hand, moving forward, this module will define a CCK field instead of a content type, so access will be managed differently. Does anyone know how to code for CCK so we can move forward with this quickly (it has been requested several times).

ok, I will commit some small final changes to 1.x, release 1.0 and create 2.x sometime this week. When I create 2.x I will try to make it a little modular so that you can more easily adapt it to CCK, and then you can rewrite it however you think it's necessary. I can help too, especially if you create the CCK functions. We can continue this discussion under #319069: Adding this to CCK, where I will make an announcement as soon as I create the new branch.

I don't know what to do about the view permission. I think I will leave it like this for the 1.0 release, but I will apply the patch and have it return NULL for the other permissions.

Any word on when the patch will be committed and the 1.0 release created?

Thanks,
Ben

I've just committed a version that has all the changes suggested by Jody Lynn in the patch, except for 'view ipapers', which continues to have

  case 'view':
    return user_access('view ipapers');

changing that to
return user_access('view ipapers)' ? TRUE : NULL;
would mean falling back to 'access content', which would no longer justify having a separate permission. I guess we could take out that permission, but probably most users have gotten used to it, and we are planning a new version (2.x) anyway so we might as well leave this the way it is. I agree nonetheless that view permission handling should normally be left to specialized party modules. I'll mark this as fixed but if there are enough people that want the future 1.x releases to deprecate this permission. In the meantime, I guess it's easy to change the code to suit your need if you have a problem with nodeaccess.

Reopening this because, as pointed out in #910164: Integration - Content Access and ACL, this is still an issue.
I ultimately agree that having a view permission for this module is problematic, but I'm concerned that if we remove this permission, people who have depended on it will have problems.
In the meantime, there is no way to have this work as designed without removing the permission entirely:

return user_access('view ipapers') ? TRUE : FALSE;
current implementation; does not play well with node access modules at all

return user_access('view ipapers') ? TRUE : NULL;
problem: Users with this permission will have access irrespective of what other overrides happen in other access modules.

return user_access('view ipapers') ? NULL: FALSE; or return user_access('view ipapers') ? NULL: NULL;
problem: access modules will work, however, anyone who has 'access content' will also be able to see ipapers; this defeats the purpose of having a separate permission for this type of content and is the same as not having the permission at all.

So, the only sustainable solution is to eliminate this completely. However, I will need to write a hook_update script for that, and I don't have time to do that at the moment.
So, in the meantime, the solution for anyone who wants to use this module together with content_access, node_access etc is to remove the following 2 lines from ipaper.module:

    case 'view':
      return user_access('view ipapers') ? TRUE : NULL;

Sorry for being ( probably) stupid rares, but do you mean this whole block?

case 'view':
if ($node->uid == $account->uid && $account->uid != 0)
return TRUE;
if ($node->status == 0)
return user_access('administer nodes');
return user_access('view ipapers');

I looked for

return user_access('view ipapers') ? TRUE : NULL;

in the ipaper.module file, but it isn't there.

File header is:

// $Id: ipaper.module,v 1.1.2.43 2009/10/09 14:44:06 rares Exp $
// $Name: DRUPAL-6--1-1 $
/*
* @file
* iPaper module hooks and functions
* Extends iPaper content type, calls funtions in scribd.php to implement Scribd API
*/

Peladon

my mistake. 6.x-1.1 has the code that you mentioned. in that case, yes, remove that whole thing.
in 6.x-1.x-dev there is some different code, based on what Jody Lynn suggested. I guess you could try to see if that code works as you need it to; my guess is that it will not.

My thanks. I'll try that tonight...

Peladon

Ah, well. If I did it right:

Edited block out of ipaper.module

Rebuilt permissions

However... access is still blocked by the module.

No matter. This still seems the best way for me to display the types of files I need to... so I'll keep massing around with it in case I've banjaxed it somewhere and cross my fingers :-).

My thanks for the input and support!

Peladon

subscribing, the approach taken in the support module may be helpful, http://drupal.org/node/768354