upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-05-19 08:25:56 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 14

Merge pull request #59796 from nextcloud/backport/59785/stable26
Some checks failed
Cypress / init (push) Has been cancelled
Details
Lint eslint / eslint (push) Has been cancelled
Details
Lint php / php-lint (push) Has been cancelled
Details
Node / versions (push) Has been cancelled
Details
Node / node (push) Has been cancelled
Details
S3 primary storage integration tests / php8.0-objectstore-minio (push) Has been cancelled
Details
S3 primary storage integration tests / php8.0-objectstore_multibucket-minio (push) Has been cancelled
Details
S3 primary storage / php8.0-objectstore-minio (push) Has been cancelled
Details
S3 primary storage / php8.0-objectstore_multibucket-minio (push) Has been cancelled
Details
Psalm static code analysis / static-code-analysis (push) Has been cancelled
Details
Psalm static code analysis / static-code-analysis-security (push) Has been cancelled
Details
Psalm static code analysis / static-code-analysis-ocp (push) Has been cancelled
Details
Cypress / runner 1 (push) Has been cancelled
Details
Cypress / runner 2 (push) Has been cancelled
Details
Cypress / runner component (push) Has been cancelled
Details
Cypress / cypress-summary (push) Has been cancelled
Details
Lint php / php-lint-summary (push) Has been cancelled
Details
Node / test (push) Has been cancelled
Details
Node / jsunit (push) Has been cancelled
Details
Node / handlebars (push) Has been cancelled
Details
S3 primary storage integration tests / s3-primary-integration-summary (push) Has been cancelled
Details
S3 primary storage / s3-primary-summary (push) Has been cancelled
Details

Browse source 
[stable26] fix: Reduce the mixups between apptokens and session ids
This commit is contained in:
Louis 2026-04-23 14:02:52 +02:00 committed by GitHub
commit a7b0c65f8e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 59 additions and 22 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

52
lib/private/User/Session.php
View file

