r68501 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r68500‎ | r68501 | r68502 >
Date:03:04, 24 June 2010
Author:laner
Status:deferred (Comments)
Tags:
Comment:
* Fixed issue with single domains, and non-auto-authentication domains being non-operational due to security fix in r68436
* Fixed another issue with mail me a password not working properly
Modified paths:
  • /trunk/extensions/LdapAuthentication/LdapAuthentication.php (modified) (history)

Diff [purge]

Index: trunk/extensions/LdapAuthentication/LdapAuthentication.php
——@@ -39,7 +39,7 @@
4040 # Support is available at http://www.mediawiki.org/wiki/Extension_talk:LDAP_Authentication
4141 #
4242
43 -if ( !defined( 'MEDIAWIKI' ) ) exit;
 43+//if ( !defined( 'MEDIAWIKI' ) ) exit;
4444
4545 $wgLDAPDomainNames = array();
4646 $wgLDAPServerNames = array();
——@@ -94,7 +94,7 @@
9595 $wgExtensionCredits['other'][] = array(
9696 'path' => __FILE__,
9797 'name' => 'LDAP Authentication Plugin',
98 - 'version' => '1.2b',
 98+ 'version' => '1.2c',
9999 'author' => 'Ryan Lane',
100100 'descriptionmsg' => 'ldapauthentication-desc',
101101 'url' => 'http://www.mediawiki.org/wiki/Extension:LDAP_Authentication',
——@@ -456,7 +456,7 @@
457457 array_push( $tempDomArr, 'local' );
458458 }
459459
460 - if ( isset( $wgLDAPAutoAuthDomain ) ) {
 460+ if ( isset( $wgLDAPAutoAuthDomain ) && $wgLDAPAutoAuthDomain != "" ) {
461461 $this->printDebug( "Allowing auto-authentication login, removing the domain from the list.", NONSENSITIVE );
462462
463463 // There is no reason for people to log in directly to the wiki if the are using an
——@@ -919,7 +919,7 @@
920920
921921 $this->printDebug( "Entering strict.", NONSENSITIVE );
922922
923 - if ( $wgLDAPUseLocal || $wgLDAPMailPassword ) {
 923+ if ( $wgLDAPUseLocal || ( isset( $wgLDAPMailPassword[$_SESSION['wsDomain']] ) && $wgLDAPMailPassword[$_SESSION['wsDomain']] ) ) {
924924 $this->printDebug( "Returning false in strict().", NONSENSITIVE );
925925 return false;
926926 } else {
——@@ -1836,16 +1836,16 @@
18371837 $wgAuth->printDebug( "Entering AutoAuthSetup.", NONSENSITIVE );
18381838
18391839 // Set configuration options for backwards compatibility
1840 - if ( isset( $wgLDAPSSLUsername ) ) {
 1840+ if ( isset( $wgLDAPSSLUsername ) && $wgLDAPSSLUsername != "" ) {
18411841 $wgAuth->printDebug( 'Setting $wgLDAPAutoAuthUsername to $wgLDAPSSLUsername; please change your configuration to fix this deprecated configuration variable.', NONSENSITIVE );
18421842 $wgLDAPAutoAuthUsername = $wgLDAPSSLUsername;
18431843 }
1844 - if ( isset( $wgLDAPSmartcardDomain ) ) {
 1844+ if ( isset( $wgLDAPSmartcardDomain ) && $wgLDAPSmartcardDomain != "" ) {
18451845 $wgAuth->printDebug( 'Setting $wgLDAPAutoAuthDomain to $wgLDAPSmartcardDomain; please change your configuration to fix this deprecated configuration variable.', NONSENSITIVE );
18461846 $wgLDAPAutoAuthDomain = $wgLDAPSmartcardDomain;
18471847 }
18481848
1849 - if ( $wgLDAPAutoAuthUsername != null ) {
 1849+ if ( $wgLDAPAutoAuthUsername != "" ) {
18501850 $wgAuth->printDebug( "wgLDAPAutoAuthUsername is not null, adding hooks.", NONSENSITIVE );
18511851 if ( version_compare( $wgVersion, '1.14.0', '<' ) ) {
18521852 if ( version_compare( $wgVersion, '1.13.0', '<' ) ) {

Follow-up revisions

RevisionCommit summaryAuthorDate
r68550Fixed $wgLDAPAutoAuthUsername check, and uncommented the MEDIAWIKI check, per...laner03:44, 25 June 2010

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r68436Fix possible remote_global vulnerabilities, and fix for r68288laner00:41, 23 June 2010

Comments

#Comment by Nikerabbit (talk | contribs)   05:52, 24 June 2010 
+//if ( !defined( 'MEDIAWIKI' ) ) exit;

Intentional? 

if ( $wgLDAPAutoAuthUsername != "" ) {

Might fail if username consists of only zeros depending which data type PHP chooses.

#Comment by Nikerabbit (talk | contribs)   05:52, 24 June 2010 
+//if ( !defined( 'MEDIAWIKI' ) ) exit;

Intentional? 

if ( $wgLDAPAutoAuthUsername != "" ) {

Might fail if username consists of only zeros depending which data type PHP chooses.

#Comment by Nikerabbit (talk | contribs)   05:57, 24 June 2010

Is there a bug in Code Review? I'm pretty sure I didn't post this twice.

#Comment by Reedy (talk | contribs)   06:09, 24 June 2010

I've had a few double post, not a regular occurrence...

#Comment by Ryan lane (talk | contribs)   13:32, 24 June 2010

Ok. I need to stop coding at 11:00 at night. I commented that out when I was debugging something. It isn't actually needed, since calling the extension directly would have no affect, but I had it in for paranoia's sake anyway. I'll put it back in tonight.

As for the wgLDAPAutoAuthUsername check, what would be a better way of checking that? I was unaware that a zero could ever equal an empty string.

#Comment by Liangent (talk | contribs)   13:37, 24 June 2010

In my test, '0' != '' but 0 == ''. PHP 5.3.1-5

#Comment by Nikerabbit (talk | contribs)   13:39, 24 June 2010

I think !== is suitable comparison operator here.

#Comment by Ryan lane (talk | contribs)   13:39, 24 June 2010

Ok. Good to know. Will fix this tonight.

#Comment by Ryan lane (talk | contribs)   03:45, 25 June 2010

Fixed in 68550

#Comment by Ryan lane (talk | contribs)   03:45, 25 June 2010

Make that r68550

Status & tagging log