michaelryanmcneill / shibboleth

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

Shibboleth v1.8 Not Allowing Login #7

Closed jmdemuth closed 6 years ago

jmdemuth commented 6 years ago

Per request I am cross-referencing an issue I posted to WordPress.org. This is all a bit new to me so I hope this is what you wanted:

https://wordpress.org/support/topic/shibboleth-v1-8-not-allowing-login/

michaelryanmcneill commented 6 years ago

@jmdemuth, thanks for this. I've been reviewing this and I'm not seeing where the issue is coming in. I've just tested on PHP 5.6 without issue. According to the documentation on getenv():

This function is useful (compared to $_SERVER, $_ENV) because it searches $varname key in those array case-insensitive manner. For example on Windows $_SERVER['Path'] is like you see Capitalized, not 'PATH' as you expected. So just: <?php getenv('path') ?>

I'm going to do some additional diagnostic work, but any additional details (operating system, SP configuration, web server, etc.) on your environment would be super helpful.

jmdemuth commented 6 years ago

Our web-hosting service uses RHEL 6.9 (kernel build 2.6.32-696), Apache 2.4.x, PHP 5.6.31. Unfortunately I don't have any Shib SP config data handy. All I can say is that if I add the following var_dump to shibboleth_authenticate_user() in shibboleth.php at the point shortly before I get the error message (just after line 343):

echo "<pre>";
var_dump(   $shib_headers['username']['name'],
            $_SERVER["uid"], 
            $username,
            $user
        );
echo "</pre>";

Then I get this corresponding output on the login page (the 'xxxxx' is a substitution here for the valid Shib login account on the system):

string(3) "uid" string(8) "xxxxx" bool(false) bool(false)

So it looks like Shib is setting the $_SERVER array values correctly, but something about our environment causes shibboleth_getenv() to set $username to false.

I also note that if I dump $_SERVER["UID"] I get a NULL value. So I have to wonder if getenv() is truly searching in a case-insensitive manner.

Hope this helps. I'd be happy to provide any more info if needed.

michaelryanmcneill commented 6 years ago

Thanks for the additional info. Can you do me a favor and also provide a var_dump of getenv('uid') and getenv('UID')? There is logic to account for the various uppercase and lowercase options, but maybe I'm missing something there: https://github.com/michaelryanmcneill/shibboleth/blob/master/shibboleth.php#L30-L43

jmdemuth commented 6 years ago

Both var_dump's return bool(false)

michaelryanmcneill commented 6 years ago

Interesting, but you see it in $_SERVER. I'm going to do some more testing...

In the meantime, can you try another variable populated by Shibboleth?

$_SERVER['Shib-Session-ID'] getenv('Shib-Session-ID')

jmdemuth commented 6 years ago

$_SERVER['Shib-Session-ID'] = NULL getenv('Shib-Session-ID') = bool(false)

michaelryanmcneill commented 6 years ago

Interesting...are you sure you have an active Shibboleth session? If you visit https://example.com/Shibboleth.sso/Session do you see an active session?

jmdemuth commented 6 years ago

Well, I see the following which I assume indicates an active session (some actual values replaced with 'xxxxx'):

Miscellaneous Session Expiration (barring inactivity): 478 minute(s) Client Address: xxx.xxx.xxx.xxx SSO Protocol: urn:oasis:names:tc:SAML:2.0:protocol Identity Provider: https://xxxx.xxxx.xxxx/idp/shibboleth Authentication Time: 2017-09-07T12:24:16.204Z Authentication Context Class: urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport Authentication Context Decl: (none)

Attributes eppn: 1 value(s) persistent-id: 1 value(s) pubcookie-user: 1 value(s) uid: 1 value(s) wiscEduPVI: 1 value(s)

michaelryanmcneill commented 6 years ago

Interesting, can you do a dump of all Shibboleth related $_SERVER data and sanitize and post it here?

jmdemuth commented 6 years ago

Here you go! I do have to head out for the day shortly, but if you need anything else I'll be checking in on this again 2017-09-08 0800 USCDT

array(58) { ["REQUEST_URI"]=> string(31) "/wp-login.php?action=shibboleth" ["SCRIPT_NAME"]=> string(13) "/wp-login.php" ["QUERY_STRING"]=> string(17) "action=shibboleth" ["AUTH_TYPE"]=> string(10) "Shibboleth" ["REMOTE_USER"]=> string(8) "xxxxx" ["uid"]=> string(8) "xxxxx" ["pubcookie_user"]=> string(8) "xxxxx" ["persistent_id"]=> string(118) "https://xxx.xxx.xxx/idp/shibboleth!https://x.x.x.x/shibboleth!Ypa9SgLHutumpn3wq5Mepf8MlyI=" ["eppn"]=> string(17) "xxxxx@xxx.xxx" ["Shib_Session_Index"]=> string(33) "_70b17a18e5fe0f125d660cdce8d023c2" ["Shib_AuthnContext_Class"]=> string(65) "urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport" ["Shib_Authentication_Method"]=> string(65) "urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport" ["Shib_Authentication_Instant"]=> string(24) "2017-09-07T12:24:16.204Z" ["Shib_Identity_Provider"]=> string(39) "https://xxx.xxx.xxx/idp/shibboleth" ["Shib_Session_ID"]=> string(33) "_70f70583f9457a5f59ca234a813cd5a9" ["Shib_Application_ID"]=> string(30) "xxx.xxx.xxx" ["Shib_Handler"]=> string(53) "https://xxx.xxx.xxx.xxx/Shibboleth.sso" }

michaelryanmcneill commented 6 years ago

Ah, underscores, my bad. No worries, I'll spend some time troubleshooting this evening and let you know what I find.

michaelryanmcneill commented 6 years ago

I've still been unable to reproduce this, but have made a change that may fix it. I didn't have an available install to test on tonight, so this may or may not work. If you have a development environment that you can test on, can you please take a copy of the issue-7 branch (https://github.com/michaelryanmcneill/shibboleth/tree/issue-7) and try running it on your environment and let me know the results?

jmdemuth commented 6 years ago

Hi Michael -

I am pleased to report that the fix appears to solve the problem! I reapplied the entire v1.8 update, and then applied your modified shibboleth.php file. I did this on my three test sites and login seems to be working properly on all three. I'll probably apply the patch to production systems early next week.

I'm going to contact the sysadmins for our web-hosting site to see if they might have any idea why getenv() wasn't doing what we thought it should be doing. If I get anything meaningful I'll update this post.

Thank you for addressing this so quickly!

michaelryanmcneill commented 6 years ago

That is excellent news, I've done some additional testing and I plan to release this (and another small fix) as 1.8.1 later today.

michaelryanmcneill commented 6 years ago

This has been released as 1.8.1 as of 2 minutes ago. Thanks again for reporting!

jmdemuth commented 6 years ago

This is just a follow-up: I did confirm with our web-hosting sysadmins that in our environment we restrict Apache from accessing environment variables for security reasons. This is why using getenv() (as opposed to $_SERVER) in the plugin failed to return any values (for us).

michaelryanmcneill commented 6 years ago

Ah, thanks for the follow-up. Good to know that this only impacts some users with special configurations, glad we were able to push out a fix quickly.