From 1a8006317dd2c52e4f70d10f585800e8efeb5b1a Mon Sep 17 00:00:00 2001 From: Tyler Anthony Romeo Date: Tue, 27 May 2014 14:00:21 -0400 Subject: [PATCH] Move token login to separate page Rather than have an extraneous form on the login page, move the token input to a separate page. The actual logic for logging in is identical, the only difference is that the token is added to the form data on a second page request. Bug: 53195 Change-Id: I39859cc59f1811de42b72f6167d332ea48812f97 --- OATHAuth.hooks.php | 86 ++++++----------------- extension.json | 7 +- i18n/en.json | 4 +- i18n/qqq.json | 1 + special/SpecialOATH.php | 30 ++++++-- special/SpecialOATHDisable.php | 1 - special/SpecialOATHEnable.php | 1 - special/SpecialOATHLogin.php | 124 +++++++++++++++++++++++++++++++++ 8 files changed, 175 insertions(+), 79 deletions(-) create mode 100644 special/SpecialOATHLogin.php diff --git a/OATHAuth.hooks.php b/OATHAuth.hooks.php index c65fdf66..4e78c0af 100644 --- a/OATHAuth.hooks.php +++ b/OATHAuth.hooks.php @@ -6,23 +6,6 @@ * @ingroup Extensions */ class OATHAuthHooks { - /** - * @param $template UserloginTemplate - * @return bool - */ - static function ModifyUITemplate( &$template ) { - $input = '
' - . Html::input( 'wpOATHToken', null, 'text', array( - 'class' => 'loginText', 'id' => 'wpOATHToken', 'tabindex' => '3', 'size' => '20' - ) ) . '
'; - - $template->set( 'extrafields', $template->get( 'extrafields', '' ) . $input ); - - return true; - } - /** * Get the singleton OATH user repository * @@ -59,7 +42,19 @@ class OATHAuthHooks { * @return bool */ static function AbortChangePassword( $user, $password, $newpassword, &$errorMsg ) { - $result = self::authenticate( $user ); + global $wgRequest; + + $token = $wgRequest->getText( 'wpOATHToken' ); + $oathrepo = self::getOATHUserRepository(); + $oathuser = $oathrepo->findByUser( $user ); + # Though it's weird to default to true, we only want to deny + # users who have two-factor enabled and have validated their + # token. + $result = true; + + if ( $oathuser->getKey() !== null ) { + $result = $oathuser->getKey()->verifyToken( $token, $oathuser ); + } if ( $result ) { return true; @@ -78,57 +73,18 @@ class OATHAuthHooks { * @return bool */ static function AbortLogin( $user, $password, &$abort, &$errorMsg ) { - $result = self::authenticate( $user ); - if ( $result ) { - return true; - } else { - $abort = LoginForm::ABORTED; - $errorMsg = 'oathauth-abortlogin'; - return false; - } - } + $context = RequestContext::getMain(); + $request = $context->getRequest(); + $output = $context->getOutput(); - /** - * @param $user User - * @return bool - */ - static function authenticate( $user ) { - global $wgRequest; + $oathrepo = self::getOATHUserRepository(); + $oathuser = $oathrepo->findByUser( $user ); - $token = $wgRequest->getText( 'wpOATHToken' ); - $oathuser = self::getOATHUserRepository()->findByUser( $user ); - # Though it's weird to default to true, we only want to deny - # users who have two-factor enabled and have validated their - # token. - $result = true; - - if ( $oathuser->getKey() !== null ) { - $result = $oathuser->getKey()->verifyToken( $token, $oathuser ); - } - - return $result; - } - - /** - * Determine if two-factor authentication is enabled for $wgUser - * - * @param bool &$isEnabled Will be set to true if enabled, false otherwise - * - * @return bool False if enabled, true otherwise - */ - static function TwoFactorIsEnabled( &$isEnabled ) { - global $wgUser; - - $user = self::getOATHUserRepository()->findByUser( $wgUser ); - if ( $user && $user->getKey() !== null ) { - $isEnabled = true; - # This two-factor extension is enabled by the user, - # we don't need to check others. + if ( $oathuser->getKey() !== null && !$request->getCheck( 'token' ) ) { + $request->setSessionData( 'oath_login', $request->getValues() ); + $output->redirect( SpecialPage::getTitleFor( 'OATH' )->getFullURL( '', false, PROTO_CURRENT ) ); return false; } else { - $isEnabled = false; - # This two-factor extension isn't enabled by the user, - # but others may be. return true; } } diff --git a/extension.json b/extension.json index e367775c..29151b2d 100644 --- a/extension.json +++ b/extension.json @@ -16,6 +16,7 @@ "SpecialOATH": "special/SpecialOATH.php", "SpecialOATHEnable": "special/SpecialOATHEnable.php", "SpecialOATHDisable": "special/SpecialOATHDisable.php", + "SpecialOATHLogin": "special/SpecialOATHLogin.php", "ProxySpecialPage": "special/ProxySpecialPage.php" }, "ExtensionMessagesFiles": { @@ -28,15 +29,9 @@ "AbortLogin": [ "OATHAuthHooks::AbortLogin" ], - "UserLoginForm": [ - "OATHAuthHooks::ModifyUITemplate" - ], "ChangePasswordForm": [ "OATHAuthHooks::ChangePasswordForm" ], - "TwoFactorIsEnabled": [ - "OATHAuthHooks::TwoFactorIsEnabled" - ], "LoadExtensionSchemaUpdates": [ "OATHAuthHooks::OATHAuthSchemaUpdates" ], diff --git a/i18n/en.json b/i18n/en.json index 5413063a..da3e6cf5 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -17,11 +17,12 @@ "oathauth-verify": "Verify two-factor token", "openstackmanager-scratchtokens": "The following list is a list of one-time use scratch tokens. These tokens can only be used once, and are for emergency use. Please write these down and keep them in a secure location. If you lose your phone, these tokens are the only way to rescue your account. These tokens will never be shown again.", "oathauth-reset": "Reset two-factor credentials", - "oathauth-donotdeleteoldsecret": "Please do not delete your old credentials until you have validated your new credentials.", + "oathauth-donotdeleteoldsecret": "Please do not delete your old credentials until you have successfully validated your new credentials.", "oathauth-token": "Token", "oathauth-currenttoken": "Current token", "oathauth-newtoken": "New token", "oathauth-disable": "Disable two-factor authentication", + "oathauth-login": "Login with two-factor authentication", "oathauth-displayoathinfo": "two-factor authentication options", "oathauth-validatedoath": "Validated two-factor credentials. Two-factor authentication will now be enforced.", "oathauth-backtopreferences": "Back to preferences.", @@ -36,6 +37,7 @@ "oathauth-mustbeloggedin": "You must be logged in to perform this action.", "oathauth-prefs-label": "Two-factor authentication:", "oathauth-abortlogin": "The two-factor authentication token provided was invalid.", + "oathauth-abortlogin": "The two-factor authentication token provided was invalid.", "oathauth-step1": "Step 1: Download the app", "oathauth-step1-test": "Download a mobile app for two-factor authentication (such as Google Authenticator) on to your phone.", "oathauth-step2": "Step 2: Scan the QR code", diff --git a/i18n/qqq.json b/i18n/qqq.json index b525c379..f983b7f3 100644 --- a/i18n/qqq.json +++ b/i18n/qqq.json @@ -26,6 +26,7 @@ "oathauth-currenttoken": "HTMLForm label, found on Special:OATH, when verifying OATH.", "oathauth-newtoken": "HTMLForm label, found on Special:OATH, when verifying OATH.", "oathauth-disable": "Page title on Special:OATH while disabling OATH.\n\nSee [https://en.wikipedia.org/wiki/Two_factor_authentication two factor authentication]", + "oathauth-login": "Page title on Special:OATH while loggin in with OATH.", "oathauth-displayoathinfo": "Page title on Special:OATH when no parameters are passed.\n\nSee [https://en.wikipedia.org/wiki/Two_factor_authentication two factor authentication]", "oathauth-validatedoath": "Plain text found on Special:OATH after a token has been validated.\n\nSee [https://en.wikipedia.org/wiki/Two_factor_authentication two factor authentication]", "oathauth-backtopreferences": "Used as link text. Link found on Special:OATH after any action has completed.", diff --git a/special/SpecialOATH.php b/special/SpecialOATH.php index 96214cf3..7f8a5807 100644 --- a/special/SpecialOATH.php +++ b/special/SpecialOATH.php @@ -8,17 +8,37 @@ class SpecialOATH extends ProxySpecialPage { * If the user already has OATH enabled, show them a page to disable * If the user has OATH disabled, show them a page to enable * - * @return SpecialOATHDisable|SpecialOATHEnable|SpecialPage + * @return SpecialOATHDisable|SpecialOATHEnable|SpecialOATHLogin|SpecialPage */ protected function getTargetPage() { $repo = OATHAuthHooks::getOATHUserRepository(); - $user = $repo->findByUser( $this->getUser() ); - if ( $user->getKey() === null ) { - return new SpecialOATHEnable( $repo, $user ); + /** @var array $sessionUser */ + $loginInfo = $this->getRequest()->getSessionData( 'oath_login' ); + + /** @var SpecialOATHDisable|SpecialOATHEnable|SpecialOATHLogin|SpecialPage $page */ + $page = null; + if ( $this->getUser()->isAnon() && $loginInfo !== null ) { + // User is anonymous, so they are logging in + $page = new SpecialOATHLogin( + $repo->findByUser(User::newFromName( $loginInfo['wpName'] ) ), + new DerivativeRequest( + $this->getRequest(), + $loginInfo, + $this->getRequest()->wasPosted() + ) + ); } else { - return new SpecialOATHDisable( $repo, $user ); + $user = $repo->findByUser( $this->getUser() ); + + if ( $user->getKey() === null ) { + $page = new SpecialOATHEnable( $repo, $user ); + } else { + $page = new SpecialOATHDisable( $repo, $user ); + } } + + return $page; } protected function getGroupName() { diff --git a/special/SpecialOATHDisable.php b/special/SpecialOATHDisable.php index a0db4aef..3029de81 100644 --- a/special/SpecialOATHDisable.php +++ b/special/SpecialOATHDisable.php @@ -3,7 +3,6 @@ /** * Special page to display key information to the user * - * @file * @ingroup Extensions */ class SpecialOATHDisable extends FormSpecialPage { diff --git a/special/SpecialOATHEnable.php b/special/SpecialOATHEnable.php index 4e331fd9..a6e9290c 100644 --- a/special/SpecialOATHEnable.php +++ b/special/SpecialOATHEnable.php @@ -3,7 +3,6 @@ /** * Special page to display key information to the user * - * @file * @ingroup Extensions */ class SpecialOATHEnable extends FormSpecialPage { diff --git a/special/SpecialOATHLogin.php b/special/SpecialOATHLogin.php new file mode 100644 index 00000000..28d32562 --- /dev/null +++ b/special/SpecialOATHLogin.php @@ -0,0 +1,124 @@ +OATHUser = $oathuser; + $this->loginForm = new LoginForm( $oldRequest ); + $this->loginForm->setContext( $this->getContext() ); + } + + /** + * Set the page title and add JavaScript RL modules + * + * @param HTMLForm $form + */ + public function alterForm( HTMLForm $form ) { + $form->setMessagePrefix( 'oathauth' ); + $form->setWrapperLegend( false ); + $form->getOutput()->setPagetitle( $this->msg( 'oathauth-login' ) ); + $form->getOutput()->addModules( 'ext.oathauth' ); + } + + /** + * @return string + */ + public function getDisplayFormat() { + return 'vform'; + } + + /** + * @return bool + */ + public function requiresUnblock() { + return false; + } + + /** + * @return array[] + */ + protected function getFormFields() { + return array( + 'token' => array( + 'type' => 'text', + 'default' => '', + 'label-message' => 'oathauth-entertoken', + 'name' => 'token', + 'required' => true + ), + 'returnto' => array( + 'type' => 'hidden', + 'default' => $this->getRequest()->getVal( 'returnto' ), + 'name' => 'returnto', + ), + 'returntoquery' => array( + 'type' => 'hidden', + 'default' => $this->getRequest()->getVal( 'returntoquery' ), + 'name' => 'returntoquery', + ) + ); + } + + /** + * Stub function: the only purpose of this form is to add more data into + * the login form + * + * @param array $formData + * + * @return true + */ + public function onSubmit( array $formData ) { + $this->getRequest()->setSessionData( 'oath_login', null ); + $this->token = $formData['token']; + + return true; + } + + public function onSuccess() { + $this->loginForm->execute( $this->par ); + } + + /** + * @param User $user + * @param $password + * @param $abort + * @param $errorMsg + * + * @return bool + */ + public function onAbortLogin( User $user, $password, &$abort, &$errorMsg ) { + $result = $this->OATHUser + ->getKey() + ->verifyToken( $this->getRequest()->getVal( 'token' ), $this->OATHUser ); + + if ( $result ) { + return true; + } else { + $abort = LoginForm::WRONG_PASS; + + return false; + } + } +}