Shibboleth plugin overrides logout redirect url even user did not log in through Shibboleth

Here is the code that fix this:

diff -u -r1.29 auth.php
--- auth/shibboleth/auth.php    17 Feb 2011 00:48:40 -0000      1.29
+++ auth/shibboleth/auth.php    3 Mar 2011 01:29:48 -0000
@@ -193,11 +193,12 @@
      *
      */
     function logoutpage_hook() {
-        global $redirect;
+        global $USER, $redirect;

         // Only do this if logout handler is defined
         if (
-              isset($this->config->logout_handler)
+              $USER->auth == $this->authtype
+              && isset($this->config->logout_handler)
               && !empty($this->config->logout_handler)
            ){
             // Check if there is an alternative logout return url defined

I'm submitting a fix for peer review which is a slight variation of the one proposed above. Instead of checking $USER->auth, I check for $SESSION->shibsessionid, which is set on Shibboleth login. We've been using a similar solution on our installation for approx. 1 year.

Hi guys,

Thanks for the work on this issue.
The solution looks good thanks Henning, feel free to put this up for integration when you want.

There was one thing I noted for interest sake, the isset($this->config->logout_handler) check is necessary as the following !empty check will in turn check the variable is set as part of its operation.

Cheers
Sam

Hi Sam, thanks for reviewing this so fast. Unfortunately, I can't send fixes to integration directly; you'll need to push the button for me. Cheers!

Thanks Henning, I've put this up for integration now and I've even gone as far as inserting it into the current integration with Apu assigned as the integrator.

All yours Apu

Cheers
Sam

Thanks for the patch.
This has been stylishly integrated

Received the following errors while testing this on 2.0:

Parse error: syntax error, unexpected ')' in /m20/auth/shibboleth/auth.php on line 200

The following error occurs in 2.0, 2.1 and master

Notice: Undefined property: stdClass::$alt_login in /m20/auth/shibboleth/config.html on line 44

Note: Functional test is still in progress.

Found error in the code.

Test failed

sorry! thats been fixed. please test again.
that $alt_login notice is probably a separate issue.

Hi, Carson.

Would you be able to get the latest integrated version and run the test given above?

You can get it from the integration repository on git.moodle.org/integration.git or from the git repositories listed above.

Let us know if it succeeded by commenting here.

Got Shibboleth IDP + SP + apache_mod working here, really funny. Testing...

Just defined the logout handler as /LOGOUT.

One local user, on logout, is redirected to main page (no LOGOUT).

One shibboleth user, on logout, is redirected to /LOGOUT.

YTC !

(aka, yay, thanks and ciao ) Closing.