Repository navigation
Email comparisons should be case insensitive #39
Description
Centralized IT may have shibbolized email addresses differently than what a departmental web admin may have for his/her local users, even though they're both technically correct. One entity may use mixed case versus another. Comparisons of email addresses should be case-insensitive when a user tries to manually link his/her account with Shibboleth.
diff -Naur public_html/wp-content/plugins/shibboleth/options-user.php public_html_modified/wp-content/plugins/shibboleth/options-user.php
--- public_html/wp-content/plugins/shibboleth/options-user.php 2018-03-12 14:39:51.000000000 -0600
+++ public_html_modified/wp-content/plugins/shibboleth/options-user.php 2018-03-13 16:40:15.386361724 -0600
@@ -218,7 +218,7 @@
wp_safe_redirect( get_edit_user_link() . '?shibboleth=failed' );
exit;
}
- } elseif ( $user->user_email == $email && $allowed === 'bypass' ) {
+ } elseif ( $user->user_email === $email && $allowed === 'bypass' ) {
update_user_meta( $user->ID, 'shibboleth_account', true );
wp_safe_redirect( get_edit_user_link() . '?shibboleth=linked' );
exit;
Activity
jrchamp commentedon Mar 14, 2018
It sounds like you are asking for the check to be switched to a case insensitive check. Looking at the code you provided, I want to note that the difference between
==and===is a type check, not a case insensitive check. Instead, you could trystrcasecmp( $user->user_email, $email ) === 0mrbrown8 commentedon Mar 15, 2018
I /am/ asking for a case insensitive check. My php-fu is not strong. I crudely went by the advise given here:
https://stackoverflow.com/questions/3333353/string-comparison-using-vs-strcmp
My rudimentary printf-style testing indicated to me that === would help where an IdP would format an address one way and a local webmaster would format other.
Whichever is the proper forward-thinking way of doing this is fine, but I am advocating to have this kind check put in.
Thanks in advance.
mrbrown8 commentedon Mar 15, 2018
Suggestions from here:
https://stackoverflow.com/questions/5473542/case-insensitive-string-comparison
indicate you could also call strtolower() on both variables before applying the == test.
[-]email comparisons should be case-INsensitive[/-][+]Email comparisons should be case insensitive[/+]michaelryanmcneill commentedon Mar 16, 2018
I worry about using
strcasecmp()because it has problems with multibyte characters which are allowed in emails now. Instead I think we should dostrtolower()on both variables and then apply a strict match (i.e.===). I think that it would be best to go through the code sometime and switch from loose match (i.e.==) to strict match (i.e.===), just to be safe, but that is for another day.jrchamp commentedon Mar 16, 2018
I had the same concerns. The question would be: what are other systems doing? Sure, email is supposed to be case-insensitive, but does that only apply to the English alphabet characters as in
strtolower()or does it extend to the full Unicode alphabetic set? That might encourage us to usemb_strtolower().As far as strict matches, I agree that everything should default to strict matches and loose matches should be the exception.
michaelryanmcneill commentedon May 15, 2018
After looking at the docs and the RFC for SMTP, I think using
mb_strtolower()is best, but we need to make sure to account for PHP which might not be compiled with --enable-mbstring. I believe most standard packages do compile with that flag, but I'd rather not introduce an incompatibility there. I'll work to get it done.Adding new case insensitive matching (#39)
Making email comparisons for manual account merging case insensitive
michaelryanmcneill commentedon May 16, 2018
So, after working on this more, I decided that because of portability issues, using
mb_strtolower()was more of a hassle than it was worth. I proceeded with usingstrtolower()which should address this use case.mrbrown8 commentedon May 16, 2018
Thank you both for your consideration.