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

Change subject: Fix bugs with handling of missing request parameters
......................................................................


Fix bugs with handling of missing request parameters

* make MWOAuthDAO methods type-safe in the face of MySQL non-strict
  mode weirdness
* throw an exception when cancelled with an invalid consumer key
  (should not happen under normal circumstances as the authorization
  dialog won't display)
* fix phpdoc so it's easier to follow in an IDE what happens

Bug: T147414
Change-Id: Ibb938ccb9bfae6c52444f7676dc475f6a3024cd8
---
M backend/MWOAuthConsumer.php
M backend/MWOAuthConsumerAcceptance.php
M backend/MWOAuthDAO.php
M backend/MWOAuthDataStore.php
M backend/MWOAuthServer.php
M frontend/specialpages/SpecialMWOAuth.php
6 files changed, 21 insertions(+), 10 deletions(-)

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



diff --git a/backend/MWOAuthConsumer.php b/backend/MWOAuthConsumer.php
index 2289888..d63fde3 100644
--- a/backend/MWOAuthConsumer.php
+++ b/backend/MWOAuthConsumer.php
@@ -163,7 +163,7 @@
        public static function newFromKey( \DBConnRef $db, $key, $flags = 0 ) {
                $row = $db->selectRow( static::getTable(),
                        array_values( static::getFieldColumnMap() ),
-                       array( 'oarc_consumer_key' => $key ),
+                       array( 'oarc_consumer_key' => (string)$key ),
                        __METHOD__,
                        ( $flags & self::READ_LOCKING ) ? array( 'FOR UPDATE' ) 
: array()
                );
@@ -190,7 +190,11 @@
        ) {
                $row = $db->selectRow( static::getTable(),
                        array_values( static::getFieldColumnMap() ),
-                       array( 'oarc_name' => $name, 'oarc_version' => 
$version, 'oarc_user_id' => $userId ),
+                       array(
+                               'oarc_name' => (string)$name,
+                               'oarc_version' => (string)$version,
+                               'oarc_user_id' => (int)$userId
+                       ),
                        __METHOD__,
                        ( $flags & self::READ_LOCKING ) ? array( 'FOR UPDATE' ) 
: array()
                );
diff --git a/backend/MWOAuthConsumerAcceptance.php 
b/backend/MWOAuthConsumerAcceptance.php
index 1851278..474f992 100644
--- a/backend/MWOAuthConsumerAcceptance.php
+++ b/backend/MWOAuthConsumerAcceptance.php
@@ -81,7 +81,7 @@
        public static function newFromToken( \DBConnRef $db, $token, $flags = 0 
) {
                $row = $db->selectRow( static::getTable(),
                        array_values( static::getFieldColumnMap() ),
-                       array( 'oaac_access_token' => $token ),
+                       array( 'oaac_access_token' => (string)$token ),
                        __METHOD__,
                        ( $flags & self::READ_LOCKING ) ? array( 'FOR UPDATE' ) 
: array()
                );
@@ -108,9 +108,10 @@
        ) {
                $row = $db->selectRow( static::getTable(),
                        array_values( static::getFieldColumnMap() ),
-                       array( 'oaac_user_id' => $userId,
+                       array(
+                               'oaac_user_id' => (int)$userId,
                                'oaac_consumer_id' => $consumer->get( 'id' ),
-                               'oaac_wiki' => $wiki
+                               'oaac_wiki' => (string)$wiki
                        ),
                        __METHOD__,
                        ( $flags & self::READ_LOCKING ) ? array( 'FOR UPDATE' ) 
: array()
diff --git a/backend/MWOAuthDAO.php b/backend/MWOAuthDAO.php
index 6b3d76f..62683e9 100644
--- a/backend/MWOAuthDAO.php
+++ b/backend/MWOAuthDAO.php
@@ -75,7 +75,7 @@
        final public static function newFromId( \DBConnRef $db, $id, $flags = 0 
) {
                $row = $db->selectRow( static::getTable(),
                        array_values( static::getFieldColumnMap() ),
-                       array( static::getIdColumn() => $id ),
+                       array( static::getIdColumn() => (int)$id ),
                        __METHOD__,
                        ( $flags & self::READ_LOCKING ) ? array( 'FOR UPDATE' ) 
: array()
                );
diff --git a/backend/MWOAuthDataStore.php b/backend/MWOAuthDataStore.php
index 2dd2d81..76de10f 100644
--- a/backend/MWOAuthDataStore.php
+++ b/backend/MWOAuthDataStore.php
@@ -149,7 +149,7 @@
         * Return a consumer key associated with the given request token.
         *
         * @param MWOAuthToken $requestToken the request token
-        * @return String the consumer key
+        * @return string|false the consumer key or false if nothing is stored 
for the request token
         */
        public function getConsumerKey( $requestToken ) {
                $cacheKey = MWOAuthUtils::getCacheKey( 'consumer', 'request', 
$requestToken );
@@ -163,10 +163,10 @@
         * A stored callback URL parameter is deleted from the cache once read 
for the first
         * time.
         *
-        * @param string the consumer key
+        * @param string $consumerKey the consumer key
         * @param string $requestKey original request key from /initiate
         * @throws MWOAuthException
-        * @return String the stored callback URL parameter
+        * @return string|false the stored callback URL parameter
         */
        public function getCallbackUrl( $consumerKey, $requestKey ) {
                $cacheKey = MWOAuthUtils::getCacheKey( 'callback', 
$consumerKey, 'request', $requestKey );
diff --git a/backend/MWOAuthServer.php b/backend/MWOAuthServer.php
index fa178cc..e643ebf 100644
--- a/backend/MWOAuthServer.php
+++ b/backend/MWOAuthServer.php
@@ -3,11 +3,14 @@
 namespace MediaWiki\Extensions\OAuth;
 
 class MWOAuthServer extends OAuthServer {
+       /** @var MWOAuthDataStore */
+       protected $data_store;
+
        /**
         * Return a consumer key associated with the given request token.
         *
         * @param MWOAuthToken $requestToken the request token
-        * @return String the consumer key
+        * @return string|false the consumer key or false if nothing is stored 
for the request token
         */
        public function getConsumerKey( $requestToken ) {
                return $this->data_store->getConsumerKey( $requestToken );
diff --git a/frontend/specialpages/SpecialMWOAuth.php 
b/frontend/specialpages/SpecialMWOAuth.php
index 0397987..8b39881 100644
--- a/frontend/specialpages/SpecialMWOAuth.php
+++ b/frontend/specialpages/SpecialMWOAuth.php
@@ -218,6 +218,9 @@
                        MWOAuthConsumer::newFromKey( $dbr, $consumerKey ),
                        $this->getContext()
                );
+               if ( !$cmr ) {
+                       throw new MWOAuthException( 
'mwoauth-invalid-consumer-key' );
+               }
 
                $this->getOutput()->addSubtitle( $this->msg( 'mwoauth-desc' 
)->escaped() );
                $this->getOutput()->addWikiMsg(

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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ibb938ccb9bfae6c52444f7676dc475f6a3024cd8
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/OAuth
Gerrit-Branch: master
Gerrit-Owner: Gergő Tisza <gti...@wikimedia.org>
Gerrit-Reviewer: Anomie <bjor...@wikimedia.org>
Gerrit-Reviewer: Bartosz Dziewoński <matma....@gmail.com>
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