michaelryanmcneill / shibboleth

Shibboleth plugin for WordPress
https://wordpress.org/plugins/shibboleth/
21 stars 12 forks source link

Shib plugin not targeting the current site's domain name in Multisite setup. #33

Closed themantimeforgot closed 6 years ago

themantimeforgot commented 6 years ago

In a multi-site setup the plugin seems to always use the main blog (#1) as the selected site. This causes problems in my setup (we use domain names for subsites) and always redirects users logging in to the admin section of the main site.The issue seems to stem from the use of $GLOBALS[‘current_site’]->blog_id in the shibboleth_session_initiator_url function. This has been happening for quite some time now (Since at least version 1.6) and I had to create my own function (See below) to get around the problem. I was wondering if a more permanent resolution could be put in place. Thank you.

function shibboleth_select_current_site() {
    if ( !empty( $GLOBALS[‘current_blog’]->blog_id ) && $GLOBALS[‘current_blog’]->blog_id > 1 ) {
        return $GLOBALS[‘current_blog’]->blog_id;
    } else {
        return $GLOBALS[‘current_site’]->blog_id;
    }
}

Issue was originally reported on wordpress.org

jrchamp commented 6 years ago

I've seen this behavior recently and a similar solution was brought up in the past, but it sent me into in a redirect loop: https://github.com/mitcho/shibboleth/pull/15/files

I'm really hoping we can get this solved and soon. We're definitely going to look at this soon ourselves.

Now, because there can be multiple networks (see https://developer.wordpress.org/reference/functions/get_main_site_id/ ), I'm hoping to not hard-code the blog id 1. We might be able to use the $GLOBALS['current_site']->site_id in place, resulting in:

    if ( !empty( $GLOBALS['current_blog']->blog_id ) && $GLOBALS['current_blog']->blog_id !== $GLOBALS['current_site']->site_id ) {
        return $GLOBALS['current_blog']->blog_id;
    } else {
        return $GLOBALS['current_site']->blog_id;
    }

Testing is definitely needed to make sure this works with single-network multi-site, multi-network multi-site and traditional single-site.

themantimeforgot commented 6 years ago

@jrchamp, I can confirm that works in my multisite setup (single-network multi-site). Single site will be easy to test but I have never set up a multi-network setup. I wouldn't mind doing so for the purpose of getting this fixed but it will take me some time to setup.

ghost commented 6 years ago

@michaelryanmcneill, I see that this fix is slated for a 2.1 release of which there are several open issues remaining. Any chance this can be folded into an earlier release? This fixes a longstanding issue at our university that is perfectly described in #35 by @jrchamp, and it would be fantastic if we could point others to this plugin without having to apply manual patches in the interim. Thanks!