Gergő Tisza has uploaded a new change for review.
https://gerrit.wikimedia.org/r/294861
Change subject: [WIP] Use AuthManager data change methods
......................................................................
[WIP] Use AuthManager data change methods
TODO:
* token reset
* handle returnto
* T132329
* make it look less horrible in oouiform
* cannot call HtmlForm::setMessagePrefix
* T122691
Bug: T110457
Change-Id: I2c1a9bf87ca5643cc7c52acc92955177aa4bdfad
Depends-On: I4d796a46d99daa8841fcdcbe76dddf9db46af9ed
---
A HTMLOATHField.php
M OATHAuth.hooks.php
M auth/TOTPAuthenticationRequest.php
A auth/TOTPDisableAuthenticationRequest.php
A auth/TOTPEnableAuthenticationRequest.php
M auth/TOTPSecondaryAuthenticationProvider.php
M extension.json
M i18n/en.json
M i18n/qqq.json
9 files changed, 326 insertions(+), 16 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/OATHAuth
refs/changes/61/294861/1
diff --git a/HTMLOATHField.php b/HTMLOATHField.php
new file mode 100644
index 0000000..e727bfd
--- /dev/null
+++ b/HTMLOATHField.php
@@ -0,0 +1,98 @@
+<?php
+
+class HTMLOATHField extends HTMLFormField {
+ /** @var OATHAuthKey */
+ protected $oathKey;
+
+ /** @var OATHUser */
+ protected $oathUser;
+
+ /**
+ * Constructs a verification form field for a given OATH key
+ * - oathKey: OATHAuthKey
+ * - oathUser: OATHUser
+ * @param array $params
+ */
+ public function __construct( array $params ) {
+ parent::__construct( $params );
+ $this->oathKey = $params['oathKey'];
+ $this->oathUser = $params['oathUser'];
+ }
+
+ public function hasVisibleOutput() {
+ return false;
+ }
+
+ public function getInputHTML( $value ) {
+ $output = $this->mParent ? $this->mParent->getOutput() :
RequestContext::getMain()->getOutput();
+ $output->addModules( 'ext.oathauth' );
+ $output->addHTML( ResourceLoader::makeInlineScript(
Xml::encodeJsCall( 'mw.loader.using', array(
+ array( 'ext.oathauth' ),
+ new XmlJsCode( 'function () {' .
'$("#qrcode").qrcode("otpauth://totp/' .
+ rawurlencode( $this->oathUser->getAccount() ) .
'?secret='
+ . $this->oathKey->getSecret() . '");' . '}' )
+ ) ) ) );
+ return '';
+ }
+
+ public static function getSubFields( OATHAuthKey $oathKey, OATHUser
$oathUser ) {
+ return array(
+ 'OATHModule' => array(
+ 'class' => self::class,
+ 'oathKey' => $oathKey,
+ 'oathUser' => $oathUser,
+ ),
+ 'OATHAppInfo' => array(
+ 'type' => 'info',
+ 'default' => wfMessage( 'oathauth-step1-test'
)->escaped(),
+ 'raw' => true,
+ 'section' => 'step1',
+ ),
+ 'OATHQRCode' => array(
+ 'type' => 'info',
+ 'default' => '<div id="qrcode"></div>',
+ 'raw' => true,
+ 'section' => 'step2',
+ ),
+ 'OATHScratchCodes' => array(
+ 'type' => 'info',
+ 'label-message' => 'oathauth-step2alt',
+ 'default' =>
+ '<strong>' . wfMessage(
'oathauth-account' )->escaped() . '</strong><br/>'
+ . $oathUser->getAccount() . '<br/><br/>'
+ . '<strong>' . wfMessage(
'oathauth-secret' )->escaped() . '</strong><br/>'
+ . $oathKey->getSecret() . '<br/>',
+ 'raw' => true,
+ 'section' => 'step2',
+ ),
+ 'OATHScratchTokens' => array(
+ 'type' => 'info',
+ 'default' =>
+ wfMessage(
'openstackmanager-scratchtokens' )
+ . self::createResourceList(
$oathKey->getScratchTokens() ),
+ 'raw' => true,
+ 'section' => 'step3',
+ ),
+ 'OATHToken' => array(
+ 'type' => 'text',
+ 'default' => '',
+ 'label-message' => 'oathauth-entertoken',
+ 'name' => 'token',
+ 'section' => 'step4',
+ 'persistent' => false,
+ ),
+ );
+ }
+
+ /**
+ * @param $resources array
+ * @return string
+ */
+ protected static function createResourceList( $resources ) {
+ $resourceList = '';
+ foreach ( $resources as $resource ) {
+ $resourceList .= Html::rawElement( 'li', array(),
$resource );
+ }
+ return Html::rawElement( 'ul', array(), $resourceList );
+ }
+}
diff --git a/OATHAuth.hooks.php b/OATHAuth.hooks.php
index 66fbfd4..7d24410 100644
--- a/OATHAuth.hooks.php
+++ b/OATHAuth.hooks.php
@@ -65,16 +65,32 @@
public static function onAuthChangeFormFields(
array $requests, array $fieldInfo, array &$formDescriptor,
$action
) {
- if ( isset( $fieldInfo['OATHToken'] ) ) {
- $formDescriptor['OATHToken'] += [
- 'cssClass' => 'loginText',
- 'id' => 'wpOATHToken',
- 'size' => 20,
- 'autofocus' => true,
- 'persistent' => false,
- ];
+ if ( !isset( $fieldInfo['OATHToken'] ) ) {
+ return;
+ };
+
+ // handle enable request
+ $req = AuthenticationRequest::getRequestByClass( $requests,
+ TOTPEnableAuthenticationRequest::class );
+ if ( $req ) {
+ // info fields are for the API, HTMLOATHField already
contains the same information
+ unset( $formDescriptor['OATHSecret'],
$formDescriptor['OATHScratchCodes'],
+ $formDescriptor['OATHToken'] );
+
+ $repo = OATHAuthHooks::getOATHUserRepository();
+ $oathuser = $repo->findByUser( User::newFromName(
$req->username ) );
+ $formDescriptor += HTMLOATHField::getSubFields(
$req->key, $oathuser );
+ return;
}
- return true;
+
+ // disable and login is the same, just a field for the token
+ $formDescriptor['OATHToken'] += [
+ 'cssClass' => 'loginText',
+ 'id' => 'wpOATHToken',
+ 'size' => 20,
+ 'autofocus' => true,
+ 'persistent' => false,
+ ];
}
/**
@@ -117,9 +133,12 @@
}
$oathUser = self::getOATHUserRepository()->findByUser( $user );
+ $isEnabled = $oathUser->getKey() !== null;
- $title = SpecialPage::getTitleFor( 'OATH' );
- $msg = $oathUser->getKey() !== null ? 'oathauth-disable' :
'oathauth-enable';
+ $title = SpecialPage::getTitleFor( 'ChangeCredentials',
$isEnabled
+ ? TOTPDisableAuthenticationRequest::class
+ : TOTPEnableAuthenticationRequest::class );
+ $msg = $isEnabled ? 'oathauth-disable' : 'oathauth-enable';
$preferences[$msg] = array(
'type' => 'info',
diff --git a/auth/TOTPAuthenticationRequest.php
b/auth/TOTPAuthenticationRequest.php
index 99220ce..8fda60d 100644
--- a/auth/TOTPAuthenticationRequest.php
+++ b/auth/TOTPAuthenticationRequest.php
@@ -7,6 +7,7 @@
* that is generated from the current time indepdendently by the server and
the client.
*/
class TOTPAuthenticationRequest extends AuthenticationRequest {
+ /** @var string */
public $OATHToken;
public function describeCredentials() {
diff --git a/auth/TOTPDisableAuthenticationRequest.php
b/auth/TOTPDisableAuthenticationRequest.php
new file mode 100644
index 0000000..ebe437c
--- /dev/null
+++ b/auth/TOTPDisableAuthenticationRequest.php
@@ -0,0 +1,30 @@
+<?php
+
+use MediaWiki\Auth\AuthenticationRequest;
+
+/**
+ * AuthManager value object for disabling TOTP second-factor checks.
+ * There are two methods to disable: externally via ACTION_CHANGE and
internally via ACTION_REMOVE.
+ * The former requires a successful TOTP key verification.
+ */
+class TOTPDisableAuthenticationRequest extends AuthenticationRequest {
+ /** @var string */
+ public $OATHToken;
+
+ public function describeCredentials() {
+ return [
+ 'provider' => wfMessage( 'oathauth-describe-provider' ),
+ 'account' => new \RawMessage( '$1', [ $this->username ]
),
+ ] + parent::describeCredentials();
+ }
+
+ public function getFieldInfo() {
+ return array(
+ 'OATHToken' => array(
+ 'type' => 'string',
+ 'label' => wfMessage( 'oathauth-entertoken' ),
+ 'help' => wfMessage( 'oathauth-auth-token-help'
),
+ ),
+ );
+ }
+}
diff --git a/auth/TOTPEnableAuthenticationRequest.php
b/auth/TOTPEnableAuthenticationRequest.php
new file mode 100644
index 0000000..3f158a7
--- /dev/null
+++ b/auth/TOTPEnableAuthenticationRequest.php
@@ -0,0 +1,58 @@
+<?php
+
+use MediaWiki\Auth\AuthenticationRequest;
+
+/**
+ * AuthManager value object for the TOTP secret key + scratch codes. Used to
pass the secret key and
+ * the codes to the user and get back a one-time token as a proof that the
user managed to configure
+ * their token generator (and enabling two-factor authentication won't lock
them out of their
+ * account).
+ */
+class TOTPEnableAuthenticationRequest extends AuthenticationRequest {
+ /** @var string */
+ public $OATHToken;
+
+ /** @var OATHAuthKey */
+ public $key;
+
+ public function __construct( OATHAuthKey $key ) {
+ $this->key = $key;
+ }
+
+ public function describeCredentials() {
+ return [
+ 'provider' => wfMessage( 'oathauth-describe-provider' ),
+ 'account' => new \RawMessage( '$1', [ $this->username ]
),
+ ] + parent::describeCredentials();
+ }
+
+ public function getFieldInfo() {
+ return array(
+ 'OATHSecret' => array(
+ 'type' => 'null',
+ 'value' => new RawMessage( '$1', array(
$this->key->getSecret() ) ),
+ 'label' => wfMessage( 'oathauth-secret-label' ),
+ 'help' => wfMessage( 'oathauth-secret-help' ),
+ ),
+ 'OATHScratchCodes' => array(
+ 'type' => 'null',
+ 'value' => new RawMessage( '$1', array(
implode( wfMessage( 'comma-separator' ),
+ $this->key->getScratchTokens() ) ) ),
+ 'label' => wfMessage(
'oathauth-scratch-codes-label' ),
+ 'help' => wfMessage(
'oathauth-scratch-codes-help' ),
+ ),
+ 'OATHToken' => array(
+ 'type' => 'string',
+ 'label' => wfMessage(
'oathauth-auth-token-label' ),
+ 'help' => wfMessage( 'oathauth-auth-token-help'
),
+ ),
+ );
+ }
+
+ public function getMetadata() {
+ return [
+ 'OATHSecret' => $this->key->getSecret(),
+ 'OATHScratchCodes' => $this->key->getScratchTokens(),
+ ];
+ }
+}
diff --git a/auth/TOTPSecondaryAuthenticationProvider.php
b/auth/TOTPSecondaryAuthenticationProvider.php
index 2ca930a..981d102 100644
--- a/auth/TOTPSecondaryAuthenticationProvider.php
+++ b/auth/TOTPSecondaryAuthenticationProvider.php
@@ -23,6 +23,26 @@
case AuthManager::ACTION_LOGIN:
// don't ask for anything initially so the
second factor is on a separate screen
return [];
+ case AuthManager::ACTION_CHANGE:
+ case AuthManager::ACTION_REMOVE:
+ $user = User::newFromName( $options['username']
);
+ if ( !$user->getId() ) {
+ return [];
+ }
+
+ $repo = OATHAuthHooks::getOATHUserRepository();
+ $oathuser = $repo->findByUser( $user );
+ $key = $oathuser->getKey();
+
+ if ( !$key && $action ===
AuthManager::ACTION_CHANGE ) {
+ // add a key
+ return [ new
TOTPEnableAuthenticationRequest( $this->getPendingKey() ) ];
+ } elseif ( $key ) {
+ // remove the key - on REMOVE it is
unconditional and only allowed internally,
+ // on CHANGE it requires a verification
token
+ return [ new
TOTPDisableAuthenticationRequest() ];
+ }
+ return [];
default:
return [];
}
@@ -83,4 +103,72 @@
public function beginSecondaryAccountCreation( $user, $creator, array
$reqs ) {
return AuthenticationResponse::newAbstain();
}
+
+ public function providerAllowsAuthenticationDataChange(
+ AuthenticationRequest $req, $checkData = true
+ ) {
+ if ( $req instanceof TOTPEnableAuthenticationRequest ) {
+ $repo = OATHAuthHooks::getOATHUserRepository();
+ $oathuser = $repo->findByUser( User::newFromName(
$req->username ) );
+ $key = $this->getPendingKey();
+
+ if ( $oathuser->getKey() ) {
+ return StatusValue::newFatal(
'oathauth-alreadyenabled' );
+ } elseif ( !$key || $key != $req->key ) {
+ // sanity check, should not be possible
+ return StatusValue::newFatal(
'oathauth-failedtovalidateoauth' );
+ } elseif ( $checkData && !$key->verifyToken(
$req->OATHToken, $oathuser ) ) {
+ return StatusValue::newFatal(
'oathauth-failedtovalidateoauth' );
+ }
+
+ return StatusValue::newGood();
+ } elseif ( $req instanceof TOTPDisableAuthenticationRequest ) {
+ $repo = OATHAuthHooks::getOATHUserRepository();
+ $oathuser = $repo->findByUser( User::newFromName(
$req->username ) );
+ $key = $oathuser->getKey();
+
+ if ( !$key ) {
+ return StatusValue::newFatal(
'oathauth-notenabled' );
+ } elseif ( $checkData &&
!$oathuser->getKey()->verifyToken( $req->OATHToken, $oathuser ) ) {
+ return StatusValue::newFatal(
'oathauth-failedtovalidateoauth' );
+ }
+
+ return StatusValue::newGood();
+ }
+ return StatusValue::newGood( 'ignored' );
+ }
+
+ public function providerChangeAuthenticationData( AuthenticationRequest
$req ) {
+ if ( $req instanceof TOTPEnableAuthenticationRequest ) {
+ $repo = OATHAuthHooks::getOATHUserRepository();
+ $oathuser = $repo->findByUser( User::newFromName(
$req->username ) );
+
+ $oathuser->setKey( $req->key );
+ $repo->persist( $oathuser );
+ $this->manager->getRequest()->getSession()->remove(
'oathauth-key' );
+ // T132329 - should return 'oathauth-validatedoath'
+ } elseif ( $req instanceof TOTPDisableAuthenticationRequest ) {
+ $repo = OATHAuthHooks::getOATHUserRepository();
+ $oathuser = $repo->findByUser( User::newFromName(
$req->username ) );
+
+ $oathuser->setKey( null );
+ $repo->remove( $oathuser );
+ // T132329 - should return 'oathauth-disabledoath'
+ }
+ }
+
+ /**
+ * Fetches (and if necessary creates and stores) the pending TOTP
secret key that the user is
+ * about to enable but has not confirmed yet.
+ * @return OATHAuthKey
+ */
+ protected function getPendingKey() {
+ $session = $this->manager->getRequest()->getSession();
+ $key = $session->getSecret( 'oathauth-key' );
+ if ( $key === null ) {
+ $key = OATHAuthKey::newFromRandom();
+ $session->setSecret( 'oathauth-key', $key );
+ }
+ return $key;
+ }
}
diff --git a/extension.json b/extension.json
index f4081c9..bb85c32 100644
--- a/extension.json
+++ b/extension.json
@@ -21,7 +21,10 @@
"SpecialOATHLogin": "special/SpecialOATHLogin.php",
"ProxySpecialPage": "special/ProxySpecialPage.php",
"TOTPAuthenticationRequest":
"auth/TOTPAuthenticationRequest.php",
- "TOTPSecondaryAuthenticationProvider":
"auth/TOTPSecondaryAuthenticationProvider.php"
+ "TOTPEnableAuthenticationRequest":
"auth/TOTPEnableAuthenticationRequest.php",
+ "TOTPDisableAuthenticationRequest":
"auth/TOTPDisableAuthenticationRequest.php",
+ "TOTPSecondaryAuthenticationProvider":
"auth/TOTPSecondaryAuthenticationProvider.php",
+ "HTMLOATHField": "HTMLOATHField.php"
},
"ExtensionMessagesFiles": {
"OATHAuthAlias": "OATHAuth.alias.php"
@@ -72,5 +75,8 @@
"oathauth-enable": true
}
},
+ "RemoveCredentialsBlacklist": [
+ "TOTPDisableAuthenticationRequest"
+ ],
"manifest_version": 1
}
diff --git a/i18n/en.json b/i18n/en.json
index 3a8e123..8c15c40 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -14,6 +14,7 @@
"oathauth-enable": "Enable two-factor authentication",
"oathauth-failedtoenableoauth": "Failed to enable two-factor
authentication.",
"oathauth-alreadyenabled": "Two-factor authentication is already
enabled.",
+ "oathauth-notenabled": "Two-factor authentication is not enabled.",
"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",
@@ -51,5 +52,9 @@
"oathauth-auth-ui": "Please enter verification code from your mobile
app",
"oathauth-throttled": "Too many verification attempts! Please wait $1.",
"oathauth-login-failed": "Verification failed.",
- "oathauth-describe-provider": "Two-factor authentication (OATH)."
+ "oathauth-describe-provider": "Two-factor authentication (OATH)",
+ "oathauth-secret-label": "Secret key",
+ "oathauth-secret-help": "Two-factor secret key used to generate the
one-time passwords.",
+ "oathauth-scratch-codes-label": "Scratch codes",
+ "oathauth-scratch-codes-help": "A list of one-time passwords in case
the secret key gets lost."
}
diff --git a/i18n/qqq.json b/i18n/qqq.json
index 3a28752..e85e20b 100644
--- a/i18n/qqq.json
+++ b/i18n/qqq.json
@@ -17,8 +17,9 @@
"oathauth-legend": "Label for the legend on Special:OATH",
"oathauth-secret": "Plain text found on Special:OATH while enabling
OATH\n\nSee [https://en.wikipedia.org/wiki/Two_factor_authentication two factor
authentication]",
"oathauth-enable": "Page title on Special:OATH, when enabling
OATH.\n\nSee [https://en.wikipedia.org/wiki/Two_factor_authentication two
factor authentication]",
- "oathauth-failedtoenableoauth": "Plain text, found on Special:OATH when
failing to enable OATH.\n\nSee
[https://en.wikipedia.org/wiki/Two_factor_authentication two factor
authentication]",
- "oathauth-alreadyenabled": "Plain text, found on Special:OATH when
failing to enable OATH.\n\nSee
[https://en.wikipedia.org/wiki/Two_factor_authentication two factor
authentication]",
+ "oathauth-failedtoenableoauth": "Plain text, shown when failing to
enable OATH.\n\nSee [https://en.wikipedia.org/wiki/Two_factor_authentication
two factor authentication]",
+ "oathauth-alreadyenabled": "Plain text, shown when failing to disable
OATH.\n\nSee [https://en.wikipedia.org/wiki/Two_factor_authentication two
factor authentication]",
+ "oathauth-notenabled": "Plain text Two-factor authentication is not
enabled.",
"oathauth-verify": "Link, found on Special:OATH when no parameters are
passed.\n\nSee [https://en.wikipedia.org/wiki/Two_factor_authentication two
factor authentication]",
"openstackmanager-scratchtokens": "Plain text, found on Special:OATH
while enabling OATH.",
"oathauth-reset": "Page title for Special:OATH, when resetting
OATH.\n\nSee [https://en.wikipedia.org/wiki/Two_factor_authentication two
factor authentication]",
@@ -56,5 +57,9 @@
"oathauth-auth-ui": "Shown on top of the login form when second factor
is required",
"oathauth-throttled": "Error message when throttling limit is
hit.\n\nParameters:\n* $1 - throttle block duration",
"oathauth-login-failed": "Error message when verifying the second
factor failed.",
- "oathauth-describe-provider": "Description of the extension as an
authentication provider."
+ "oathauth-describe-provider": "Description of the extension as an
authentication provider.",
+ "oathauth-secret-label": "Field label for the OATH secret key field.",
+ "oathauth-secret-help": "API help message for the OATH secret key
field.",
+ "oathauth-scratch-codes-label": "Field label for the OATH scratch code
field.",
+ "oathauth-scratch-codes-help": "API help message for the OATH scratch
code field."
}
--
To view, visit https://gerrit.wikimedia.org/r/294861
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2c1a9bf87ca5643cc7c52acc92955177aa4bdfad
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/OATHAuth
Gerrit-Branch: master
Gerrit-Owner: Gergő Tisza <gti...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
