Reedy has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/336838 )

Change subject: API: Insist authn parameters be in the POST body
......................................................................

API: Insist authn parameters be in the POST body

Passwords should always be submitted in the POST body, not in the query
string. Thus, a warning will now be returned if the password for
action=login.

Change-Id: Ifb2c684bb28c9acc004be2b0c2fef839eb7624aa
---
M RELEASE-NOTES-1.23
M includes/api/ApiBase.php
M includes/api/ApiLogin.php
M includes/api/ApiMain.php
4 files changed, 57 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/38/336838/1

diff --git a/RELEASE-NOTES-1.23 b/RELEASE-NOTES-1.23
index 4e1116a..55558ae 100644
--- a/RELEASE-NOTES-1.23
+++ b/RELEASE-NOTES-1.23
@@ -7,6 +7,9 @@
 === Changes since 1.23.15 ===
 * (T68404) CSS3 attr() function with url type is no longer allowed
   in inline styles.
+* Submitting the lgtoken and lgpassword parameters in the query string to
+  action=login is now deprecated and outputs a warning. They should be 
submitted
+  in the POST body instead.
 
 == MediaWiki 1.23.15 ==
 
diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php
index cf96ac2..b985101 100644
--- a/includes/api/ApiBase.php
+++ b/includes/api/ApiBase.php
@@ -845,6 +845,39 @@
        }
 
        /**
+        * Die if any of the specified parameters were found in the query part 
of
+        * the URL rather than the post body.
+        * @since 1.28
+        * @param string[] $params Parameters to check
+        * @param string $prefix Set to 'noprefix' to skip calling 
$this->encodeParamName()
+        */
+       public function requirePostedParameters( $params, $prefix = 'prefix' ) {
+               // Skip if $wgDebugAPI is set or we're in internal mode
+               if ( $this->getConfig()->get( 'DebugAPI' ) || 
$this->getMain()->isInternalMode() ) {
+                       return;
+               }
+
+               $queryValues = $this->getRequest()->getQueryValues();
+               $badParams = [];
+               foreach ( $params as $param ) {
+                       if ( $prefix !== 'noprefix' ) {
+                               $param = $this->encodeParamName( $param );
+                       }
+                       if ( array_key_exists( $param, $queryValues ) ) {
+                               $badParams[] = $param;
+                       }
+               }
+
+               if ( $badParams ) {
+                       $this->dieUsage(
+                               'The following parameters were found in the 
query string, but must be in the POST body: '
+                                       . join( ', ', $badParams ),
+                               'mustpostparams'
+                       );
+               }
+       }
+
+       /**
         * Generates the possible errors requireAtLeastOneParameter() can die 
with
         *
         * @since 1.23
@@ -2036,8 +2069,19 @@
         * Indicates if this module needs maxlag to be checked
         * @return bool
         */
+<<<<<<< HEAD
        public function shouldCheckMaxlag() {
                return true;
+=======
+       public function logFeatureUsage( $feature ) {
+               $request = $this->getRequest();
+               $s = '"' . addslashes( $feature ) . '"' .
+                       ' "' . wfUrlencode( str_replace( ' ', '_', 
$this->getUser()->getName() ) ) . '"' .
+                       ' "' . $request->getIP() . '"' .
+                       ' "' . addslashes( $request->getHeader( 'Referer' ) ) . 
'"' .
+                       ' "' . addslashes( $this->getMain()->getUserAgent() ) . 
'"';
+               wfDebugLog( 'api-feature-usage', $s, 'private' );
+>>>>>>> 6a068d18... API: Insist authn parameters be in the POST body
        }
 
        /**
diff --git a/includes/api/ApiLogin.php b/includes/api/ApiLogin.php
index f2a9d1a..9a6ee1d 100644
--- a/includes/api/ApiLogin.php
+++ b/includes/api/ApiLogin.php
@@ -56,6 +56,14 @@
                        return;
                }
 
+               try {
+                       $this->requirePostedParameters( [ 'password', 'token' ] 
);
+               } catch ( UsageException $ex ) {
+                       // Make this a warning for now, upgrade to an error in 
1.29.
+                       $this->setWarning( $ex->getMessage() );
+                       $this->logFeatureUsage( 'login-params-in-query-string' 
);
+               }
+
                $params = $this->extractRequestParams();
 
                $result = array();
diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php
index baabaec..9d7d63a 100644
--- a/includes/api/ApiMain.php
+++ b/includes/api/ApiMain.php
@@ -805,6 +805,8 @@
                                $this->dieUsageMsg( array( 'missingparam', 
'token' ) );
                        }
 
+                       $module->requirePostedParameters( [ 'token' ] );
+
                        if ( !$this->getUser()->matchEditToken(
                                $moduleParams['token'],
                                $salt,

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifb2c684bb28c9acc004be2b0c2fef839eb7624aa
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: REL1_23
Gerrit-Owner: Reedy <[email protected]>
Gerrit-Reviewer: Anomie <[email protected]>

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

Reply via email to