Subscriptions to restricted material should not be permitted

killes had said he was going to take a look at this... perhaps he can comment on the status here?

Sorry, the project I wanted to do this for has been postponed. The required changes are mentioned in the upgrade guide. http://drupal.org/node/7176

Thanks for the quick response. I'll look into making the necessary changes.

Is there still a constituency for this? If so, what are the indicators of a private or restricted node?

Yes, there is still a need for this. This can happen especially in the case where one has a subscription to a term, but some nodes that are tagged to the term are not available to the member. In that case foks get notifications to nodes they can not access.

I'm not an expert on the api's, but I believe the table/mechanism in play is "node_access".

This is a nice module and the recent improvements have been great! thanks so much.

I've got a possible fix for this if you'd care to test it. In the attached file, I've added the following code:

// determine if target reciever has access to the node
  	node_access("view", $nid, $subscriptions->uid) ? $nodeaccess = true : $nodeaccess = false;
    if ($selftest && $nodeaccess ....

I think that should handle it, but I really don't have any experience with restricting access to nodes. As such, I wasn't able to come up with a good test for this. Care to test the attached code to see if this addresses the problem?

Thanks. I know this is the right path and I've actually got a version of this in play, but there are a few issues. Here is what I have found:

1. I think the second argument is a node object, not a node id. Because node_access depends upon a node versus a nid, we would need to change the signature for the subscriptions_mailvars to pass the node instead of the nid, because in the case of a new node, I don't think the node has been saved until after subscriptions has been called, and therefore can't be retrieved unless passed.

2. There appears to be some problems with the node_access routine not respecting the uid argument consistently. For instance, it does not use the uid argument in looking at "administer nodes" or "access content" checks. This is fixable but I believe there appear to be other similar issues in downstream hook calls. Because of the inconsistency in using uid in some cases and the user object in other cases (in node access and user access routines), repairs are a bit tedious too. I've been reluctant to do anything about this because it is core and as I say, it looks like there are actually several areas that do not observer uid properly.

For now, I have a version running that 1) only runs with cron (since I can lookup a node in that case, versus having to change the code to pass the node), and 2) a basic patch to node_access to observe the uid. Until we make an issue for node_access and user_access to observe uid consistently, its going to still be questionable about getting this fixed.

One work around to the bugs in node_access is to change the current $user to the person whose subscription you are examining before the call to node_access, and then change it back. However I worry that if there was a failure of some sort, the user might not get changed back and now the posting user could become the failed subscriber user. Maybe this is not worrying about, but maybe it's a big thing to worry about. I don't know.

RE: "1. I think the second argument is a node object, not a node id. "

Right you are. No need to change the signature - I just grabed the node using "node_load()" and that part should be good, now (see attached).

RE: "One work around to the bugs in node_access is to change the current $user to the person whose subscription you are examining before the call to node_access, and then change it back."

Is that really necessary? I'm passing it the userid from the subscriptiosn table, not the user object. In any case, I still have problems with "access content", but this is probably as good as it gets until the problems in core are addressed. Is the bug logged?

Ok, as you indicate it's getting about as good as it's going to for now, which works fine with some minor caveats. No, I haven't logged the node_access issue yet. I will try to do that soon.

When I was indicating I didn't think the node was saved yet, thus needed actual node, yes I was wrong on that so the in-line fetch seems to work. One part still not working yet, and thus why it works for me on cron but not in real-time... I'm think the way nodeapi works for the insert call, I assume is to call subscriptions first, then taxonomy. I think this creates a problem when one uses taxonomy_access (which node_access uses) because if the taxonomy has not been written when node_access is called (by subscriptions), node_access will never see the taxonomy based security, and thus may not send notifications to subscribers that are granted access in the case the taxonomy was the driver to give access to certain roles. This is not an issue with cron because the taxonomy has been written by then. I don't know if there is a way to force subscriptions to fire after taxonomy instead of before (assuming that's the problem).

In any case, this is a workable solution, so long as the node_access issues don't get in the way, and one uses the cron option for taxonomy_access sites. I'll submit the request to look at the issues on node_access.

I have put equivalent code in production for a couple of days, but not the exact patch you have given yet, but will put your patch into production over the weekend yet sometime and will be sure to let you know if I observe any issues.

Your help has been very appreciated.

Thanks
Don R.

This fix has been committed. Problems in core are outside the scope of this module.

I've installed the module and tested it to see if it obeys the node_access rights, and am still having issues.

I note that there is still an outstanding issue with the order in which things are called when creating a node, meaning that the access checks don't currently work when notifications are being sent on node update - ie they send out notifications whether you're authorised to see those nodes or not. However, when I change to send the notifications by cron, none get sent out at all - and no errors are reported. Cron also updates the event log to say it has completed successfully.

There is a possible (and pretty easy) solution to the problems in core relative to node_access (http://drupal.org/node/72487). I realize that it has no direct bearing here, but if there are individuals who have been waiting for a fix and who get thread update notifications, this could be useful info.

Turning project over to new owner.

Closing ancient issue -- please help test the new 5.x-2.0-BETA...