From 1d38dd6fa06667abe4d41ee4a3fcc6cf45075054 Mon Sep 17 00:00:00 2001 From: Git'Fellow <12234510+solracsf@users.noreply.github.com> Date: Fri, 30 Jan 2026 18:34:41 +0100 Subject: [PATCH 1/2] refactor(session): Code simplification Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com> --- lib/private/User/Session.php | 128 ++++++++++-------- lib/private/User/User.php | 256 ++++++++++++++++++----------------- 2 files changed, 204 insertions(+), 180 deletions(-) diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index 27c9d42d1983c..3115f8e7e12d0 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -67,8 +67,7 @@ class Session implements IUserSession, Emitter { use TTransactional; - /** @var User $activeUser */ - protected $activeUser; + protected ?IUser $activeUser = null; public function __construct( private Manager $manager, @@ -83,9 +82,6 @@ public function __construct( ) { } - /** - * @param IProvider $provider - */ public function setTokenProvider(IProvider $provider) { $this->tokenProvider = $provider; } @@ -93,7 +89,6 @@ public function setTokenProvider(IProvider $provider) { /** * @param string $scope * @param string $method - * @param callable $callback */ public function listen($scope, $method, callable $callback) { $this->manager->listen($scope, $method, $callback); @@ -102,7 +97,6 @@ public function listen($scope, $method, callable $callback) { /** * @param string $scope optional * @param string $method optional - * @param callable $callback optional */ public function removeListener($scope = null, $method = null, ?callable $callback = null) { $this->manager->removeListener($scope, $method, $callback); @@ -145,7 +139,7 @@ public function setSession(ISession $session) { * @param IUser|null $user */ public function setUser($user) { - if (is_null($user)) { + if ($user === null) { $this->session->remove('user_id'); } else { $this->session->set('user_id', $user->getUID()); @@ -173,17 +167,22 @@ public function getUser() { if (OC_User::isIncognitoMode()) { return null; } - if (is_null($this->activeUser)) { - $uid = $this->session->get('user_id'); - if (is_null($uid)) { - return null; - } - $this->activeUser = $this->manager->get($uid); - if (is_null($this->activeUser)) { - return null; - } - $this->validateSession(); + + if ($this->activeUser !== null) { + return $this->activeUser; } + + $uid = $this->session->get('user_id'); + if ($uid === null) { + return null; + } + + $this->activeUser = $this->manager->get($uid); + if ($this->activeUser === null) { + return null; + } + + $this->validateSession(); return $this->activeUser; } @@ -194,17 +193,12 @@ public function getUser() { * - For browsers, the session token validity is checked */ protected function validateSession() { - $token = null; $appPassword = $this->session->get('app_password'); - if (is_null($appPassword)) { - try { - $token = $this->session->getId(); - } catch (SessionNotAvailableException $ex) { - return; - } - } else { - $token = $appPassword; + try { + $token = $appPassword ?? $this->session->getId(); + } catch (SessionNotAvailableException $ex) { + return; } if (!$this->validateToken($token)) { @@ -220,7 +214,7 @@ protected function validateSession() { */ public function isLoggedIn() { $user = $this->getUser(); - if (is_null($user)) { + if ($user === null) { return false; } @@ -233,7 +227,7 @@ public function isLoggedIn() { * @param string|null $loginName for the logged in user */ public function setLoginName($loginName) { - if (is_null($loginName)) { + if ($loginName === null) { $this->session->remove('loginname'); } else { $this->session->set('loginname', $loginName); @@ -246,12 +240,12 @@ public function setLoginName($loginName) { * @return ?string */ public function getLoginName() { - if ($this->activeUser) { + if ($this->activeUser !== null) { return $this->session->get('loginname'); } $uid = $this->session->get('user_id'); - if ($uid) { + if ($uid !== null) { $this->activeUser = $this->manager->get($uid); return $this->session->get('loginname'); } @@ -273,7 +267,6 @@ public function setImpersonatingUserID(bool $useCurrentUser = true): void { } $currentUser = $this->getUser(); - if ($currentUser === null) { throw new \OC\User\NoUserException(); } @@ -348,19 +341,21 @@ public function completeLogin(IUser $user, array $loginDetails, $regenerateSessi $loginDetails['password'], $isToken )); + $this->manager->emit('\OC\User', 'postLogin', [ $user, $loginDetails['loginName'], $loginDetails['password'], $isToken, ]); - if ($this->isLoggedIn()) { - $this->prepareUserLogin($firstTimeLogin, $regenerateSessionId); - return true; + + if (!$this->isLoggedIn()) { + $message = \OCP\Util::getL10N('lib')->t('Login canceled by app'); + throw new LoginException($message); } - $message = \OCP\Util::getL10N('lib')->t('Login canceled by app'); - throw new LoginException($message); + $this->prepareUserLogin($firstTimeLogin, $regenerateSessionId); + return true; } /** @@ -458,7 +453,7 @@ private function handleLoginFailed(IThrottler $throttler, int $currentDelay, str } protected function supportsCookies(IRequest $request) { - if (!is_null($request->getCookie('cookie_test'))) { + if ($request->getCookie('cookie_test') !== null) { return true; } setcookie('cookie_test', 'test', $this->timeFactory->getTime() + 3600); @@ -476,7 +471,7 @@ protected function isTwoFactorEnforced($username) { ['uid' => &$username] ); $user = $this->manager->get($username); - if (is_null($user)) { + if ($user === null) { $users = $this->manager->getByEmail($username); if (empty($users)) { return false; @@ -619,7 +614,7 @@ private function loginWithToken($token) { $this->manager->emit('\OC\User', 'preLogin', [$dbToken->getLoginName(), $password]); $user = $this->manager->get($uid); - if (is_null($user)) { + if ($user === null) { // user does not exist return false; } @@ -645,7 +640,7 @@ private function loginWithToken($token) { * @return boolean */ public function createSessionToken(IRequest $request, $uid, $loginName, $password = null, $remember = IToken::DO_NOT_REMEMBER) { - if (is_null($this->manager->get($uid))) { + if ($this->manager->get($uid) === null) { // User does not exist return false; } @@ -675,7 +670,7 @@ public function createSessionToken(IRequest $request, $uid, $loginName, $passwor * @return string|null the password or null if none was set in the token */ private function getPassword($password) { - if (is_null($password)) { + if ($password === null) { // This is surely no token ;-) return null; } @@ -714,7 +709,7 @@ private function checkTokenCredentials(IToken $dbToken, $token) { } catch (PasswordlessTokenException $ex) { // Token has no password - if (!is_null($this->activeUser) && !$this->activeUser->isEnabled()) { + if ($this->activeUser !== null && !$this->activeUser->isEnabled()) { $this->tokenProvider->invalidateToken($token); return false; } @@ -723,7 +718,7 @@ private function checkTokenCredentials(IToken $dbToken, $token) { } // Invalidate token if the user is no longer active - if (!is_null($this->activeUser) && !$this->activeUser->isEnabled()) { + if ($this->activeUser !== null && !$this->activeUser->isEnabled()) { $this->tokenProvider->invalidateToken($token); return false; } @@ -764,7 +759,7 @@ private function validateToken($token, $user = null) { return false; } - if (!is_null($user) && !$this->validateTokenLoginName($user, $dbToken)) { + if ($user !== null && !$this->validateTokenLoginName($user, $dbToken)) { return false; } @@ -791,7 +786,7 @@ private function validateTokenLoginName(?string $loginName, IToken $token): bool if (mb_strtolower($token->getLoginName()) !== mb_strtolower($loginName ?? '')) { // TODO: this makes it impossible to use different login names on browser and client // e.g. login by e-mail 'user@example.com' on browser for generating the token will not - // allow to use the client token with the login name 'user'. + // allow to use the client token with the login name 'user'. $this->logger->error('App token login name does not match', [ 'tokenLoginName' => $token->getLoginName(), 'sessionLoginName' => $loginName, @@ -869,7 +864,7 @@ public function loginWithCookie($uid, $currentToken, $oldSessionId) { $this->session->regenerateId(); $this->manager->emit('\OC\User', 'preRememberedLogin', [$uid]); $user = $this->manager->get($uid); - if (is_null($user)) { + if ($user === null) { // user does not exist return false; } @@ -949,6 +944,7 @@ public function createRememberMeToken(IUser $user) { public function logout() { $user = $this->getUser(); $this->manager->emit('\OC\User', 'logout', [$user]); + if ($user !== null) { try { $token = $this->session->getId(); @@ -957,16 +953,18 @@ public function logout() { 'user' => $user->getUID(), ]); } catch (SessionNotAvailableException $ex) { + // Session not available, continue logout } } $this->logger->debug('Logging out', [ - 'user' => $user === null ? null : $user->getUID(), + 'user' => $user?->getUID(), ]); $this->setUser(null); $this->setLoginName(null); $this->setToken(null); $this->unsetMagicInCookie(); $this->session->clear(); + $this->manager->emit('\OC\User', 'postLogout', [$user]); } @@ -985,6 +983,7 @@ public function setMagicInCookie($username, $token) { $domain = $this->config->getSystemValueString('cookie_domain'); $maxAge = $this->config->getSystemValueInt('remember_login_cookie_lifetime', 60 * 60 * 24 * 15); + \OC\Http\CookieHelper::setCookie( 'nc_username', $username, @@ -995,6 +994,7 @@ public function setMagicInCookie($username, $token) { true, \OC\Http\CookieHelper::SAMESITE_LAX ); + \OC\Http\CookieHelper::setCookie( 'nc_token', $token, @@ -1005,6 +1005,7 @@ public function setMagicInCookie($username, $token) { true, \OC\Http\CookieHelper::SAMESITE_LAX ); + try { \OC\Http\CookieHelper::setCookie( 'nc_session_id', @@ -1028,18 +1029,31 @@ public function unsetMagicInCookie() { //TODO: DI for cookies and IRequest $secureCookie = OC::$server->getRequest()->getServerProtocol() === 'https'; $domain = $this->config->getSystemValueString('cookie_domain'); + $expireTime = $this->timeFactory->getTime() - 3600; + + //TODO: DI + unset($_COOKIE['nc_username'], $_COOKIE['nc_token'], $_COOKIE['nc_session_id']); + + $cookieOptions = [ + 'expires' => $expireTime, + 'path' => OC::$WEBROOT, + 'domain' => $domain, + 'secure' => $secureCookie, + 'httponly' => true, + 'samesite' => 'Lax', + ]; + + // Clear cookies with current path + setcookie('nc_username', '', $cookieOptions); + setcookie('nc_token', '', $cookieOptions); + setcookie('nc_session_id', '', $cookieOptions); - unset($_COOKIE['nc_username']); //TODO: DI - unset($_COOKIE['nc_token']); - unset($_COOKIE['nc_session_id']); - setcookie('nc_username', '', $this->timeFactory->getTime() - 3600, OC::$WEBROOT, $domain, $secureCookie, true); - setcookie('nc_token', '', $this->timeFactory->getTime() - 3600, OC::$WEBROOT, $domain, $secureCookie, true); - setcookie('nc_session_id', '', $this->timeFactory->getTime() - 3600, OC::$WEBROOT, $domain, $secureCookie, true); // old cookies might be stored under /webroot/ instead of /webroot // and Firefox doesn't like it! - setcookie('nc_username', '', $this->timeFactory->getTime() - 3600, OC::$WEBROOT . '/', $domain, $secureCookie, true); - setcookie('nc_token', '', $this->timeFactory->getTime() - 3600, OC::$WEBROOT . '/', $domain, $secureCookie, true); - setcookie('nc_session_id', '', $this->timeFactory->getTime() - 3600, OC::$WEBROOT . '/', $domain, $secureCookie, true); + $cookieOptions['path'] = OC::$WEBROOT . '/'; + setcookie('nc_username', '', $cookieOptions); + setcookie('nc_token', '', $cookieOptions); + setcookie('nc_session_id', '', $cookieOptions); } /** diff --git a/lib/private/User/User.php b/lib/private/User/User.php index 1de44193cbc97..452e3ccfc545d 100644 --- a/lib/private/User/User.php +++ b/lib/private/User/User.php @@ -25,6 +25,7 @@ use OCP\IUser; use OCP\IUserBackend; use OCP\Notification\IManager as INotificationManager; +use OCP\Server; use OCP\User\Backend\IGetHomeBackend; use OCP\User\Backend\IPasswordHashBackend; use OCP\User\Backend\IProvideAvatarBackend; @@ -48,28 +49,16 @@ class User implements IUser { private IConfig $config; private IURLGenerator $urlGenerator; + private IAccountManager $accountManager; + private IAvatarManager $avatarManager; - /** @var IAccountManager */ - protected $accountManager; - - /** @var string|null */ - private $displayName; - - /** @var bool|null */ - private $enabled; - - /** @var Emitter|Manager|null */ - private $emitter; - - /** @var string */ - private $home; - + private ?string $displayName = null; + private ?bool $enabled = null; + private Emitter|Manager|null $emitter = null; + private string $home = ''; private ?int $lastLogin = null; private ?int $firstLogin = null; - /** @var IAvatarManager */ - private $avatarManager; - public function __construct( private string $uid, private ?UserInterface $backend, @@ -79,8 +68,10 @@ public function __construct( $urlGenerator = null, ) { $this->emitter = $emitter; - $this->config = $config ?? \OCP\Server::get(IConfig::class); - $this->urlGenerator = $urlGenerator ?? \OCP\Server::get(IURLGenerator::class); + $this->config = $config ?? Server::get(IConfig::class); + $this->urlGenerator = $urlGenerator ?? Server::get(IURLGenerator::class); + $this->accountManager = Server::get(IAccountManager::class); + $this->avatarManager = Server::get(IAvatarManager::class); } /** @@ -98,22 +89,20 @@ public function getUID() { * @return string */ public function getDisplayName() { - if ($this->displayName === null) { - $displayName = ''; - if ($this->backend && $this->backend->implementsActions(Backend::GET_DISPLAYNAME)) { - // get display name and strip whitespace from the beginning and end of it - $backendDisplayName = $this->backend->getDisplayName($this->uid); - if (is_string($backendDisplayName)) { - $displayName = trim($backendDisplayName); - } - } + if ($this->displayName !== null) { + return $this->displayName; + } - if (!empty($displayName)) { - $this->displayName = $displayName; - } else { - $this->displayName = $this->uid; + $displayName = ''; + if ($this->backend?->implementsActions(Backend::GET_DISPLAYNAME)) { + // get display name and strip whitespace from the beginning and end of it + $backendDisplayName = $this->backend->getDisplayName($this->uid); + if (is_string($backendDisplayName)) { + $displayName = trim($backendDisplayName); } } + + $this->displayName = $displayName ?: $this->uid; return $this->displayName; } @@ -128,18 +117,28 @@ public function getDisplayName() { */ public function setDisplayName($displayName) { $displayName = trim($displayName); + if (empty($displayName)) { + return false; + } + $oldDisplayName = $this->getDisplayName(); - if ($this->backend->implementsActions(Backend::SET_DISPLAYNAME) && !empty($displayName) && $displayName !== $oldDisplayName) { - /** @var ISetDisplayNameBackend $backend */ - $backend = $this->backend; - $result = $backend->setDisplayName($this->uid, $displayName); - if ($result) { - $this->displayName = $displayName; - $this->triggerChange('displayName', $displayName, $oldDisplayName); - } - return $result !== false; + if ($displayName === $oldDisplayName) { + return false; + } + + if (!$this->backend?->implementsActions(Backend::SET_DISPLAYNAME)) { + return false; + } + + /** @var ISetDisplayNameBackend $backend */ + $backend = $this->backend; + $result = $backend->setDisplayName($this->uid, $displayName); + if ($result) { + $this->displayName = $displayName; + $this->triggerChange('displayName', $displayName, $oldDisplayName); } - return false; + + return $result !== false; } /** @@ -256,7 +255,7 @@ public function updateLastLoginTimestamp(): bool { */ public function delete() { if ($this->backend === null) { - \OCP\Server::get(LoggerInterface::class)->error('Cannot delete user: No backend set'); + Server::get(LoggerInterface::class)->error('Cannot delete user: No backend set'); return false; } @@ -269,6 +268,7 @@ public function delete() { // Set delete flag on the user - this is needed to ensure that the user data is removed if there happen any exception in the backend // because we can not restore the user meaning we could not rollback to any stable state otherwise. $this->config->setUserValue($this->uid, 'core', 'deleted', 'true'); + // We also need to backup the home path as this can not be reconstructed later if the original backend uses custom home paths $this->config->setUserValue($this->uid, 'core', 'deleted.home-path', $this->getHome()); @@ -281,7 +281,7 @@ public function delete() { } // We have to delete the user from all groups - $groupManager = \OCP\Server::get(IGroupManager::class); + $groupManager = Server::get(IGroupManager::class); foreach ($groupManager->getUserGroupIds($this) as $groupId) { $group = $groupManager->get($groupId); if ($group) { @@ -291,32 +291,35 @@ public function delete() { } } - $commentsManager = \OCP\Server::get(ICommentsManager::class); + $commentsManager = Server::get(ICommentsManager::class); $commentsManager->deleteReferencesOfActor('users', $this->uid); $commentsManager->deleteReadMarksFromUser($this); - $avatarManager = \OCP\Server::get(AvatarManager::class); + $avatarManager = Server::get(AvatarManager::class); $avatarManager->deleteUserAvatar($this->uid); - $notificationManager = \OCP\Server::get(INotificationManager::class); + $notificationManager = Server::get(INotificationManager::class); $notification = $notificationManager->createNotification(); $notification->setUser($this->uid); $notificationManager->markProcessed($notification); - $accountManager = \OCP\Server::get(AccountManager::class); + $accountManager = Server::get(AccountManager::class); $accountManager->deleteUser($this); - $database = \OCP\Server::get(IDBConnection::class); + $database = Server::get(IDBConnection::class); try { // We need to create a transaction to make sure we are in a defined state // because if all user values are removed also the flag is gone, but if an exception happens (e.g. database lost connection on the set operation) // exactly here we are in an undefined state as the data is still present but the user does not exist on the system anymore. $database->beginTransaction(); + // Remove all user settings $this->config->deleteAllUserValues($this->uid); + // But again set flag that this user is about to be deleted $this->config->setUserValue($this->uid, 'core', 'deleted', 'true'); $this->config->setUserValue($this->uid, 'core', 'deleted.home-path', $this->getHome()); + // Commit the transaction so we are in a defined state: either the preferences are removed or an exception occurred but the delete flag is still present $database->commit(); } catch (\Throwable $e) { @@ -345,36 +348,35 @@ public function delete() { */ public function setPassword($password, $recoveryPassword = null) { $this->dispatcher->dispatchTyped(new BeforePasswordUpdatedEvent($this, $password, $recoveryPassword)); - if ($this->emitter) { - $this->emitter->emit('\OC\User', 'preSetPassword', [$this, $password, $recoveryPassword]); + $this->emitter?->emit('\\OC\\User', 'preSetPassword', [$this, $password, $recoveryPassword]); + + if (!$this->backend?->implementsActions(Backend::SET_PASSWORD)) { + return false; } - if ($this->backend->implementsActions(Backend::SET_PASSWORD)) { - /** @var ISetPasswordBackend $backend */ - $backend = $this->backend; - $result = $backend->setPassword($this->uid, $password); - if ($result !== false) { - $this->dispatcher->dispatchTyped(new PasswordUpdatedEvent($this, $password, $recoveryPassword)); - if ($this->emitter) { - $this->emitter->emit('\OC\User', 'postSetPassword', [$this, $password, $recoveryPassword]); - } - } + /** @var ISetPasswordBackend $backend */ + $backend = $this->backend; + $result = $backend->setPassword($this->uid, $password); - return !($result === false); - } else { + if ($result === false) { return false; } + + $this->dispatcher->dispatchTyped(new PasswordUpdatedEvent($this, $password, $recoveryPassword)); + $this->emitter?->emit('\\OC\\User', 'postSetPassword', [$this, $password, $recoveryPassword]); + + return true; } public function getPasswordHash(): ?string { - if (!($this->backend instanceof IPasswordHashBackend)) { + if (!$this->backend instanceof IPasswordHashBackend) { return null; } return $this->backend->getPasswordHash($this->uid); } public function setPasswordHash(string $passwordHash): bool { - if (!($this->backend instanceof IPasswordHashBackend)) { + if (!$this->backend instanceof IPasswordHashBackend) { return false; } return $this->backend->setPasswordHash($this->uid, $passwordHash); @@ -386,14 +388,21 @@ public function setPasswordHash(string $passwordHash): bool { * @return string */ public function getHome() { - if (!$this->home) { - /** @psalm-suppress UndefinedInterfaceMethod Once we get rid of the legacy implementsActions, psalm won't complain anymore */ - if (($this->backend instanceof IGetHomeBackend || $this->backend->implementsActions(Backend::GET_HOME)) && $home = $this->backend->getHome($this->uid)) { + if ($this->home) { + return $this->home; + } + + /** @psalm-suppress UndefinedInterfaceMethod Once we get rid of the legacy implementsActions, psalm won't complain anymore */ + if ($this->backend instanceof IGetHomeBackend || $this->backend?->implementsActions(Backend::GET_HOME)) { + $home = $this->backend->getHome($this->uid); + if ($home) { $this->home = $home; - } else { - $this->home = $this->config->getSystemValueString('datadirectory', \OC::$SERVERROOT . '/data') . '/' . $this->uid; + return $this->home; } } + + $this->home = $this->config->getSystemValueString('datadirectory', \OC::$SERVERROOT . '/data') . '/' . $this->uid; + return $this->home; } @@ -460,17 +469,18 @@ public function canChangeEmail(): bool { */ public function isEnabled() { $queryDatabaseValue = function (): bool { - if ($this->enabled === null) { - $enabled = $this->config->getUserValue($this->uid, 'core', 'enabled', 'true'); - $this->enabled = $enabled === 'true'; + if ($this->enabled !== null) { + return $this->enabled; } + + $enabled = $this->config->getUserValue($this->uid, 'core', 'enabled', 'true'); + $this->enabled = ($enabled === 'true'); return $this->enabled; }; - if ($this->backend instanceof IProvideEnabledStateBackend) { - return $this->backend->isUserEnabled($this->uid, $queryDatabaseValue); - } else { - return $queryDatabaseValue(); - } + + return $this->backend instanceof IProvideEnabledStateBackend + ? $this->backend->isUserEnabled($this->uid, $queryDatabaseValue) + : $queryDatabaseValue(); } /** @@ -480,23 +490,27 @@ public function isEnabled() { */ public function setEnabled(bool $enabled = true) { $oldStatus = $this->isEnabled(); + $setDatabaseValue = function (bool $enabled): void { $this->config->setUserValue($this->uid, 'core', 'enabled', $enabled ? 'true' : 'false'); $this->enabled = $enabled; }; - if ($this->backend instanceof IProvideEnabledStateBackend) { - $queryDatabaseValue = function (): bool { - if ($this->enabled === null) { - $enabled = $this->config->getUserValue($this->uid, 'core', 'enabled', 'true'); - $this->enabled = $enabled === 'true'; - } + + $queryDatabaseValue = function (): bool { + if ($this->enabled !== null) { return $this->enabled; - }; - $enabled = $this->backend->setUserEnabled($this->uid, $enabled, $queryDatabaseValue, $setDatabaseValue); - if ($oldStatus !== $enabled) { - $this->triggerChange('enabled', $enabled, $oldStatus); } - } elseif ($oldStatus !== $enabled) { + + $enabled = $this->config->getUserValue($this->uid, 'core', 'enabled', 'true'); + $this->enabled = ($enabled === 'true'); + return $this->enabled; + }; + + if ($this->backend instanceof IProvideEnabledStateBackend) { + $enabled = $this->backend->setUserEnabled($this->uid, $enabled, $queryDatabaseValue, $setDatabaseValue); + } + + if ($oldStatus !== $enabled) { $setDatabaseValue($enabled); $this->triggerChange('enabled', $enabled, $oldStatus); } @@ -539,26 +553,32 @@ public function getQuota() { $event = new GetQuotaEvent($this); $this->dispatcher->dispatchTyped($event); $overwriteQuota = $event->getQuota(); + if ($overwriteQuota) { - $quota = $overwriteQuota; - } else { - $quota = $this->config->getUserValue($this->uid, 'files', 'quota', 'default'); - } - if ($quota === 'default') { - $quota = $this->config->getAppValue('files', 'default_quota', 'none'); - - // if unlimited quota is not allowed => avoid getting 'unlimited' as default_quota fallback value - // use the first preset instead - $allowUnlimitedQuota = $this->config->getAppValue('files', 'allow_unlimited_quota', '1') === '1'; - if (!$allowUnlimitedQuota) { - $presets = $this->config->getAppValue('files', 'quota_preset', '1 GB, 5 GB, 10 GB'); - $presets = array_filter(array_map('trim', explode(',', $presets))); - $quotaPreset = array_values(array_diff($presets, ['default', 'none'])); - if (count($quotaPreset) > 0) { - $quota = $this->config->getAppValue('files', 'default_quota', $quotaPreset[0]); - } - } + return $overwriteQuota; + } + + $quota = $this->config->getUserValue($this->uid, 'files', 'quota', 'default'); + if ($quota !== 'default') { + return $quota; + } + + $quota = $this->config->getAppValue('files', 'default_quota', 'none'); + $allowUnlimitedQuota = $this->config->getAppValue('files', 'allow_unlimited_quota', '1') === '1'; + if ($allowUnlimitedQuota) { + return $quota; + } + + // if unlimited quota is not allowed => avoid getting 'unlimited' as default_quota fallback value + // use the first preset instead + $presets = $this->config->getAppValue('files', 'quota_preset', '1 GB, 5 GB, 10 GB'); + $presets = array_filter(array_map('trim', explode(',', $presets))); + $quotaPreset = array_values(array_diff($presets, ['default', 'none'])); + + if (count($quotaPreset) > 0) { + return $this->config->getAppValue('files', 'default_quota', $quotaPreset[0]); } + return $quota; } @@ -585,17 +605,19 @@ public function getQuotaBytes(): int|float { */ public function setQuota($quota) { $oldQuota = $this->config->getUserValue($this->uid, 'files', 'quota', ''); - if ($quota !== 'none' && $quota !== 'default') { + if (!in_array($quota, ['none', 'default'], true)) { $bytesQuota = \OCP\Util::computerFileSize($quota); if ($bytesQuota === false) { throw new InvalidArgumentException('Failed to set quota to invalid value ' . $quota); } $quota = \OCP\Util::humanFileSize($bytesQuota); } + if ($quota !== $oldQuota) { $this->config->setUserValue($this->uid, 'files', 'quota', $quota); $this->triggerChange('quota', $quota, $oldQuota); } + \OC_Helper::clearStorageInfo('/' . $this->uid . '/files'); } @@ -628,18 +650,8 @@ public function setManagerUids(array $uids): void { * @since 9.0.0 */ public function getAvatarImage($size) { - // delay the initialization - if (is_null($this->avatarManager)) { - $this->avatarManager = \OC::$server->get(IAvatarManager::class); - } - $avatar = $this->avatarManager->getAvatar($this->uid); - $image = $avatar->get($size); - if ($image) { - return $image; - } - - return null; + return $avatar->get($size) ?: null; } /** @@ -668,8 +680,6 @@ private function removeProtocolFromUrl(string $url): string { public function triggerChange($feature, $value = null, $oldValue = null) { $this->dispatcher->dispatchTyped(new UserChangedEvent($this, $feature, $value, $oldValue)); - if ($this->emitter) { - $this->emitter->emit('\OC\User', 'changeUser', [$this, $feature, $value, $oldValue]); - } + $this->emitter?->emit('\OC\User', 'changeUser', [$this, $feature, $value, $oldValue]); } } From 133a94ffa4cbd7d389b5df550e56b3fe8607daa5 Mon Sep 17 00:00:00 2001 From: Git'Fellow <12234510+solracsf@users.noreply.github.com> Date: Fri, 30 Jan 2026 19:27:41 +0100 Subject: [PATCH 2/2] fix: tests Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com> --- lib/private/User/User.php | 41 ++++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/lib/private/User/User.php b/lib/private/User/User.php index 452e3ccfc545d..1bf11f455e8f6 100644 --- a/lib/private/User/User.php +++ b/lib/private/User/User.php @@ -49,16 +49,28 @@ class User implements IUser { private IConfig $config; private IURLGenerator $urlGenerator; - private IAccountManager $accountManager; - private IAvatarManager $avatarManager; - private ?string $displayName = null; - private ?bool $enabled = null; - private Emitter|Manager|null $emitter = null; - private string $home = ''; + /** @var IAccountManager */ + protected $accountManager; + + /** @var string|null */ + private $displayName; + + /** @var bool|null */ + private $enabled; + + /** @var Emitter|Manager|null */ + private $emitter; + + /** @var string */ + private $home; + private ?int $lastLogin = null; private ?int $firstLogin = null; + /** @var IAvatarManager */ + private $avatarManager; + public function __construct( private string $uid, private ?UserInterface $backend, @@ -68,10 +80,8 @@ public function __construct( $urlGenerator = null, ) { $this->emitter = $emitter; - $this->config = $config ?? Server::get(IConfig::class); - $this->urlGenerator = $urlGenerator ?? Server::get(IURLGenerator::class); - $this->accountManager = Server::get(IAccountManager::class); - $this->avatarManager = Server::get(IAvatarManager::class); + $this->config = $config ?? \OCP\Server::get(IConfig::class); + $this->urlGenerator = $urlGenerator ?? \OCP\Server::get(IURLGenerator::class); } /** @@ -650,8 +660,17 @@ public function setManagerUids(array $uids): void { * @since 9.0.0 */ public function getAvatarImage($size) { + // delay the initialization + if ($this->avatarManager === null) { + $this->avatarManager = \OC::$server->get(IAvatarManager::class); + } + $avatar = $this->avatarManager->getAvatar($this->uid); - return $avatar->get($size) ?: null; + if ($image = $avatar->get($size)) { + return $image; + } + + return null; } /**