Aaron Schulz has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/74087


Change subject: Made MWOAuthManageMyGrantsPager page on ID instead of access 
token
......................................................................

Made MWOAuthManageMyGrantsPager page on ID instead of access token

* This adds an ID column to the consumer acceptance table
* The consumer access token is no longer exposed anywhere (just for sanity)
* Also fixed typo in Pager classes (mDB => mDb) to unbreak them

Change-Id: I91d2e03f399c1f4c96c8a13d08d2aa391eefe04a
---
M backend/MWOAuthConsumer.php
M backend/MWOAuthConsumerAcceptance.php
M backend/MWOAuthServer.php
M backend/schema/mysql/OAuth.sql
M control/MWOAuthConsumerAcceptanceSubmitControl.php
M frontend/specialpages/SpecialMWOAuthConsumerRegistration.php
M frontend/specialpages/SpecialMWOAuthManageConsumers.php
M frontend/specialpages/SpecialMWOAuthManageMyGrants.php
8 files changed, 33 insertions(+), 32 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/OAuth 
refs/changes/87/74087/1

diff --git a/backend/MWOAuthConsumer.php b/backend/MWOAuthConsumer.php
index e36b288..630f5fc 100644
--- a/backend/MWOAuthConsumer.php
+++ b/backend/MWOAuthConsumer.php
@@ -22,7 +22,7 @@
  * Representation of an OAuth consumer request
  */
 class MWOAuthConsumer extends MWOAuthDAO {
-       /** @var integer */
+       /** @var integer Unique ID */
        protected $id;
        /** @var string Hex token */
        protected $consumerKey;
@@ -42,7 +42,7 @@
        protected $wiki;
        /** @var string TS_MW timestamp of proposal */
        protected $registration;
-       /** @var string Secret hmac key */
+       /** @var string Secret HMAC key */
        protected $secretKey;
        /** @var string Public RSA key */
        protected $rsaKey;
diff --git a/backend/MWOAuthConsumerAcceptance.php 
b/backend/MWOAuthConsumerAcceptance.php
index f370ef0..3d3b5f4 100644
--- a/backend/MWOAuthConsumerAcceptance.php
+++ b/backend/MWOAuthConsumerAcceptance.php
@@ -22,6 +22,8 @@
  * Representation of an OAuth consumer acceptance
  */
 class MWOAuthConsumerAcceptance extends MWOAuthDAO {
+       /** @var integer Unique ID */
+       protected $id;
        /** @var string Wiki ID the application can be used on (or "*" for all) 
*/
        protected $wiki;
        /** @var integer Publisher user ID (on central wiki) */
@@ -30,7 +32,7 @@
        protected $consumerId;
        /** @var string Hex token */
        protected $accessToken;
-       /** @var string Hex token */
+       /** @var string Secret HMAC key */
        protected $secretToken;
        /** @var array List of grants */
        protected $grants;
@@ -41,6 +43,7 @@
                return array(
                        'table'          => 'oauth_accepted_consumer',
                        'fieldColumnMap' => array(
+                               'id'           => 'oaac_id',
                                'wiki'         => 'oaac_wiki',
                                'userId'       => 'oaac_user_id',
                                'consumerId'   => 'oaac_consumer_id',
@@ -49,7 +52,8 @@
                                'grants'       => 'oaac_grants',
                                'accepted'     => 'oaac_accepted'
                        ),
-                       'idField'        => 'accessToken',
+                       'idField'        => 'id',
+                       'autoIncrField'  => 'id',
                );
        }
 
diff --git a/backend/MWOAuthServer.php b/backend/MWOAuthServer.php
index 73ed605..b8af02a 100644
--- a/backend/MWOAuthServer.php
+++ b/backend/MWOAuthServer.php
@@ -110,7 +110,7 @@
                // Generate and Update the tokens:
                // * Generate Access token, and add a pointer to it in the 
request token
                // * Generate a new Verification code, and add it to the 
request token
-               // * Resave Request token with 
+               // * Resave Request token with
                $accessToken = MWOAuthDataStore::newToken();
                $verifyCode = MWCryptRand::generateHex( 32, true);
                $requestToken = $this->data_store->lookup_token( $consumer, 
'request', $requestTokenKey );
@@ -127,6 +127,7 @@
 
                // Add the Authorization to the database
                $cmra = MWOAuthConsumerAcceptance::newFromArray( array(
+                       'id'           => null,
                        'wiki'         => $consumer->get( 'wiki' ),
                        'userId'       => $mwUser->getId(),
                        'consumerId'   => $consumer->get( 'id' ),
diff --git a/backend/schema/mysql/OAuth.sql b/backend/schema/mysql/OAuth.sql
index 3bf1197..f5bb205 100644
--- a/backend/schema/mysql/OAuth.sql
+++ b/backend/schema/mysql/OAuth.sql
@@ -56,6 +56,7 @@
 
 -- Grant approvals by users for consumers
 CREATE TABLE IF NOT EXISTS /*_*/oauth_accepted_consumer (
+    oaac_id integer unsigned NOT NULL auto_increment PRIMARY KEY,
     -- The name of a wiki or "*"
     oaac_wiki varchar(255) binary NOT NULL,
     -- Key to the user who approved the consumer (on the central wiki)
@@ -65,7 +66,7 @@
     -- Tokens for the consumer to act on behave of the user
     oaac_access_token varbinary(32) NOT NULL,
     oaac_access_secret varbinary(32) NOT NULL,
-    --- JSON blob of actually accepted grants
+    -- JSON blob of actually accepted grants
     oaac_grants blob NOT NULL,
     -- Timestamp of grant approval by the user
     oaac_accepted varbinary(14) NOT NULL
@@ -77,3 +78,4 @@
     ON /*_*/oauth_accepted_consumer (oaac_user_id,oaac_consumer_id,oaac_wiki);
 CREATE INDEX /*i*/oaac_consumer_user
     ON /*_*/oauth_accepted_consumer (oaac_consumer_id,oaac_user_id);
+CREATE INDEX /*i*/oaac_user_id ON /*_*/oauth_accepted_consumer 
(oaac_user_id,oaac_id);
diff --git a/control/MWOAuthConsumerAcceptanceSubmitControl.php 
b/control/MWOAuthConsumerAcceptanceSubmitControl.php
index 435acd9..c952b93 100644
--- a/control/MWOAuthConsumerAcceptanceSubmitControl.php
+++ b/control/MWOAuthConsumerAcceptanceSubmitControl.php
@@ -38,7 +38,7 @@
                                }
                        ),
                        'update'   => array(
-                               'accessToken' => '/^[0-9a-f]{32}$/',
+                               'acceptanceId' => '/^\d+$/',
                                'wiki'        => function( $s ) {
                                        return WikiMap::getWiki( $s ) || $s === 
'*'; },
                                'grants'      => function( $s ) {
@@ -47,7 +47,7 @@
                                }
                        ),
                        'renounce' => array(
-                               'accessToken' => '/^[0-9a-f]{32}$/',
+                               'acceptanceId' => '/^\d+$/',
                        ),
                );
        }
