fix(Session): avoid race conditions on clustered setups

- re-stablishes old behaviour with cache to return null instead of throwing
  an InvalidTokenException when the token is cached as non-existing
- token invalidation and re-generation are bundled in a DB transaction now

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>

3 files changed, 19 insertions, 46 deletions

diff --git a/lib/private/Authentication/Token/PublicKeyTokenProvider.php b/lib/private/Authentication/Token/PublicKeyTokenProvider.php
index 767ece1e551..18b850b9377 100644
--- a/lib/private/Authentication/Token/PublicKeyTokenProvider.php
+++ b/lib/private/Authentication/Token/PublicKeyTokenProvider.php
@@ -171,7 +171,7 @@ class PublicKeyTokenProvider implements IProvider {
 	private function getTokenFromCache(string $tokenHash): ?PublicKeyToken {
 		$serializedToken = $this->cache->get($tokenHash);
 		if ($serializedToken === false) {
-			throw new InvalidTokenException('Token does not exist: ' . $tokenHash);
+			return null;
 		}
 
 		if ($serializedToken === null) {
diff --git a/lib/private/Server.php b/lib/private/Server.php
index bcdf482f02d..7ac63bdb5b7 100644
--- a/lib/private/Server.php
+++ b/lib/private/Server.php
@@ -506,7 +506,7 @@ class Server extends ServerContainer implements IServerContainer {
 				$c->get(ISecureRandom::class),
 				$c->get('LockdownManager'),
 				$c->get(LoggerInterface::class),
-				$c->get(IEventDispatcher::class)
+				$c->get(IEventDispatcher::class),
 			);
 			/** @deprecated 21.0.0 use BeforeUserCreatedEvent event with the IEventDispatcher instead */
 			$userSession->listen('\OC\User', 'preCreateUser', function ($uid, $password) {
diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php
index e5d27172519..64e8f6dfdc8 100644
--- a/lib/private/User/Session.php
+++ b/lib/private/User/Session.php
@@ -19,6 +19,7 @@ use OC\Security\CSRF\CsrfTokenManager;
 use OC_User;
 use OC_Util;
 use OCA\DAV\Connector\Sabre\Auth;
+use OCP\AppFramework\Db\TTransactional;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\Authentication\Exceptions\ExpiredTokenException;
 use OCP\Authentication\Exceptions\InvalidTokenException;
@@ -26,6 +27,7 @@ use OCP\EventDispatcher\GenericEvent;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Files\NotPermittedException;
 use OCP\IConfig;
+use OCP\IDBConnection;
 use OCP\IRequest;
 use OCP\ISession;
 use OCP\IUser;
@@ -62,53 +64,22 @@ use Psr\Log\LoggerInterface;
  * @package OC\User
  */
 class Session implements IUserSession, Emitter {
-	/** @var Manager $manager */
-	private $manager;
-
-	/** @var ISession $session */
-	private $session;
-
-	/** @var ITimeFactory */
-	private $timeFactory;
-
-	/** @var IProvider */
-	private $tokenProvider;
-
-	/** @var IConfig */
-	private $config;
+	use TTransactional;
 
 	/** @var User $activeUser */
 	protected $activeUser;
 
-	/** @var ISecureRandom */
-	private $random;
-
-	/** @var ILockdownManager  */
-	private $lockdownManager;
-
-	private LoggerInterface $logger;
-	/** @var IEventDispatcher */
-	private $dispatcher;
-
-	public function __construct(Manager $manager,
-		ISession $session,
-		ITimeFactory $timeFactory,
-		?IProvider $tokenProvider,
-		IConfig $config,
-		ISecureRandom $random,
-		ILockdownManager $lockdownManager,
-		LoggerInterface $logger,
-		IEventDispatcher $dispatcher
+	public function __construct(
+		private Manager          $manager,
+		private ISession         $session,
+		private ITimeFactory     $timeFactory,
+		private ?IProvider       $tokenProvider,
+		private IConfig          $config,
+		private ISecureRandom    $random,
+		private ILockdownManager $lockdownManager,
+		private LoggerInterface  $logger,
+		private IEventDispatcher $dispatcher,
 	) {
-		$this->manager = $manager;
-		$this->session = $session;
-		$this->timeFactory = $timeFactory;
-		$this->tokenProvider = $tokenProvider;
-		$this->config = $config;
-		$this->random = $random;
-		$this->lockdownManager = $lockdownManager;
-		$this->logger = $logger;
-		$this->dispatcher = $dispatcher;
 	}
 
 	/**
@@ -674,8 +645,10 @@ class Session implements IUserSession, Emitter {
 			$sessionId = $this->session->getId();
 			$pwd = $this->getPassword($password);
 			// Make sure the current sessionId has no leftover tokens
-			$this->tokenProvider->invalidateToken($sessionId);
-			$this->tokenProvider->generateToken($sessionId, $uid, $loginName, $pwd, $name, IToken::TEMPORARY_TOKEN, $remember);
+			$this->atomic(function () use ($sessionId, $uid, $loginName, $pwd, $name, $remember) {
+				$this->tokenProvider->invalidateToken($sessionId);
+				$this->tokenProvider->generateToken($sessionId, $uid, $loginName, $pwd, $name, IToken::TEMPORARY_TOKEN, $remember);
+			}, \OCP\Server::get(IDBConnection::class));
 			return true;
 		} catch (SessionNotAvailableException $ex) {
 			// This can happen with OCC, where a memory session is used