Skip to content

Commit

Permalink
fix: Handle exception when clearing previously removed two factor tokens
Browse files Browse the repository at this point in the history
If a token was already removed from the database but not from the
configuration clearing the tokens will try to remove it again from the
database, which caused a DoesNotExistException to be thrown.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
  • Loading branch information
danxuliu authored and nickvergessen committed Nov 5, 2024
1 parent 381a2aa commit 5ea5b2d
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 1 deletion.
6 changes: 5 additions & 1 deletion lib/private/Authentication/TwoFactorAuth/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use Exception;
use OC\Authentication\Token\IProvider as TokenProvider;
use OCP\Activity\IManager;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\Authentication\Exceptions\InvalidTokenException;
use OCP\Authentication\TwoFactorAuth\IActivatableAtLogin;
Expand Down Expand Up @@ -368,7 +369,10 @@ public function clearTwoFactorPending(string $userId) {
foreach ($tokensNeeding2FA as $tokenId) {
$this->config->deleteUserValue($userId, 'login_token_2fa', $tokenId);

$this->tokenProvider->invalidateTokenById($userId, (int)$tokenId);
try {
$this->tokenProvider->invalidateTokenById($userId, (int)$tokenId);
} catch (DoesNotExistException $e) {
}
}
}
}
32 changes: 32 additions & 0 deletions tests/lib/Authentication/TwoFactorAuth/ManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use OC\Authentication\TwoFactorAuth\ProviderLoader;
use OCP\Activity\IEvent;
use OCP\Activity\IManager;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\Authentication\TwoFactorAuth\IActivatableAtLogin;
use OCP\Authentication\TwoFactorAuth\IProvider;
Expand Down Expand Up @@ -727,4 +728,35 @@ public function testClearTwoFactorPending() {

$this->manager->clearTwoFactorPending('theUserId');
}

public function testClearTwoFactorPendingTokenDoesNotExist() {
$this->config->method('getUserKeys')
->with('theUserId', 'login_token_2fa')
->willReturn([
'42', '43', '44'
]);

$this->config->expects($this->exactly(3))
->method('deleteUserValue')
->withConsecutive(
['theUserId', 'login_token_2fa', '42'],
['theUserId', 'login_token_2fa', '43'],
['theUserId', 'login_token_2fa', '44'],
);

$this->tokenProvider->expects($this->exactly(3))
->method('invalidateTokenById')
->withConsecutive(
['theUserId', 42],
['theUserId', 43],
['theUserId', 44],
)
->willReturnCallback(function ($user, $tokenId) {
if ($tokenId === 43) {
throw new DoesNotExistException('token does not exist');
}
});

$this->manager->clearTwoFactorPending('theUserId');
}
}

0 comments on commit 5ea5b2d

Please # to comment.