Merge pull request #17939 from nextcloud/fix/token-insert-conflict-handling

Handle token insert conflicts

2 files changed, 68 insertions, 19 deletions

diff --git a/lib/private/Authentication/Token/Manager.php b/lib/private/Authentication/Token/Manager.php
index ea94efce54d..0642d4426c4 100644
--- a/lib/private/Authentication/Token/Manager.php
+++ b/lib/private/Authentication/Token/Manager.php
@@ -23,6 +23,7 @@ declare(strict_types=1);
 
 namespace OC\Authentication\Token;
 
+use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
 use OC\Authentication\Exceptions\ExpiredTokenException;
 use OC\Authentication\Exceptions\InvalidTokenException;
 use OC\Authentication\Exceptions\PasswordlessTokenException;
@@ -60,15 +61,29 @@ class Manager implements IProvider {
 								  string $name,
 								  int $type = IToken::TEMPORARY_TOKEN,
 								  int $remember = IToken::DO_NOT_REMEMBER): IToken {
-		return $this->publicKeyTokenProvider->generateToken(
-			$token,
-			$uid,
-			$loginName,
-			$password,
-			$name,
-			$type,
-			$remember
-		);
+		try {
+			return $this->publicKeyTokenProvider->generateToken(
+				$token,
+				$uid,
+				$loginName,
+				$password,
+				$name,
+				$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);
+			}
+			return $existing;
+		}
 	}
 
 	/**
diff --git a/tests/lib/Authentication/Token/ManagerTest.php b/tests/lib/Authentication/Token/ManagerTest.php
index 48876cade6b..1f3fda1b15d 100644
--- a/tests/lib/Authentication/Token/ManagerTest.php
+++ b/tests/lib/Authentication/Token/ManagerTest.php
@@ -1,4 +1,5 @@
-<?php
+<?php declare(strict_types=1);
+
 /**
  * @copyright Copyright (c) 2018 Roeland Jago Douma <roeland@famdouma.nl>
  *
@@ -23,6 +24,7 @@
 
 namespace Test\Authentication\Token;
 
+use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
 use OC\Authentication\Exceptions\InvalidTokenException;
 use OC\Authentication\Exceptions\PasswordlessTokenException;
 use OC\Authentication\Token\DefaultToken;
@@ -31,21 +33,15 @@ use OC\Authentication\Token\ExpiredTokenException;
 use OC\Authentication\Token\IToken;
 use OC\Authentication\Token\Manager;
 use OC\Authentication\Token\PublicKeyToken;
-use OC\Authentication\Token\PublicKeyTokenMapper;
 use OC\Authentication\Token\PublicKeyTokenProvider;
-use OCP\AppFramework\Db\DoesNotExistException;
-use OCP\AppFramework\Utility\ITimeFactory;
-use OCP\IConfig;
-use OCP\ILogger;
-use OCP\IUser;
-use OCP\Security\ICrypto;
+use PHPUnit\Framework\MockObject\MockObject;
 use Test\TestCase;
 
 class ManagerTest extends TestCase {
 
-	/** @var PublicKeyTokenProvider|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var PublicKeyTokenProvider|MockObject */
 	private $publicKeyTokenProvider;
-	/** @var DefaultTokenProvider|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var DefaultTokenProvider|MockObject */
 	private $defaultTokenProvider;
 	/** @var Manager */
 	private $manager;
@@ -92,6 +88,44 @@ class ManagerTest extends TestCase {
 		$this->assertSame($token, $actual);
 	}
 
+	public function testGenerateConflictingToken() {
+		/** @var MockObject|UniqueConstraintViolationException $exception */
+		$exception = $this->createMock(UniqueConstraintViolationException::class);
+		$this->defaultTokenProvider->expects($this->never())
+			->method('generateToken');
+
+		$token = new PublicKeyToken();
+		$token->setUid('uid');
+
+		$this->publicKeyTokenProvider->expects($this->once())
+			->method('generateToken')
+			->with(
+				'token',
+				'uid',
+				'loginName',
+				'password',
+				'name',
+				IToken::TEMPORARY_TOKEN,
+				IToken::REMEMBER
+			)->willThrowException($exception);
+		$this->publicKeyTokenProvider->expects($this->once())
+			->method('getToken')
+			->with('token')
+			->willReturn($token);
+
+		$actual = $this->manager->generateToken(
+			'token',
+			'uid',
+			'loginName',
+			'password',
+			'name',
+			IToken::TEMPORARY_TOKEN,
+			IToken::REMEMBER
+		);
+
+		$this->assertSame($token, $actual);
+	}
+
 	public function tokenData(): array {
 		return [
 			[new DefaultToken()],