From bfc4e41636aca33b943f8522024bd9f8eeac1977 Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Wed, 9 May 2018 14:53:32 -0400 Subject: [PATCH] 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 --- RELEASE-NOTES-1.32 | 7 ++- includes/specialpage/FormSpecialPage.php | 43 ++++++++++++++++++- includes/specialpage/SpecialPage.php | 54 ++++++++++++++++++++++-- 3 files changed, 97 insertions(+), 7 deletions(-) diff --git a/RELEASE-NOTES-1.32 b/RELEASE-NOTES-1.32 index 702b28916c..bdb3882c3c 100644 --- a/RELEASE-NOTES-1.32 +++ b/RELEASE-NOTES-1.32 @@ -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 === * … @@ -54,7 +58,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. diff --git a/includes/specialpage/FormSpecialPage.php b/includes/specialpage/FormSpecialPage.php index 66c7d47ea9..81a0036e83 100644 --- 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 6828b4a400..317aa0d7c8 100644 --- 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(); } -- 2.20.1