jenkins-bot has submitted this change and it was merged.
Change subject: Cleanup localuser records when wiki is missing data
......................................................................
Cleanup localuser records when wiki is missing data
Under some (undefined) circumstances the localuser table can record that
a user has been attached to a wiki but the wiki's own db is missing
a record for the user. There is a maintenance script to find and cleanup
these dangling records, but that's not of much help for a user who is
effected as they will not be able to authenticate to any wiki until the
next time someone runs the script.
Add logic to catch this problem in the most likely places and proactively
clean up the dangling attachment record. The cleanup is done by running
a job which cleans up the record in a safe manner and clears caches as
appropriate. Using a job makes this cleanup safe for GET requests.
The fact that the error was encountered is logged as this is not
a desired state. The logging uses the PSR-3 logger directly so that
structured log data and a stack trace can be attached.
Bug: T119736
Change-Id: I3d219cfff0dc835faa72d5fe72a80e57235dcb04
(cherry picked from commit a24ca11e277afd8b5259d44b2d645d4dbb99502f)
---
M extension.json
M includes/CentralAuthHooks.php
A includes/CentralAuthUnattachUserJob.php
M includes/CentralAuthUser.php
A includes/LocalUserNotFoundException.php
5 files changed, 147 insertions(+), 34 deletions(-)
Approvals:
Gergő Tisza: Looks good to me, approved
jenkins-bot: Verified
diff --git a/extension.json b/extension.json
index b60bf2b..0ab0911 100644
--- a/extension.json
+++ b/extension.json
@@ -70,7 +70,8 @@
"LocalRenameUserJob": "LocalRenameUserJob",
"LocalUserMergeJob": "LocalUserMergeJob",
"LocalPageMoveJob": "LocalPageMoveJob",
- "CentralAuthCreateLocalAccountJob":
"CentralAuthCreateLocalAccountJob"
+ "CentralAuthCreateLocalAccountJob":
"CentralAuthCreateLocalAccountJob",
+ "CentralAuthUnattachUserJob": "CentralAuthUnattachUserJob"
},
"LogTypes": [
"globalauth",
@@ -153,7 +154,9 @@
"CentralAuthHooks": "includes/CentralAuthHooks.php",
"CentralAuthPreAuthManagerHooks":
"includes/CentralAuthPreAuthManagerHooks.php",
"CentralAuthSuppressUserJob": "includes/SuppressUserJob.php",
+ "CentralAuthUnattachUserJob":
"includes/CentralAuthUnattachUserJob.php",
"CentralAuthCreateLocalAccountJob":
"includes/CreateLocalAccountJob.php",
+ "LocalUserNotFoundException":
"includes/LocalUserNotFoundException.php",
"WikiSet": "includes/WikiSet.php",
"SpecialCentralAutoLogin":
"includes/specials/SpecialCentralAutoLogin.php",
"CentralAuthUserArray": "includes/CentralAuthUserArray.php",
diff --git a/includes/CentralAuthHooks.php b/includes/CentralAuthHooks.php
index ba9f293..dd1e111 100644
--- a/includes/CentralAuthHooks.php
+++ b/includes/CentralAuthHooks.php
@@ -1355,33 +1355,10 @@
$central = CentralAuthUser::getInstance( $user );
if ( $central->exists() ) {
- try {
- $localPolicyGroups = array_intersect(
- array_keys(
$wgCentralAuthGlobalPasswordPolicies ),
- $central->getLocalGroups()
- );
- } catch ( Exception $e ) {
- // T104615 - race condition in attaching user
and creating local
- // wiki account can cause this Exception from
- // CentralAuthUser::localUserData. Allow the
password for now, and
- // we'll catch them next login if their
password isn't valid.
- // And T119736 - if localuser table gets out of
sync, don't
- // deny logins
- if ( substr( $e->getMessage(), 0 , 34 )
- === 'Could not find local user data for'
- ) {
- wfDebugLog(
- 'CentralAuth',
- sprintf( 'Bug T104615 hit for
%s@%s',
- $user->getName(),
- wfWikiId()
- )
- );
- return true;
- }
-
- throw $e;
- }
+ $localPolicyGroups = array_intersect(
+ array_keys(
$wgCentralAuthGlobalPasswordPolicies ),
+ $central->getLocalGroups()
+ );
$effectivePolicy =
UserPasswordPolicy::getPoliciesForGroups(
$wgCentralAuthGlobalPasswordPolicies,
diff --git a/includes/CentralAuthUnattachUserJob.php
b/includes/CentralAuthUnattachUserJob.php
new file mode 100644
index 0000000..49609a0
--- /dev/null
+++ b/includes/CentralAuthUnattachUserJob.php
@@ -0,0 +1,61 @@
+<?php
+/**
+ * @section LICENSE
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+/**
+ * A job to unattach a user.
+ *
+ * @copyright © 2016 Wikimedia Foundation and contributors.
+ */
+class CentralAuthUnattachUserJob extends Job {
+
+ /**
+ * Constructor
+ *
+ * @param Title $title Associated title
+ * @param array $params Job parameters
+ */
+ public function __construct( $title, $params ) {
+ parent::__construct( 'CentralAuthUnattachUserJob', $title,
$params );
+ $this->removeDuplicates = true;
+ }
+
+ /**
+ * Execute the job
+ *
+ * @return bool
+ */
+ public function run() {
+ $username = $this->params['username'];
+ $wiki = $this->params['wiki'];
+ $user = User::newFromName( $username );
+ if ( $user->getId() !== 0 ) {
+ // User has been created since this job was queued.
+ // Races are fun!
+ return true;
+ }
+ $causer = CentralAuthUser::getMasterInstanceByName( $username );
+ $causer->removeLocalName( $wiki );
+ if ( $causer->exists() ) {
+ $causer->adminUnattach( array( $wiki ) );
+ }
+ return true;
+ }
+}
diff --git a/includes/CentralAuthUser.php b/includes/CentralAuthUser.php
index aa635ac..ac65e8a 100644
--- a/includes/CentralAuthUser.php
+++ b/includes/CentralAuthUser.php
@@ -10,6 +10,8 @@
*/
+use MediaWiki\Logger\LoggerFactory;
+
class CentralAuthUser extends AuthPluginUser implements IDBAccessObject {
/** Cache of loaded CentralAuthUsers */
private static $loadedUsers = null;
@@ -1391,6 +1393,23 @@
}
/**
+ * Queue a job to unattach this user from a named wiki.
+ *
+ * @param string $wikiId
+ */
+ protected function queueAdminUnattachJob( $wikiId ) {
+ $job = Job::factory(
+ 'CentralAuthUnattachUserJob',
+ Title::makeTitleSafe( NS_USER, $this->getName() ),
+ array(
+ 'username' => $this->getName(),
+ 'wiki' => $wikiId,
+ )
+ );
+ JobQueueGroup::singleton( $wikiId )->push( $job );
+ }
+
+ /**
* Delete a global account and log what happened
*
* @param $reason string Reason for the deletion
@@ -2236,8 +2255,17 @@
$wikis = $this->queryAttachedBasic();
foreach ( $wikis as $wikiId => $_ ) {
- $localUser = $this->localUserData( $wikiId );
- $wikis[$wikiId] = array_merge( $wikis[$wikiId],
$localUser );
+ try {
+ $localUser = $this->localUserData( $wikiId );
+ $wikis[$wikiId] = array_merge( $wikis[$wikiId],
$localUser );
+ } catch ( LocalUserNotFoundException $e ) {
+ // T119736: localuser table told us that the
user was attached
+ // from $wikiId but there is no data in the
master or slaves
+ // that corroborates that.
+ unset( $wikis[$wikiId] );
+ // Queue a job to delete the bogus attachment
record.
+ $this->queueAdminUnattachJob( $wikiId );
+ }
}
$this->mAttachedInfo = $wikis;
@@ -2298,8 +2326,16 @@
$items = array();
foreach ( $wikiIDs as $wikiID ) {
- $data = $this->localUserData( $wikiID );
- $items[$wikiID] = $data;
+ try {
+ $data = $this->localUserData( $wikiID );
+ $items[$wikiID] = $data;
+ } catch ( LocalUserNotFoundException $e ) {
+ // T119736: localnames table told us that the
name was
+ // unattached on $wikiId but there is no data
in the master
+ // or slaves that corroborates that.
+ // Queue a job to delete the bogus record.
+ $this->queueAdminUnattachJob( $wikiID );
+ }
}
return $items;
@@ -2311,7 +2347,7 @@
* Returns most data in the user and ipblocks tables, user groups, and
editcount.
*
* @param $wikiID String
- * @throws Exception if local user not found
+ * @throws LocalUserNotFoundException if local user not found
* @return array
*/
protected function localUserData( $wikiID ) {
@@ -2335,7 +2371,17 @@
}
if ( !$row ) {
$lb->reuseConnection( $db );
- throw new Exception( "Could not find local user data
for {$this->mName}@{$wikiID}" );
+ $ex = new LocalUserNotFoundException(
+ "Could not find local user data for
{$this->mName}@{$wikiID}" );
+ LoggerFactory::getInstance( 'CentralAuth' )->warning(
+ 'Could not find local user data for
{username}@{wikiId}',
+ array(
+ 'username' => $this->mName,
+ 'wikiId' => $wikiID,
+ 'exception' => $ex,
+ )
+ );
+ throw $ex;
}
/** @var $row object */
diff --git a/includes/LocalUserNotFoundException.php
b/includes/LocalUserNotFoundException.php
new file mode 100644
index 0000000..5f804f3
--- /dev/null
+++ b/includes/LocalUserNotFoundException.php
@@ -0,0 +1,26 @@
+<?php
+/**
+ * @section LICENSE
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+/**
+ * @copyright © 2016 Wikimedia Foundation and contributors
+ */
+class LocalUserNotFoundException extends Exception {
+}
--
To view, visit https://gerrit.wikimedia.org/r/299896
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I3d219cfff0dc835faa72d5fe72a80e57235dcb04
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/CentralAuth
Gerrit-Branch: REL1_27
Gerrit-Owner: Gergő Tisza <[email protected]>
Gerrit-Reviewer: BryanDavis <[email protected]>
Gerrit-Reviewer: Gergő Tisza <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
