Allow simplenews cron to send nodes that anonymous users cannot access

The cause of the problem is, as you mention, the access restriction to the node. This is not solved by only leaving out the db_rewrite_sql here.
I found a solution in the mailhandler module. It switches the user when collecting data with restricted access. When simplenews sends emails we need to switch to the author user, load the node and switch the user back. But, this requires some caution not to create any security holes.

Please try the attached patch to see if it fixes the problem.

This patch fixed the problem for me. Will this be rolled into a future release?

I consider it. Please explain how you have restricted the access to the newsletter. I still need to understand the problem, before I decide for this solution. And I want more people to test this patch.

Ah ok I guess someone should have explained this. :) I don't know the situation for others, but for us newsletter access is being restricted using the Nodeaccess module.

For example, in Access Control the simplenews module is set for *no* anonymous users to have access to newsletters, then we use Nodeaccess to control who can receive certain types of Newsletters (using the Simplenews Template module to create templates for registered, paid, etc.).

Updating to the version we are using.

This is the kind of situation where the patch is intended for.
1. try so send using cron and see if the nodes appear in the email.
2. if the nodes do not get through, try the #3 patch and pls report the results.

I'm sorry, I thought my comment in #4 answered it? The patch in #3 worked for us. That's why I was asking if it would be rolled into a future release.

I like to see more test-result before rolling the patch.

My problem is when using simplenews with relatedcontent and taxonomyaccess.
I have some nodes with restricted access using taxonomy; when I send test all the nodes get inserted in the newsletter, but when they are send using cron, the restricted ones aren't included in the newsletter.

I think it is a great idea to save the user, switch to another user, and then restore the initial user; but I would rather switch to the uid=1 user than the author of the node.

@enboig: please test the #3 patch. I want to see more test before this gets in. And, please explain why you want to switch to user/1? I assumed that the node author has full access to the node.

Well, it should work with the node author; but I think its simplier to use always the user 1 than look always for the node author.

I will test the patch now.

after applying the patch I recive this error:

Fatal error: Call to undefined function session_save_session() in /home/escolcat/public_html/sites/all/modules/simplenews/simplenews.module on line 2864

Following #1 I changed:
$result = db_query(db_rewrite_sql('SELECT n.nid, s.vid, s.tid, n.created FROM {node} n INNER JOIN {simplenews_newsletters} s ON n.nid = s.nid WHERE s.s_status = %d ORDER BY n.created ASC'), 1);
to
$result = db_query('SELECT n.nid, s.vid, s.tid, n.created FROM {node} n INNER JOIN {simplenews_newsletters} s ON n.nid = s.nid WHERE s.s_status = %d ORDER BY n.created ASC', 1);
And now It appear to work. Does this modification has security issues?

The patch does exactly change this code (and more) perhaps you had errors during patching. It is very likely that the patch does not apply cleanly anymore.

I applied manually the patch, the version of simplenews I am using is a little different of this one; what I don't understand is why cron.php fails when calling a function ("session_save_session") that is in api.drupal.org (version 5).

doesn't cron.php do a full bootstrap?

