Re-instate "Add some logging of OATHAuth actions"

This reverts commit 69b6292c12.

Bug: T151010
Change-Id: I6f610551bc4bd1e78c0282011b80a3f3e70b8885

includes/OATHAuthKey.php

@@ -16,6 +16,9 @@
  * http://www.gnu.org/copyleft/gpl.html
  */
 
+use Psr\Log\LoggerInterface;
+use \MediaWiki\Logger\LoggerFactory;
+
 /**
  * Class representing a two-factor key
  *
@@ -119,12 +122,21 @@ class OATHAuthKey {
 		// or trimmeable whitespace
 		$token = preg_replace( '/\s+/', '', $token );
 
+		$clientIP = $user->getUser()->getRequest()->getIP();
+
+		$logger = $this->getLogger();
+
 		// Check to see if the user's given token is in the list of tokens generated
 		// for the time window.
 		foreach ( $results as $window => $result ) {
 			if ( $window > $lastWindow && $result->toHOTP( 6 ) === $token ) {
 				$lastWindow = $window;
 				$retval = self::MAIN_TOKEN;
+
+				$logger->info( 'OATHAuth user {user} entered a valid OTP from {clientip}', [
+					'user' => $user->getAccount(),
+					'clientip' => $clientIP,
+				] );
 				break;
 			}
 		}
@@ -133,16 +145,22 @@ class OATHAuthKey {
 		if ( !$retval ) {
 			$length = count( $this->scratchTokens );
 			// Detect condition where all scratch tokens have been used
-			if ( $length == 1 && "" === $this->scratchTokens[0] ) {
+			if ( $length === 1 && $this->scratchTokens[0] === "" ) {
 				$retval = false;
 			} else {
 				for ( $i = 0; $i < $length; $i++ ) {
 					if ( $token === $this->scratchTokens[$i] ) {
 						// If there is a scratch token, remove it from the scratch token list
 						unset( $this->scratchTokens[$i] );
+
+						$logger->info( 'OATHAuth user {user} used a scratch token from {clientip}', [
+							'user' => $user->getAccount(),
+							'clientip' => $clientIP,
+						] );
+
 						$oathrepo = OATHAuthHooks::getOATHUserRepository();
 						$user->setKey( $this );
-						$oathrepo->persist( $user );
+						$oathrepo->persist( $user, $clientIP );
 						// Only return true if we removed it from the database
 						$retval = self::SCRATCH_TOKEN;
 						break;
@@ -158,6 +176,12 @@ class OATHAuthKey {
 				$this->secret['period'] * ( 1 + 2 * $wgOATHAuthWindowRadius )
 			);
 		} else {
+
+			$logger->info( 'OATHAuth user {user} failed OTP/scratch token from {clientip}', [
+				'user' => $user->getAccount(),
+				'clientip' => $clientIP,
+			] );
+
 			// Increase rate limit counter for failed request
 			$user->getUser()->pingLimiter( 'badoath' );
 		}
@@ -184,4 +208,11 @@ class OATHAuthKey {
 		$token = preg_replace( '/\s+/', '', $token );
 		return in_array( $token, $this->scratchTokens, true );
 	}
+
+	/**
+	 * @return LoggerInterface
+	 */
+	private function getLogger() {
+		return LoggerFactory::getInstance( 'authentication' );
+	}
 }

includes/OATHUserRepository.php

@@ -16,6 +16,7 @@
  * http://www.gnu.org/copyleft/gpl.html
  */
 
+use Psr\Log\LoggerInterface;
 use Wikimedia\Rdbms\ILoadBalancer;
 use Wikimedia\Rdbms\DBConnRef;
 