@ -45,6 +45,7 @@ use OC\Authentication\Exceptions\PasswordlessTokenException;
use OC\Authentication\Exceptions\PasswordLoginForbiddenException;
use OC\Authentication\Token\IProvider;
use OC\Authentication\Token\IToken;
use OC\Authentication\Token\PublicKeyToken;
use OC\Hooks\Emitter;
use OC\Hooks\PublicEmitter;
use OC_User;
@ -439,8 +440,15 @@ class Session implements IUserSession, Emitter {
}
try {
$isTokenPassword = $this->isTokenPassword($password);
} catch (ExpiredTokenException $e) {
$dbToken = $this->getTokenFromPassword($password);
$isTokenPassword = $dbToken !== null;
if (($dbToken instanceof PublicKeyToken)
&& ($dbToken->getType() !== IToken::PERMANENT_TOKEN)
) {
// Refuse session tokens here, only app tokens are handled
return false;
}
} catch (ExpiredTokenException) {
// Just return on an expired token no need to check further or record a failed login
return false;
}
@ -461,7 +469,6 @@ class Session implements IUserSession, Emitter {
}
if ($isTokenPassword) {
$dbToken = $this->tokenProvider->getToken($password);
$userFromToken = $this->manager->get($dbToken->getUID());
$isValidEmailLogin = $userFromToken->getEMailAddress() === $user
&& $this->validateTokenLoginName($userFromToken->getEMailAddress(), $dbToken);
@ -551,6 +558,24 @@ class Session implements IUserSession, Emitter {
}
}
/**
* Check if the given 'password' is actually a device token
*
* @throws ExpiredTokenException
*/
private function getTokenFromPassword(string $password): ?IToken {
try {
return $this->tokenProvider->getToken($password);
} catch (ExpiredTokenException $e) {
throw $e;
} catch (InvalidTokenException $ex) {
$this->logger->debug('Token is not valid: ' . $ex->getMessage(), [
'exception' => $ex,
]);
return null;
}
}
protected function prepareUserLogin($firstTimeLogin, $refreshCsrfToken = true) {
if ($refreshCsrfToken) {
// TODO: mock/inject/use non-static
@ -855,6 +880,7 @@ class Session implements IUserSession, Emitter {
*/
public function tryTokenLogin(IRequest $request) {
$authHeader = $request->getHeader('Authorization');
$tokenFromCookie = false;
if (strpos($authHeader, 'Bearer ') === 0) {
$token = substr($authHeader, 7);
} elseif ($request->getCookie($this->config->getSystemValueString('instanceid')) !== null) {
@ -862,6 +888,7 @@ class Session implements IUserSession, Emitter {
// session and the request has a session cookie
try {
$token = $this->session->getId();
$tokenFromCookie = true;
} catch (SessionNotAvailableException $ex) {
return false;
}
@ -869,6 +896,18 @@ class Session implements IUserSession, Emitter {
return false;
}
try {
$dbToken = $this->tokenProvider->getToken($token);
} catch (InvalidTokenException $e) {
// Can't really happen but better safe than sorry
return false;
}
if ($dbToken instanceof PublicKeyToken && $dbToken->getType() === IToken::TEMPORARY_TOKEN && !$tokenFromCookie) {
// Session token but from Bearer header, not allowed
return false;
}
if (!$this->loginWithToken($token)) {
return false;
}
@ -876,13 +915,6 @@ class Session implements IUserSession, Emitter {
return false;
}
try {
$dbToken = $this->tokenProvider->getToken($token);
} catch (InvalidTokenException $e) {
// Can't really happen but better save than sorry
return true;
}
// Set the session variable so we know this is an app password
if ($dbToken instanceof \OC\Authentication\Token\PublicKeyToken && $dbToken->getType() === IToken::PERMANENT_TOKEN) {
$this->session->set('app_password', $token);

29
tests/lib/User/SessionTest.php
View file

@ -405,16 +405,18 @@ class SessionTest extends \Test\TestCase {
$manager = $this->createMock(Manager::class);
$session = $this->createMock(ISession::class);
$request = $this->createMock(IRequest::class);
$token = $this->createMock(IToken::class);
/** @var Session $userSession */
$userSession = $this->getMockBuilder(Session::class)
->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher])
->setMethods(['isTokenPassword', 'login', 'supportsCookies', 'createSessionToken', 'getUser'])
->onlyMethods(['login', 'supportsCookies', 'createSessionToken', 'getUser'])
->getMock();
$userSession->expects($this->once())
->method('isTokenPassword')
->willReturn(true);
$this->tokenProvider->expects($this->once())
->method('getToken')
->with('I-AM-AN-APP-PASSWORD')
->willReturn($token);
$userSession->expects($this->once())
->method('login')
->with('john', 'I-AM-AN-APP-PASSWORD')
@ -1154,16 +1156,18 @@ class SessionTest extends \Test\TestCase {
$manager = $this->createMock(Manager::class);
$session = $this->createMock(ISession::class);
$request = $this->createMock(IRequest::class);
$token = $this->createMock(IToken::class);
/** @var Session $userSession */
$userSession = $this->getMockBuilder(Session::class)
->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher])
->setMethods(['isTokenPassword', 'login', 'supportsCookies', 'createSessionToken', 'getUser'])
->onlyMethods(['login', 'supportsCookies', 'createSessionToken', 'getUser'])
->getMock();
$userSession->expects($this->once())
->method('isTokenPassword')
->willReturn(true);
$this->tokenProvider->expects($this->once())
->method('getToken')
->with('I-AM-AN-PASSWORD')
->willReturn($token);
$userSession->expects($this->once())
->method('login')
->with('john', 'I-AM-AN-PASSWORD')
@ -1204,12 +1208,13 @@ class SessionTest extends \Test\TestCase {
/** @var Session $userSession */
$userSession = $this->getMockBuilder(Session::class)
->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher])
->setMethods(['isTokenPassword', 'login', 'supportsCookies', 'createSessionToken', 'getUser'])
->onlyMethods(['login', 'supportsCookies', 'createSessionToken', 'getUser'])
->getMock();
$userSession->expects($this->once())
->method('isTokenPassword')
->willReturn(false);
$this->tokenProvider->expects($this->once())
->method('getToken')
->with('I-AM-AN-PASSWORD')
->willThrowException(new InvalidTokenException());
$userSession->expects($this->once())
->method('login')
->with('john@foo.bar', 'I-AM-AN-PASSWORD')