The attached patch is the D6 version of the same functionality.
(it does not address the #14 error)

Any updates in this issue? I am using Drupal5 and I couldn't apply the patch provided.

Thanks

Are the patches applied to the dev version so I could test them?

no, patches are not committed.
How to patch: http://drupal.org/patch/apply

I could apply the patch "manually". My problem was because old versions of Drupal5 session_save_session(); updated the whole site and now it works ok.

Thanks.

Change version to HEAD, new functionality will now only be added to HEAD.
Patch requires minor rework.

Patch reworked for HEAD.
Changes:
- changed comment
- select element changed to radios

oops, mixed up with other patch.

I was thinking that maybe the user which should be used to create the mail should be the "receiver" and not the creator of the newsletter.
In my association I have multiple users with different role and access to different content using "Taxonomy access". The way mails are generated now I understand some users could get a mail where they cannot click on "Read more" to read the full article.

Tried the patch on the latest rc, where it applies with fuzz.

When cron runs, I get this:

    * warning: array_fill() [function.array-fill]: Number of elements must be positive in /Users/Sheila/Sites/drupal-name/includes/database.inc on line 241.
    * warning: implode() [function.implode]: Invalid arguments passed in /Users/Sheila/Sites/drupal-name/includes/database.inc on line 241.
    * warning: array_keys() [function.array-keys]: The first argument should be an array in /Users/Sheila/Sites/drupal-name/modules/user/user.module on line 502.
    * user warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ')' at line 1 query: SELECT p.perm FROM role r INNER JOIN permission p ON p.rid = r.rid WHERE r.rid IN () in /Users/Sheila/Sites/drupal-name/modules/user/user.module on line 502.

I suspect this is because the newsletter is made by uid 1 and I've not bothered to give the superuser any roles.

I'm not sure about the use of the 'cron user' setting -- why woud you want to send as the cron anonymous user? It just adds a complication because the way the newsletter is send is different depending on whether cron is being used or not.

Also -- how do I reproduce this anyway?
Does running cron with devel module run it as superuser? In which case that's no good for testing.
I've reversed the patch and emails are getting through, running cron on the command line with curl. Totally confused now!!!

While the function simplenews_switch_user in the patches above is declared as
function simplenews_switch_user($uid = NULL) {
it is called from simplenews_mail_mail for the user switch with
simplenews_switch_user($node->nid, $node->vid);

This will obviously not work. The single parameter should be $uid.

In addition, I support #26. This would open up the publication of a "public" and a "internal" version of the same newsletter, where one user sees some nodes, and the other not. Also, if someone who has received a newsletter with one role and now gets another role or additional rights, he could get the same newsletter in another version without having to unsubribe from the old and subscribing to a new series. (Think of a chronological list of events with public and internal events intermixed)

This patch is re-rolled for the current HEAD and has the error found by ccprog corrected.

Patch reworked for the current DRUPAL-6--1 branch.

The simplenews.d6_256795_2.patch had the same issue pointed out by ccprog. I am attaching a modified version with this issue fixed.

I'm having the same issue. I've applied the patch in #31, and tried setting the cron user to be the newsletter author, but at cron time it doesn't send, it's still permanently pending. When I run cron manually, it sends immediately, although it's sending it twice. Does the patch work for others - am I missing something in set up?

All my content, and therefore the newsletter, is private.

The current 6.x-1.x-dev branch has simplenews.module revision 1.76.2.136, but the patch applies correctly only on 1.76.2.134. With the latest branch I got the same error as you.

I'm actually using the latest stable release 6.x-1.0-rc6 rather than dev. This has simplenews.module revision 1.76.2.129. Is it worth updating to dev if it's still not solved the cron issue?

@Kweeks, sgabe: Try testing without Mime mail or other mail back-end like SMTP. Try debugging the code to see why the emails are not sent by cron.

Thanks for the revised patch Sutharsan.

I'm just using plain not MIME emails. My limited cron debugging skills seem to show:

• module_invoke_all in includes/module.inc shows that simplenews_cron is being called at the 10 minute cron run
• But the watchdog I put in simplenews_cron itself is not being called (which makes no sense to me?!)
• Running cron.php manually calls the watchdog I put in simplenews_cron though, and the emails are sent

So it seems to me that the cron run is not calling hook_cron properly but the manual cron is. I can't see how that can be the case?!

Make sure you logout before callin cron.php manually; to ensure it is not a permission problem.

I've managed to solve my problem now and the patch works great, thanks Sutharsan.

@enboig, thanks for that reminder, I hadn't tried that. It worked even when logged out so wasn't a permission problem at least not that side.

I eventually tracked it down to my crontab - it was in root's crontab and needed to be in apache's to run properly. So actually cron on the site had not been running at all for any hook_crons, and it was only simplenews that alerted me to the fact. (the emails being sent twice was a silly oversight on my part - two test subscribers with the same address :-))

@Kweeks: Thanks very much for reporting the cause of your problem! (believe it or not, but not many people report back once their problem is fixed.) Can you explain a bit more on "it was in root's crontab and needed to be in apache's to run properly." Was cron triggered by user 'root' instead of user 'apache'? How did you notice and how did you solve? Perhaps you want to add a description to the (simplenews) handbook.

It's exactly as you said. The cron was in the crontab of the Linux root user, when it needed to be in the crontab for the user 'apache' in order to run with the way our web servers are set up.

I added a watchdog into module_invoke_all in includes/module.inc to see what hook_crons were being called at cron time (a very useful tip from a DrupalCon Paris session, Debugging Drupal).

From that I could see that no cron functions were being called on the scheduled run, only when I ran it manually - meaning there's probably something wrong with the crontab. I hadn't even realised it wasn't running properly. Also because the emails sent manually (because that was the user 'apache' calling it) it wasn't a problem with cron just crashing on some module.

The cron log files (usually in /var/log/cron) show who's running the cron tab - it was root so I changed the crontabs and it worked.

Good idea I'll add something to the handbook - it might help someone else debug their cron problems even if it's not the same issue.

Also tested by BazzeFTW in #597264: Simplenews: Test mailing works fine, but real mailing not.
I will have a final look at the patch and add it to HEAD and the 2.x simplenews branch.

Hi,

I applied the patch manually and it worked.
I understand how it switches to the required user, but I don't see where does it switch back to the last user.
I have elysia cron and when I am logged in as admin and I run force a run, I get kicked out of admin.

Since we're getting in sync with permissions, shouldn't we do a node_access() call against the recipient's permissions? In other words, if the recipient doesn't have permissions to see the node, they shouldn't get sent the newsletter.

This may be a (future) option, but should certainly not be the default. A newsletter as email is usually not restricted content. Currently problems arise when the author has full access but the cron user (aka anonymous visitor) has limited access. This is the use case where this issue comes from.

I think this is probably ready to go. I patched it and used it to solve some very real problems (using premium module) and it did the job. I still applies and works fine with simplenews-6.x-1.0.

rfay, have you checked the observation in #42?

re: #42, #47:

I have run simplenews cron sessions as an anon user and as a logged in user. In both cases the behavior appears to be correct, both during newsletter processing and the resultant situation of the browser.

Logged in, I ran cron.php. It delivered the newsletters, I was still logged in.

Logged out (anonymous) I ran cron.php. It delivered the newsletters (and processed them correctly). After that I was still logged out.

I suspect the code could be a little cleaner in setting and unsetting the delivery user, as it's repeated a couple of times.

Can we get this in?

Thanks!

Would like to enquire to the status of getting this committed - we regularly have to apply it to production sites and it'd be great to be able to update to the latest versions without that tedium.

the patch on #35 did not apply on 6.x-2.x it was probably created for the 6.x-1.x version
here is a direct translation

but it not solving the problem for me #943980: newsletter send ignores subscriber's language preference

Thank you for this progress. Here are my current thoughts.

Note that temporarily switching the global user object is very very sensitive...

If somehow the script breaks (e.g. php timeout) the user session might remain in the switched user state.
Drupal 7 introduced the session save switch to prevent from possible issues...

Anyway - i recently added the switch in 6.2 to the anonymous user (see patch #471594: Insert View module shows Views administration links in mails sent by Simplenews ) for all cases of sending a newsletter and this seems to be a very clean base to me. Also security is not so much an issue since it ultimately could result in loosing user state and finally result in an anonymous user.
http://api.drupal.org/api/function/drupal_cron_run/7
And the finally new API introduced for dangeous operations:
http://api.drupal.org/api/function/drupal_save_session/7

If we switch to the author, a newsletter is being built with the author permissions. This will e.g. break the edit-link issues like #471594: Insert View module shows Views administration links in mails sent by Simplenews and related ones if the user is somehow administrator.
As stated above: anonymous user is not perfect but it will always result in very predictable results.

I'm thinking if we should switch to the delivery user, but that would result in performance issues since the resulting build node content could not be stored anymore. Think about the case that field visibility/permissions might be different per user.

Another solution would be to allow to define a sending user per newsletter or per issue.

However: switching to the author seems to me to result in higher complexity and a complex situation for users to understand permission application. Also the result will be wrong in some cases (regarding visibility of fields and functions).

So i won't apply this patch before we didn't think about better solutions and - if possible i'll wait for a cleaner solution.

Any further movement on this? Perhaps some clean separation of that functionality is in order so that sub-modules can be implemented to deal with it on a custom basis without having to hack simplenews?

We will need to introduce a new rendering mechanism to allow this without even more issues.

See my laste comment in
#700308: Allow separate theming of each individual outgoing email instead of caching the prepared newsletter

Right, we'll need to add APIs to controle the situation.

Is this issue still relevant?