TOTPKey: simplify verify() method

* Do not set the last window cache value when using a scratch token.
* Always return a boolean.

Change-Id: I60ce99ed3c70de73794ebafedd434adfcbf86ffc
This commit is contained in:
Taavi Väänänen 2023-01-31 19:50:59 +02:00
parent b843d75e96
commit 8890a44a31
No known key found for this signature in database
GPG key ID: EF242F709F912FBE
2 changed files with 28 additions and 46 deletions

View file

@ -10,7 +10,7 @@ interface IAuthKey extends JsonSerializable {
/**
* @param array|stdClass $data
* @param OATHUser $user
* @return mixed
* @return bool
*/
public function verify( $data, OATHUser $user );

View file

@ -39,18 +39,6 @@ use Psr\Log\LoggerInterface;
* @ingroup Extensions
*/
class TOTPKey implements IAuthKey {
/**
* Represents that a token corresponds to the main secret
* @see verify
*/
private const MAIN_TOKEN = 1;
/**
* Represents that a token corresponds to a scratch token
* @see verify
*/
private const SCRATCH_TOKEN = -1;
/** @var array Two factor binary secret */
private $secret;
@ -150,7 +138,6 @@ class TOTPKey implements IAuthKey {
$key = $store->makeKey( 'oathauth-totp', 'usedtokens', $uid );
$lastWindow = (int)$store->get( $key );
$retval = false;
$results = HOTP::generateByTimeWindow(
Base32::decode( $this->secret['secret'] ),
$this->secret['period'],
@ -171,53 +158,48 @@ class TOTPKey implements IAuthKey {
foreach ( $results as $window => $result ) {
if ( $window > $lastWindow && hash_equals( $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;
$store->set(
$key,
$lastWindow,
$this->secret['period'] * ( 1 + 2 * $wgOATHAuthWindowRadius )
);
return true;
}
}
// See if the user is using a scratch token
if ( !$retval ) {
foreach ( $this->scratchTokens as $i => $scratchToken ) {
if ( hash_equals( $token, $scratchToken ) ) {
// If we used a scratch token, remove it from the scratch token list.
// This is saved below via OATHUserRepository::persist, TOTP::getDataFromUser.
array_splice( $this->scratchTokens, $i, 1 );
foreach ( $this->scratchTokens as $i => $scratchToken ) {
if ( hash_equals( $token, $scratchToken ) ) {
// If we used a scratch token, remove it from the scratch token list.
// This is saved below via OATHUserRepository::persist, TOTP::getDataFromUser.
array_splice( $this->scratchTokens, $i, 1 );
$logger->info( 'OATHAuth user {user} used a scratch token from {clientip}', [
'user' => $user->getAccount(),
'clientip' => $clientIP,
] );
$logger->info( 'OATHAuth user {user} used a scratch token from {clientip}', [
'user' => $user->getAccount(),
'clientip' => $clientIP,
] );
$moduleRegistry = MediaWikiServices::getInstance()->getService( 'OATHAuthModuleRegistry' );
$module = $moduleRegistry->getModuleByKey( 'totp' );
$moduleRegistry = MediaWikiServices::getInstance()->getService( 'OATHAuthModuleRegistry' );
$module = $moduleRegistry->getModuleByKey( 'totp' );
/** @var OATHUserRepository $userRepo */
$userRepo = MediaWikiServices::getInstance()->getService( 'OATHUserRepository' );
$user->addKey( $this );
$user->setModule( $module );
$userRepo->persist( $user, $clientIP );
// Only return true if we removed it from the database
$retval = self::SCRATCH_TOKEN;
break;
}
/** @var OATHUserRepository $userRepo */
$userRepo = MediaWikiServices::getInstance()->getService( 'OATHUserRepository' );
$user->addKey( $this );
$user->setModule( $module );
$userRepo->persist( $user, $clientIP );
return true;
}
}
if ( $retval ) {
$store->set(
$key,
$lastWindow,
$this->secret['period'] * ( 1 + 2 * $wgOATHAuthWindowRadius )
);
}
return $retval;
return false;
}
public function regenerateScratchTokens() {