Various minor cleanup

Change-Id: Idbf84a1f49f1afbd2d3a342cedd72895c5378bc6

maintenance/UpdateForMultipleDevicesSupport.php

@@ -88,7 +88,7 @@ class UpdateForMultipleDevicesSupport extends LoggedUpdateMaintenance {
 				$decodedData = FormatJson::decode( $row->data, true );
 
 				if ( isset( $decodedData['keys'] ) ) {
-					$updated += 1;
+					$updated++;
 
 					foreach ( $decodedData['keys'] as $keyData ) {
 						$toAdd[] = [

maintenance/disableOATHAuthForUser.php

@@ -22,8 +22,9 @@ class DisableOATHAuthForUser extends Maintenance {
 	public function execute() {
 		$username = $this->getArg( 0 );
 
-		$user = User::newFromName( $username );
-		if ( $user && $user->getId() === 0 ) {
+		$user = MediaWikiServices::getInstance()->getUserFactory()
+			->newFromName( $username );
+		if ( $user === null || $user->getId() === 0 ) {
 			$this->fatalError( "User $username doesn't exist!" );
 		}
 
@@ -35,8 +36,9 @@ class DisableOATHAuthForUser extends Maintenance {
 		}
 
 		$repo->remove( $oathUser, 'Maintenance script', false );
-		// Kill all existing sessions. If this disable was social-engineered by an attacker,
-		// the legitimate user will hopefully login again and notice that the second factor
+		// Kill all existing sessions.
+		// If this request to disable 2FA was social-engineered by an attacker,
+		// the legitimate user will hopefully log in again to the wiki, and notice that the second factor
 		// is missing or different, and alert the operators.
 		SessionManager::singleton()->invalidateSessionsForUser( $user );
 

maintenance/updateDatabaseValueFormat.php

@@ -37,7 +37,7 @@ require_once "$IP/maintenance/Maintenance.php";
 class UpdateDatabaseValueFormat extends Maintenance {
 	public function __construct() {
 		parent::__construct();
-		$this->addDescription( 'Script to convert old, TOTP specific, column values to new structure' );
+		$this->addDescription( 'Script to convert old, TOTP specific, column values to a newer structure' );
 		$this->requireExtension( 'OATHAuth' );
 	}
 

src/Api/Module/ApiOATHValidate.php

@@ -24,7 +24,6 @@ use FormatJson;
 use MediaWiki\Extension\OATHAuth\IModule;
 use MediaWiki\Logger\LoggerFactory;
 use MediaWiki\MediaWikiServices;
-use User;
 use Wikimedia\ParamValidator\ParamValidator;
 
 /**
@@ -36,17 +35,17 @@ use Wikimedia\ParamValidator\ParamValidator;
 class ApiOATHValidate extends ApiBase {
 	public function execute() {
 		$this->requirePostedParameters( [ 'token', 'data' ] );
+		$this->checkUserRightsAny( 'oathauth-api-all' );
 
 		$params = $this->extractRequestParams();
 		if ( $params['user'] === null ) {
-			$params['user'] = $this->getUser()->getName();
-		}
-
-		$this->checkUserRightsAny( 'oathauth-api-all' );
-
-		$user = User::newFromName( $params['user'] );
-		if ( $user === false ) {
-			$this->dieWithError( 'noname' );
+			$user = $this->getUser();
+		} else {
+			$user = MediaWikiServices::getInstance()->getUserFactory()
+				->newFromName( $params['user'] );
+			if ( $user === null ) {
+				$this->dieWithError( 'noname' );
+			}
 		}
 
 		// Don't increase pingLimiter, just check for limit exceeded.
@@ -112,7 +111,7 @@ class ApiOATHValidate extends ApiBase {
 			],
 			'data' => [
 				ParamValidator::PARAM_TYPE => 'string',
-				ApiBase::PARAM_REQUIRED => true,
+				ParamValidator::PARAM_REQUIRED => true,
 			]
 		];
 	}

src/Api/Module/ApiQueryOATH.php

@@ -23,7 +23,6 @@ use ApiQueryBase;
 use ApiResult;
 use ManualLogEntry;
 use MediaWiki\MediaWikiServices;
-use User;
 use Wikimedia\ParamValidator\ParamValidator;
 
 /**
@@ -46,13 +45,10 @@ class ApiQueryOATH extends ApiQueryBase {
 	}
 
 	public function execute() {
-		$params = $this->extractRequestParams();
-		if ( $params['user'] === null ) {
-			$params['user'] = $this->getUser()->getName();
-		}
-
 		$this->checkUserRightsAny( [ 'oathauth-api-all', 'oathauth-verify-user' ] );
 
+		$params = $this->extractRequestParams();
+
 		$hasOAthauthApiAll = $this->getPermissionManager()
 			->userHasRight(
 				$this->getUser(),
@@ -64,9 +60,14 @@ class ApiQueryOATH extends ApiQueryBase {
 			$this->dieWithError( [ 'apierror-missingparam', 'reason' ] );
 		}
 
-		$user = User::newFromName( $params['user'] );
-		if ( $user === false ) {
-			$this->dieWithError( 'noname' );
+		if ( $params['user'] === null ) {
+			$user = $this->getUser();
+		} else {
+			$user = MediaWikiServices::getInstance()->getUserFactory()
+				->newFromName( $params['user'] );
+			if ( $user === null ) {
+				$this->dieWithError( 'noname' );
+			}
 		}
 
 		$result = $this->getResult();

src/Auth/SecondaryAuthenticationProvider.php

@@ -48,8 +48,8 @@ class SecondaryAuthenticationProvider extends AbstractSecondaryAuthenticationPro
 			return AuthenticationResponse::newAbstain();
 		}
 
-		$provider = $this->getProviderForModule( $module );
-		return $provider->beginSecondaryAuthentication( $user, $reqs );
+		return $this->getProviderForModule( $module )
+			->beginSecondaryAuthentication( $user, $reqs );
 	}
 
 	/**

src/Auth/TOTPSecondaryAuthenticationProvider.php

@@ -45,13 +45,8 @@ class TOTPSecondaryAuthenticationProvider extends AbstractSecondaryAuthenticatio
 	 * @return array
 	 */
 	public function getAuthenticationRequests( $action, array $options ) {
-		switch ( $action ) {
-			case AuthManager::ACTION_LOGIN:
-				// don't ask for anything initially so the second factor is on a separate screen
-				return [];
-			default:
-				return [];
-		}
+		// don't ask for anything initially, so the second factor is on a separate screen
+		return [];
 	}
 
 	/**
@@ -68,10 +63,12 @@ class TOTPSecondaryAuthenticationProvider extends AbstractSecondaryAuthenticatio
 
 		if ( !( $authUser->getModule() instanceof TOTP ) ) {
 			return AuthenticationResponse::newAbstain();
-		} else {
-			return AuthenticationResponse::newUI( [ new TOTPAuthenticationRequest() ],
-				wfMessage( 'oathauth-auth-ui' ), 'warning' );
 		}
+
+		return AuthenticationResponse::newUI(
+			[ new TOTPAuthenticationRequest() ],
+			wfMessage( 'oathauth-auth-ui' ),
+		);
 	}
 
 	/**
@@ -91,8 +88,9 @@ class TOTPSecondaryAuthenticationProvider extends AbstractSecondaryAuthenticatio
 		$token = $request->OATHToken;
 
 		if ( !( $authUser->getModule() instanceof TOTP ) ) {
-			$this->logger->warning( 'Two-factor authentication was disabled mid-authentication for '
-				. $user->getName() );
+			$this->logger->warning( 'Two-factor authentication was disabled mid-authentication for {user}', [
+				'user' => $user->getName(),
+			] );
 			return AuthenticationResponse::newAbstain();
 		}
 
@@ -109,18 +107,21 @@ class TOTPSecondaryAuthenticationProvider extends AbstractSecondaryAuthenticatio
 
 		if ( $authUser->getModule()->verify( $authUser, [ 'token' => $token ] ) ) {
 			return AuthenticationResponse::newPass();
-		} else {
-			// Increase rate limit counter for failed request
-			$user->pingLimiter( 'badoath' );
-
-			$this->logger->info( 'OATHAuth user {user} failed OTP/scratch token from {clientip}', [
-				'user'     => $user,
-				'clientip' => $user->getRequest()->getIP(),
-			] );
-
-			return AuthenticationResponse::newUI( [ new TOTPAuthenticationRequest() ],
-				wfMessage( 'oathauth-login-failed' ), 'error' );
 		}
+
+		// Increase rate limit counter for failed request
+		$user->pingLimiter( 'badoath' );
+
+		$this->logger->info( 'OATHAuth user {user} failed OTP/scratch token from {clientip}', [
+			'user'     => $user->getName(),
+			'clientip' => $user->getRequest()->getIP(),
+		] );
+
+		return AuthenticationResponse::newUI(
+			[ new TOTPAuthenticationRequest() ],
+			wfMessage( 'oathauth-login-failed' ),
+			'error'
+		);
 	}
 
 	/**

src/HTMLForm/OATHAuthOOUIHTMLForm.php

@@ -102,10 +102,7 @@ abstract class OATHAuthOOUIHTMLForm extends OOUIHTMLForm implements IManageForm
 		return [];
 	}
 
-	/**
-	 * @return LoggerInterface
-	 */
-	private function getLogger() {
+	private function getLogger(): LoggerInterface {
 		return LoggerFactory::getInstance( 'authentication' );
 	}
 
@@ -114,15 +111,12 @@ abstract class OATHAuthOOUIHTMLForm extends OOUIHTMLForm implements IManageForm
 	 */
 	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( [
+		$layout = new PanelLayout( [
 			'expanded' => false,
-			'padded' => true,
-			'framed' => false,
 			'infusable' => false,
-		], [
 			'padded' => $this->panelPadded,
-			'framed' => $this->panelFramed
-		] ) );
+			'framed' => $this->panelFramed,
+		] );
 
 		$layout->appendContent(
 			new FieldsetLayout( [

src/HTMLForm/TOTPDisableForm.php

@@ -7,7 +7,7 @@ use MediaWiki\Logger\LoggerFactory;
 use Message;
 use MWException;
 
-class TOTPDisableForm extends OATHAuthOOUIHTMLForm implements IManageForm {
+class TOTPDisableForm extends OATHAuthOOUIHTMLForm {
 	/**
 	 * Add content to output when operation was successful
 	 */
@@ -39,7 +39,7 @@ class TOTPDisableForm extends OATHAuthOOUIHTMLForm implements IManageForm {
 	 * @throws MWException
 	 */
 	public function onSubmit( array $formData ) {
-		// Don't increase pingLimiter, just check for limit exceeded.
+		// Don't increase pingLimiter, instead check for the limit being exceeded.
 		if ( $this->oathUser->getUser()->pingLimiter( 'badoath', 0 ) ) {
 			// Arbitrary duration given here
 			LoggerFactory::getInstance( 'authentication' )->info(
@@ -52,20 +52,21 @@ class TOTPDisableForm extends OATHAuthOOUIHTMLForm implements IManageForm {
 		}
 
 		$module = $this->oathUser->getModule();
-		if ( $module instanceof TOTP ) {
-			if ( !$module->verify( $this->oathUser, [ 'token' => $formData['token'] ] ) ) {
-				LoggerFactory::getInstance( 'authentication' )->info(
-					'OATHAuth {user} failed to provide a correct token while disabling 2FA from {clientip}', [
-						'user' => $this->getUser()->getName(),
-						'clientip' => $this->getRequest()->getIP(),
-					]
-				);
+		if (
+			( $module instanceof TOTP ) &&
+			!$module->verify( $this->oathUser, [ 'token' => $formData['token'] ] )
+		) {
+			LoggerFactory::getInstance( 'authentication' )->info(
+				'OATHAuth {user} failed to provide a correct token while disabling 2FA from {clientip}', [
+					'user' => $this->getUser()->getName(),
+					'clientip' => $this->getRequest()->getIP(),
+				]
+			);
 
-				// Increase rate limit counter for failed request
-				$this->getUser()->pingLimiter( 'badoath' );
+			// Increase rate limit counter for failed request
+			$this->getUser()->pingLimiter( 'badoath' );
 
-				return [ 'oathauth-failedtovalidateoath' ];
-			}
+			return [ 'oathauth-failedtovalidateoath' ];
 		}
 
 		$this->oathUser->setKeys();

src/HTMLForm/TOTPEnableForm.php

@@ -2,12 +2,14 @@
 
 namespace MediaWiki\Extension\OATHAuth\HTMLForm;
 
+use ConfigException;
 use Html;
 use MediaWiki\Extension\OATHAuth\Key\TOTPKey;
 use MediaWiki\Logger\LoggerFactory;
+use MWException;
 use Status;
 
-class TOTPEnableForm extends OATHAuthOOUIHTMLForm implements IManageForm {
+class TOTPEnableForm extends OATHAuthOOUIHTMLForm {
 	/**
 	 * @param array|bool|Status|string $submitResult
 	 * @return string
@@ -151,8 +153,8 @@ class TOTPEnableForm extends OATHAuthOOUIHTMLForm implements IManageForm {
 	/**
 	 * @param array $formData
 	 * @return array|bool
-	 * @throws \ConfigException
-	 * @throws \MWException
+	 * @throws ConfigException
+	 * @throws MWException
 	 */
 	public function onSubmit( array $formData ) {
 		$keyData = $this->getRequest()->getSessionData( 'oathauth_totp_key' ) ?? [];

src/Hook/HookHandler.php

@@ -14,6 +14,7 @@ use MediaWiki\SpecialPage\Hook\AuthChangeFormFieldsHook;
 use MediaWiki\Title\Title;
 use MediaWiki\User\Hook\UserEffectiveGroupsHook;
 use MediaWiki\User\UserGroupManager;
+use Message;
 use OOUI\ButtonWidget;
 use OOUI\HorizontalLayout;
 use OOUI\LabelWidget;
@@ -97,9 +98,8 @@ class HookHandler implements
 	public function onGetPreferences( $user, &$preferences ) {
 		$oathUser = $this->userRepo->findByUser( $user );
 
-		// If there is no existing module in user, and the user is not allowed to enable it,
+		// If there is no existing module for the user, and the user is not allowed to enable it,
 		// we have nothing to show.
-
 		if (
 			$oathUser->getModule() === null &&
 			!$this->permissionManager->userHasRight( $user, 'oathauth-enable' )
@@ -149,9 +149,9 @@ class HookHandler implements
 			$disabledInfo = [ 'oathauth-disabledgroups' => [
 				'type' => 'info',
 				'label-message' => [ 'oathauth-prefs-disabledgroups',
-					\Message::numParam( count( $disabledGroups ) ) ],
+					Message::numParam( count( $disabledGroups ) ) ],
 				'help-message' => [ 'oathauth-prefs-disabledgroups-help',
-					\Message::numParam( count( $disabledGroups ) ), $user->getName() ],
+					Message::numParam( count( $disabledGroups ) ), $user->getName() ],
 				'default' => $info,
 				'raw' => true,
 				'section' => 'personal/info',
@@ -185,9 +185,9 @@ class HookHandler implements
 		if ( $oathUser->getModule() === null ) {
 			// Not enabled, strip the groups
 			return $intersect;
-		} else {
-			return [];
 		}
+
+		return [];
 	}
 
 	/**
@@ -242,7 +242,7 @@ class HookHandler implements
 
 		$dbGroups = $this->userGroupManager->getUserGroups( $user );
 		if ( $this->getDisabledGroups( $user, $dbGroups ) ) {
-			// Has some disabled groups, add oathauth-enable
+			// User has some disabled groups, add oathauth-enable
 			$rights[] = 'oathauth-enable';
 		}
 	}

src/Hook/UpdateTables.php

@@ -107,7 +107,7 @@ class UpdateTables implements LoadExtensionSchemaUpdatesHook {
 	}
 
 	/**
-	 * Helper function for converting single TOTP keys to multi-key system
+	 * Helper function for converting single TOTP keys to the multi-key system
 	 * @param DatabaseUpdater $updater
 	 * @return bool
 	 * @throws ConfigException
@@ -117,7 +117,7 @@ class UpdateTables implements LoadExtensionSchemaUpdatesHook {
 	}
 
 	/**
-	 * Helper function for converting single TOTP keys to multi-key system
+	 * Helper function for converting single TOTP keys to the multi-key system
 	 * @param DatabaseUpdater $updater
 	 * @return bool
 	 * @throws ConfigException
@@ -127,7 +127,7 @@ class UpdateTables implements LoadExtensionSchemaUpdatesHook {
 	}
 
 	/**
-	 * Converts old, TOTP specific, column values to new structure
+	 * Converts old, TOTP specific, column values to a newer structure
 	 * @param IMaintainableDatabase $db
 	 * @return bool
 	 * @throws ConfigException
@@ -250,6 +250,7 @@ class UpdateTables implements LoadExtensionSchemaUpdatesHook {
 					$updated = true;
 				}
 			}
+			unset( $k );
 
 			if ( !$updated ) {
 				continue;

src/IModule.php

@@ -10,6 +10,7 @@ use Message;
 interface IModule {
 	/**
 	 * Name of the module
+	 *
 	 * @return string
 	 */
 	public function getName();
@@ -20,7 +21,6 @@ interface IModule {
 	public function getDisplayName();
 
 	/**
-	 *
 	 * @param array $data
 	 * @return IAuthKey
 	 */
@@ -32,8 +32,9 @@ interface IModule {
 	public function getSecondaryAuthProvider();
 
 	/**
-	 * Is this module currently enabled for the given user
-	 * Arguably, module is enabled just by the fact its set on user
+	 * Is this module currently enabled for the given user?
+	 *
+	 * Arguably, module is enabled just by the fact its set on user,
 	 * but it might not be true for all future modules
 	 *
 	 * @param OATHUser $user
@@ -66,14 +67,15 @@ interface IModule {
 
 	/**
 	 * Return Message object for the short text to be displayed as description
+	 *
 	 * @return Message
 	 */
 	public function getDescriptionMessage();
 
 	/**
-	 * Module-specific text that will be shown when user is disabling
+	 * Module-specific text that will be shown when the user is disabling
 	 * the module, to warn of data-loss.
-	 * This will be shown alongside generic warning message.
+	 * This will be shown alongside the generic warning message.
 	 *
 	 * @return Message|null if no additional text is needed
 	 */

src/Key/TOTPKey.php

@@ -92,7 +92,7 @@ class TOTPKey implements IAuthKey {
 	 * @param array $scratchTokens
 	 */
 	public function __construct( $secret, array $scratchTokens ) {
-		// Currently hardcoded values; might be used in future
+		// Currently hardcoded values; might be used in the future
 		$this->secret = [
 			'mode' => 'hotp',
 			'secret' => $secret,
@@ -155,7 +155,6 @@ class TOTPKey implements IAuthKey {
 		);
 
 		// Remove any whitespace from the received token, which can be an intended group separator
-		// or trimmeable whitespace
 		$token = preg_replace( '/\s+/', '', $token );
 
 		$clientIP = $user->getUser()->getRequest()->getIP();
@@ -220,7 +219,7 @@ class TOTPKey implements IAuthKey {
 	}
 
 	/**
-	 * Check if a token is one of the scratch tokens for this two factor key.
+	 * Check if a token is one of the scratch tokens for this two-factor key.
 	 *
 	 * @param string $token Token to verify
 	 *

src/Module/TOTP.php

@@ -3,18 +3,15 @@
 namespace MediaWiki\Extension\OATHAuth\Module;
 
 use IContextSource;
-use MediaWiki\Auth\AbstractSecondaryAuthenticationProvider;
 use MediaWiki\Extension\OATHAuth\Auth\TOTPSecondaryAuthenticationProvider;
 use MediaWiki\Extension\OATHAuth\HTMLForm\IManageForm;
 use MediaWiki\Extension\OATHAuth\HTMLForm\TOTPDisableForm;
 use MediaWiki\Extension\OATHAuth\HTMLForm\TOTPEnableForm;
-use MediaWiki\Extension\OATHAuth\IAuthKey;
 use MediaWiki\Extension\OATHAuth\IModule;
 use MediaWiki\Extension\OATHAuth\Key\TOTPKey;
 use MediaWiki\Extension\OATHAuth\OATHUser;
 use MediaWiki\Extension\OATHAuth\OATHUserRepository;
 use MediaWiki\Extension\OATHAuth\Special\OATHManage;
-use Message;
 use MWException;
 
 class TOTP implements IModule {
@@ -22,25 +19,18 @@ class TOTP implements IModule {
 		return new static();
 	}
 
-	/**
-	 * Name of the module
-	 * @return string
-	 */
+	/** @inheritDoc */
 	public function getName() {
 		return "totp";
 	}
 
-	/**
-	 * @return Message
-	 */
+	/** @inheritDoc */
 	public function getDisplayName() {
 		return wfMessage( 'oathauth-module-totp-label' );
 	}
 
 	/**
-	 *
-	 * @param array $data
-	 * @return IAuthKey
+	 * @inheritDoc
 	 * @throws MWException
 	 */
 	public function newKey( array $data ) {
@@ -55,7 +45,7 @@ class TOTP implements IModule {
 	}
 
 	/**
-	 * @return AbstractSecondaryAuthenticationProvider
+	 * @return TOTPSecondaryAuthenticationProvider
 	 */
 	public function getSecondaryAuthProvider() {
 		return new TOTPSecondaryAuthenticationProvider();
@@ -79,7 +69,7 @@ class TOTP implements IModule {
 	}
 
 	/**
-	 * Is this module currently enabled for the given user
+	 * Is this module currently enabled for the given user?
 	 *
 	 * @param OATHUser $user
 	 * @return bool

src/Notifications/DisablePresentationModel.php

@@ -20,11 +20,12 @@
 
 namespace MediaWiki\Extension\OATHAuth\Notifications;
 
-use EchoEventPresentationModel;
+use MediaWiki\Extension\Notifications\Formatters\EchoEventPresentationModel;
 use MediaWiki\Title\Title;
 use SpecialPage;
 
 class DisablePresentationModel extends EchoEventPresentationModel {
+
 	/**
 	 * @inheritDoc
 	 */

src/Notifications/EnablePresentationModel.php

@@ -20,11 +20,12 @@
 
 namespace MediaWiki\Extension\OATHAuth\Notifications;
 
-use EchoEventPresentationModel;
+use MediaWiki\Extension\Notifications\Formatters\EchoEventPresentationModel;
 use MediaWiki\Title\Title;
 use SpecialPage;
 
 class EnablePresentationModel extends EchoEventPresentationModel {
+
 	/**
 	 * @inheritDoc
 	 */

src/Notifications/Manager.php

@@ -20,8 +20,8 @@
 
 namespace MediaWiki\Extension\OATHAuth\Notifications;
 
-use EchoEvent;
 use ExtensionRegistry;
+use MediaWiki\Extension\Notifications\Model\Event;
 use MediaWiki\Extension\OATHAuth\OATHUser;
 use SpecialPage;
 
@@ -29,6 +29,7 @@ use SpecialPage;
  * Manages logic for configuring and sending out notifications with Echo
  */
 class Manager {
+
 	/**
 	 * Whether Echo is installed and can be used
 	 *
@@ -48,7 +49,7 @@ class Manager {
 		if ( !self::isEnabled() ) {
 			return;
 		}
-		EchoEvent::create( [
+		Event::create( [
 			'type' => 'oathauth-disable',
 			'title' => SpecialPage::getTitleFor( 'Preferences' ),
 			'agent' => $oUser->getUser(),
@@ -67,7 +68,7 @@ class Manager {
 		if ( !self::isEnabled() ) {
 			return;
 		}
-		EchoEvent::create( [
+		Event::create( [
 			'type' => 'oathauth-enable',
 			'title' => SpecialPage::getTitleFor( 'Preferences' ),
 			'agent' => $oUser->getUser()

src/OATHAuthDatabase.php

@@ -34,12 +34,9 @@ class OATHAuthDatabase {
 	public const CONSTRUCTOR_OPTIONS = [
 		'OATHAuthDatabase',
 	];
+	private ServiceOptions $options;
 
-	/** @var ServiceOptions */
-	private $options;
-
-	/** @var LBFactory */
-	private $lbFactory;
+	private LBFactory $lbFactory;
 
 	/**
 	 * @param ServiceOptions $options

src/OATHAuthModuleRegistry.php

@@ -24,7 +24,6 @@ use Exception;
 
 class OATHAuthModuleRegistry {
 
-	/** @var OATHAuthDatabase */
 	private OATHAuthDatabase $database;
 
 	/** @var array */
@@ -33,10 +32,6 @@ class OATHAuthModuleRegistry {
 	/** @var array|null */
 	private $moduleIds;
 
-	/**
-	 * @param OATHAuthDatabase $database
-	 * @param array $modules
-	 */
 	public function __construct(
 		OATHAuthDatabase $database,
 		array $modules
@@ -45,10 +40,6 @@ class OATHAuthModuleRegistry {
 		$this->modules = $modules;
 	}
 
-	/**
-	 * @param string $key
-	 * @return IModule|null
-	 */
 	public function getModuleByKey( string $key ): ?IModule {
 		if ( isset( $this->getModules()[$key] ) ) {
 			$module = call_user_func_array( $this->getModules()[$key], [] );
@@ -80,6 +71,7 @@ class OATHAuthModuleRegistry {
 
 	/**
 	 * Returns the numerical ID for the module with the specified key.
+	 *
 	 * @param string $key
 	 * @return int
 	 */

src/OATHUser.php

@@ -26,8 +26,7 @@ use User;
  * @ingroup Extensions
  */
 class OATHUser {
-	/** @var User */
-	private $user;
+	private User $user;
 
 	/** @var IAuthKey[] */
 	private $keys;
@@ -84,7 +83,7 @@ class OATHUser {
 
 	/**
 	 * Useful for modules that operate on single-key premise,
-	 * as well as testing the key type, since first key is
+	 * as well as testing the key type, since the first key is(?)
 	 * necessarily the same type as others
 	 *
 	 * @return IAuthKey|null

src/OATHUserRepository.php

@@ -22,6 +22,7 @@ use BagOStuff;
 use ConfigException;
 use FormatJson;
 use MediaWiki\Config\ServiceOptions;
+use MediaWiki\Extension\OATHAuth\Notifications\Manager;
 use MediaWiki\User\CentralId\CentralIdLookupFactory;
 use MWException;
 use Psr\Log\LoggerAwareInterface;
@@ -31,22 +32,16 @@ use RuntimeException;
 use User;
 
 class OATHUserRepository implements LoggerAwareInterface {
-	/** @var ServiceOptions */
 	private ServiceOptions $options;
 
-	/** @var OATHAuthDatabase */
 	private OATHAuthDatabase $database;
 
-	/** @var BagOStuff */
 	private BagOStuff $cache;
 
-	/** @var OATHAuthModuleRegistry */
 	private OATHAuthModuleRegistry $moduleRegistry;
 
-	/** @var CentralIdLookupFactory */
 	private CentralIdLookupFactory $centralIdLookupFactory;
 
-	/** @var LoggerInterface */
 	private LoggerInterface $logger;
 
 	/** @internal Only public for service wiring use. */
@@ -54,16 +49,6 @@ class OATHUserRepository implements LoggerAwareInterface {
 		'OATHAuthMultipleDevicesMigrationStage',
 	];
 
-	/**
-	 * OATHUserRepository constructor.
-	 *
-	 * @param ServiceOptions $options
-	 * @param OATHAuthDatabase $database
-	 * @param BagOStuff $cache
-	 * @param OATHAuthModuleRegistry $moduleRegistry
-	 * @param CentralIdLookupFactory $centralIdLookupFactory
-	 * @param LoggerInterface $logger
-	 */
 	public function __construct(
 		ServiceOptions $options,
 		OATHAuthDatabase $database,
@@ -91,8 +76,8 @@ class OATHUserRepository implements LoggerAwareInterface {
 	/**
 	 * @param User $user
 	 * @return OATHUser
-	 * @throws \ConfigException
-	 * @throws \MWException
+	 * @throws ConfigException
+	 * @throws MWException
 	 */
 	public function findByUser( User $user ) {
 		$oathUser = $this->cache->get( $user->getName() );
@@ -251,7 +236,7 @@ class OATHUserRepository implements LoggerAwareInterface {
 	/**
 	 * @param OATHUser $user
 	 * @param string $clientInfo
-	 * @param bool $self Whether they disabled it themselves
+	 * @param bool $self Whether the user disabled the 2FA themselves
 	 */
 	public function remove( OATHUser $user, $clientInfo, bool $self ) {
 		$userId = $this->centralIdLookupFactory->getLookup()
@@ -280,7 +265,7 @@ class OATHUserRepository implements LoggerAwareInterface {
 			'clientip' => $clientInfo,
 			'oathtype' => $user->getModule()->getName(),
 		] );
-		Notifications\Manager::notifyDisabled( $user, $self );
+		Manager::notifyDisabled( $user, $self );
 	}
 
 	private function getMultipleDevicesMigrationStage(): int {

src/Special/DisableOATHForUser.php

@@ -17,11 +17,10 @@ use UserBlockedError;
 use UserNotLoggedIn;
 
 class DisableOATHForUser extends FormSpecialPage {
-	/** @var OATHUserRepository */
-	private $userRepo;
 
-	/** @var UserFactory */
-	private $userFactory;
+	private OATHUserRepository $userRepo;
+
+	private UserFactory $userFactory;
 
 	/**
 	 * @param OATHUserRepository $userRepo

src/Special/OATHManage.php

@@ -93,7 +93,6 @@ class OATHManage extends SpecialPage {
 
 	/**
 	 * @param null|string $subPage
-	 * @return void
 	 */
 	public function execute( $subPage ) {
 		$this->getOutput()->enableOOUI();
@@ -277,13 +276,7 @@ class OATHManage extends SpecialPage {
 	}
 
 	private function shouldShowGenericButtons() {
-		if ( !$this->requestedModule instanceof IModule ) {
-			return true;
-		}
-		if ( !$this->isGenericAction() ) {
-			return true;
-		}
-		return false;
+		return !$this->requestedModule instanceof IModule || !$this->isGenericAction();
 	}
 
 	private function isModuleRequested( IModule $module ) {
@@ -301,7 +294,7 @@ class OATHManage extends SpecialPage {
 	}
 
 	/**
-	 * Checks if given form instance fulfills required conditions
+	 * Verifies if the given form instance fulfills the required conditions
 	 *
 	 * @param mixed $form
 	 * @return bool
@@ -333,7 +326,7 @@ class OATHManage extends SpecialPage {
 
 	/**
 	 * When performing an action on a module (like enable/disable),
-	 * page should contain only form for that action
+	 * page should contain only the form for that action
 	 */
 	private function clearPage() {
 		if ( $this->isGenericAction() ) {
@@ -349,7 +342,7 @@ class OATHManage extends SpecialPage {
 	}
 
 	/**
-	 * Actions enable and disable are generic and all modules must
+	 * The enable and disable actions are generic, and all modules must
 	 * implement them, while all other actions are module-specific
 	 * @return bool
 	 */

src/Special/VerifyOATHForUser.php

@@ -16,12 +16,8 @@ class VerifyOATHForUser extends FormSpecialPage {
 
 	private const OATHAUTH_IS_ENABLED = 'enabled';
 	private const OATHAUTH_NOT_ENABLED = 'disabled';
-
-	/** @var OATHUserRepository */
-	private $userRepo;
-
-	/** @var UserFactory */
-	private $userFactory;
+	private OATHUserRepository $userRepo;
+	private UserFactory $userFactory;
 
 	/** @var string */
 	private $enabledStatus;