Drupal give cached content before checking IP

I have a very similar problem with version 5.x-1.2. I use the gallery module and I grant access to the gallery only to logged in users and users in a certain IP range. Access control in the gallery module is done in its implementation of hook_menu() via user_access(). Now the problem is that menus in drupal are cached (and afaik independently from the caching setting mentioned above, but I'm not sure about that).

So when an anonymous user accesses the site the menu will be cached and all anonymous users will see the same menu and have the same access rights no matter what IP they have. Probably it is not enough to just add the new roles to $user->roles in ipauth_init(). But I haven't figured out yet what else has to be done.

I will have a deeper look into it but probably not next week and the week after. I'm quite new to drupal development so I don't know yet if I can fix it.

In Drupal 5, I have worked around the issue by adding:

function hook_init() {
  if (user_access('access purchase via ipAuthenticated')) {
    cache_clear_all('*', 'cache_menu', TRUE);
  }
}

Inside my hook_init() function (inside a module), this code checks to see if a specific role returns true. Notice the user_access() function call. So if the role 'access purchase via ipAuthenticated' has been assigned to the current session, then clear the menu cache.

Now the tricky part is to clear the menu cache early enough in the bootstrap or else this may not work; the module's name started with a 'p'. That seemed to clear the menu cache at the right point.

I'm sorry about the sloppy work around. You maybe able to add the conditional statement into the template.php file.

I wounder if somebody could add:

  if (user_access('access purchase via ipAuthenticated')) {
    cache_clear_all('*', 'cache_menu', TRUE);
  }

Into the ipauth_init() function and see if it corrects the menu caching issue. hmm You would have to replace the role (access purchase via ipAuthenticated) is replaced with the role that you're assigning your authenticated ips too. In the example above, I've used a distinct role for optimization reasons.

Please tell me how it works out. Thank you so much for helping out.

I wonder if I change the weight of the module in the system database table, if it would correct the caching issue. Because the modules are loaded-up in alphabetical order, I probably would need to change this since 'i' is late in the game. I also wonder if Drupal 5 and Drupal 6 support weighted modules.

My latest investigations yield that unfortunately ipAuthenticator is fundamentally incompatible with Drupal 5 page caching. There is no way to execute custom code before the page is fetched from cache. Drupal will then call the init hook but it's already too late. I will continue and see what can be done about menu caching.

Ok, so to make it work with menu caching we would have to execute a cache_clear_all() in ipauth_init() and in ipauth_exit(). The first one is to make sure that people in the special ip region see the normally hidden menu items. The second one is to make sure that a stranger doesn't see those items. There is no need to use the user_access function to test for a role because we can use ipauth_get_ip_authenticators() to check if the user is in a privileged ip region.
The code can be made more efficient if we don't delete the whole menu cache but only the row for the current user.

The problem I see here is that of a race condition. When a privileged ip user accesses the site the menu cache will be emptied and rebuilt (containing privileged entries). What if a second, unprivileged user requests a page before the first page request has reached the exit hook? The menu cache will still contain the privileged entries and the unprivileged user will see the privileged entries anyhow.

Any ideas?

I may have a solution for both 5 and 6 versions of Drupal:

Is the settings.php file considered core? As far as I know that file isn't edited or touched when updating Drupal. Moreover, this file is loaded on each page request even if aggressive caching is turned on (this is true in both Drupal 6 and Drupal 5). So it would seem that we can add some code to do the authentication in the settings.php file that would authenticate the user and turn off the page cache for that page visit. This would also eliminate the race issue that you described above.

Now at the point where the settings.php file is included, common drupal functions are not available. In fact, I think that only the bootstrap.inc functions are available to use. So the code in the settings.php file probably should only disable the cache if the ip is authenticated.

The benefit is a workaround for all forms of caching.
The issues include, how to authenticate (database calls), how complicated is it for non-programmers to add this code into the settings.php file, and how can we do this in a way that doesn't pollute the file?

I know it isn't a traditional approach, but what are your thoughts?

So here is the proposed code to insert into the settings.php file:

/**
 * Use by ipAuthenticator module.  To be inserted at bottom of the settings.php file.
 */

// Get the database function loaded up (the ipauth_get_ip_authenticators() function uses it).
require_once './includes/database.inc';

// Load the ipAuthenticator functions into memory.
require_once realpath('./sites/all/modules/ipAuthenticator/ipauth.module');

db_set_active();

// if the ip address is authenticated, then disable the cache.
if ( (bool) db_result( ipauth_get_ip_authenticators($_SERVER['REMOTE_ADDR'])) ) {
  $conf['cache'] = CACHE_DISABLED;
}

I don't understand every single implication of this code and I am sure that it needs to be tested. But I am very interested in your findings.

I didn't think about settings.php. That's a good point! I agree that it is not Drupal core as it contains settings that have to survive the update procedure. So an update wouldn't touch our code. The downside is that it is a bit illogical to put code into a file called "settings.php". But as we need to execute our code before the first hook is called I think that might be the only solution. Maybe we can get an "early hook" into core if there is no other way to do it.

Disabling the cache at that point is not enough because the menu cache is independent from the cache setting. We could use beekerstudios idea to make an automatic log in. If we assigned a user depending on IP even menu caching would be activated and working.

I am not available on this week-end so see you next week.

Perhaps the module needs to be modified to assign a User id (uid) to the ip authenticated user.

So upon install, the module adds a user and stores that uid in the variables database table to refer back to. Then when an ip authenticated user comes in, they will assigned the uid in the variables table. Hence, beekerstudios' idea.

So in the ipauth_init() function if the user is authenticated, I will assign the $user->uid equal to variable_get("ipauth_generated_uid", 0). I don't know what unpredictable results this will cause since I don't exactly know how the drupal login page is coded.

I've committed the changes. You can now specify either a user name or multiple roles to authenticate to. The changes should be on the cvs and will be updated on the next Drupal packaging run. This update requires you to run the /update.php script.

Would this solution help in my case where I don't want IPAutenticated users to be served non-cached content. For now, I have a block that I only want to show to "authenticated" users. I assign that role through IpAuthentication. If I turn page caching (normal) on, IpAuthenticateds don't see the block. If I add in the logic suggested in this thread in my settings.php file, everything works great and they see the block, but my CPU usage skyrockets because several hundred users are being treated as authenticated and thus not being served cached content.

Any suggestions?

@jonfrancisskydiver: I just want to let you know that I'm still working on this subject. Unfortunately I have run into a problem I can't resolve yet (#332219: Can't overwrite menu entries in _menu_append_contextual_items()). I've had a look at your code to authenticate as a user. Does it work? I'm wondering because I don't see any parts that let you login if you're already logged in by ipauthenticator. Also I want to block ip authenticated users from editing their account because it is not their personal one but one shared among multiple users. That's where the issue mentioned above stalls me. There's my current code attached if you want to have a look. It works only for normal caching but it doesn't need settings.php. And I've put the weight of the module to be 5 in the system table to overwrite some menu entries of the user module.

@pcorbett: As far as I understand there is only one page cache in Drupal 5. So you can't serve different cached content (authenticated/not authenticated) for the same url. So what you can do is to add a query string in settings.php to the requested url based on the role the user gets authenticated to. This changes the url and thus a different page can be served. You could do this as well in an init hook as I did (see attached file) but this has two downsides. First: In the init hook the page is already fetched from cache. So to get another one we need to reload via drupal_goto. This has an impact on speed and server load (two pages must be served instead of one at every request) but also on security. If you use drupal_goto then the query string is sent back to the client. So everybody who knows the right query string can access the restricted content you want to show via ipauthenticator. The same is true for the settings.php solution. But there the query string is not sent out of the server, so you have to make sure that nobody can guess it.
Unfortunately you can't use the "login as user via ipauthenticator"-functionality I'm working on as Drupal 5 can't cache on a per user basis. It only caches for anonymous users.

Hope that helps

I've had a look at the remember_me module and there they deny access to certain menu items in the init hook. I did the same and it works. I'm back on the road and optimistic to get what I want in a couple of days even if my solution seems to be more and more a kludge.

Finally,
voilà my proposition. Please have a look at it and tell me what you think.

Regards,
hajo

[some minor changes added]

Wow, I've been taking a look at all of the changes. Thank you. You have certainly paid attention to many more details than I have.

I've been looking at your issue with the drupal_goto function. Though I don't completely understand why the security issue exists, I may have away around the drupal_goto call (btw, here's away around including the common.inc file: you could use header("location: ..."); instead ).

Here's the idea to avoid the drupal_goto function altogether:

1. In the ipauth_init() We could hijack the $cache variable used in _drupal_cache_init() by adding a global $cache.
2. Rebuild the cache for the page with the authenticated view -- but here's the key... don't alter the database in doing so (is that possible?).... meaning lets overwrite the $cache variable with the authenticated page content.

Now how do we rebuild the cache variable... we could do a file( URL ? ipauth_no_cache=identifier ) which would create a second request but avoiding the use of drupal_goto and create more server load... or we could mimic the Drupal bootstrap process and capture all the output into the $cache object (it isn't so much mimic but forcing the full bootstrap process).

Is all this possible? Thoughts? (I've been sick so sorry for not responding in the past few days.)

well, in step one, I am not sure if we can capture the $cache object properly. I wonder if there is a php way of altering a parent function's variables.

I've been looking at your issue with the drupal_goto function. Though I don't completely understand why the security issue exists

The security issue does not exist in my proposal but in a "solution" I sketched for pcorbett. He wants to serve cached pages also for ip authenticated users. However, Drupal can serve cached pages only for user 0 (anonymous).
Here a scenario how the query string based approach could work:

User requests index.php?q=node
In ipauth_init() he is assigned role_a based on IP
drupal_goto is executed, new page: index.php?q=node&ipauth=role_a

if page index.php?q=node&ipauth=role_a exists in cache then
page for role_a is fetched from cache and served
else
page is built, saved in cache and served

But as an attacker I could directly request index.php?q=node&ipauth=role_a and see what only ip authenticated users should see. So this approach doesn't work.

In the ipauth_init() We could hijack the $cache variable used in _drupal_cache_init() by adding a global $cache.

Unfortunately, I'm not sure if this is possible via a global variable. As far as I know the cache variable in the bootstrap code has to be made global so that we can access it.
But the drupal goto is not a big thing as long as we're happy with the fact that ip authenticated users don't get cached pages. If the user has cookies enabled then the drupal_goto() is only issued once at the start of his session. Afterwards the session is set with uid!=0 and drupal does not try to serve cached pages anymore.

Another idea to avoid drupal_goto(): Replace drupal_goto() by
call exit hooks
eval(index.php);
exit();
So we could restart processing without the need to tell the browser. I don't know if this will work, though.

Hey, I've committed the code you posted above with a few revisions (http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/ipAuthentic...). I basically copied the code for drupal_goto and pasted in place of the function call and common.inc include.

In drupal_goto() I found this interesting tid bit: session_write_close(); So in addition to all instances of the module_invoke_all('exit', $url), we should add session_write_close(). I haven't done that yet... just wanted to know your thoughts.

Also that was a good catch with accessing the user login pages (the last few functions).

Lastly, I kept the unsigned int's for the ip1 and ip2, because I am pretty sure that I have fixed the issue with the negative integers.

About the eval(index.php)... I thought it through a bit. We would have to escape the requested URL and include the session identifier. using the exec type functions are pretty dangerous if you're not careful. I'll keep pondering it.

Please let me know what you think. I will return Monday!

Thanks for comitting. I had made some changes to the files attached to comment #16 after you have downloaded it. Would be kind if you incorporated that as well.

Regarding the session_write_close(): I haven't understood why it is necessary in drupal_goto(). But it probably won't hurt putting it in ipauth.

I'm not sure about the eval function. But my intent was to keep the variable environment the same, so the path will be set in the $_GET['q'] variable and will be sane, etc. So just restart from the beginning with everything else left set. I don't know if eval behaves this way though. I'll have to look this up.

Regards,
hajo

I've just found a bug. If you are logged in by ipauth and try to login with your personal account you don't get an error message if you type in the wrong password or username. This is due to the way drupal core code handles logins. So maybe we should handle things more like the ip login module does. Let the user see that he is logged in, ie don't show a login block etc and provide a logout link. To avoid that the user is logged in by ipauth right away we could use a session variable as the ip login module does.

Good catch. If the user both was IP authenticated and has a user account, it'd be nice if the real Drupal account would inherit any extra permissions once the user has logged in.

So if they could login while being ip authenticated, that would be the best solution (instead of having to logout). I'll review your code more closely and see what I can come up with. I wonder if something in drupal core prevented ip_login from doing this.

I also merged your more recent changes. Do you have any thing against how I modified the code that you posted? If not, I was hoping that we could work off of the development version of the module. That way I don't have to by hand merge the differences in code.

Hajo,
I am curious to why you added the code that checks for blocked users.
Jonathan

I am curious to why you added the code that checks for blocked users.

My idea behind this is that I don't want ipauthenticated users to be in the authenticated user role. For example in my case I don't want them to post comments unless they are logged in with their personal account.
If now somebody uses an account to log in that is used by ipauthenticator as well than the account is used with different user roles set and I think that this is a bad idea because of caching issues.
So in order to avoid that an account is used both for normal login and ipauthenticator I let only blocked user accounts be used for ipauthenticator.

Regards,
hajo