"Check with the CAS server to see if the user is already logged in?" option does not work with caching

I tried renaming cas_init() to cas_boot() but things broke pretty badly. Matir is correct though, we should be doing this in hook_boot().

It's probably a good idea to use hook_boot(), but then you'll need to fix cas_login_check(), which relies on "user" module functions and common.inc:url().

It seems to me that cas_login_check() will need to be broken into two parts:

• From hook_boot(): The first part should check whether a user is already authenticated, and whether a user should be logged in, such as after returning from phpCAS::forceAuthentication(). A custom cas_url() will probably need to be written, or common.inc will need to be included in order to access the url() function.
• From hook_init(): The second part should register and/or log in authenticated users.

If, from hook_boot() you decide you need the second part, then you could call:

cache_clear_all($GLOBALS['base_root'] . request_uri(), 'cache_page');

That would force the current page to be rebuilt and allow code in hook_init() to be executed.

It seems to me that clearing the cache like that would have undesirable performance implications.... Instead maybe we could model persistent login, which also has to deal with this issue.

Regardless of the implementation strategy, patches are always welcome.

Hi, I've run into the same issue and I have found a solution :
I've added a hook_boot to the cas.module which detects if a CAS gateway authentication is needed.
If it's needed, its overrides the cache settings only for this time and then resume the bootstrap phase.
I check if the CAS gateway request is needed by using the _cas_allow_check_for_login() function and a static var so it didn't try to do a Gateway call if its a webcrawler or an excluded URI.

Patch provided

Better with a non empty patch (first patch with git on windows ... I miss my macbook so much :p )!

+++ b/cas.moduleundefined
@@ -15,6 +15,26 @@ define('CAS_LOGIN_DRUPAL_INVITE_DEFAULT', 'Cancel CAS login');
+    unset($conf['cache']);
+    $conf['cache']= CACHE_DISABLED;

I think we should be able to skip the "unset" line here.

Any chance you can review the patch in #869926: Use phpCAS::setCacheTimesForAuthRecheck() instead of a custom cookie. and comment as well? Also, since I think we will commit that patch soon, can you see if this patch plays nice with it?

@see http://drupal.org/node/1280474

We don't need a link to this issue in the source code. If we do need to include a reference, it should be to a documentation page, i.e., a child of http://drupal.org/node/1084302.

Also, why do we need the $cas_boot_perform variable? Is there some case in which cas_boot would be called twice? Other similar uses, like persistent_login don't seem to need such a flag.

Agree for the comment.
As you said, the cas_boot_perform is here to prevent hook cas_boot to be called twice.
With what I have seen in the code of drupal_bootstrap, it shouldnt be called twice if we remove this check, but I think that leave it could prevent side effects in some use case (that I dont even know ) that would alter the standard bootstrap mechanism.
Tell me if you want a new patch with updated comments, and without this check !

Regards,

David

As you said, the cas_boot_perform is here to prevent hook cas_boot to be called twice.
With what I have seen in the code of drupal_bootstrap, it shouldnt be called twice if we remove this check, but I think that leave it could prevent side effects in some use case (that I dont even know ) that would alter the standard bootstrap mechanism.

Let's remove the check. No other module is going to call cas_boot(), and the bootstrap phase will only call it once.

Yes, please roll a new patch.

Also it'd be good to describe any environment in which you've tested this, since the automated testing system isn't going to tell us much about whether or not this works. In addition we need to ensure that there aren't big performance implications for including this patch.

Here is the new patch !
I have the following environment :

- Jasig Cas server : 3.4.11

patch tested with :
Drupal 6.25
pressflow 6.19.92
with the 6.x-3.x-dev version of cas module

Settings :

- protocol SAML version 1.1.1
- "Check with the CAS server to see if the user is already logged in?" is checked

Performance issues :
- Will work with no cache or normal caching but not with agressive caching mechanism (because of hook_boot) and neither with external caching (if using pressflow).
- Module will perform a gateway call only once and then for each next request the cost in perf is just a check for a cookie, pure CPU and few instructions, so it should'nt be a problem.

David

Hi again,
Is there something wrong with this new patch ?
Are we close to have it committed ?
Thanks in advance for your answer,

David

Just installed CAS and ran into this issue. We have caching enabled on most of our sites. There doesn't seem to be much activity here so I hope you don't mind if I change the version to 7.x-1.x-dev and offer a new patch. It's working on our test site without any issues but I've only tested it for a few hours.

A small update to the patch - hook_boot can also be called from _drupal_bootstrap_page_header() and we only want to run this code if called from _drupal_bootstrap_page_cache().

Silly question but is the patched pulled in yet?
I would love to see this patched because it might help with #869926: Use phpCAS::setCacheTimesForAuthRecheck() instead of a custom cookie.

Thanks.

I think this is much-needed functionality and I'm using this on a production site with no ill effects *but* it does have a minor impact on performance for cached pages and so at least requires a system variable (and associated configuration option) so that the impact is as minimal as possible.

The ideal solution with almost no impact (for those that weren't using the feature) would be a $conf setting in the settings.php file such that this functionality is off by default unless the setting is present.

Is there any chance at all that a maintainer would consider this?

Expanding on my previous comment, if there was a $conf setting, this would have close to zero impact on a site that did not have the relevant $conf variable.

All that would be required would be changing one line of my patch from:

if (!isset($_COOKIE[session_name()])) {

To:

if (!empty($conf['cas_check_on_cached_page']) && !isset($_COOKIE[session_name()])) {

We have a reasonably popular site so caching is essential. We can clearly see that forcing the user to click a login button even though they already have an active cas session from one of our other sites has a significant effect on bounce rates.

After looking at this a while and testing quite a bit, it seems this patch is not disabling page caching properly - it's more complicated than just setting $conf['cache'] = FALSE; and continuing the bootstrap phase. The problem is that all hook_boot implementations will be invoked twice:

1. From within _drupal_bootstrap_page_cache
2. From within _drupal_bootstrap_page_header

I see that the patch has a check to avoid a call to the second invocation by backing out if the session has been initialized (which would indicate that it's past the _drupal_bootstrap_page_cache phase), but what about other modules that implement hook_boot? It could cause some seriously unexpected behavior.

I confirmed this by creating a basic module that implements hook_boot and logging when it is called. With page caching enabled w/ a page cache hit, it's called twice before the user is redirected to the CAS server for the gateway implementation.

This is a hard problem. I wrote a blog post about this a few months ago which describes the use of the Dynamic Cache module which is used specifically to do what we are trying to do here. Perhaps we make that module a requirement for this feature instead?

@dazweeja, curious what version of PHP CAS library are you using?

I'm using 1.3.2, and at least in that version phpCAS::checkAuthentication() will set a session variable $_SESSION['phpCAS']['unauth_count'] if the gateway check shows that the user is not logged into the CAS server. As long as a session variable is set, Drupal will NOT served cached pages. See _drupal_bootstrap_page_cache() in bootstrap.inc.

I'm curious how you gateway method working at all while still serving cached pages?

The work in #1420170: Login during hook_init interferes with other modules (like Rules) dramatically reduces the amount of code in hook_init, so we may be able to revisit this and try to just move the remaining code in hook_init into hook_boot.

phpCAS will always set $_SESSION variables during a gateway check, and as soon as that happens, cached pages won't be displayed. So in that regard, caching will never work with this module, unless phpCAS is ripped out and we implement it's functionality without using sessions.

But in the other sense, if we can move into hook_boot, we can at least get gateway redirects working for existing cached pages, whereas now it doesn't work.