@@ -26,6 +27,9 @@ class OATHUserRepository {
 	/** @var BagOStuff */
 	protected $cache;
 
+	/** @var LoggerInterface */
+	private $logger;
+
 	/**
 	 * OATHUserRepository constructor.
 	 * @param ILoadBalancer $lb
@@ -34,6 +38,15 @@ class OATHUserRepository {
 	public function __construct( ILoadBalancer $lb, BagOStuff $cache ) {
 		$this->lb = $lb;
 		$this->cache = $cache;
+
+		$this->setLogger( \MediaWiki\Logger\LoggerFactory::getInstance( 'authentication' ) );
+	}
+
+	/**
+	 * @param LoggerInterface $logger
+	 */
+	public function setLogger( LoggerInterface $logger ) {
+		$this->logger = $logger;
 	}
 
 	/**
@@ -64,8 +77,11 @@ class OATHUserRepository {
 
 	/**
 	 * @param OATHUser $user
+	 * @param string $clientInfo
 	 */
-	public function persist( OATHUser $user ) {
+	public function persist( OATHUser $user, $clientInfo ) {
+		$prevUser = $this->findByUser( $user->getUser() );
+
 		$this->getDB( DB_MASTER )->replace(
 			'oathauth_users',
 			[ 'id' ],
@@ -76,23 +92,46 @@ class OATHUserRepository {
 			],
 			__METHOD__
 		);
-		$this->cache->set( $user->getUser()->getName(), $user );
+
+		$userName = $user->getUser()->getName();
+		$this->cache->set( $userName, $user );
+
+		if ( $prevUser !== false ) {
+			$this->logger->info( 'OATHAuth updated for {user} from {clientip}', [
+				'user' => $userName,
+				'clientip' => $clientInfo,
+			] );
+		} else {
+			// If findByUser() has returned false, there was no user row or cache entry
+			$this->logger->info( 'OATHAuth enabled for {user} from {clientip}', [
+				'user' => $userName,
+				'clientip' => $clientInfo,
+			] );
+		}
 	}
 
 	/**
 	 * @param OATHUser $user
+	 * @param string $clientInfo
 	 */
-	public function remove( OATHUser $user ) {
+	public function remove( OATHUser $user, $clientInfo ) {
 		$this->getDB( DB_MASTER )->delete(
 			'oathauth_users',
 			[ 'id' => CentralIdLookup::factory()->centralIdFromLocalUser( $user->getUser() ) ],
 			__METHOD__
 		);
-		$this->cache->delete( $user->getUser()->getName() );
+
+		$userName = $user->getUser()->getName();
+		$this->cache->delete( $userName );
+
+		$this->logger->info( 'OATHAuth disabled for {user} from {clientip}', [
+			'user' => $userName,
+			'clientip' => $clientInfo,
+		] );
 	}
 
 	/**
-	 * @param integer $index DB_MASTER/DB_REPLICA
+	 * @param int $index DB_MASTER/DB_REPLICA
 	 * @return DBConnRef
 	 */
 	private function getDB( $index ) {

includes/special/SpecialDisableOATHForUser.php

@@ -91,7 +91,15 @@ class SpecialDisableOATHForUser extends FormSpecialPage {
 		}
 
 		$oathUser->setKey( null );
-		$this->OATHRepository->remove( $oathUser );
+		$this->OATHRepository->remove( $oathUser, $this->getRequest()->getIP() );
+
+		\MediaWiki\Logger\LoggerFactory::getInstance( 'authentication' )->info(
+			'OATHAuth disabled for {usertarget} by {user} from {clientip}', [
+				'user' => $this->getUser()->getName(),
+				'usertarget' => $formData['user'],
+				'clientip' => $this->getRequest()->getIP(),
+			]
+		);
 
 		return true;
 	}

includes/special/SpecialOATHDisable.php

@@ -116,15 +116,27 @@ class SpecialOATHDisable extends FormSpecialPage {
 		// Don't increase pingLimiter, just check for limit exceeded.
 		if ( $this->OATHUser->getUser()->pingLimiter( 'badoath', 0 ) ) {
 			// Arbitrary duration given here
+			\MediaWiki\Logger\LoggerFactory::getInstance( 'authentication' )->info(
+				'OATHAuth {user} rate limited while disabling 2FA from {clientip}', [
+					'user' => $this->getUser()->getName(),
+					'clientip' => $this->getRequest()->getIP(),
+				]
+			);
 			return [ 'oathauth-throttled', Message::durationParam( 60 ) ];
 		}
 
 		if ( !$this->OATHUser->getKey()->verifyToken( $formData['token'], $this->OATHUser ) ) {
+			\MediaWiki\Logger\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(),
+				]
+			);
 			return [ 'oathauth-failedtovalidateoath' ];
 		}
 
 		$this->OATHUser->setKey( null );
-		$this->OATHRepository->remove( $this->OATHUser );
+		$this->OATHRepository->remove( $this->OATHUser, $this->getRequest()->getIP() );
 
 		return true;
 	}

includes/special/SpecialOATHEnable.php

@@ -160,7 +160,8 @@ class SpecialOATHEnable extends FormSpecialPage {
 			'returntoquery' => [
 				'type' => 'hidden',
 				'default' => $this->getRequest()->getVal( 'returntoquery' ),
-				'name' => 'returntoquery', ]
+				'name' => 'returntoquery',
+			]
 		];
 	}
 
@@ -175,15 +176,27 @@ class SpecialOATHEnable extends FormSpecialPage {
 
 		if ( $key->isScratchToken( $formData['token'] ) ) {
 			// A scratch token is not allowed for enrollement
+			\MediaWiki\Logger\LoggerFactory::getInstance( 'authentication' )->info(
+				'OATHAuth {user} attempted to enable 2FA using a scratch token from {clientip}', [
+					'user' => $this->getUser()->getName(),
+					'clientip' => $this->getRequest()->getIP(),
+				]
+			);
 			return [ 'oathauth-noscratchforvalidation' ];
 		}
 		if ( !$key->verifyToken( $formData['token'], $this->OATHUser ) ) {
+			\MediaWiki\Logger\LoggerFactory::getInstance( 'authentication' )->info(
+				'OATHAuth {user} failed to provide a correct token while enabling 2FA from {clientip}', [
+					'user' => $this->getUser()->getName(),
+					'clientip' => $this->getRequest()->getIP(),
+				]
+			);
 			return [ 'oathauth-failedtovalidateoath' ];
 		}
 
 		$this->getRequest()->setSessionData( 'oathauth_key', null );
 		$this->OATHUser->setKey( $key );
-		$this->OATHRepository->persist( $this->OATHUser );
+		$this->OATHRepository->persist( $this->OATHUser, $this->getRequest()->getIP() );
 
 		return true;
 	}

maintenance/disableOATHAuthForUser.php

@@ -31,7 +31,7 @@ class DisableOATHAuthForUser extends Maintenance {
 			$this->error( "User $username doesn't have OATHAuth enabled!", 1 );
 		}
 
-		$repo->remove( $oathUser );
+		$repo->remove( $oathUser, 'Maintenance script' );
 		$this->output( "OATHAuth disabled for $username.\n" );
 	}
 }