From 131dbafa5b7e33f67fecee8c3166fedb16effdad Mon Sep 17 00:00:00 2001 From: "Grigorii K. Shartsev" Date: Tue, 30 Apr 2024 15:55:49 +0200 Subject: [PATCH] fix(Authentication): correctly catch uniq constraint on token insert Signed-off-by: Grigorii K. Shartsev --- lib/private/Authentication/Token/Manager.php | 28 +++++++++++-------- .../lib/Authentication/Token/ManagerTest.php | 6 ++-- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/lib/private/Authentication/Token/Manager.php b/lib/private/Authentication/Token/Manager.php index e0b0e2dd14b..9a8d5100abb 100644 --- a/lib/private/Authentication/Token/Manager.php +++ b/lib/private/Authentication/Token/Manager.php @@ -27,7 +27,6 @@ declare(strict_types=1); */ namespace OC\Authentication\Token; -use Doctrine\DBAL\Exception\UniqueConstraintViolationException; use OC\Authentication\Exceptions\InvalidTokenException as OcInvalidTokenException; use OC\Authentication\Exceptions\PasswordlessTokenException; use OCP\Authentication\Exceptions\ExpiredTokenException; @@ -35,6 +34,7 @@ use OCP\Authentication\Exceptions\InvalidTokenException; use OCP\Authentication\Exceptions\WipeTokenException; use OCP\Authentication\Token\IProvider as OCPIProvider; use OCP\Authentication\Token\IToken as OCPIToken; +use OCP\DB\Exception; class Manager implements IProvider, OCPIProvider { /** @var PublicKeyTokenProvider */ @@ -77,18 +77,22 @@ class Manager implements IProvider, OCPIProvider { $type, $remember ); - } catch (UniqueConstraintViolationException $e) { - // It's rare, but if two requests of the same session (e.g. env-based SAML) - // try to create the session token they might end up here at the same time - // because we use the session ID as token and the db token is created anew - // with every request. - // - // If the UIDs match, then this should be fine. - $existing = $this->getToken($token); - if ($existing->getUID() !== $uid) { - throw new \Exception('Token conflict handled, but UIDs do not match. This should not happen', 0, $e); + } catch (Exception $e) { + if ($e->getReason() === Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { + // It's rare, but if two requests of the same session (e.g. env-based SAML) + // try to create the session token they might end up here at the same time + // because we use the session ID as token and the db token is created anew + // with every request. + // + // If the UIDs match, then this should be fine. + $existing = $this->getToken($token); + if ($existing->getUID() !== $uid) { + throw new \Exception('Token conflict handled, but UIDs do not match. This should not happen', 0, $e); + } + return $existing; } - return $existing; + + throw $e; } } diff --git a/tests/lib/Authentication/Token/ManagerTest.php b/tests/lib/Authentication/Token/ManagerTest.php index 3dd889dcae2..5f320747e5b 100644 --- a/tests/lib/Authentication/Token/ManagerTest.php +++ b/tests/lib/Authentication/Token/ManagerTest.php @@ -32,6 +32,7 @@ use OC\Authentication\Token\IToken; use OC\Authentication\Token\Manager; use OC\Authentication\Token\PublicKeyToken; use OC\Authentication\Token\PublicKeyTokenProvider; +use OC\DB\Exceptions\DbalException; use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; @@ -79,8 +80,9 @@ class ManagerTest extends TestCase { } public function testGenerateConflictingToken() { - /** @var MockObject|UniqueConstraintViolationException $exception */ - $exception = $this->createMock(UniqueConstraintViolationException::class); + /** @var MockObject|UniqueConstraintViolationException $parent */ + $parent = $this->createMock(UniqueConstraintViolationException::class); + $exception = DbalException::wrap($parent); $token = new PublicKeyToken(); $token->setUid('uid');