UI upgrade

Help messages for 2FA in general and for TOTP module are taken from Wikipedia.
Those could probably be improved, any suggestions are welcome

Bug: T218214
Bug: T226056
Change-Id: Ifc81a3c0e1adc9f6d0d49e7eee086714fc2c0f81

i18n/en.json

@@ -56,7 +56,7 @@
 	"oathauth-module-totp-label": "TOTP (one-time token)",
 	"oathauth-ui-manage": "Manage",
 	"oathmanage": "Manage Two-factor authentication",
-	"oathauth-ui-not-enabled-modules": "Available methods",
+	"oathauth-ui-not-enabled-modules": "Switch to an alternative method",
 	"oathauth-ui-enabled-module": "Enabled authentication method",
 	"oathauth-enable-generic": "Enable",
 	"oathauth-disable-generic": "Disable",
@@ -64,5 +64,8 @@
 	"oathauth-invalid-key-type": "Key set on the user does not match the required type for selected auth method",
 	"oathauth-disable-page-title": "Disable $1",
 	"oathauth-enable-page-title": "Enable $1",
-	"oathauth-action-exclusive-to-2fa": "This action can only be performed by users with Two-factor authentication enabled."
+	"oathauth-action-exclusive-to-2fa": "This action can only be performed by users with Two-factor authentication enabled.",
+	"oathauth-ui-available-modules": "Available methods",
+	"oathauth-ui-general-help": "'''Multi-factor authentication''' ('''MFA''') is an authentication method in which a computer user is granted access only after successfully presenting two or more pieces of evidence (or factors) to an authentication mechanism: knowledge (something the user and only the user knows), possession (something the user and only the user has), and inherence (something the user and only the user is). [https://en.wikipedia.org/wiki/Multi-factor_authentication Read more]",
+	"oathauth-totp-description": "The Time-based One-Time Password algorithm (TOTP) is an extension of the HMAC-based One-time Password algorithm (HOTP) generating a one-time password by instead taking uniqueness from the current time."
 }

i18n/qqq.json

@@ -65,7 +65,7 @@
 	"oathauth-module-totp-label": "User preference value when the TOTP module is enabled",
 	"oathauth-ui-manage": "Button on Special:Preferences, that leads to [[Special:OATHManage]]",
 	"oathmanage": "{{optional}}\n{{doc-special|OATHManage}}",
-	"oathauth-ui-not-enabled-modules": "Header on Special:OATHManage under which all available, but not currently enabled, modules are listed",
+	"oathauth-ui-not-enabled-modules": "Header on Special:OATHManage under which all modules that are not currently enabled are listed. This is used only if user already has 2FA enabled",
 	"oathauth-ui-enabled-module": "Header on Special:OATHManage under which is the currently selected 2FA module shown",
 	"oathauth-enable-generic": "Button label that enables the module\n{{Identical|Enable}}",
 	"oathauth-disable-generic": "Button label that disables the module\n{{Identical|Disable}}",
@@ -73,5 +73,8 @@
 	"oathauth-invalid-key-type": "Error message when key class is unexpected",
 	"oathauth-disable-page-title": "Page title for [[Special:OATHManage]] when user is disabling a module. \n$1 - display name of the module",
 	"oathauth-enable-page-title": "Page title for [[Special:OATHManage]] when user is enabling a module. \n$1 - display name of the module",
-	"oathauth-action-exclusive-to-2fa": "Error messages when user action is blocked due to the permission being exclusive to users with 2FA enabled"
+	"oathauth-action-exclusive-to-2fa": "Error messages when user action is blocked due to the permission being exclusive to users with 2FA enabled",
+	"oathauth-ui-available-modules": "Header on Special:OATHManage under which all 2FA modules that can be enabled. Used only when user does not have 2FA enabled",
+	"oathauth-ui-general-help": "Help on what is two-factor authentication, displayed on Special:OATHManage when user does not have any module enabled",
+	"oathauth-totp-description": "Description of TOTP module, shown on Special:OATHManage"
 }

