fix(Authentication): correctly catch uniq constraint on token insert

Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
fix/auth-token-uniq-constraint-violation-handling
Grigorii K. Shartsev 1 month ago
parent a73773b1ef
commit 131dbafa5b

@ -27,7 +27,6 @@ declare(strict_types=1);
*/ */
namespace OC\Authentication\Token; namespace OC\Authentication\Token;
use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
use OC\Authentication\Exceptions\InvalidTokenException as OcInvalidTokenException; use OC\Authentication\Exceptions\InvalidTokenException as OcInvalidTokenException;
use OC\Authentication\Exceptions\PasswordlessTokenException; use OC\Authentication\Exceptions\PasswordlessTokenException;
use OCP\Authentication\Exceptions\ExpiredTokenException; use OCP\Authentication\Exceptions\ExpiredTokenException;
@ -35,6 +34,7 @@ use OCP\Authentication\Exceptions\InvalidTokenException;
use OCP\Authentication\Exceptions\WipeTokenException; use OCP\Authentication\Exceptions\WipeTokenException;
use OCP\Authentication\Token\IProvider as OCPIProvider; use OCP\Authentication\Token\IProvider as OCPIProvider;
use OCP\Authentication\Token\IToken as OCPIToken; use OCP\Authentication\Token\IToken as OCPIToken;
use OCP\DB\Exception;
class Manager implements IProvider, OCPIProvider { class Manager implements IProvider, OCPIProvider {
/** @var PublicKeyTokenProvider */ /** @var PublicKeyTokenProvider */
@ -77,18 +77,22 @@ class Manager implements IProvider, OCPIProvider {
$type, $type,
$remember $remember
); );
} catch (UniqueConstraintViolationException $e) { } catch (Exception $e) {
// It's rare, but if two requests of the same session (e.g. env-based SAML) if ($e->getReason() === Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
// try to create the session token they might end up here at the same time // It's rare, but if two requests of the same session (e.g. env-based SAML)
// because we use the session ID as token and the db token is created anew // try to create the session token they might end up here at the same time
// with every request. // 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 the UIDs match, then this should be fine.
if ($existing->getUID() !== $uid) { $existing = $this->getToken($token);
throw new \Exception('Token conflict handled, but UIDs do not match. This should not happen', 0, $e); 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;
} }
} }

@ -32,6 +32,7 @@ use OC\Authentication\Token\IToken;
use OC\Authentication\Token\Manager; use OC\Authentication\Token\Manager;
use OC\Authentication\Token\PublicKeyToken; use OC\Authentication\Token\PublicKeyToken;
use OC\Authentication\Token\PublicKeyTokenProvider; use OC\Authentication\Token\PublicKeyTokenProvider;
use OC\DB\Exceptions\DbalException;
use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\MockObject\MockObject;
use Test\TestCase; use Test\TestCase;
@ -79,8 +80,9 @@ class ManagerTest extends TestCase {
} }
public function testGenerateConflictingToken() { public function testGenerateConflictingToken() {
/** @var MockObject|UniqueConstraintViolationException $exception */ /** @var MockObject|UniqueConstraintViolationException $parent */
$exception = $this->createMock(UniqueConstraintViolationException::class); $parent = $this->createMock(UniqueConstraintViolationException::class);
$exception = DbalException::wrap($parent);
$token = new PublicKeyToken(); $token = new PublicKeyToken();
$token->setUid('uid'); $token->setUid('uid');

Loading…
Cancel
Save