Merge "Add getLoginSecurityLevel() support to FormSpecialPage"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Thu, 7 Jun 2018 14:50:14 +0000 (14:50 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Thu, 7 Jun 2018 14:50:14 +0000 (14:50 +0000)
RELEASE-NOTES-1.32
includes/specialpage/FormSpecialPage.php
includes/specialpage/SpecialPage.php

index 65d79f3..ad62af4 100644 (file)
@@ -37,6 +37,10 @@ production.
 * (T152462) A cookie can now be set when an IP user is blocked to track that user if
   they move to a new IP address. This is disabled by default.
 * (T194950) Added 'ApiMaxLagInfo' hook.
+* SpecialPage::checkLoginSecurityLevel() will now preserve POST data when
+  reauthenticating.
+* FormSpecialPage::execute() will now call checkLoginSecurityLevel() if
+  getLoginSecurityLevel() returns non-false.
 
 === External library changes in 1.32 ===
 * …
@@ -55,7 +59,8 @@ production.
 * …
 
 === Bug fixes in 1.32 ===
-* …
+* SpecialPage::execute() will now only call checkLoginSecurityLevel() if
+  getLoginSecurityLevel() returns non-false.
 
 === Action API changes in 1.32 ===
 * Added templated parameters.
index 66c7d47..81a0036 100644 (file)
@@ -35,6 +35,12 @@ abstract class FormSpecialPage extends SpecialPage {
         */
        protected $par = null;
 
+       /**
+        * @var array|null POST data preserved across re-authentication
+        * @since 1.32
+        */
+       protected $reauthPostData = null;
+
        /**
         * Get an HTMLForm descriptor array
         * @return array
@@ -89,13 +95,31 @@ abstract class FormSpecialPage extends SpecialPage {
         * @return HTMLForm|null
         */
        protected function getForm() {
+               $context = $this->getContext();
+               $onSubmit = [ $this, 'onSubmit' ];
+
+               if ( $this->reauthPostData ) {
+                       // Restore POST data
+                       $context = new DerivativeContext( $context );
+                       $oldRequest = $this->getRequest();
+                       $context->setRequest( new DerivativeRequest(
+                               $oldRequest, $this->reauthPostData + $oldRequest->getQueryValues(), true
+                       ) );
+
+                       // But don't treat it as a "real" submission just in case of some
+                       // crazy kind of CSRF.
+                       $onSubmit = function () {
+                               return false;
+                       };
+               }
+
                $form = HTMLForm::factory(
                        $this->getDisplayFormat(),
                        $this->getFormFields(),
-                       $this->getContext(),
+                       $context,
                        $this->getMessagePrefix()
                );
-               $form->setSubmitCallback( [ $this, 'onSubmit' ] );
+               $form->setSubmitCallback( $onSubmit );
                if ( $this->getDisplayFormat() !== 'ooui' ) {
                        // No legend and wrapper by default in OOUI forms, but can be set manually
                        // from alterForm()
@@ -151,6 +175,11 @@ abstract class FormSpecialPage extends SpecialPage {
                // This will throw exceptions if there's a problem
                $this->checkExecutePermissions( $this->getUser() );
 
+               $securityLevel = $this->getLoginSecurityLevel();
+               if ( $securityLevel !== false && !$this->checkLoginSecurityLevel( $securityLevel ) ) {
+                       return;
+               }
+
                $form = $this->getForm();
                if ( $form->show() ) {
                        $this->onSuccess();
@@ -199,4 +228,14 @@ abstract class FormSpecialPage extends SpecialPage {
        public function requiresUnblock() {
                return true;
        }
+
+       /**
+        * Preserve POST data across reauthentication
+        *
+        * @since 1.32
+        * @param array $data
+        */
+       protected function setReauthPostData( array $data ) {
+               $this->reauthPostData = $data;
+       }
 }
index 6828b4a..317aa0d 100644 (file)
@@ -352,6 +352,23 @@ class SpecialPage implements MessageLocalizer {
                return false;
        }
 
+       /**
+        * Record preserved POST data after a reauthentication.
+        *
+        * This is called from checkLoginSecurityLevel() when returning from the
+        * redirect for reauthentication, if the redirect had been served in
+        * response to a POST request.
+        *
+        * The base SpecialPage implementation does nothing. If your subclass uses
+        * getLoginSecurityLevel() or checkLoginSecurityLevel(), it should probably
+        * implement this to do something with the data.
+        *
+        * @since 1.32
+        * @param array $data
+        */
+       protected function setReauthPostData( array $data ) {
+       }
+
        /**
         * Verifies that the user meets the security level, possibly reauthenticating them in the process.
         *
@@ -378,16 +395,42 @@ class SpecialPage implements MessageLocalizer {
         */
        protected function checkLoginSecurityLevel( $level = null ) {
                $level = $level ?: $this->getName();
+               $key = 'SpecialPage:reauth:' . $this->getName();
+               $request = $this->getRequest();
+
                $securityStatus = AuthManager::singleton()->securitySensitiveOperationStatus( $level );
                if ( $securityStatus === AuthManager::SEC_OK ) {
+                       $uniqueId = $request->getVal( 'postUniqueId' );
+                       if ( $uniqueId ) {
+                               $key = $key . ':' . $uniqueId;
+                               $session = $request->getSession();
+                               $data = $session->getSecret( $key );
+                               if ( $data ) {
+                                       $session->remove( $key );
+                                       $this->setReauthPostData( $data );
+                               }
+                       }
                        return true;
                } elseif ( $securityStatus === AuthManager::SEC_REAUTH ) {
-                       $request = $this->getRequest();
                        $title = self::getTitleFor( 'Userlogin' );
+                       $queryParams = $request->getQueryValues();
+
+                       if ( $request->wasPosted() ) {
+                               $data = array_diff_assoc( $request->getValues(), $request->getQueryValues() );
+                               if ( $data ) {
+                                       // unique ID in case the same special page is open in multiple browser tabs
+                                       $uniqueId = MWCryptRand::generateHex( 6 );
+                                       $key = $key . ':' . $uniqueId;
+                                       $queryParams['postUniqueId'] = $uniqueId;
+                                       $session = $request->getSession();
+                                       $session->persist(); // Just in case
+                                       $session->setSecret( $key, $data );
+                               }
+                       }
+
                        $query = [
                                'returnto' => $this->getFullTitle()->getPrefixedDBkey(),
-                               'returntoquery' => wfArrayToCgi( array_diff_key( $request->getQueryValues(),
-                                       [ 'title' => true ] ) ),
+                               'returntoquery' => wfArrayToCgi( array_diff_key( $queryParams, [ 'title' => true ] ) ),
                                'force' => $level,
                        ];
                        $url = $title->getFullURL( $query, false, PROTO_HTTPS );
@@ -568,7 +611,10 @@ class SpecialPage implements MessageLocalizer {
        public function execute( $subPage ) {
                $this->setHeaders();
                $this->checkPermissions();
-               $this->checkLoginSecurityLevel( $this->getLoginSecurityLevel() );
+               $securityLevel = $this->getLoginSecurityLevel();
+               if ( $securityLevel !== false && !$this->checkLoginSecurityLevel( $securityLevel ) ) {
+                       return;
+               }
                $this->outputHeader();
        }