Add getLoginSecurityLevel() support to FormSpecialPage

The base SpecialPage will handle reauthentication automatically if you
just implement getLoginSecurityLevel() to return an appropriate string.

But it doesn't work with FormSpecialPage, and if you try calling
checkLoginSecurityLevel() manually it'll lose any post data if the
reauth happens when the form is posted.

So this patch has SpecialPage::checkLoginSecurityLevel() preserve post
data across reauth (using logic similar to that in AuthManagerSpecialPage),
and has FormSpecialPage call checkLoginSecurityLevel() in the same
way the base SpecialPage does.

It also fixes the SpecialPage logic to not call
checkLoginSecurityLevel() when the special page doesn't implement
getLoginSecurityLevel(), as was the originally-intended behavior.
Apparently almost nothing actually gets to SpecialPage::execute() or
this would probably have been noticed already.

Change-Id: Ic89dc1b6583aaecd2efe3f5109896148a188c271
(cherry picked from commit bfc4e41636aca33b943f8522024bd9f8eeac1977)

diff --git a/RELEASE-NOTES-1.31 b/RELEASE-NOTES-1.31
index 3e0efe4..54672be 100644 (file)
--- a/RELEASE-NOTES-1.31
+++ b/RELEASE-NOTES-1.31
@@ -79,6 +79,10 @@ Required PHP version has been increased from 7.0.0 to 7.0.13.
   saveFileDependencies().
 * (T224374) Fix message parameters so that the message that says SQLite is out of date
   makes sense.
+* SpecialPage::checkLoginSecurityLevel() will now preserve POST data when
+  reauthenticating.
+* FormSpecialPage::execute() will now call checkLoginSecurityLevel() if
+  getLoginSecurityLevel() returns non-false.
 
 == MediaWiki 1.31.1 ==
 

diff --git a/includes/specialpage/FormSpecialPage.php b/includes/specialpage/FormSpecialPage.php
index 66c7d47..81a0036 100644 (file)
--- a/includes/specialpage/FormSpecialPage.php
+++ b/includes/specialpage/FormSpecialPage.php
@@ -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;
+       }
 }

diff --git a/includes/specialpage/SpecialPage.php b/includes/specialpage/SpecialPage.php
index 6828b4a..317aa0d 100644 (file)
--- a/includes/specialpage/SpecialPage.php
+++ b/includes/specialpage/SpecialPage.php
@@ -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();
        }