Module does not work with caching (reload needed)

bajnokk - July 2, 2009 - 16:09
Project:Shibboleth authentication
Version:6.x-3.x-dev
Component:Code
Category:bug report
Priority:critical
Assigned:Unassigned
Status:closed
Description

The module does not work with caching enabled. For normal caching, the user has to hit the reload button, but with aggressive caching, it fails constantly.

At least it is now documented...

#1

bajnokk - July 2, 2009 - 16:16
Priority:critical» normal

Adjusting priority to be align with the Issue queue handbook. However we take this bug very seriously.

#2

MOREnetWebmaster - July 29, 2009 - 21:38

We have encountered this as well.

We would like to add our +1 to requesting this be addressed.

Thanks!

#3

jbkc85 - August 11, 2009 - 19:44

After looking around, I was able to SOMEWHAT Fix this.

Inside the Shibboleth Coding, around line 307 (after I edited at least), you will see the variable declaration of $actuallocation.

I was able to change this, KNOWING I was redirecting after the shibboleth.

$actuallocation = urlencode([Everything until ://] . '://direct.to.me'; // Got rid of wayuri or whatever else was here.

I then deleted ALL of the Caching and Cookies in my browser, and Shibboleth worked with memcache enabled. I am sorry if this wasnt too descriptive, but I am unable to access the actual file to copy/paste the code. But, this code actually worked with memcache, GIVEN the cache/cookies were deleted on the browser after the caching was turned on (also, i cleared drupal cache right after I turned it on just as a precaution).

Hope it helps someone else...

#4

dorion - August 14, 2009 - 09:32
Priority:normal» critical
Status:active» needs review

Hi!

I am an official devenloper this module and I created a patch for the lastest dev version.
I think this patch SOLVE the caching problem both normal and aggressive cache.

The patch includes the more useable improved debug feature too.

Please review it!

If this patch is slove this issue we will release a new stable version.

AttachmentSize
cache_solution_drupal_org.patch 7.89 KB

#5

Stevel - September 5, 2009 - 16:33

I've found an issue with the patch provided:
When the url of doesn't end with a slash, the resulting url doesn't work. This is the case for example with a multilingual site, where url('<front>') resolves to e.g. '/en' for english or '/nl' for dutch. Appending a / if needed seems to solve the problem.

    $front = url('<front>');
    if (substr($front, -1) != '/') $front .= '/';

    //$actuallocation: the path where the Shibboleth should return
    $actuallocation = (isset($_SERVER['HTTPS']) ? 'https' : 'http')
                              .'://'. $_SERVER['HTTP_HOST']
                              . $front
                              . $url_prefix
                              .'shib_login/'
                              . $_GET['q'];

#6

Stevel - September 15, 2009 - 18:45

I've been running the patch with the slash-adjustment in my previous post for a while now and haven't encountered any issue related to caching.

#7

snorkers - October 1, 2009 - 23:30

I tried applying the patch to latest DEV (1 Oct 09) and it failed. So then applied patch to 3.1 - which passed - but did not cure the caching problem for me. 'Login' style link remains on previously cached page.

What is the status of this patch against DEV?

#8

snorkers - October 2, 2009 - 17:57

Using the /shib_auth/ caching workaround currently in DEV, I've encountered a few double slashes in the generated URL. Rather than use the link generated by the module to login with Shibboleth, I generate a custom login link (I don't want users to be confused by the conventional login texfields, and most of them will have no idea what 'Login with Shibboleth' means). So I've got my own functionality in template.php > preprocess_page which goes along the line of:

function theme_preprocess_page(&$variables) {

global $user;
global $base_url;

        //get drupal site root and add trailing slash if not there
$siteRoot = $base_url;
if (substr($siteRoot, -1) != '/') $siteRoot .= '/';

        // Create a custom variable to store the member login link, if the user is not already logged in. In page.tpl.php, remember to 'print $member_login;' in the right place!
if ($variables['logged_in']) {
$variables['member_login'] = l(t('Logout'), 'logout', array('attributes' => array('class' => 'memberLogin', 'title' => 'Log out from the system. Note you may subsequently need to close your browser to completely log out from all University online services.')));
$variables['member_account_link'] = l(t('My account'), 'user/'.$user->uid);
}
else {
$variables['member_login'] = l(t('Login'), $siteHome.'Shibboleth.sso/Login?target='.$siteRoot.'shib_login/'.$_GET['q'], array('attributes' => array('class' => 'memberLogin', 'title' => 'Log in using University online ID')));
$variables['member_account_link'] = '';
}
}

So I don't see a reason to use PHP urlencode (mentioned at #3 in #543602: The return location should be encoded); and is there a problem with using $base_url to get the site root (after all, shouldn't we be using all the Drupal functions and variables for 'safe' code)? The current DEV Shib module also assumes that $front is the site root too, which I think is an incorrect assumption.

This issue is also related to #442170: shib_auth incompatible with caching (even normal mode).

#9

bajnokk - October 19, 2009 - 10:10
Status:needs review» closed

@#7: No, the -dev (and 3.2) already contains the fix
@#8:
* I've opened a new bug for the double slash problem: #608358: Get rid of double slashes at the return url
* I've opened a new feature request for overriding the login link #608364: Allow customizing login link (about to be fixed in 4.x)
* We don't want to use templates at the moment, so this will need manual coding if you need more that just changing the login link
* You can get rid of the login form elements
** on the front page by disabling the user login block
** on the logged-in user page: use the userprotect module
** on anonymous /user page: we don't want to hide the conventional login textfields, because this is the only way the admin can log in while using the module
* I've opened a new bug for the $base_url misinterpretation #608368: Misinterpretation of $base_url

 
 

Drupal is a registered trademark of Dries Buytaert.