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','');

Comments

treksler’s picture

Status: Patch (to be ported) » Postponed (maintainer needs more info)

do we need an option to choose 'service' vs 'url'?
'service' works fine for me, while 'url' does not

redndahead’s picture

StatusFileSize
new802 bytes

Url is wrong also. It should be destination. Here is the patch to apply to HEAD

redndahead’s picture

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.

redndahead’s picture

Status: Postponed (maintainer needs more info) » Needs review
redndahead’s picture

Version: 5.x-2.2 » master

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?

metzlerd’s picture

url is what works for me as well. Not sure how to proceed. Perhaps we need to rethink this whole thing?

Dave

metzlerd’s picture

StatusFileSize
new1.75 KB

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?

redndahead’s picture

StatusFileSize
new1.75 KB

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.

metzlerd’s picture

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

redndahead’s picture

Status: Needs review » Reviewed & tested by the community

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.

redndahead’s picture

Is there a reason it is not using the phpCAS logout method?

redndahead’s picture

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.

metzlerd’s picture

Geat, Am away on vacation, will be back on the 10th at which point I'll commit this.

Dave

metzlerd’s picture

Assigned: Unassigned » metzlerd
Status: Reviewed & tested by the community » Fixed

Committed in Head.

Anonymous’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.

etaroza’s picture

Status: Closed (fixed) » Active
  // 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?

metzlerd’s picture

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

etaroza’s picture

The settings are rather ordinary:

  • CAS server is on the localhost
  • "Check to see if the user is logged in" - marked
  • Require CAS login for all pages except specific pages - empty
  • Redirect user on logout - marked
  • Logout destination http://www.google.com

I am using ?q=caslogout to logout.

metzlerd’s picture

StatusFileSize
new822 bytes

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.

metzlerd’s picture

Status: Active » Needs review
adamfranco’s picture

StatusFileSize
new719 bytes

I have run into another issue with logouts not redirecting when caslogout is called with a destination parameter, e.g:
http://www.example.com/caslogout?destination=http://www.example.com/my/path/

What happens in this case is that drupal_goto() sends the user to the URL in $_REQUEST['destination'], rather than the url passed as an argument. This has the effect of logging the user out of Drupal, then sending them directly to their destination page without hitting the CAS-logout page. By skipping the CAS-logout page, the user is logged out of Drupal, but still logged in to CAS. The next time the user clicks a log-in link, CAS won't prompt them for their password because they are still logged in to CAS.

The intended sequence (based on $logout_destination):
A page (user clicks logout link)
» drupal/caslogout (redirected to...)
» CAS server/logout (a page with logout message and destination link, user clicks link)
» destination.

What is happening instead:
A page (user clicks logout link)
» drupal/caslogout (redirected to...)
» destination.

Solution

Before calling drupal_goto($logout_destination) we need to unset the $_REQUEST['destination'] so that drupal_goto() will use our $logout_destination.

  // Add the log out destination if we have one
  // The three variables are a hack because cas server implementations don't seem to be consistent with
  // its use. 
  if ($destination) {
    $logout_destination .= '?destination='. $destination .'&service='. $destination .'&url='. $destination;
  }
  
+  // Remove our original destination from the request array so that it won't cause
+  // drupal_goto to re-write the url.
+  unset($_REQUEST['destination']);
  
  // Go to the constructed logout destination
  drupal_goto($logout_destination);

A patch off of the CVS HEAD is attached.

- Adam

metzlerd’s picture

Status: Needs review » Fixed

Tests ok, and doesn't appear to break code, so committing.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

  • Commit 75044da on 6.x-3.x, 7.x-1.x, master, 8.x-1.x by metzlerd:
    #193460 Revised fix for drupal_goto behavior.