src/HTMLForm/IManageForm.php

@@ -3,6 +3,7 @@
 namespace MediaWiki\Extension\OATHAuth\HTMLForm;
 
 use HTMLForm;
+use OOUI\Layout;
 use Title;
 use Status;
 
@@ -34,9 +35,10 @@ interface IManageForm {
 	public function setSubmitCallback( $cb );
 
 	/**
+	 * @param Layout|null $layout
 	 * @return bool|Status
 	 */
-	public function show();
+	public function show( $layout = null );
 
 	/**
 	 * @param array $formData

src/HTMLForm/OATHAuthOOUIHTMLForm.php

@@ -6,6 +6,11 @@ use MediaWiki\Extension\OATHAuth\IModule;
 use MediaWiki\Extension\OATHAuth\OATHUser;
 use MediaWiki\Extension\OATHAuth\OATHUserRepository;
 use MediaWiki\Logger\LoggerFactory;
+use OOUI\FieldsetLayout;
+use OOUI\HtmlSnippet;
+use OOUI\Layout;
+use OOUI\PanelLayout;
+use OOUI\Widget;
 use OOUIHTMLForm;
 use Psr\Log\LoggerInterface;
 
@@ -28,6 +33,23 @@ abstract class OATHAuthOOUIHTMLForm extends OOUIHTMLForm implements IManageForm
 	 */
 	protected $logger;
 
+	/**
+	 * @var Layout|null
+	 */
+	protected $layoutContainer = null;
+
+	/**
+	 * Make the form-wrapper panel padded
+	 * @var bool
+	 */
+	protected $panelPadded = true;
+
+	/**
+	 * Make the form-wrapper panel framed
+	 * @var bool
+	 */
+	protected $panelFramed = true;
+
 	/**
 	 * Initialize the form
 	 *
@@ -44,6 +66,27 @@ abstract class OATHAuthOOUIHTMLForm extends OOUIHTMLForm implements IManageForm
 		parent::__construct( $this->getDescriptors(), null, "oathauth" );
 	}
 
+	/**
+	 * @inheritDoc
+	 */
+	public function show( $layout = null ) {
+		$this->layoutContainer = $layout;
+		return parent::show();
+	}
+
+	/**
+	 * @inheritDoc
+	 */
+	public function displayForm( $submitResult ) {
+		if ( !$this->layoutContainer instanceof Layout ) {
+			return parent::displayForm( $submitResult );
+		}
+
+		$this->layoutContainer->appendContent( new HtmlSnippet(
+			$this->getHTML( $submitResult )
+		) );
+	}
+
 	/**
 	 * @return array
 	 */
@@ -58,6 +101,32 @@ abstract class OATHAuthOOUIHTMLForm extends OOUIHTMLForm implements IManageForm
 		return LoggerFactory::getInstance( 'authentication' );
 	}
 
+	protected function wrapFieldSetSection( $legend, $section, $attributes, $isRoot ) {
+		// to get a user visible effect, wrap the fieldset into a framed panel layout
+		$layout = new PanelLayout( array_merge( [
+			'expanded' => false,
+			'padded' => true,
+			'framed' => false,
+			'infusable' => false,
+		], [
+			'padded' => $this->panelPadded,
+			'framed' => $this->panelFramed
+		] ) );
+
+		$layout->appendContent(
+			new FieldsetLayout( [
+				'label' => $legend,
+				'infusable' => false,
+				'items' => [
+					new Widget( [
+						'content' => new HtmlSnippet( $section )
+					] ),
+				],
+			] + $attributes )
+		);
+		return $layout;
+	}
+
 	/**
 	 * @param array $formData
 	 * @return array|bool

src/IModule.php

@@ -4,6 +4,7 @@ namespace MediaWiki\Extension\OATHAuth;
 
 use MediaWiki\Auth\SecondaryAuthenticationProvider;
 use MediaWiki\Extension\OATHAuth\HTMLForm\IManageForm;
+use Message;
 
 interface IModule {
 	/**
@@ -13,7 +14,7 @@ interface IModule {
 	public function getName();
 
 	/**
-	 * @return \Message
+	 * @return Message
 	 */
 	public function getDisplayName();
 
