Redirect user on logout : doesn't work
qphrenqp - November 19, 2007 - 10:58
| Project: | CAS |
| Version: | HEAD |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | metzlerd |
| Status: | needs review |
Description
The error is just the use of a bad parameter name. url is described in the protocol page af CAS http://www.ja-sig.org/products/cas/overview/protocol/index.html and service is used by the CAS module.
Here is just the patch that solves this. I'm not sure about the patch format, though, so i just copy it, if someone wants to validate it...
520c520
< $logout_destination .= '?service=' . variable_get('cas_logout_destination','');
---
> $logout_destination .= '?url=' . variable_get('cas_logout_destination','');
#1
do we need an option to choose 'service' vs 'url'?
'service' works fine for me, while 'url' does not
#2
Url is wrong also. It should be destination. Here is the patch to apply to HEAD
#3
Now that I think of it maybe it's a difference between cas 1.0 and cas 2.0. I am using cas 2 and I use destination.
#4
#5
The attached patch was against head so I'm changing the version to head. Dave, any time to look at getting these patches applied to head?
#6
url is what works for me as well. Not sure how to proceed. Perhaps we need to rethink this whole thing?
Dave
#7
Here's a revised patch that combines an ugly hack do deal with the differences with a strategy submitted under a different issue that also allows for a supplied destination= logout url. Thoughts on this approach?
#8
It feels like your screaming "YOU WILL REDIRECT IF YOU LIKE IT OR NOT!" ;)
Can you explain the code below more? What are valid entries? I'm confused on how the override actually works.
// Add destination override so that a destination can be specified on the logout link// e.g. caslogout?destination=http://foo.bar.com/foobar
// do not accept caslogout as a valid destination, since that may cause a redirect loop
$destination = preg_replace("/(destination=|caslogout)/", "", drupal_get_destination());
Attached is a revised patch that fixes some documentation spelling mistakes. The explanation in the settings page should be revised and probably be added to this patch before committing.
#9
This lets site content providers create different pages put links on their pages like http://example.com/caslogout?destination=http://anothersite.example.com and have their pages redirect to a different url than the one configured.
The code you're talking about just is an extra check to make sure that users cant specifiy destination=caslogout. Which would cause a redirection loop. It just nulls out destination if caslogout is contained in the url.
The normal default destination logic would still apply if denstination= is not specified.
Dave
#10
Aaaah I think I get it so if on my user page I specifically want a logout tab I can create it and point it to http://example.com/caslogout?destination=node/5 and it will go to that node instead of the global chosen page.
So no documentation is needed on the destination setting.
I would commit as is it looks good. I have tested it and it is working fine for me. Once this is committed I will work on re-rolling my other patches to reflect the updates.
#11
Is there a reason it is not using the phpCAS logout method?
#12
FYI I wrote to the phpCAS list about the current logout not supporting destination, url and service. He put it into version 1.0.0RC2. I can now see why you didn't use the phpCAS logout method, because logout wouldn't always work. Once 1.0.0 is released it might be good to only support that version to clean up these nuances.
#13
Geat, Am away on vacation, will be back on the 10th at which point I'll commit this.
Dave
#14
Committed in Head.
#15
Automatically closed -- issue fixed for two weeks with no activity.
#16
// Add destination override so that a destination can be specified on the logout link
// e.g. caslogout?desination=http://foo.bar.com/foobar
// do not accept caslogout as a valid destination, since that may cause a redirect loop
$destination = preg_replace("/(destination=|caslogout)/", "", drupal_get_destination());
//Make it an absolute url. This will also convert <front> to the front page.
$destination = url($destination, array('absolute' => TRUE));
// If there was no override and admin has set and enabled a logout destination, look for it
if (empty($destination) && variable_get('cas_logout_redirect', 0)) {
$destination = variable_get('cas_logout_destination', '');
// redirecti is enabled but no url is given - default to baseurl
if (empty($destination)) {
$destination = $base_url;
}
}
For me redirect to a specific destination does not work. I tried to watchdog before and after the //Make it an absolute url code and the thing is that that when the $destination is empty making it absolute will return the url. Therefore the variable cas_logout_redirect value will never be applied.
Do others have the same issue?
#17
Could you post what your settings are. Part of the problem may be that the "Require cas for all pages except" is conflicting with where you send the logout url. Also note the confusion others have had about using the "CAS logout" menu item vs. the normal drupal logout menu item. You need to make sure you're using the CAS provided logout menu item in order for any of this to work.
Dave
#18
The settings are rather ordinary:
I am using ?q=caslogout to logout.
#19
I don't have the same issue but that may be because of drupal revision or configuration differences. If what you say is correct then we ought to patch this by putting an if ($destination) clause in front of the $destination= URL call. Here's the simple patch that would fix that condition.
#20