@@ -104,7 +104,7 @@
                        $cmra->save();
                */
                case 'update':
-                       $cmra = MWOAuthConsumerAcceptance::newFromToken( $dbw, 
$this->vals['accessToken'] );
+                       $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() ) {
@@ -121,7 +121,7 @@
 
                        return $this->success( $cmra );
                case 'renounce':
-                       $cmra = MWOAuthConsumerAcceptance::newFromToken( $dbw, 
$this->vals['accessToken'] );
+                       $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() ) {
diff --git a/frontend/specialpages/SpecialMWOAuthConsumerRegistration.php 
b/frontend/specialpages/SpecialMWOAuthConsumerRegistration.php
index f9efda2..f14bea1 100644
--- a/frontend/specialpages/SpecialMWOAuthConsumerRegistration.php
+++ b/frontend/specialpages/SpecialMWOAuthConsumerRegistration.php
@@ -390,7 +390,7 @@
                        $this->mConds['oarc_deleted'] = 0;
                }
 
-               $this->mDB = MWOAuthUtils::getCentralDB( DB_SLAVE );
+               $this->mDb = MWOAuthUtils::getCentralDB( DB_SLAVE );
                parent::__construct();
 
                # Treat 20 as the default limit, since each entry takes up 5 
rows.
diff --git a/frontend/specialpages/SpecialMWOAuthManageConsumers.php 
b/frontend/specialpages/SpecialMWOAuthManageConsumers.php
index 2185830..d3b8286 100644
--- a/frontend/specialpages/SpecialMWOAuthManageConsumers.php
+++ b/frontend/specialpages/SpecialMWOAuthManageConsumers.php
@@ -504,7 +504,7 @@
                        $this->mConds['oarc_deleted'] = 0;
                }
 
-               $this->mDB = MWOAuthUtils::getCentralDB( DB_SLAVE );
+               $this->mDb = MWOAuthUtils::getCentralDB( DB_SLAVE );
                parent::__construct();
 
                # Treat 20 as the default limit, since each entry takes up 5 
rows.
diff --git a/frontend/specialpages/SpecialMWOAuthManageMyGrants.php 
b/frontend/specialpages/SpecialMWOAuthManageMyGrants.php
index 7e0e90d..47fdc66 100644
--- a/frontend/specialpages/SpecialMWOAuthManageMyGrants.php
+++ b/frontend/specialpages/SpecialMWOAuthManageMyGrants.php
@@ -51,11 +51,11 @@
                // Format is 
