Use phpCAS methods for CAS logout.

That sounds okay to me. Patches are welcome.

Hmm, I didn't notice the $base_url in your original post. So this issue is about two things:

1. Using phpCAS::logout...() instead of drupal_goto()
2. Using a different default logout destination.

Okay, let's drop the $base_url part. If you want to redirect back to the main page, you should enter <front> as the redirection destination. If that's not clear from the config page we should clarify the help.

+++ cas.module	2012-03-07 17:37:51.000000000 +0100
@@ -685,9 +679,8 @@ function cas_logout($invoke_hook = TRUE)
+  // Logout on cas server and redirect to drupal if logout redirects is enabled on cas server.
+  phpCAS::logoutWithRedirectService($destination_url);

What if $destination_url is now empty? Does phpCAS::logoutWithRedirectService('') work? Or do we need to do something instead like:

if (!empty($destination_url)) {
  phpCAS::logoutWithRedirectService($destination_url);
}
else {
  phpCAS::logout();
}

Thanks for the testing!

I can't speak to all the issues, but some are due to the fact that in not all cases do you garantee that $destination_url is set. So you get an undefined notice error in that case. Did you look the view details on the test link above? Remember that the code needs to work if no destination logout url is set as well.

Revised patch attached.

Ahh, I see. The CAS tests expected logoutWithUrl, but the patch was using logoutWithRedirectService. I think we should maintain both flavors so as not change the functionality.

If you want to test it manually and report back with success, I'll go ahead and commit this. :)

Can you try the patch I suggested in post #23? It does use phpCAS::logout() (and not phpCAS::logoutWithRedirectService()).

Testing a backport of this patch to 6.x-3.x.

Committed to 6.x-3.x and 7.x-1.x. Thanks for the report and initial patch.

Hi there,
I'm reopening this ticket because I just read the phpCAS class, and we should stick with the original phpCAS::logoutWithRedirectService instead of logoutWithUrl

As you can see:

    public static function logoutWithUrl($url)
    {
        trigger_error('Function deprecated for cas servers >= 3.3.5.1', E_USER_DEPRECATED);
        ...
    }

And all over the place it says :

     * @deprecated The url parameter has been removed from the CAS server as of
     * version 3.3.5.1

What do you think about that?

@Stanto: I think it's fine because the patch we ended up using calls:

phpCAS::logout(array('service' => ..., 'url => ...));

@bfroehle : Thanks for the quick answer. And as you said the function phpCAS::logout() doesn't say anything about the url parameter being deprecated, but if you look at the function logoutWithUrl($url) (which basically launch phpCAS::logout() ) is deprecated, as well as the $url argument everywhere else (here for example phpCAS::logoutWithRedirectServiceAndUrl()).

Basically this url parameter seems to be annoying with (at least) rubycas-server : https://github.com/rubycas/rubycas-server/issues/86

And as you can see in the CAS specs (http://www.jasig.org/cas/protocol at 2.3.1) the "url" parameter is (was?) used to display a blurb of text like : "The application you just logged out of has provided a link it would like you to follow. Please click [link]here to continue". Its purpose is definitely not to redirect the user to an other page.

I feel that the straightforward approach would be to use phpCAS::logoutWithRedirectService instead of phpCAS::logout, but if you want to keep the url option, what do you think about adding a setting in the admin UI ?

@Stanto: Historically we've always provided the url and service parameters. And in the past we even sent a destination parameter.

Given that the 'url' option was in the official Jasig CAS protocol not too long ago (and might still be?), I don't think we need to stop sending it. No admin option for this, it's not worth the extra complexity. I'm sorry to hear that rubcas-server can't handle it, but frankly I think the burden lies with them.

Dave, what do you think?

My point is just that sending the "url" parameter at the same time as sending the "service" parameter doesn't really make sense.
On one hand the service parameter defines basically a redirection, while the url parameter define some link to put in the logout page.

The "URL" parameter should not be used for any redirection since that's what "service" is meant for.

I completely agree with you on the fact that the Ruby implementation should handle properly those use cases, but that doesn't change the fact that we're sending wrong values to the CAS server.
And it's pretty confusing.. How does it know if it should display the logout message or redirect?

The reason that both parameters were added was that different CAS server versions were responding to different parameters. If you had an older version of the cas server it might respond to one vs. the other based on the way the cas server was setup or compiled.

If we picked one at the exclusion of the other, it would work for some sites but not others. How the server handles the parameters is a server implementation decision. Many cas servers are customized and will use the service parameter at login to decide what level of trust. It may or may not allow redirection after logout based on the requesting service at login. In such cases the cas clients would often desire that the link appear on the logout page instead, hence the desire for the url parameter.

The cas protocol specification is full of "should's" vs. "must" which are interpreted differently based on the server implementation. As always, we're just trying to figure out what will work in the 80% use cases. The current handling of this was the result of such a discussion. We switched to service= and took away the url and it broke the cas module for another user, so we elected to send both.