From 44ed7190279391004ba600a5631bb0a9ce8f1cfe Mon Sep 17 00:00:00 2001 From: Daniel Calviño Sánchez Date: Mon, 28 Oct 2024 10:14:29 +0100 Subject: fix: Clear pending two factor tokens also from configuration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Otherwise as the tokens were removed from the database but not from the configuration the next time that the tokens were cleared the previous tokens were still got from the configuration, and trying to remove them again from the database ended in a DoesNotExistException being thrown. Signed-off-by: Daniel Calviño Sánchez --- .../Authentication/TwoFactorAuth/ManagerTest.php | 26 ++++++++++++++++++++++ 1 file changed, 26 insertions(+) (limited to 'tests') diff --git a/tests/lib/Authentication/TwoFactorAuth/ManagerTest.php b/tests/lib/Authentication/TwoFactorAuth/ManagerTest.php index a2655f58649..c741ff068ac 100644 --- a/tests/lib/Authentication/TwoFactorAuth/ManagerTest.php +++ b/tests/lib/Authentication/TwoFactorAuth/ManagerTest.php @@ -715,4 +715,30 @@ class ManagerTest extends TestCase { $this->assertFalse($this->manager->needsSecondFactor($user)); } + + public function testClearTwoFactorPending() { + $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], + ); + + $this->manager->clearTwoFactorPending('theUserId'); + } } -- cgit v1.2.3 From 4d9cc7dd8d95cdb212e0ce19e22ea53fdefff817 Mon Sep 17 00:00:00 2001 From: Daniel Calviño Sánchez Date: Mon, 28 Oct 2024 10:15:16 +0100 Subject: fix: Handle exception when clearing previously removed two factor tokens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../Authentication/TwoFactorAuth/Manager.php | 6 +++- .../Authentication/TwoFactorAuth/ManagerTest.php | 32 ++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/lib/private/Authentication/TwoFactorAuth/Manager.php b/lib/private/Authentication/TwoFactorAuth/Manager.php index 209b32d46a3..d3d5486da94 100644 --- a/lib/private/Authentication/TwoFactorAuth/Manager.php +++ b/lib/private/Authentication/TwoFactorAuth/Manager.php @@ -31,6 +31,7 @@ use BadMethodCallException; 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; @@ -387,7 +388,10 @@ class Manager { 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) { + } } } } diff --git a/tests/lib/Authentication/TwoFactorAuth/ManagerTest.php b/tests/lib/Authentication/TwoFactorAuth/ManagerTest.php index c741ff068ac..23ae5d93fdd 100644 --- a/tests/lib/Authentication/TwoFactorAuth/ManagerTest.php +++ b/tests/lib/Authentication/TwoFactorAuth/ManagerTest.php @@ -29,6 +29,7 @@ use OC\Authentication\TwoFactorAuth\MandatoryTwoFactor; 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; @@ -741,4 +742,35 @@ class ManagerTest extends TestCase { $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'); + } } -- cgit v1.2.3