Cron should run as anonymous when invoked via the run-cron link on the status report page

subscribing

This is obviously a bug in Drupal core, but seems like a serious design issue in content permission, too ;)

Issue mentioned in content_permissions
http://drupal.org/node/510744

Here's a patch, sets $GLOBALS['user'] = drupal_anonymous_user() in drupal_cron_run() and prevents session being saved.

Critical because this blocks #523894: Comment indexing does not include the fully rendered comment which affects indexing fields on comments, which in turn blocks comment body as field (and etc.).

Spoke to DamZ in irc, inverted the two calls.

HEAD was a couple of hours stale.

That's a real bug - if run via the ui, system_run_cron() does a drupal_set_message() to report success/fail. So, we now restore the user after all jobs have run, and don't prevent session information being saved. The other option is to remove those drupal_set_message() calls, but having to look at the status table to see if it ran properly or not isn't ideal, so would rather keep them in.

It's ok to restore the user at the end of the process, but you still have to prevent the session from being saved during the time the other is changed.

Here.

I guess I have a different perspective. Search should have access to index all the content that it is supposed to.

So whether cron is run as anonymous or as an authenticated user with privileges should be irrelevant.

Search results on the other hand should respect the access modules in place. Usually in the form of a contrib module, such as apachesolr_nodeaccess (in http://drupal.org/project/apachesolr) that will modify the query of search results to exclude those nodes the user does not have access to.

Because search results may show restricted content does not mean that that restricted to anonymous content should not get indexed at all, and therefore not be available to those that have access, to view them in search.

For apache solr field access control, I will start http://drupal.org/project/apachesolr_fieldaccess

Moving to new cron system component.

We should probably require #287292: Add functionality to impersonate a user to land in core first.

Too late for D7. :(

hrm. Actually, reading the OP once more, I wonder why this is a public security issue. Moving back.

@aufumy

You concern is off-topic here. Cron normally runs as anonymous (from the command0line), so we need to ensure uniform behavior.

Here's a patch that passes, but I had to alter the verifyCron() test for dbLog to make it do so.

+++ includes/common.inc	26 Feb 2010 23:02:01 -0000
@@ -4373,6 +4373,14 @@ function drupal_cron_run() {
+  // Prevent session information from being saved while the cron is running.

... while cron is running.

+++ includes/common.inc	26 Feb 2010 23:02:01 -0000
@@ -4447,6 +4458,10 @@ function drupal_cron_cleanup() {
+  drupal_save_session(TRUE);

Usually, this is called right after reverting the user. Is there a specific reason for delaying this by putting it in drupal_cron_cleanup()?

+++ includes/common.inc	26 Feb 2010 23:02:01 -0000
@@ -4447,6 +4458,10 @@ function drupal_cron_cleanup() {
+  return TRUE;

drupal_cron_cleanup() should not return any value.

+++ modules/dblog/dblog.test	26 Feb 2010 23:02:04 -0000
@@ -74,9 +74,7 @@ class DBLogTestCase extends DrupalWebTes
-    $this->drupalGet('admin/reports/status/run-cron');
-    $this->assertResponse(200);
-    $this->assertText(t('Cron ran successfully'), t('Cron ran successfully'));

I don't think we can just get rid of the "Cron ran successfully" message when running cron manually through the interface. Users expect this kind of message when performing actions through the interface. Since the message is set in system_cron() after calling drupal_run_cron(), the message should be persisted if the user is reverted and drupal_save_session(TRUE) called at the end of drupal_run_cron(), right?

Powered by Dreditor.

Could be. Let's try it.

+++ includes/common.inc	10 Mar 2010 21:51:02 -0000
@@ -4385,6 +4385,14 @@ function drupal_cron_run() {
+  // Prevent session information from being saved while the cron is running.

1. Is it 'the cron' or just 'cron'?

2. If we forced the user to the anonymous user, do we still care about sessions? I'm not sure the phpDoc provides enough context to understand why sessions need to be disabled (for the anonymous user).

@Dries: IMO, yes, we care about the session: if we do not disable session saving during impersonation and the cron run is aborted half-way through, the user will logged out.

Thinking about this does bring up an interesting point though. What if an individual hook_cron implementation does impersonation itself. When reverting, it calls session_save_session(TRUE); and switches session saving back on...

mr.baileys: that sounds like it would be a bug in the module then. I think this is RTBC. I'll fix the comment and commit it.

Committed to CVS HEAD.