Special:MWOAuthManageMyGrants[/list|/manage/<accesstoken>]
                $navigation = explode( '/', $par );
                $typeKey = isset( $navigation[0] ) ? $navigation[0] : null;
-               $accessToken = isset( $navigation[1] ) ? $navigation[1] : null;
+               $acceptanceId = isset( $navigation[1] ) ? $navigation[1] : null;
 
                switch ( $typeKey ) {
                case 'manage':
-                       $this->handleConsumerForm( $accessToken );
+                       $this->handleConsumerForm( $acceptanceId );
                        break;
                case 'list':
                        // fall through
@@ -64,7 +64,7 @@
                        break;
                }
 
-               $this->addSubtitleLinks( $accessToken );
+               $this->addSubtitleLinks( $acceptanceId );
 
                $this->getOutput()->addModules( 'ext.MWOAuth' ); // CSS
        }
@@ -72,12 +72,12 @@
        /**
         * Show other parent page link
         *
-        * @param type $accessToken
+        * @param type $acceptanceId
         * @return void
         */
-       protected function addSubtitleLinks( $accessToken ) {
+       protected function addSubtitleLinks( $acceptanceId ) {
                $listLinks = array();
-               if ( $accessToken ) {
+               if ( $acceptanceId ) {
                        $listLinks[] = Linker::linkKnown(
                                $this->getTitle( 'proposed' ),
                                $this->msg( 'mwoauthmanagemygrants-showlist' 
)->escaped() );
@@ -95,15 +95,15 @@
        /**
         * Show the form to approve/reject/disable/re-enable consumers
         *
-        * @param string $accessToken
+        * @param string $acceptanceId
         * @return void
         */
-       protected function handleConsumerForm( $accessToken ) {
+       protected function handleConsumerForm( $acceptanceId ) {
                $user = $this->getUser();
                $db = MWOAuthUtils::getCentralDB( DB_SLAVE );
 
                $cmra = MWOAuthDAOAccessControl::wrap(
-                       MWOAuthConsumerAcceptance::newFromToken( $db, 
$accessToken ), $this->getContext() );
+                       MWOAuthConsumerAcceptance::newFromId( $db, 
$acceptanceId ), $this->getContext() );
                if ( !$cmra ) {
                        $this->getOutput()->addHtml( $this->msg( 
'mwoauth-invalid-access-token' )->escaped() );
                        return;
@@ -117,11 +117,6 @@
 
                $form = new HTMLForm(
                        array(
-                               'accessTokenShown' => array(
-                                       'type' => 'info',
-                                       'label-message' => 
'mwoauth-consumer-accesstoken',
-                                       'default' => $cmra->get( 'accessToken' )
-                               ),
                                'name' => array(
                                        'type' => 'info',
                                        'label-message' => 
'mwoauth-consumer-name',
@@ -183,9 +178,9 @@
                                                $this->msg( 
'mwoauthmanagemygrants-update' )->escaped() => 'update',
                                                $this->msg( 
'mwoauthmanagemygrants-renounce' )->escaped() => 'renounce' )
                                ),
-                               'accessToken' => array(
+                               'acceptanceId' => array(
                                        'type' => 'hidden',
-                                       'default' => $cmra->get( 'accessToken' )
+                                       'default' => $cmra->get( 'id' )
                                ),
                        ),
                        $this->getContext()
@@ -216,7 +211,6 @@
        /**
         * Show a paged list of consumers with links to details
         *
-        * @param string $accessToken
         * @return void
         */
        protected function showConsumerList() {
@@ -245,7 +239,7 @@
                $stageKey = self::$stageKeyMap[$cmr->get( 'stage' )];
 
                $link = Linker::linkKnown(
-                       $this->getTitle( 'manage/' . $cmra->get( 'accessToken' 
) ),
+                       $this->getTitle( 'manage/' . $cmra->get( 'id' ) ),
                        $this->msg( 'mwoauthmanagemygrants-review' )->escaped()
                );
 
@@ -305,7 +299,7 @@
                        $this->mConds['oarc_deleted'] = 0;
                }
 
-               $this->mDB = MWOAuthUtils::getCentralDB( DB_SLAVE );
+               $this->mDb = MWOAuthUtils::getCentralDB( DB_SLAVE );
                parent::__construct();
 
                # Treat 20 as the default limit, since each entry takes up 5 
rows.
@@ -365,6 +359,6 @@
         * @return string
         */
        function getIndexField() {
-               return 'oaac_accepted';
+               return 'oaac_consumer_id';
        }
 }

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I91d2e03f399c1f4c96c8a13d08d2aa391eefe04a
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/OAuth
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to