michaelryanmcneill / shibboleth

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

Add Session ID header override #97

Closed spfncer closed 8 months ago

spfncer commented 8 months ago

Hi there! I've been working for a while trying to configure this plugin for a site at the University of Florida, but we continued running into an infinite redirect upon authentication. Through debugging, I found that our Session ID header was different than all the ones this plugin checked for, but the plugin does not currently provide a means to override the default Session ID header -- causing the infinite redirect. I've added that override feature in this PR, which can be controlled via a PHP constant or wp-admin option (like existing options). If the new option is left blank, it will continue using the default Shib-Session-ID header.

I've tested this on our test site, and it appears to work as intended. It may be worth a bit of testing to ensure it does not disrupt other setups.

Thank you to the team that maintains this repo! I hope this feature can help more people use this great plugin.

jrchamp commented 8 months ago

Hi @spfncer! Thank you for using this plugin and for contributing back a change that addresses an issue you encountered.

Now that you've highlighted the check in the plugin for Shib-Session-ID, I'm wondering why we check that at all. We don't use the session value, and if it's going to be different on different systems, it doesn't seem like it's a good universal value to use.

Instead, maybe we should be checking for the one environment variable that we actually need and using that in place of Shib-Session-ID. Specifically, line 588 and line 592: https://github.com/michaelryanmcneill/shibboleth/blob/7e94bd21fffb3009d0a7f9e6ee447bdd664b4cf9/shibboleth.php#L588-L592

I think that may simplify the solution significantly while leveraging the configuration that already exists. Does that work in your environment?

spfncer commented 8 months ago

Good morning @jrchamp! Yes that worked; I modified the first few lines of shibboleth_session_active to check for the username as you directed and continued to see successful authentication. I wondered if there may have been some other reason (maybe future development) why the Session IDs were pulled, but if they're not needed that's a much simpler fix. Thank you!

mmatthiesencsc commented 4 months ago

Hello, for what it is worth, this created an issue for me. My WP instance is behind a proxy and uses HTTP Headers. Now the Shibboleth plugin uses EPPN for authentication but also uses MAIL if EPPN is not found. In our case we mapped EPPN on the proxy to a different variable, i.e. it was not accessible for the plugin, but we never noticed since the plugin then uses MAIL. SHIB-Session-ID worked as intended. This change broke our installation. After I found the missing EPPN, it was easy to fix, but I just want to point out that you probably want to check for EPPN or MAIL to account for an active session, since you use either to authenticate. Otherwise thanks for the very useful plugin!