Skip to content

Commit 38a6c50

Browse files
authored
Merge pull request #46435 from nextcloud/backport/46398/stable27
[stable27] fix(Session): avoid race conditions on clustered setups
2 parents 6268273 + e39be84 commit 38a6c50

File tree

4 files changed

+22
-46
lines changed

4 files changed

+22
-46
lines changed

build/psalm-baseline.xml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3587,6 +3587,9 @@
35873587
<code><![CDATA[$request->server]]></code>
35883588
<code><![CDATA[$request->server]]></code>
35893589
</NoInterfaceProperties>
3590+
<RedundantCondition>
3591+
<code><![CDATA[$this->manager instanceof PublicEmitter]]></code>
3592+
</RedundantCondition>
35903593
<TooManyArguments>
35913594
<code>dispatch</code>
35923595
</TooManyArguments>

lib/private/Authentication/Token/PublicKeyTokenProvider.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ public function getToken(string $tokenId): IToken {
190190
private function getTokenFromCache(string $tokenHash): ?PublicKeyToken {
191191
$serializedToken = $this->cache->get($tokenHash);
192192
if ($serializedToken === false) {
193-
throw new InvalidTokenException('Token does not exist: ' . $tokenHash);
193+
return null;
194194
}
195195

196196
if ($serializedToken === null) {

lib/private/Server.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -582,7 +582,7 @@ public function __construct($webRoot, \OC\Config $config) {
582582
$c->get(ISecureRandom::class),
583583
$c->getLockdownManager(),
584584
$c->get(LoggerInterface::class),
585-
$c->get(IEventDispatcher::class)
585+
$c->get(IEventDispatcher::class),
586586
);
587587
/** @deprecated 21.0.0 use BeforeUserCreatedEvent event with the IEventDispatcher instead */
588588
$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
@@ -51,10 +51,12 @@
5151
use OC_User;
5252
use OC_Util;
5353
use OCA\DAV\Connector\Sabre\Auth;
54+
use OCP\AppFramework\Db\TTransactional;
5455
use OCP\AppFramework\Utility\ITimeFactory;
5556
use OCP\EventDispatcher\IEventDispatcher;
5657
use OCP\Files\NotPermittedException;
5758
use OCP\IConfig;
59+
use OCP\IDBConnection;
5860
use OCP\IRequest;
5961
use OCP\ISession;
6062
use OCP\IUser;
@@ -91,53 +93,22 @@
9193
* @package OC\User
9294
*/
9395
class Session implements IUserSession, Emitter {
94-
/** @var Manager $manager */
95-
private $manager;
96-
97-
/** @var ISession $session */
98-
private $session;
99-
100-
/** @var ITimeFactory */
101-
private $timeFactory;
102-
103-
/** @var IProvider */
104-
private $tokenProvider;
105-
106-
/** @var IConfig */
107-
private $config;
96+
use TTransactional;
10897

10998
/** @var User $activeUser */
11099
protected $activeUser;
111100

112-
/** @var ISecureRandom */
113-
private $random;
114-
115-
/** @var ILockdownManager */
116-
private $lockdownManager;
117-
118-
private LoggerInterface $logger;
119-
/** @var IEventDispatcher */
120-
private $dispatcher;
121-
122-
public function __construct(Manager $manager,
123-
ISession $session,
124-
ITimeFactory $timeFactory,
125-
?IProvider $tokenProvider,
126-
IConfig $config,
127-
ISecureRandom $random,
128-
ILockdownManager $lockdownManager,
129-
LoggerInterface $logger,
130-
IEventDispatcher $dispatcher
101+
public function __construct(
102+
private Manager $manager,
103+
private ISession $session,
104+
private ITimeFactory $timeFactory,
105+
private ?IProvider $tokenProvider,
106+
private IConfig $config,
107+
private ISecureRandom $random,
108+
private ILockdownManager $lockdownManager,
109+
private LoggerInterface $logger,
110+
private IEventDispatcher $dispatcher
131111
) {
132-
$this->manager = $manager;
133-
$this->session = $session;
134-
$this->timeFactory = $timeFactory;
135-
$this->tokenProvider = $tokenProvider;
136-
$this->config = $config;
137-
$this->random = $random;
138-
$this->lockdownManager = $lockdownManager;
139-
$this->logger = $logger;
140-
$this->dispatcher = $dispatcher;
141112
}
142113

143114
/**
@@ -693,8 +664,10 @@ public function createSessionToken(IRequest $request, $uid, $loginName, $passwor
693664
$sessionId = $this->session->getId();
694665
$pwd = $this->getPassword($password);
695666
// Make sure the current sessionId has no leftover tokens
696-
$this->tokenProvider->invalidateToken($sessionId);
697-
$this->tokenProvider->generateToken($sessionId, $uid, $loginName, $pwd, $name, IToken::TEMPORARY_TOKEN, $remember);
667+
$this->atomic(function () use ($sessionId, $uid, $loginName, $pwd, $name, $remember) {
668+
$this->tokenProvider->invalidateToken($sessionId);
669+
$this->tokenProvider->generateToken($sessionId, $uid, $loginName, $pwd, $name, IToken::TEMPORARY_TOKEN, $remember);
670+
}, \OCP\Server::get(IDBConnection::class));
698671
return true;
699672
} catch (SessionNotAvailableException $ex) {
700673
// This can happen with OCC, where a memory session is used

0 commit comments

Comments
 (0)