Skip to content

Commit 6a783d9

Browse files
committed
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 <[email protected]>
1 parent e31f474 commit 6a783d9

File tree

3 files changed

+19
-46
lines changed

3 files changed

+19
-46
lines changed

lib/private/Authentication/Token/PublicKeyTokenProvider.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ public function getToken(string $tokenId): OCPIToken {
171171
private function getTokenFromCache(string $tokenHash): ?PublicKeyToken {
172172
$serializedToken = $this->cache->get($tokenHash);
173173
if ($serializedToken === false) {
174-
throw new InvalidTokenException('Token does not exist: ' . $tokenHash);
174+
return null;
175175
}
176176

177177
if ($serializedToken === null) {

lib/private/Server.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -506,7 +506,7 @@ public function __construct($webRoot, \OC\Config $config) {
506506
$c->get(ISecureRandom::class),
507507
$c->get('LockdownManager'),
508508
$c->get(LoggerInterface::class),
509-
$c->get(IEventDispatcher::class)
509+
$c->get(IEventDispatcher::class),
510510
);
511511
/** @deprecated 21.0.0 use BeforeUserCreatedEvent event with the IEventDispatcher instead */
512512
$userSession->listen('\OC\User', 'preCreateUser', function ($uid, $password) {

lib/private/User/Session.php

Lines changed: 17 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,15 @@
1919
use OC_User;
2020
use OC_Util;
2121
use OCA\DAV\Connector\Sabre\Auth;
22+
use OCP\AppFramework\Db\TTransactional;
2223
use OCP\AppFramework\Utility\ITimeFactory;
2324
use OCP\Authentication\Exceptions\ExpiredTokenException;
2425
use OCP\Authentication\Exceptions\InvalidTokenException;
2526
use OCP\EventDispatcher\GenericEvent;
2627
use OCP\EventDispatcher\IEventDispatcher;
2728
use OCP\Files\NotPermittedException;
2829
use OCP\IConfig;
30+
use OCP\IDBConnection;
2931
use OCP\IRequest;
3032
use OCP\ISession;
3133
use OCP\IUser;
@@ -62,53 +64,22 @@
6264
* @package OC\User
6365
*/
6466
class Session implements IUserSession, Emitter {
65-
/** @var Manager $manager */
66-
private $manager;
67-
68-
/** @var ISession $session */
69-
private $session;
70-
71-
/** @var ITimeFactory */
72-
private $timeFactory;
73-
74-
/** @var IProvider */
75-
private $tokenProvider;
76-
77-
/** @var IConfig */
78-
private $config;
67+
use TTransactional;
7968

8069
/** @var User $activeUser */
8170
protected $activeUser;
8271

83-
/** @var ISecureRandom */
84-
private $random;
85-
86-
/** @var ILockdownManager */
87-
private $lockdownManager;
88-
89-
private LoggerInterface $logger;
90-
/** @var IEventDispatcher */
91-
private $dispatcher;
92-
93-
public function __construct(Manager $manager,
94-
ISession $session,
95-
ITimeFactory $timeFactory,
96-
?IProvider $tokenProvider,
97-
IConfig $config,
98-
ISecureRandom $random,
99-
ILockdownManager $lockdownManager,
100-
LoggerInterface $logger,
101-
IEventDispatcher $dispatcher
72+
public function __construct(
73+
private Manager $manager,
74+
private ISession $session,
75+
private ITimeFactory $timeFactory,
76+
private ?IProvider $tokenProvider,
77+
private IConfig $config,
78+
private ISecureRandom $random,
79+
private ILockdownManager $lockdownManager,
80+
private LoggerInterface $logger,
81+
private IEventDispatcher $dispatcher,
10282
) {
103-
$this->manager = $manager;
104-
$this->session = $session;
105-
$this->timeFactory = $timeFactory;
106-
$this->tokenProvider = $tokenProvider;
107-
$this->config = $config;
108-
$this->random = $random;
109-
$this->lockdownManager = $lockdownManager;
110-
$this->logger = $logger;
111-
$this->dispatcher = $dispatcher;
11283
}
11384

11485
/**
@@ -674,8 +645,10 @@ public function createSessionToken(IRequest $request, $uid, $loginName, $passwor
674645
$sessionId = $this->session->getId();
675646
$pwd = $this->getPassword($password);
676647
// Make sure the current sessionId has no leftover tokens
677-
$this->tokenProvider->invalidateToken($sessionId);
678-
$this->tokenProvider->generateToken($sessionId, $uid, $loginName, $pwd, $name, IToken::TEMPORARY_TOKEN, $remember);
648+
$this->atomic(function () use ($sessionId, $uid, $loginName, $pwd, $name, $remember) {
649+
$this->tokenProvider->invalidateToken($sessionId);
650+
$this->tokenProvider->generateToken($sessionId, $uid, $loginName, $pwd, $name, IToken::TEMPORARY_TOKEN, $remember);
651+
}, \OCP\Server::get(IDBConnection::class));
679652
return true;
680653
} catch (SessionNotAvailableException $ex) {
681654
// This can happen with OCC, where a memory session is used

0 commit comments

Comments
 (0)