jenkins-bot has submitted this change and it was merged.

Change subject: Sanitize user auto creation in CentralAuth
......................................................................


Sanitize user auto creation in CentralAuth

Minor changes:
* Avoid double calls to functions
* Use the respecting User class function rather than a Title
  one to sanitize user names.

Most important change is that we use CentralAuthUser::getInstance
on the initial User object to make sure it is already set and sane.
With that change I hope to strangle bug 39996 silently.

Bug: 39996
Change-Id: Ie87319134d92fe6ecb9cf46cf3aba408724bd361
---
M CentralAuthHooks.php
M CentralAuthPlugin.php
M CentralAuthUser.php
3 files changed, 16 insertions(+), 15 deletions(-)

Approvals:
  CSteipp: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/CentralAuthHooks.php b/CentralAuthHooks.php
index 3733186..bb9b750 100644
--- a/CentralAuthHooks.php
+++ b/CentralAuthHooks.php
@@ -241,15 +241,19 @@
                }
 
                // Clean up username
-               $title = Title::makeTitleSafe( NS_USER, $userName );
-               if ( !$title ) {
+               $userName = User::getCanonicalName( $userName, 'valid' );
+               if ( !$userName ) {
                        wfDebug( __METHOD__ . ": invalid username\n" );
                        return true;
                }
-               $userName = $title->getText();
+               $user->setName( $userName );
 
                // Try the central user
-               $centralUser = new CentralAuthUser( $userName );
+               $centralUser = CentralAuthUser::getInstance( $user );
+               if ( !$centralUser->exists() ) {
+                       wfDebug( __METHOD__ . ": global account doesn't 
exist\n" );
+                       return true;
+               }
                if ( $centralUser->authenticateWithToken( $token ) != 'ok' ) {
                        wfDebug( __METHOD__ . ": token mismatch\n" );
                        return true;
@@ -273,7 +277,7 @@
 
                if ( !$localId ) {
                        // User does not exist locally, attempt to create it
-                       if ( !self::attemptAddUser( $user, $userName ) ) {
+                       if ( !self::attemptAddUser( $user ) ) {
                                // Can't create user, give up now
                                return true;
                        }
@@ -474,12 +478,13 @@
         * Attempt to add a user to the database
         * Does the required authentication checks and updates for auto-creation
         * @param $user User
-        * @param $userName string
         * @throws MWException
         * @return bool Success
         */
-       static function attemptAddUser( $user, $userName ) {
+       static function attemptAddUser( $user ) {
                global $wgAuth, $wgCentralAuthCreateOnView;
+
+               $userName = $user->getName();
                // Denied by configuration?
                if ( !$wgAuth->autoCreate() ) {
                        wfDebug( __METHOD__ . ": denied by configuration\n" );
@@ -519,7 +524,7 @@
                }
 
                // Check for validity of username
-               if ( !User::isValidUserName( $userName ) ) {
+               if ( !User::isCreatableName( $userName ) ) {
                        wfDebug( __METHOD__ . ": Invalid username\n" );
                        $session['auto-create-blacklist'][] = wfWikiID();
                        CentralAuthUser::setSession( $session );
@@ -576,7 +581,6 @@
                $user->addNewUserLogEntryAutoCreate();
 
                $wgAuth->initUser( $user, true );
-               $wgAuth->updateUser( $user );
 
                # Notify hooks (e.g. Newuserlog)
                wfRunHooks( 'AuthPluginAutoCreate', array( $user ) );
diff --git a/CentralAuthPlugin.php b/CentralAuthPlugin.php
index 2790d15..4cb77d5 100644
--- a/CentralAuthPlugin.php
+++ b/CentralAuthPlugin.php
@@ -206,7 +206,6 @@
                return true;
        }
 
-
        /**
         * Return true to prevent logins that don't authenticate here from being
         * checked against the local database's password fields.
diff --git a/CentralAuthUser.php b/CentralAuthUser.php
index e776703..daf6ef5 100644
--- a/CentralAuthUser.php
+++ b/CentralAuthUser.php
@@ -1354,7 +1354,8 @@
                                'lu_attached_timestamp' => $dbw->timestamp(),
                                'lu_attached_method'    => $method ),
                        __METHOD__,
-                       array( 'IGNORE' ) );
+                       array( 'IGNORE' )
+               );
 
                if ( $dbw->affectedRows() == 0 ) {
                        wfDebugLog( 'CentralAuth',
@@ -2222,11 +2223,8 @@
                if ( !$this->mDelayInvalidation ) {
                        wfDebugLog( 'CentralAuth', "Updating cache for global 
user {$this->mName}" );
 
-                       // Reload the state
+                       // Reload the state and overwrite the cache.
                        $this->loadStateNoCache();
-
-                       // Overwrite the cache.
-                       $this->saveToCache();
                } else {
                        wfDebugLog( 'CentralAuth', "Deferring cache 
invalidation because we're in a transaction" );
                }

-- 
To view, visit https://gerrit.wikimedia.org/r/57671
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ie87319134d92fe6ecb9cf46cf3aba408724bd361
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/CentralAuth
Gerrit-Branch: master
Gerrit-Owner: Hoo man <h...@online.de>
Gerrit-Reviewer: Anomie <bjor...@wikimedia.org>
Gerrit-Reviewer: CSteipp <cste...@wikimedia.org>
Gerrit-Reviewer: Hoo man <h...@online.de>
Gerrit-Reviewer: Tim Starling <tstarl...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to