@@ -61,4 +62,10 @@ interface IModule {
 	 * @return IManageForm|null if no form is available for given action
 	 */
 	public function getManageForm( $action, OATHUser $user, OATHUserRepository $repo );
+
+	/**
+	 * Return Message object for the short text to be displayed as description
+	 * @return Message
+	 */
+	public function getDescriptionMessage();
 }

src/Module/TOTP.php

@@ -123,4 +123,11 @@ class TOTP implements IModule {
 	public function getConfig() {
 		return null;
 	}
+
+	/**
+	 * @inheritDoc
+	 */
+	public function getDescriptionMessage() {
+		return wfMessage( 'oathauth-totp-description' );
+	}
 }

src/Special/OATHManage.php

@@ -29,9 +29,15 @@ use OOUI\ButtonWidget;
 use OOUI\HorizontalLayout;
 use Message;
 use Html;
+use OOUI\HtmlSnippet;
+use OOUI\PanelLayout;
 use SpecialPage;
 use OOUI\LabelWidget;
 use HTMLForm;
+use ConfigException;
+use MWException;
+use PermissionsError;
+use UserNotLoggedIn;
 
 class OATHManage extends SpecialPage {
 	const ACTION_ENABLE = 'enable';
@@ -49,28 +55,20 @@ class OATHManage extends SpecialPage {
 	 * @var OATHUser
 	 */
 	protected $authUser;
-	/**
-	 * @var string
-	 */
-	protected $enabledModule;
 	/**
 	 * @var string
 	 */
 	protected $action;
 	/**
-	 * @var string
+	 * @var IModule
 	 */
 	protected $requestedModule;
-	/**
-	 * @var bool
-	 */
-	protected $genericActionHandled = false;
 
 	/**
 	 * Initializes a page to manage available 2FA modules
 	 *
-	 * @throws \ConfigException
-	 * @throws \MWException
+	 * @throws ConfigException
+	 * @throws MWException
 	 */
 	public function __construct() {
 		parent::__construct( 'OATHManage', 'oathauth-enable' );
@@ -83,6 +81,7 @@ class OATHManage extends SpecialPage {
 
 	/**
 	 * @param null|string $subPage
+	 * @return void
 	 */
 	public function execute( $subPage ) {
 		parent::execute( $subPage );
@@ -90,29 +89,38 @@ class OATHManage extends SpecialPage {
 		$this->getOutput()->enableOOUI();
 		$this->setAction();
 		$this->setModule();
-		$this->addEnabledModule();
-		if ( $this->isGenericAction() && $this->genericActionHandled ) {
-			// If generic action is handled by the enabled module
+
+		if ( $this->requestedModule instanceof IModule ) {
+			// Performing an action on a requested module
+			$this->clearPage();
+			return $this->addModuleHTML( $this->requestedModule );
+		}
+
+		$this->addGeneralHelp();
+		if ( $this->hasEnabled() ) {
+			$this->addEnabledHTML();
+			if ( $this->hasAlternativeModules() ) {
+				$this->addAlternativesHTML();
+			}
 			return;
 		}
-		$this->addNotEnabledModules();
+		$this->nothingEnabled();
 	}
 
 	/**
-	 * @throws \PermissionsError
-	 * @throws \UserNotLoggedIn
+	 * @throws PermissionsError
+	 * @throws UserNotLoggedIn
 	 */
 	public function checkPermissions() {
 		$this->requireLogin();
 
 		$canEnable = $this->getUser()->isAllowed( 'oathauth-enable' );
 
-		if ( $this->action && $this->action === static::ACTION_ENABLE && !$canEnable ) {
+		if ( $this->action === static::ACTION_ENABLE && !$canEnable ) {
 			$this->displayRestrictionError();
 		}
 
-		$hasEnabled = $this->authUser->getModule() instanceof IModule;
-		if ( !$hasEnabled && !$canEnable ) {
+		if ( !$this->hasEnabled() && !$canEnable ) {
 			// No enabled module and cannot enable - nothing to do
 			$this->displayRestrictionError();
 		}
@@ -123,35 +131,114 @@ class OATHManage extends SpecialPage {
 	}
 
 	private function setModule() {
-		$this->requestedModule = $this->getRequest()->getVal( 'module', '' );
+		$moduleKey = $this->getRequest()->getVal( 'module', '' );
+		$this->requestedModule = $this->auth->getModuleByKey( $moduleKey );
 	}
 
-	protected function addEnabledModule() {
-		$module = $this->authUser->getModule();
-		if ( $module !== null ) {
-			if ( !$this->requestedModule ) {
-				$this->requestedModule = $module->getName();
+	private function hasEnabled() {
+		return $this->authUser->getModule() instanceof IModule;
+	}
+
+	private function getEnabled() {
+		return $this->hasEnabled() ? $this->authUser->getModule() : null;
+	}
+
+	private function addEnabledHTML() {
+		$this->addHeading( wfMessage( 'oathauth-ui-enabled-module' ) );
+		$this->addModuleHTML( $this->getEnabled() );
+	}
+
+	private function addAlternativesHTML() {
+		$this->addHeading( wfMessage( 'oathauth-ui-not-enabled-modules' ) );
+		$this->addInactiveHTML();
+	}
+
+	private function nothingEnabled() {
+		$this->addHeading( wfMessage( 'oathauth-ui-available-modules' ) );
+		$this->addInactiveHTML();
+	}
+
+	private function addInactiveHTML() {
+		foreach ( $this->auth->getAllModules() as $key => $module ) {
+			if ( $this->isModuleEnabled( $module ) ) {
+				continue;
 			}
-			$this->enabledModule = $module->getName();
-			$this->addHeading(
-				wfMessage( 'oathauth-ui-enabled-module' )
-			);
-			$this->addModule( $module, true );
+			$this->addModuleHTML( $module );
 		}
 	}
 
-	protected function addNotEnabledModules() {
-		$headerAdded = false;
-		foreach ( $this->auth->getAllModules() as $key => $module ) {
-			if ( $this->enabledModule && $key === $this->enabledModule ) {
-				continue;
-			}
-			if ( !$headerAdded ) {
-				// To avoid adding header for an empty section
-				$this->addHeading( wfMessage( 'oathauth-ui-not-enabled-modules' ) );
-				$headerAdded = true;
-			}
-			$this->addModule( $module, false );
+	private function addGeneralHelp() {
+		$this->getOutput()->addHTML( wfMessage(
+			'oathauth-ui-general-help'
+		)->parseAsBlock() );
+	}
+
+	private function addModuleHTML( IModule $module ) {
+		if ( $this->isModuleRequested( $module ) ) {
+			return $this->addCustomContent( $module );
+		}
+
+		$panel = $this->getGenericContent( $module );
+		if ( $this->isModuleEnabled( $module ) ) {
+			$this->addCustomContent( $module, $panel );
+		}
+
+		return $this->getOutput()->addHTML( (string)$panel );
+	}
+
+	/**
+	 * Get the panel with generic content for a module
+	 *
+	 * @param IModule $module
+	 * @return PanelLayout
+	 */
+	private function getGenericContent( IModule $module ) {
+		$modulePanel = new PanelLayout( [
+			'framed' => true,
+			'expanded' => false,
+			'padded' => true
+		] );
+		$headerLayout = new HorizontalLayout();
+
+		$label = new LabelWidget( [
+			'label' => $module->getDisplayName()->text()
+		] );
+		if ( $this->shouldShowGenericButtons() ) {
+			$button = new ButtonWidget( [
+				'label' => $this->isModuleEnabled( $module ) ?
+					wfMessage( 'oathauth-disable-generic' )->text() :
+					wfMessage( 'oathauth-enable-generic' )->text(),
+				'href' => $this->getOutput()->getTitle()->getLocalURL( [
+					'action' => $this->isModuleEnabled( $module ) ?
+						static::ACTION_DISABLE : static::ACTION_ENABLE,
+					'module' => $module->getName()
+				] )
+			] );
+			$headerLayout->addItems( [ $button ] );
+		}
+		$headerLayout->addItems( [ $label ] );
+
+		$modulePanel->appendContent( $headerLayout );
+		$modulePanel->appendContent( new HtmlSnippet(
+			$module->getDescriptionMessage()->parseAsBlock()
+		) );
+		return $modulePanel;
+	}
+
+	/**
+	 * @param IModule $module
+	 * @param PanelLayout|null $panel
+	 */
+	private function addCustomContent( IModule $module, $panel = null ) {
+		$form = $module->getManageForm( $this->action, $this->authUser, $this->userRepo );
+		if ( $form === null || !$this->isValidFormType( $form ) ) {
+			return;
+		}
+		$form->setTitle( $this->getOutput()->getTitle() );
+		$this->ensureRequiredFormFields( $form, $module );
+		$form->setSubmitCallback( [ $form, 'onSubmit' ] );
+		if ( $form->show( $panel ) ) {
+			$form->onSuccess();
 		}
 	}
 
@@ -159,83 +246,30 @@ class OATHManage extends SpecialPage {
 		$this->getOutput()->addHTML( Html::element( 'h2', [], $message->text() ) );
 	}
 
-	private function addModule( IModule $module, $enabled ) {
-		if ( $this->genericActionHandled ) {
-			// If generic action is handled by previous (non-enabled) module
-			return;
+	private function shouldShowGenericButtons() {
+		if ( !$this->requestedModule instanceof IModule ) {
+			return true;
 		}
-		$this->addModuleGeneric( $module, $enabled );
-		if ( $this->requestedModule === $module->getName() ) {
-			$this->addModuleCustomForm( $module, $enabled );
+		if ( !$this->isGenericAction() ) {
+			return true;
 		}
+		return false;
 	}
 
-	/**
-	 * Add module name and generic controls
-	 *
-	 * @param IModule $module
-	 * @param boolean $enabled
-	 */
-	private function addModuleGeneric( IModule $module, $enabled ) {
-		$layout = new HorizontalLayout();
-		$label = new LabelWidget( [
-			'label' => $module->getDisplayName()->text()
-		] );
-
-		if ( $this->requestedModule !== $module->getName() || !$this->isGenericAction() ) {
-			// Add a generic action button
-			$button = new ButtonWidget( [
-				'label' => $enabled ?
-					wfMessage( 'oathauth-disable-generic' )->text() :
-					wfMessage( 'oathauth-enable-generic' )->text(),
-				'href' => $this->getOutput()->getTitle()->getLocalURL( [
-					'action' => $enabled ? static::ACTION_DISABLE : static::ACTION_ENABLE,
-					'module' => $module->getName()
-				] )
-			] );
-			$layout->addItems( [ $button ] );
+	private function isModuleRequested( IModule $module ) {
+		if ( $this->requestedModule instanceof IModule ) {
+			if ( $this->requestedModule->getName() === $module->getName() ) {
+				return true;
+			}
 		}
-		$layout->addItems( [ $label ] );
-		$this->getOutput()->addHTML( (string)$layout );
+		return false;
 	}
 
-	/**
-	 * @param IModule $module
-	 * @param bool $enabled
-	 */
-	private function addModuleCustomForm( IModule $module, $enabled ) {
-		$form = $module->getManageForm( $this->action, $this->authUser, $this->userRepo );
-		if ( $form === null || !$this->isValidFormType( $form ) ) {
-			return;
+	private function isModuleEnabled( IModule $module ) {
+		if ( $this->getEnabled() instanceof IModule ) {
+			return $this->getEnabled()->getName() === $module->getName();
 		}
-		if ( $this->isGenericAction() ) {
-			$this->setUpGenericAction( $module, $enabled );
-		}
-		$form->setTitle( $this->getOutput()->getTitle() );
-		$this->ensureRequiredFormFields( $form, $module );
-		$form->setSubmitCallback( [ $form, 'onSubmit' ] );
-		if ( $form->show() ) {
-			$form->onSuccess();
-		}
-	}
-
-	/**
-	 * Generic actions (disable||enable) should not be handled in-line,
-	 * so we clear the content to display only the form
-	 *
-	 * @param IModule $module
-	 * @param bool $enabled
-	 */
-	protected function setUpGenericAction( IModule $module, $enabled ) {
-		$pageTitle = $enabled ?
-			wfMessage( 'oathauth-disable-page-title', $module->getDisplayName() )->text() :
-			wfMessage( 'oathauth-enable-page-title', $module->getDisplayName() )->text();
-
-		$this->getOutput()->clearHTML();
-		$this->getOutput()->setPageTitle( $pageTitle );
-		$this->getOutput()->addBacklinkSubtitle( $this->getOutput()->getTitle() );
-
-		$this->genericActionHandled = true;
+		return false;
 	}
 
 	/**
@@ -244,7 +278,7 @@ class OATHManage extends SpecialPage {
 	 * @param mixed $form
 	 * @return bool
 	 */
-	protected function isValidFormType( $form ) {
+	private function isValidFormType( $form ) {
 		if ( !( $form instanceof HTMLForm ) ) {
 			return false;
 		}
@@ -256,21 +290,11 @@ class OATHManage extends SpecialPage {
 		return true;
 	}
 
-	/**
-	 * Is the requested action a generic one or module-specific
-	 *
-	 * @return bool
-	 */
-	protected function isGenericAction() {
-		return $this->action &&
-			in_array( $this->action, [ static::ACTION_DISABLE, static::ACTION_ENABLE ] );
-	}
-
 	/**
 	 * @param IManageForm &$form
 	 * @param IModule $module
 	 */
-	protected function ensureRequiredFormFields( IManageForm &$form, IModule $module ) {
+	private function ensureRequiredFormFields( IManageForm &$form, IModule $module ) {
 		if ( !$form->hasField( 'module' ) ) {
 			$form->addHiddenField( 'module', $module->getName() );
 		}
@@ -278,4 +302,39 @@ class OATHManage extends SpecialPage {
 			$form->addHiddenField( 'action', $this->action );
 		}
 	}
+
+	/**
+	 * When performing an action on a module (like enable/disable),
+	 * page should contain only form for that action
+	 */
+	private function clearPage() {
+		if ( $this->isGenericAction() ) {
+			$displayName = $this->requestedModule->getDisplayName();
+			$pageTitle = $this->isModuleEnabled( $this->requestedModule ) ?
+				wfMessage( 'oathauth-disable-page-title', $displayName )->text() :
+				wfMessage( 'oathauth-enable-page-title', $displayName )->text();
+			$this->getOutput()->setPageTitle( $pageTitle );
+		}
+
+		$this->getOutput()->clearHTML();
+		$this->getOutput()->addBacklinkSubtitle( $this->getOutput()->getTitle() );
+	}
+
+	/**
+	 * Actions enable and disable are generic and all modules must
+	 * implement them, while all other actions are module-specific
+	 */
+	private function isGenericAction() {
+		return in_array( $this->action, [ static::ACTION_ENABLE, static::ACTION_DISABLE ] );
+	}
+
+	private function hasAlternativeModules() {
+		foreach ( $this->auth->getAllModules() as $key => $module ) {
+			if ( !$this->isModuleEnabled( $module ) ) {
+				return true;
+			}
+		}
+		return false;
+	}
+
 }