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