CSteipp has submitted this change and it was merged.
Change subject: Funnel all central ID <=> name/User lookups through the utils
methods
......................................................................
Funnel all central ID <=> name/User lookups through the utils methods
* Also added a MWOAuthUtils::getCentralUserNameFromId() method (the hook
supports batching)
Change-Id: Ic274ff3d65ecfa9d602fb92b8998276ce722f5b1
---
M backend/MWOAuthServer.php
M backend/MWOAuthUtils.php
M control/MWOAuthConsumerAcceptanceSubmitControl.php
M control/MWOAuthConsumerSubmitControl.php
M frontend/specialpages/SpecialMWOAuth.php
M frontend/specialpages/SpecialMWOAuthConsumerRegistration.php
M frontend/specialpages/SpecialMWOAuthManageConsumers.php
M frontend/specialpages/SpecialMWOAuthManageMyGrants.php
8 files changed, 59 insertions(+), 28 deletions(-)
Approvals:
CSteipp: Verified; Looks good to me, approved
jenkins-bot: Verified
diff --git a/backend/MWOAuthServer.php b/backend/MWOAuthServer.php
index 063a14b..fe4e211 100644
--- a/backend/MWOAuthServer.php
+++ b/backend/MWOAuthServer.php
@@ -141,8 +141,8 @@
$requestToken->addVerifyCode( $verifyCode );
// CentralAuth may abort here if there is no global account for
this user
- $userId = MWOAuthUtils::getCentralIdFromLocalUser( $mwUser );
- if ( !$userId ) {
+ $centralUserId = MWOAuthUtils::getCentralIdFromLocalUser(
$mwUser );
+ if ( !$centralUserId ) {
throw new MWOAuthException(
'mwoauthserver-invalid-user' );
}
@@ -170,7 +170,7 @@
$cmra = MWOAuthConsumerAcceptance::newFromArray( array(
'id' => null,
'wiki' => $consumer->get( 'wiki' ),
- 'userId' => $userId,
+ 'userId' => $centralUserId,
'consumerId' => $consumer->get( 'id' ),
'accessToken' => $accessToken->key,
'accessSecret' => $accessToken->secret,
diff --git a/backend/MWOAuthUtils.php b/backend/MWOAuthUtils.php
index 97ac139..1653b28 100644
--- a/backend/MWOAuthUtils.php
+++ b/backend/MWOAuthUtils.php
@@ -237,8 +237,30 @@
* Given a central wiki user ID, get a local User object
*
* @param integer $userId
+ * @return string|bool User name or false if not found
+ */
+ public static function getCentralUserNameFromId( $userId ) {
+ global $wgMWOAuthCentralWiki;
+
+ if ( MWOAuthUtils::isCentralWiki() ) {
+ $name = User::whoIs( $userId );
+ } else {
+ $namesById = array( $userId => false );
+ // Let extensions check that central wiki user ID is
attached to a global account
+ // and that return the user on this wiki that is
attached to that global account
+ wfRunHooks( 'OAuthGetUserNamesFromCentralIds',
+ array( $wgMWOAuthCentralWiki, &$namesById ) );
+ $name = $namesById[$userId];
+ }
+
+ return $name;
+ }
+
+ /**
+ * Given a central wiki user ID, get a local User object
+ *
+ * @param integer $userId
* @return User|bool User or false if not found
- * @throws MWOAuthException
*/
public static function getLocalUserFromCentralId( $userId ) {
global $wgMWOAuthCentralWiki;
@@ -261,7 +283,6 @@
*
* @param User $user
* @return integer|bool ID or false if not found
- * @throws MWOAuthException
*/
public static function getCentralIdFromLocalUser( User $user ) {
global $wgMWOAuthCentralWiki;
@@ -280,10 +301,6 @@
// Process cache the result to avoid queries
$user->oAuthUserData['centralId'] = $id;
}
- }
-
- if ( !$id ) {
- throw new MWOAuthException(
'mwoauthserver-invalid-user' );
}
return $id;
diff --git a/control/MWOAuthConsumerAcceptanceSubmitControl.php
b/control/MWOAuthConsumerAcceptanceSubmitControl.php
index 8a1d01f..c5baa06 100644
--- a/control/MWOAuthConsumerAcceptanceSubmitControl.php
+++ b/control/MWOAuthConsumerAcceptanceSubmitControl.php
@@ -84,6 +84,11 @@
$user = $this->getUser(); // proposer or admin
$dbw = $this->dbw; // convenience
+ $centralUserId = MWOAuthUtils::getCentralIdFromLocalUser( $user
);
+ if ( !$centralUserId ) { // sanity
+ return $this->failure( 'permission_denied',
'badaccess-group0' );
+ }
+
switch ( $action ) {
/*
case 'accept': // @TODO: unused (WIP)
@@ -102,7 +107,7 @@
// WIP: PUT THIS IN authorize()
$cmra = MWOAuthConsumerAcceptance::newFromArray( array(
'wiki' => $this->vals['wiki'],
- 'userId' => $this->getUser()->getId(),
+ 'userId' => $centralUserId,
'consumerId' => $cmr->getId(),
'accessToken' => ...,
'accessSecret' => ...,
@@ -116,7 +121,7 @@
$cmra = MWOAuthConsumerAcceptance::newFromId( $dbw,
$this->vals['acceptanceId'] );
if ( !$cmra ) {
return $this->failure( 'invalid_access_token',
'mwoauth-invalid-access-token' );
- } elseif ( $cmra->get( 'userId' ) !==
$this->getUser()->getId() ) {
+ } elseif ( $cmra->get( 'userId' ) !== $centralUserId ) {
return $this->failure( 'invalid_access_token',
'mwoauth-invalid-access-token' );
}
$cmr = MWOAuthConsumer::newFromId( $dbw, $cmra->get(
'consumerId' ) );
@@ -133,7 +138,7 @@
$cmra = MWOAuthConsumerAcceptance::newFromId( $dbw,
$this->vals['acceptanceId'] );
if ( !$cmra ) {
return $this->failure( 'invalid_access_token',
'mwoauth-invalid-access-token' );
- } elseif ( $cmra->get( 'userId' ) !==
$this->getUser()->getId() ) {
+ } elseif ( $cmra->get( 'userId' ) !== $centralUserId ) {
return $this->failure( 'invalid_access_token',
'mwoauth-invalid-access-token' );
}
diff --git a/control/MWOAuthConsumerSubmitControl.php
b/control/MWOAuthConsumerSubmitControl.php
index cfe212a..b7dbe9d 100644
--- a/control/MWOAuthConsumerSubmitControl.php
+++ b/control/MWOAuthConsumerSubmitControl.php
@@ -118,6 +118,11 @@
$user = $this->getUser(); // proposer or admin
$dbw = $this->dbw; // convenience
+ $centralUserId = MWOAuthUtils::getCentralIdFromLocalUser( $user
);
+ if ( !$centralUserId ) { // sanity
+ return $this->failure( 'permission_denied',
'badaccess-group0' );
+ }
+
switch ( $action ) {
case 'propose':
if ( !$user->isAllowed( 'mwoauthproposeconsumer' ) ) {
@@ -130,7 +135,7 @@
}
if ( MWOAuthConsumer::newFromNameVersionUser(
- $dbw, $this->vals['name'],
$this->vals['version'], $user->getId() ) )
+ $dbw, $this->vals['name'],
$this->vals['version'], $centralUserId ) )
{
return $this->failure( 'consumer_exists',
'mwoauth-consumer-alreadyexists' );
}
@@ -140,7 +145,7 @@
array(
'id' => null, //
auto-increment
'consumerKey' =>
MWCryptRand::generateHex( 32 ),
- 'userId' => $user->getId(),
+ 'userId' => $centralUserId,
'email' =>
$user->getEmail(),
'emailAuthenticated' => $now, // see
above
'secretKey' =>
MWCryptRand::generateHex( 32 ),
@@ -173,7 +178,7 @@
$cmr = MWOAuthConsumer::newFromKey( $dbw,
$this->vals['consumerKey'] );
if ( !$cmr ) {
return $this->failure( 'invalid_consumer_key',
'mwoauth-invalid-consumer-key' );
- } elseif ( $cmr->get( 'userId' ) !== $user->getId() ) {
+ } elseif ( $cmr->get( 'userId' ) !== $centralUserId ) {
return $this->failure( 'permission_denied',
'badaccess-group0' );
} elseif ( $cmr->get( 'stage' ) !==
MWOAuthConsumer::STAGE_APPROVED
&& $cmr->get( 'stage' ) !==
MWOAuthConsumer::STAGE_PROPOSED )
@@ -374,7 +379,7 @@
* @return Title
*/
protected function getLogTitle( DBConnRef $db, $userId ) {
- $name = $db->selectField( 'user', 'user_name', array( 'user_id'
=> $userId ) );
+ $name = MWOAuthUtils::getCentralUserNameFromId( $userId );
return Title::makeTitleSafe( NS_USER, $name );
}
}
diff --git a/frontend/specialpages/SpecialMWOAuth.php
b/frontend/specialpages/SpecialMWOAuth.php
index e739dfd..beb378a 100644
--- a/frontend/specialpages/SpecialMWOAuth.php
+++ b/frontend/specialpages/SpecialMWOAuth.php
@@ -88,7 +88,8 @@
'grants' =>
$consumer->get( 'grants' ),
'existing' => $existing,
'description' => array (
- 'user' =>
User::newFromId( $consumer->get( 'userId') )->getName(),
+ 'user' =>
MWOAuthUtils::getCentralUserNameFromId(
+
$consumer->get( 'userId' ) ),
'name' =>
$consumer->get( 'name'),
'version' =>
$consumer->get( 'version'),
'description'
=> $consumer->get( 'description'),
diff --git a/frontend/specialpages/SpecialMWOAuthConsumerRegistration.php
b/frontend/specialpages/SpecialMWOAuthConsumerRegistration.php
index 14fabf5..9107886 100644
--- a/frontend/specialpages/SpecialMWOAuthConsumerRegistration.php
+++ b/frontend/specialpages/SpecialMWOAuthConsumerRegistration.php
@@ -48,6 +48,7 @@
$user = $this->getUser();
$request = $this->getRequest();
$lang = $this->getLanguage();
+ $centralUserId = MWOAuthUtils::getCentralIdFromLocalUser( $user
);
$block = $user->getBlock();
if ( $block ) {
@@ -183,7 +184,7 @@
break;
} elseif ( $cmr->get( 'deleted' ) && !$user->isAllowed(
'mwoauthviewsuppressed' ) ) {
throw new PermissionsError(
'mwoauthviewsuppressed' );
- } elseif ( $cmr->get( 'userId' ) !== $user->getId() ) {
+ } elseif ( $cmr->get( 'userId' ) !== $centralUserId ) {
// Do not show private information to other
users
$this->getOutput()->addWikiMsg(
'mwoauth-invalid-consumer-key' );
break;
@@ -282,7 +283,7 @@
}
break;
case 'list':
- $pager = new MWOAuthListMyConsumersPager( $this,
array(), $this->getUser() );
+ $pager = new MWOAuthListMyConsumersPager( $this,
array(), $centralUserId );
if ( $pager->getNumRows() ) {
$this->getOutput()->addHTML(
$pager->getNavigationBar() );
$this->getOutput()->addHTML( $pager->getBody()
);
@@ -417,10 +418,10 @@
class MWOAuthListMyConsumersPager extends ReverseChronologicalPager {
public $mForm, $mConds;
- function __construct( $form, $conds, $user ) {
+ function __construct( $form, $conds, $centralUserId ) {
$this->mForm = $form;
$this->mConds = $conds;
- $this->mConds['oarc_user_id'] = $user instanceof $user ?
$user->getId() : $user;
+ $this->mConds['oarc_user_id'] = $centralUserId;
if ( !$this->getUser()->isAllowed( 'mwoauthviewsuppressed' ) ) {
$this->mConds['oarc_deleted'] = 0;
}
diff --git a/frontend/specialpages/SpecialMWOAuthManageConsumers.php
b/frontend/specialpages/SpecialMWOAuthManageConsumers.php
index 048e4f6..4269e8a 100644
--- a/frontend/specialpages/SpecialMWOAuthManageConsumers.php
+++ b/frontend/specialpages/SpecialMWOAuthManageConsumers.php
@@ -256,7 +256,7 @@
'user' => array(
'type' => 'info',
'label-message' =>
'mwoauth-consumer-user',
- 'default' => $cmr->get( 'userId',
'User::whoIs' )
+ 'default' => $cmr->get( 'userId',
'MWOAuthUtils::getCentralUserNameFromId' )
),
'description' => array(
'type' => 'info',
@@ -463,7 +463,7 @@
return $s . ' [' . $cmr->get( 'version'
) . ']'; } )
),
'mwoauthmanageconsumers-user' => htmlspecialchars(
- $cmr->get( 'userId', 'User::whoIs' )
+ $cmr->get( 'userId',
'MWOAuthUtils::getCentralUserNameFromId' )
),
'mwoauthmanageconsumers-description' =>
htmlspecialchars(
$cmr->get( 'description', function( $s ) use (
$lang ) {
diff --git a/frontend/specialpages/SpecialMWOAuthManageMyGrants.php
b/frontend/specialpages/SpecialMWOAuthManageMyGrants.php
index cad151e..35ac272 100644
--- a/frontend/specialpages/SpecialMWOAuthManageMyGrants.php
+++ b/frontend/specialpages/SpecialMWOAuthManageMyGrants.php
@@ -126,7 +126,7 @@
'user' => array(
'type' => 'info',
'label-message' =>
'mwoauth-consumer-user',
- 'default' => $cmr->get( 'userId',
'User::whoIs' )
+ 'default' => $cmr->get( 'userId',
'MWOAuthUtils::getCentralUserNameFromId' )
),
'version' => array(
'type' => 'info',
@@ -227,7 +227,8 @@
* @return void
*/
protected function showConsumerList() {
- $pager = new MWOAuthManageMyGrantsPager( $this, array(),
$this->getUser()->getId() );
+ $centralUserId = MWOAuthUtils::getCentralIdFromLocalUser(
$this->getUser() );
+ $pager = new MWOAuthManageMyGrantsPager( $this, array(),
$centralUserId );
if ( $pager->getNumRows() ) {
$this->getOutput()->addHTML( $pager->getNavigationBar()
);
$this->getOutput()->addHTML( $pager->getBody() );
@@ -269,7 +270,8 @@
'mwoauthmanagemygrants-name' =>
$cmr->get( 'name', function( $s ) use ( $cmr ) {
return $s . ' [' . $cmr->get( 'version'
) . ']'; } ),
- 'mwoauthmanagemygrants-user' => $cmr->get( 'userId',
array( 'User', 'whoIs' ) ),
+ 'mwoauthmanagemygrants-user' => $cmr->get( 'userId',
+ 'MWOAuthUtils::getCentralUserNameFromId' ),
'mwoauthmanagemygrants-description' =>
$cmr->get( 'description', function( $s ) use (
$lang ) {
return $lang->truncate( $s, 10024 ); }
),
@@ -305,11 +307,11 @@
class MWOAuthManageMyGrantsPager extends ReverseChronologicalPager {
public $mForm, $mConds;
- function __construct( $form, $conds, $user ) {
+ function __construct( $form, $conds, $centralUserId ) {
$this->mForm = $form;
$this->mConds = $conds;
$this->mConds[] = 'oaac_consumer_id = oarc_id';
- $this->mConds['oaac_user_id'] = $user;
+ $this->mConds['oaac_user_id'] = $centralUserId;
if ( !$this->getUser()->isAllowed( 'mwoauthviewsuppressed' ) ) {
$this->mConds['oarc_deleted'] = 0;
}
--
To view, visit https://gerrit.wikimedia.org/r/75773
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic274ff3d65ecfa9d602fb92b8998276ce722f5b1
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/OAuth
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Anomie <[email protected]>
Gerrit-Reviewer: CSteipp <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
