From e08c75a165c5d53bd7898286db32dc634bd1ac72 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 7 Apr 2026 17:17:56 +0200 Subject: [PATCH 01/13] feat: Throw an error when trying to listen to no longer used signals Signed-off-by: Carl Schwan --- lib/private/Files/Filesystem.php | 7 +++ lib/private/Files/Node/HookConnector.php | 26 ++++---- lib/private/legacy/OC_Hook.php | 79 +++++++++++++++++++++++- lib/private/legacy/OC_User.php | 4 +- 4 files changed, 100 insertions(+), 16 deletions(-) diff --git a/lib/private/Files/Filesystem.php b/lib/private/Files/Filesystem.php index 022467e754877..4bb44d822a24e 100644 --- a/lib/private/Files/Filesystem.php +++ b/lib/private/Files/Filesystem.php @@ -157,6 +157,10 @@ class Filesystem { * @param string $path */ public const signal_delete = 'delete'; + public const signal_post_delete = 'post_delete'; + + public const signal_touch = 'touch'; + public const signal_post_touch = 'post_touch'; /** * parameters definitions for signals @@ -174,6 +178,9 @@ class Filesystem { public const signal_delete_mount = 'delete_mount'; public const signal_param_mount_type = 'mounttype'; public const signal_param_users = 'users'; + public const signal_setup = 'setup'; + public const signal_pre_setup = 'preSetup'; + public const signal_post_init_mountpoints = 'post_initMountPoints'; /** * @param bool $shouldLog diff --git a/lib/private/Files/Node/HookConnector.php b/lib/private/Files/Node/HookConnector.php index a318d182fe644..bfcd200837232 100644 --- a/lib/private/Files/Node/HookConnector.php +++ b/lib/private/Files/Node/HookConnector.php @@ -45,25 +45,25 @@ public function __construct( } public function viewToNode() { - Util::connectHook('OC_Filesystem', 'write', $this, 'write'); - Util::connectHook('OC_Filesystem', 'post_write', $this, 'postWrite'); + Util::connectHook(Filesystem::CLASSNAME, Filesystem::signal_write, $this, 'write'); + Util::connectHook(Filesystem::CLASSNAME, Filesystem::signal_post_write, $this, 'postWrite'); - Util::connectHook('OC_Filesystem', 'create', $this, 'create'); - Util::connectHook('OC_Filesystem', 'post_create', $this, 'postCreate'); + Util::connectHook(Filesystem::CLASSNAME, 'create', $this, 'create'); + Util::connectHook(Filesystem::CLASSNAME, 'post_create', $this, 'postCreate'); - Util::connectHook('OC_Filesystem', 'delete', $this, 'delete'); - Util::connectHook('OC_Filesystem', 'post_delete', $this, 'postDelete'); + Util::connectHook(Filesystem::CLASSNAME, 'delete', $this, 'delete'); + Util::connectHook(Filesystem::CLASSNAME, 'post_delete', $this, 'postDelete'); - Util::connectHook('OC_Filesystem', 'rename', $this, 'rename'); - Util::connectHook('OC_Filesystem', 'post_rename', $this, 'postRename'); + Util::connectHook(Filesystem::CLASSNAME, 'rename', $this, 'rename'); + Util::connectHook(Filesystem::CLASSNAME, 'post_rename', $this, 'postRename'); - Util::connectHook('OC_Filesystem', 'copy', $this, 'copy'); - Util::connectHook('OC_Filesystem', 'post_copy', $this, 'postCopy'); + Util::connectHook(Filesystem::CLASSNAME, 'copy', $this, 'copy'); + Util::connectHook(Filesystem::CLASSNAME, 'post_copy', $this, 'postCopy'); - Util::connectHook('OC_Filesystem', 'touch', $this, 'touch'); - Util::connectHook('OC_Filesystem', 'post_touch', $this, 'postTouch'); + Util::connectHook(Filesystem::CLASSNAME, 'touch', $this, 'touch'); + Util::connectHook(Filesystem::CLASSNAME, 'post_touch', $this, 'postTouch'); - Util::connectHook('OC_Filesystem', 'read', $this, 'read'); + Util::connectHook(Filesystem::CLASSNAME, 'read', $this, 'read'); } public function write($arguments) { diff --git a/lib/private/legacy/OC_Hook.php b/lib/private/legacy/OC_Hook.php index 9d0a7bf34c7d2..f173679a6ad36 100644 --- a/lib/private/legacy/OC_Hook.php +++ b/lib/private/legacy/OC_Hook.php @@ -5,15 +5,84 @@ * SPDX-FileCopyrightText: 2016 ownCloud, Inc. * SPDX-License-Identifier: AGPL-3.0-only */ + +use OC\Files\Filesystem; use OC\ServerNotAvailableException; use OCP\HintException; use OCP\Server; +use OCP\Share; use Psr\Log\LoggerInterface; class OC_Hook { public static $thrownExceptions = []; - private static $registered = []; + private static array $registered = []; + + private static array $allowList = [ + [Filesystem::CLASSNAME, Filesystem::signal_read], + [Filesystem::CLASSNAME, Filesystem::signal_create], + [Filesystem::CLASSNAME, Filesystem::signal_post_create], + [Filesystem::CLASSNAME, Filesystem::signal_update], + [Filesystem::CLASSNAME, Filesystem::signal_post_update], + [Filesystem::CLASSNAME, Filesystem::signal_write], + [Filesystem::CLASSNAME, Filesystem::signal_post_write], + [Filesystem::CLASSNAME, Filesystem::signal_delete], + [Filesystem::CLASSNAME, Filesystem::signal_post_delete], + [Filesystem::CLASSNAME, Filesystem::signal_rename], + [Filesystem::CLASSNAME, Filesystem::signal_post_rename], + [Filesystem::CLASSNAME, Filesystem::signal_copy], + [Filesystem::CLASSNAME, Filesystem::signal_post_copy], + [Filesystem::CLASSNAME, Filesystem::signal_touch], + [Filesystem::CLASSNAME, Filesystem::signal_post_touch], + [Filesystem::CLASSNAME, Filesystem::signal_delete_mount], + [Filesystem::CLASSNAME, Filesystem::signal_create_mount], + [Filesystem::CLASSNAME, Filesystem::signal_setup], + [Filesystem::CLASSNAME, Filesystem::signal_pre_setup], + [Filesystem::CLASSNAME, Filesystem::signal_post_init_mountpoints], + [Filesystem::CLASSNAME, 'umount'], + ['\OCA\Files_Sharing\API\Server2Server', 'preLoginNameUsedAsUserName'], + [Share::class,'share_link_access'], + [Share::class,'pre_unshare'], + [Share::class,'post_unshare'], + [Share::class,'post_unshareFromSelf'], + [Share::class,'pre_shared'], + [Share::class,'post_shared'], + [Share::class,'post_set_expiration_date'], + [Share::class,'post_update_password'], + [Share::class,'post_update_permissions'], + ['\OC\Share','verifyExpirationDate'], + ['\OC\Files\Storage\Shared','fopen'], + ['\OC\Files\Storage\Shared','file_get_contents'], + ['\OC\Files\Storage\Shared','file_put_contents'], + ['\OCA\Files_Trashbin\Trashbin','post_moveToTrash'], + ['\OCA\Files_Trashbin\Trashbin','post_restore'], + ['\OCP\Trashbin','preDeleteAll'], + ['\OCP\Trashbin','deleteAll'], + ['\OCP\Versions','rollback'], + ['\OCP\Versions','preDelete'], + ['\OCP\Versions','delete'], + [OC_User::class,'pre_createUser'], + [OC_User::class,'post_createUser'], + [OC_User::class,'pre_deleteUser'], + [OC_User::class,'post_deleteUser'], + [OC_User::class,'pre_setPassword'], + [OC_User::class,'post_setPassword'], + [OC_User::class,'pre_login'], + [OC_User::class,'post_login'], + [OC_User::class,'logout'], + [OC_User::class,'changeUser'], + ['\OC\User','assignedUserId'], + ['\OC\User','preUnassignedUserId'], + ['\OC\User','postUnassignedUserId'], + ['\OC\Files\Cache\Scanner','scan_file'], + ['\OC\Files\Cache\Scanner','post_scan_file'], + ['Scanner','removeFromCache'], + ['Scanner','addToCache'], + ['Scanner','correctFolderSize'], + ['\OCP\Config','js'], + ['\OC\Core\LostPassword\Controller\LostController','post_passwordReset'], + ['\OC\Core\LostPassword\Controller\LostController','pre_passwordReset'], + ]; /** * connects a function to a hook @@ -29,6 +98,14 @@ class OC_Hook { * TODO: write example */ public static function connect($signalClass, $signalName, $slotClass, $slotName) { + $found = array_find(self::$allowList, function ($allowed) use ($signalClass, $signalName) { + [$allowedClass, $allowedSignal] = $allowed; + return $allowedClass === $signalClass && $allowedSignal === $signalName; + }) !== null; + + if (!$found) { + throw new \RuntimeException("The signal $signalClass::$signalName is no longer emitted in server. Listening to it is NOOP."); + } // If we're trying to connect to an emitting class that isn't // yet registered, register it if (!array_key_exists($signalClass, self::$registered)) { diff --git a/lib/private/legacy/OC_User.php b/lib/private/legacy/OC_User.php index bd71d7a343fa7..c4531f062c2d6 100644 --- a/lib/private/legacy/OC_User.php +++ b/lib/private/legacy/OC_User.php @@ -148,7 +148,7 @@ public static function setupBackends() { public static function loginWithApache(IApacheBackend $backend): bool { $uid = $backend->getCurrentUserId(); $run = true; - OC_Hook::emit('OC_User', 'pre_login', ['run' => &$run, 'uid' => $uid, 'backend' => $backend]); + Util::emitHook('OC_User', 'pre_login', ['run' => &$run, 'uid' => $uid, 'backend' => $backend]); if ($uid) { if (self::getUser() !== $uid) { @@ -197,7 +197,7 @@ public static function loginWithApache(IApacheBackend $backend): bool { // completed before we can safely create the users folder. // For example encryption needs to initialize the users keys first // before we can create the user folder with the skeleton files - OC_Hook::emit( + Util::emitHook( 'OC_User', 'post_login', [ From 722124f3b58678becc58a1c4034f91a436844376 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Wed, 8 Apr 2026 12:37:38 +0200 Subject: [PATCH 02/13] refactor: Remove preLoginNameUsedAsUserName hook Signed-off-by: Carl Schwan --- .../Controller/RequestHandlerController.php | 16 +---- .../lib/AddressHandler.php | 72 ++++++------------- .../lib/OCM/CloudFederationProviderFiles.php | 7 +- .../tests/AddressHandlerTest.php | 6 +- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + .../lib/Event/LoadAdditionalBackendEvent.php | 17 +++++ .../lib/Service/BackendService.php | 54 +++++--------- .../tests/Service/BackendServiceTest.php | 14 ++-- .../lib/Listener/SharesUpdatedListener.php | 2 +- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + apps/user_ldap/lib/AppInfo/Application.php | 29 +------- apps/user_ldap/lib/Helper.php | 19 ----- .../LoadAdditionalBackendListener.php | 39 ++++++++++ apps/user_ldap/lib/User_LDAP.php | 6 +- apps/user_ldap/lib/User_Proxy.php | 23 +++--- apps/user_ldap/tests/User_LDAPTest.php | 24 +++---- build/psalm-baseline.xml | 49 ------------- core/Controller/LostController.php | 7 +- core/Controller/WebAuthnController.php | 8 +-- lib/OC.php | 8 --- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + .../Listeners/LoginFailedListener.php | 7 +- lib/private/User/Database.php | 36 ++-------- lib/private/User/Manager.php | 17 +++++ lib/private/User/Session.php | 6 +- lib/private/legacy/OC_Hook.php | 1 - lib/public/IUserManager.php | 7 ++ .../IGetUserNameFromLoginNameBackend.php | 25 +++++++ tests/Core/Controller/LostControllerTest.php | 3 + .../Login/PreLoginHookCommandTest.php | 17 ++--- tests/lib/User/ManagerTest.php | 12 ++-- tests/lib/User/UserTest.php | 2 - tests/lib/Util/User/Dummy.php | 2 +- 36 files changed, 225 insertions(+), 316 deletions(-) create mode 100644 apps/files_external/lib/Event/LoadAdditionalBackendEvent.php create mode 100644 apps/user_ldap/lib/Listener/LoadAdditionalBackendListener.php create mode 100644 lib/public/User/Backend/IGetUserNameFromLoginNameBackend.php diff --git a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php index 59e88faee51fa..e3dac3e9b3199 100644 --- a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php +++ b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php @@ -43,7 +43,6 @@ use OCP\Security\Signature\IIncomingSignedRequest; use OCP\Server; use OCP\Share\Exceptions\ShareNotFound; -use OCP\Util; use Psr\Log\LoggerInterface; /** @@ -409,21 +408,12 @@ private function protocolCarriesSharedSecret(array $protocol): bool { } /** - * map login name to internal LDAP UID if a LDAP backend is in use - * - * @param string $uid - * @return string mixed + * Map login name to internal LDAP UID if an LDAP backend is in use */ - private function mapUid($uid) { - // FIXME this should be a method in the user management instead + private function mapUid(string $uid): string { $this->logger->debug('shareWith before, ' . $uid, ['app' => $this->appName]); - Util::emitHook( - '\OCA\Files_Sharing\API\Server2Server', - 'preLoginNameUsedAsUserName', - ['uid' => &$uid] - ); + $uid = $this->userManager->getUserNameFromLoginName($uid); $this->logger->debug('shareWith after, ' . $uid, ['app' => $this->appName]); - return $uid; } diff --git a/apps/federatedfilesharing/lib/AddressHandler.php b/apps/federatedfilesharing/lib/AddressHandler.php index a4502c939f8c1..5bb2c8be81be2 100644 --- a/apps/federatedfilesharing/lib/AddressHandler.php +++ b/apps/federatedfilesharing/lib/AddressHandler.php @@ -12,7 +12,7 @@ use OCP\HintException; use OCP\IL10N; use OCP\IURLGenerator; -use OCP\Util; +use OCP\IUserManager; /** * Class AddressHandler - parse, modify and construct federated sharing addresses @@ -23,26 +23,23 @@ class AddressHandler { /** * AddressHandler constructor. - * - * @param IURLGenerator $urlGenerator - * @param IL10N $l - * @param ICloudIdManager $cloudIdManager */ public function __construct( - private IURLGenerator $urlGenerator, - private IL10N $l, - private ICloudIdManager $cloudIdManager, + private readonly IURLGenerator $urlGenerator, + private readonly IL10N $l, + private readonly ICloudIdManager $cloudIdManager, + private readonly IUserManager $userManager, ) { } /** - * split user and remote from federated cloud id + * Split user and remote from federated cloud id. * * @param string $address federated share address * @return array [user, remoteURL] * @throws HintException */ - public function splitUserRemote($address) { + public function splitUserRemote(string $address): array { try { $cloudId = $this->cloudIdManager->resolveCloudId($address); return [$cloudId->getUser(), $cloudId->getRemote()]; @@ -53,55 +50,36 @@ public function splitUserRemote($address) { } /** - * generate remote URL part of federated ID + * Generate remote URL part of federated ID * * @return string url of the current server */ - public function generateRemoteURL() { + public function generateRemoteURL(): string { return $this->urlGenerator->getAbsoluteURL('/'); } /** - * check if two federated cloud IDs refer to the same user + * Check if two federated cloud IDs refer to the same user * - * @param string $user1 - * @param string $server1 - * @param string $user2 - * @param string $server2 * @return bool true if both users and servers are the same */ - public function compareAddresses($user1, $server1, $user2, $server2) { + public function compareAddresses(string $user1, string $server1, string $user2, string $server2): bool { $normalizedServer1 = strtolower($this->removeProtocolFromUrl($server1)); $normalizedServer2 = strtolower($this->removeProtocolFromUrl($server2)); - if (rtrim($normalizedServer1, '/') === rtrim($normalizedServer2, '/')) { - // FIXME this should be a method in the user management instead - Util::emitHook( - '\OCA\Files_Sharing\API\Server2Server', - 'preLoginNameUsedAsUserName', - ['uid' => &$user1] - ); - Util::emitHook( - '\OCA\Files_Sharing\API\Server2Server', - 'preLoginNameUsedAsUserName', - ['uid' => &$user2] - ); - - if ($user1 === $user2) { - return true; - } + if (rtrim($normalizedServer1, '/') !== rtrim($normalizedServer2, '/')) { + return false; } - return false; + $user1 = $this->userManager->getUserNameFromLoginName($user1); + $user2 = $this->userManager->getUserNameFromLoginName($user2); + return $user1 === $user2; } /** - * remove protocol from URL - * - * @param string $url - * @return string + * Remove protocol from URL */ - public function removeProtocolFromUrl($url) { + public function removeProtocolFromUrl(string $url): string { if (str_starts_with($url, 'https://')) { return substr($url, strlen('https://')); } elseif (str_starts_with($url, 'http://')) { @@ -112,17 +90,9 @@ public function removeProtocolFromUrl($url) { } /** - * check if the url contain the protocol (http or https) - * - * @param string $url - * @return bool + * Check if the url contain the protocol (http or https). */ - public function urlContainProtocol($url) { - if (str_starts_with($url, 'https://') - || str_starts_with($url, 'http://')) { - return true; - } - - return false; + public function urlContainProtocol(string $url): bool { + return str_starts_with($url, 'https://') || str_starts_with($url, 'http://'); } } diff --git a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php index e63bea255359d..132b81e468688 100644 --- a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php +++ b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php @@ -50,7 +50,6 @@ use OCP\Share\IManager; use OCP\Share\IProviderFactory; use OCP\Share\IShare; -use OCP\Util; use Override; use Psr\Log\LoggerInterface; use SensitiveParameter; @@ -167,11 +166,7 @@ public function shareReceived(ICloudFederationShare $share): string { if ($shareType === IShare::TYPE_USER) { $this->logger->debug('shareWith before, ' . $shareWith, ['app' => 'files_sharing']); - Util::emitHook( - '\OCA\Files_Sharing\API\Server2Server', - 'preLoginNameUsedAsUserName', - ['uid' => &$shareWith] - ); + $shareWith = $this->userManager->getUserNameFromLoginName($shareWith); $this->logger->debug('shareWith after, ' . $shareWith, ['app' => 'files_sharing']); $user = $this->userManager->get($shareWith); diff --git a/apps/federatedfilesharing/tests/AddressHandlerTest.php b/apps/federatedfilesharing/tests/AddressHandlerTest.php index 1d31ea07d4e4a..fa7f26d7e115c 100644 --- a/apps/federatedfilesharing/tests/AddressHandlerTest.php +++ b/apps/federatedfilesharing/tests/AddressHandlerTest.php @@ -33,16 +33,18 @@ protected function setUp(): void { $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->il10n = $this->createMock(IL10N::class); $this->contactsManager = $this->createMock(IManager::class); + $userManager = $this->createMock(IUserManager::class); + $userManager->method('getUserNameFromLoginName')->willReturnArgument(0); $this->cloudIdManager = new CloudIdManager( $this->createMock(ICacheFactory::class), $this->createMock(IEventDispatcher::class), $this->contactsManager, $this->urlGenerator, - $this->createMock(IUserManager::class), + $userManager, ); - $this->addressHandler = new AddressHandler($this->urlGenerator, $this->il10n, $this->cloudIdManager); + $this->addressHandler = new AddressHandler($this->urlGenerator, $this->il10n, $this->cloudIdManager, $userManager); } public static function dataTestSplitUserRemote(): array { diff --git a/apps/files_external/composer/composer/autoload_classmap.php b/apps/files_external/composer/composer/autoload_classmap.php index 6cb11dc66fe85..d70dbd0490fcd 100644 --- a/apps/files_external/composer/composer/autoload_classmap.php +++ b/apps/files_external/composer/composer/autoload_classmap.php @@ -37,6 +37,7 @@ 'OCA\\Files_External\\Controller\\StoragesController' => $baseDir . '/../lib/Controller/StoragesController.php', 'OCA\\Files_External\\Controller\\UserGlobalStoragesController' => $baseDir . '/../lib/Controller/UserGlobalStoragesController.php', 'OCA\\Files_External\\Controller\\UserStoragesController' => $baseDir . '/../lib/Controller/UserStoragesController.php', + 'OCA\\Files_External\\Event\\LoadAdditionalBackendEvent' => $baseDir . '/../lib/Event/LoadAdditionalBackendEvent.php', 'OCA\\Files_External\\Event\\StorageCreatedEvent' => $baseDir . '/../lib/Event/StorageCreatedEvent.php', 'OCA\\Files_External\\Event\\StorageDeletedEvent' => $baseDir . '/../lib/Event/StorageDeletedEvent.php', 'OCA\\Files_External\\Event\\StorageUpdatedEvent' => $baseDir . '/../lib/Event/StorageUpdatedEvent.php', diff --git a/apps/files_external/composer/composer/autoload_static.php b/apps/files_external/composer/composer/autoload_static.php index 132da2a2eff10..d6c3581dadabd 100644 --- a/apps/files_external/composer/composer/autoload_static.php +++ b/apps/files_external/composer/composer/autoload_static.php @@ -52,6 +52,7 @@ class ComposerStaticInitFiles_External 'OCA\\Files_External\\Controller\\StoragesController' => __DIR__ . '/..' . '/../lib/Controller/StoragesController.php', 'OCA\\Files_External\\Controller\\UserGlobalStoragesController' => __DIR__ . '/..' . '/../lib/Controller/UserGlobalStoragesController.php', 'OCA\\Files_External\\Controller\\UserStoragesController' => __DIR__ . '/..' . '/../lib/Controller/UserStoragesController.php', + 'OCA\\Files_External\\Event\\LoadAdditionalBackendEvent' => __DIR__ . '/..' . '/../lib/Event/LoadAdditionalBackendEvent.php', 'OCA\\Files_External\\Event\\StorageCreatedEvent' => __DIR__ . '/..' . '/../lib/Event/StorageCreatedEvent.php', 'OCA\\Files_External\\Event\\StorageDeletedEvent' => __DIR__ . '/..' . '/../lib/Event/StorageDeletedEvent.php', 'OCA\\Files_External\\Event\\StorageUpdatedEvent' => __DIR__ . '/..' . '/../lib/Event/StorageUpdatedEvent.php', diff --git a/apps/files_external/lib/Event/LoadAdditionalBackendEvent.php b/apps/files_external/lib/Event/LoadAdditionalBackendEvent.php new file mode 100644 index 0000000000000..d6401ded24609 --- /dev/null +++ b/apps/files_external/lib/Event/LoadAdditionalBackendEvent.php @@ -0,0 +1,17 @@ +dispatchTyped(new LoadAdditionalBackendEvent()); $instance->eventSent = true; } } - private function loadBackendProviders() { + private function loadBackendProviders(): void { $this->callForRegistrations(); foreach ($this->backendProviders as $provider) { - $this->registerBackends($provider->getBackends()); + foreach ($provider->getBackends() as $backend) { + $this->registerBackend($backend); + } } $this->backendProviders = []; } @@ -103,27 +107,25 @@ private function loadBackendProviders() { * Register an auth mechanism provider * * @since 9.1.0 - * @param IAuthMechanismProvider $provider */ - public function registerAuthMechanismProvider(IAuthMechanismProvider $provider) { + public function registerAuthMechanismProvider(IAuthMechanismProvider $provider): void { $this->authMechanismProviders[] = $provider; } - private function loadAuthMechanismProviders() { + private function loadAuthMechanismProviders(): void { $this->callForRegistrations(); foreach ($this->authMechanismProviders as $provider) { - $this->registerAuthMechanisms($provider->getAuthMechanisms()); + foreach ($provider->getAuthMechanisms() as $mechanism) { + $this->registerAuthMechanism($mechanism); + } } $this->authMechanismProviders = []; } /** * Register a backend - * - * @deprecated 9.1.0 use registerBackendProvider() - * @param Backend $backend */ - public function registerBackend(Backend $backend) { + private function registerBackend(Backend $backend): void { if (!$this->isAllowedUserBackend($backend)) { $backend->removeVisibility(BackendService::VISIBILITY_PERSONAL); } @@ -132,22 +134,10 @@ public function registerBackend(Backend $backend) { } } - /** - * @deprecated 9.1.0 use registerBackendProvider() - * @param Backend[] $backends - */ - public function registerBackends(array $backends) { - foreach ($backends as $backend) { - $this->registerBackend($backend); - } - } /** * Register an authentication mechanism - * - * @deprecated 9.1.0 use registerAuthMechanismProvider() - * @param AuthMechanism $authMech */ - public function registerAuthMechanism(AuthMechanism $authMech) { + private function registerAuthMechanism(AuthMechanism $authMech): void { if (!$this->isAllowedAuthMechanism($authMech)) { $authMech->removeVisibility(BackendService::VISIBILITY_PERSONAL); } @@ -156,22 +146,12 @@ public function registerAuthMechanism(AuthMechanism $authMech) { } } - /** - * @deprecated 9.1.0 use registerAuthMechanismProvider() - * @param AuthMechanism[] $mechanisms - */ - public function registerAuthMechanisms(array $mechanisms) { - foreach ($mechanisms as $mechanism) { - $this->registerAuthMechanism($mechanism); - } - } - /** * Get all backends * - * @return Backend[] + * @return array */ - public function getBackends() { + public function getBackends(): array { $this->loadBackendProviders(); // only return real identifiers, no aliases $backends = []; @@ -186,8 +166,8 @@ public function getBackends() { * * @return Backend[] */ - public function getAvailableBackends() { - $backends = array_filter($this->getBackends(), fn (Backend $backend) => $backend->checkRequiredDependencies() === [] && $backend->getDeprecateTo() === null); + public function getAvailableBackends(): array { + $backends = array_filter($this->getBackends(), fn (Backend $backend): bool => $backend->checkRequiredDependencies() === [] && $backend->getDeprecateTo() === null); uasort($backends, [Backend::class, 'lexicalCompare']); return $backends; } diff --git a/apps/files_external/tests/Service/BackendServiceTest.php b/apps/files_external/tests/Service/BackendServiceTest.php index b9c2b6defeb6a..71e1006435235 100644 --- a/apps/files_external/tests/Service/BackendServiceTest.php +++ b/apps/files_external/tests/Service/BackendServiceTest.php @@ -62,8 +62,8 @@ public function testRegisterBackend(): void { $backendAlias->method('getIdentifier') ->willReturn('identifier_real'); - $service->registerBackend($backend); - $service->registerBackend($backendAlias); + $this->invokePrivate($service, 'registerBackend', [$backend]); + $this->invokePrivate($service, 'registerBackend', [$backendAlias]); $this->assertEquals($backend, $service->getBackend('identifier:\Foo\Bar')); $this->assertEquals($backendAlias, $service->getBackend('identifier_real')); @@ -170,9 +170,9 @@ public function testUserMountingBackends(): void { $backendAlias->expects($this->never()) ->method('removeVisibility'); - $service->registerBackend($backendAllowed); - $service->registerBackend($backendNotAllowed); - $service->registerBackend($backendAlias); + $this->invokePrivate($service, 'registerBackend', [$backendAllowed]); + $this->invokePrivate($service, 'registerBackend', [$backendNotAllowed]); + $this->invokePrivate($service, 'registerBackend', [$backendAlias]); } public function testGetAvailableBackends(): void { @@ -191,8 +191,8 @@ public function testGetAvailableBackends(): void { ->getMock() ]); - $service->registerBackend($backendAvailable); - $service->registerBackend($backendNotAvailable); + $this->invokePrivate($service, 'registerBackend', [$backendAvailable]); + $this->invokePrivate($service, 'registerBackend', [$backendNotAvailable]); $availableBackends = $service->getAvailableBackends(); $this->assertArrayHasKey('identifier:\Backend\Available', $availableBackends); diff --git a/apps/files_sharing/lib/Listener/SharesUpdatedListener.php b/apps/files_sharing/lib/Listener/SharesUpdatedListener.php index 87a47698366e5..d509cba1c5072 100644 --- a/apps/files_sharing/lib/Listener/SharesUpdatedListener.php +++ b/apps/files_sharing/lib/Listener/SharesUpdatedListener.php @@ -33,7 +33,7 @@ /** * Listen to various events that can change what shares a user has access to * - * @psalm-type GroupEvents = UserAddedEvent|UserRemovedEvent|GroupDeletedEvent|BeforeGroupDeletedEvent + * @psalm-type GroupEvents = UserAddedEvent|UserRemovedEvent|GroupDeletedEvent|BeforeGroupDeletedEvent|UserDeletedEvent * @template-implements IEventListener */ class SharesUpdatedListener implements IEventListener { diff --git a/apps/user_ldap/composer/composer/autoload_classmap.php b/apps/user_ldap/composer/composer/autoload_classmap.php index 4b9858dbc984e..dee72550d8c12 100644 --- a/apps/user_ldap/composer/composer/autoload_classmap.php +++ b/apps/user_ldap/composer/composer/autoload_classmap.php @@ -58,6 +58,7 @@ 'OCA\\User_LDAP\\LDAPProvider' => $baseDir . '/../lib/LDAPProvider.php', 'OCA\\User_LDAP\\LDAPProviderFactory' => $baseDir . '/../lib/LDAPProviderFactory.php', 'OCA\\User_LDAP\\LDAPUtility' => $baseDir . '/../lib/LDAPUtility.php', + 'OCA\\User_LDAP\\Listener\\LoadAdditionalBackendListener' => $baseDir . '/../lib/Listener/LoadAdditionalBackendListener.php', 'OCA\\User_LDAP\\LoginListener' => $baseDir . '/../lib/LoginListener.php', 'OCA\\User_LDAP\\Mapping\\AbstractMapping' => $baseDir . '/../lib/Mapping/AbstractMapping.php', 'OCA\\User_LDAP\\Mapping\\GroupMapping' => $baseDir . '/../lib/Mapping/GroupMapping.php', diff --git a/apps/user_ldap/composer/composer/autoload_static.php b/apps/user_ldap/composer/composer/autoload_static.php index 07ea6c3d53583..8f7fc14d0dbcf 100644 --- a/apps/user_ldap/composer/composer/autoload_static.php +++ b/apps/user_ldap/composer/composer/autoload_static.php @@ -73,6 +73,7 @@ class ComposerStaticInitUser_LDAP 'OCA\\User_LDAP\\LDAPProvider' => __DIR__ . '/..' . '/../lib/LDAPProvider.php', 'OCA\\User_LDAP\\LDAPProviderFactory' => __DIR__ . '/..' . '/../lib/LDAPProviderFactory.php', 'OCA\\User_LDAP\\LDAPUtility' => __DIR__ . '/..' . '/../lib/LDAPUtility.php', + 'OCA\\User_LDAP\\Listener\\LoadAdditionalBackendListener' => __DIR__ . '/..' . '/../lib/Listener/LoadAdditionalBackendListener.php', 'OCA\\User_LDAP\\LoginListener' => __DIR__ . '/..' . '/../lib/LoginListener.php', 'OCA\\User_LDAP\\Mapping\\AbstractMapping' => __DIR__ . '/..' . '/../lib/Mapping/AbstractMapping.php', 'OCA\\User_LDAP\\Mapping\\GroupMapping' => __DIR__ . '/..' . '/../lib/Mapping/GroupMapping.php', diff --git a/apps/user_ldap/lib/AppInfo/Application.php b/apps/user_ldap/lib/AppInfo/Application.php index 6d7d7ea686547..aba99a2940408 100644 --- a/apps/user_ldap/lib/AppInfo/Application.php +++ b/apps/user_ldap/lib/AppInfo/Application.php @@ -9,16 +9,15 @@ namespace OCA\User_LDAP\AppInfo; -use Closure; -use OCA\Files_External\Service\BackendService; +use OCA\Files_External\Event\LoadAdditionalBackendEvent; use OCA\User_LDAP\Events\GroupBackendRegistered; use OCA\User_LDAP\Events\UserBackendRegistered; use OCA\User_LDAP\Group_Proxy; use OCA\User_LDAP\GroupPluginManager; -use OCA\User_LDAP\Handler\ExtStorageConfigHandler; use OCA\User_LDAP\Helper; use OCA\User_LDAP\ILDAPWrapper; use OCA\User_LDAP\LDAP; +use OCA\User_LDAP\Listener\LoadAdditionalBackendListener; use OCA\User_LDAP\LoginListener; use OCA\User_LDAP\Notification\Notifier; use OCA\User_LDAP\SetupChecks\LdapConnection; @@ -42,7 +41,6 @@ use OCP\Notification\IManager as INotificationManager; use OCP\Share\IManager as IShareManager; use OCP\User\Events\PostLoginEvent; -use OCP\Util; use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; @@ -78,6 +76,7 @@ function (ContainerInterface $c) { false ); $context->registerEventListener(PostLoginEvent::class, LoginListener::class); + $context->registerEventListener(LoadAdditionalBackendEvent::class, LoadAdditionalBackendListener::class); $context->registerSetupCheck(LdapInvalidUuids::class); $context->registerSetupCheck(LdapConnection::class); $context->registerSystemReportSection(SystemReportSection::class); @@ -104,32 +103,10 @@ public function boot(IBootContext $context): void { $groupManager->addBackend($groupBackend); $userBackendRegisteredEvent = new UserBackendRegistered($userBackend, $userPluginManager); - $dispatcher->dispatch('OCA\\User_LDAP\\User\\User::postLDAPBackendAdded', $userBackendRegisteredEvent); $dispatcher->dispatchTyped($userBackendRegisteredEvent); $groupBackendRegisteredEvent = new GroupBackendRegistered($groupBackend, $groupPluginManager); $dispatcher->dispatchTyped($groupBackendRegisteredEvent); } }); - - $context->injectFn(Closure::fromCallable([$this, 'registerBackendDependents'])); - - Util::connectHook( - '\OCA\Files_Sharing\API\Server2Server', - 'preLoginNameUsedAsUserName', - '\OCA\User_LDAP\Helper', - 'loginName2UserName' - ); - } - - private function registerBackendDependents(ContainerInterface $appContainer, IEventDispatcher $dispatcher): void { - $dispatcher->addListener( - 'OCA\\Files_External::loadAdditionalBackends', - function () use ($appContainer): void { - $storagesBackendService = $appContainer->get(BackendService::class); - $storagesBackendService->registerConfigHandler('home', function () use ($appContainer) { - return $appContainer->get(ExtStorageConfigHandler::class); - }); - } - ); } } diff --git a/apps/user_ldap/lib/Helper.php b/apps/user_ldap/lib/Helper.php index 6fea8035e491a..66b72661fd09a 100644 --- a/apps/user_ldap/lib/Helper.php +++ b/apps/user_ldap/lib/Helper.php @@ -282,23 +282,4 @@ public function sanitizeDN($dn) { public function DNasBaseParameter($dn) { return str_ireplace('\\5c', '\\', $dn); } - - /** - * listens to a hook thrown by server2server sharing and replaces the given - * login name by a username, if it matches an LDAP user. - * - * @param array $param contains a reference to a $uid var under 'uid' key - * @throws \Exception - */ - public static function loginName2UserName($param): void { - if (!isset($param['uid'])) { - throw new \Exception('key uid is expected to be set in $param'); - } - - $userBackend = Server::get(User_Proxy::class); - $uid = $userBackend->loginName2UserName($param['uid']); - if ($uid !== false) { - $param['uid'] = $uid; - } - } } diff --git a/apps/user_ldap/lib/Listener/LoadAdditionalBackendListener.php b/apps/user_ldap/lib/Listener/LoadAdditionalBackendListener.php new file mode 100644 index 0000000000000..a0167cedf5d7c --- /dev/null +++ b/apps/user_ldap/lib/Listener/LoadAdditionalBackendListener.php @@ -0,0 +1,39 @@ + + */ +class LoadAdditionalBackendListener implements IEventListener { + public function __construct( + private readonly BackendService $storagesBackendService, + private readonly ContainerInterface $container, + ) { + } + + #[Override] + public function handle(Event $event): void { + if (!$event instanceof LoadAdditionalBackendEvent) { + return; + } + + $this->storagesBackendService->registerConfigHandler('home', fn () => $this->container->get(ExtStorageConfigHandler::class)); + } +} diff --git a/apps/user_ldap/lib/User_LDAP.php b/apps/user_ldap/lib/User_LDAP.php index 7f4f6e6c24c8d..59771a9ec761b 100644 --- a/apps/user_ldap/lib/User_LDAP.php +++ b/apps/user_ldap/lib/User_LDAP.php @@ -72,8 +72,8 @@ public function canChangeAvatar($uid) { * @return string|false * @throws \Exception */ - public function loginName2UserName($loginName, bool $forceLdapRefetch = false) { - $cacheKey = 'loginName2UserName-' . $loginName; + public function getUserNameFromLoginName($loginName, bool $forceLdapRefetch = false) { + $cacheKey = 'getUserNameFromLoginName-' . $loginName; $username = $this->access->connection->getFromCache($cacheKey); $knownDn = $username ? $this->access->username2dn($username) : false; @@ -140,7 +140,7 @@ public function getLDAPUserByLoginName($loginName) { * @return false|string */ public function checkPassword($uid, $password) { - $username = $this->loginName2UserName($uid, true); + $username = $this->getUserNameFromLoginName($uid, true); if ($username === false) { return false; } diff --git a/apps/user_ldap/lib/User_Proxy.php b/apps/user_ldap/lib/User_Proxy.php index 080f828e0a22d..04132ebebf6ca 100644 --- a/apps/user_ldap/lib/User_Proxy.php +++ b/apps/user_ldap/lib/User_Proxy.php @@ -16,6 +16,7 @@ use OCP\Notification\IManager as INotificationManager; use OCP\User\Backend\ICountMappedUsersBackend; use OCP\User\Backend\IGetDisplayNameBackend; +use OCP\User\Backend\IGetUserNameFromLoginNameBackend; use OCP\User\Backend\ILimitAwareCountUsersBackend; use OCP\User\Backend\IPropertyPermissionBackend; use OCP\User\Backend\IProvideEnabledStateBackend; @@ -26,7 +27,16 @@ /** * @template-extends Proxy */ -class User_Proxy extends Proxy implements IUserBackend, UserInterface, IUserLDAP, ILimitAwareCountUsersBackend, ICountMappedUsersBackend, IProvideEnabledStateBackend, IGetDisplayNameBackend, IPropertyPermissionBackend { +class User_Proxy extends Proxy implements + IUserBackend, + UserInterface, + IUserLDAP, + ILimitAwareCountUsersBackend, + ICountMappedUsersBackend, + IProvideEnabledStateBackend, + IGetDisplayNameBackend, + IPropertyPermissionBackend, + IGetUserNameFromLoginNameBackend { public function __construct( Helper $helper, ILDAPWrapper $ldap, @@ -228,15 +238,10 @@ public function checkPassword($uid, $password) { return $this->handleRequest($uid, 'checkPassword', [$uid, $password]); } - /** - * returns the username for the given login name, if available - * - * @param string $loginName - * @return string|false - */ - public function loginName2UserName($loginName) { + #[Override] + public function getUserNameFromLoginName(string $loginName): string|false { $id = 'LOGINNAME,' . $loginName; - return $this->handleRequest($id, 'loginName2UserName', [$loginName]); + return $this->handleRequest($id, 'getUserNameFromLoginName', [$loginName]); } /** diff --git a/apps/user_ldap/tests/User_LDAPTest.php b/apps/user_ldap/tests/User_LDAPTest.php index b87b5fde55dec..3641cb86a2557 100644 --- a/apps/user_ldap/tests/User_LDAPTest.php +++ b/apps/user_ldap/tests/User_LDAPTest.php @@ -1073,11 +1073,11 @@ public function testLoginName2UserNameSuccess(): void { $this->connection->expects($this->exactly(2)) ->method('getFromCache') - ->with($this->equalTo('loginName2UserName-' . $loginName)) + ->with($this->equalTo('getUserNameFromLoginName-' . $loginName)) ->willReturnOnConsecutiveCalls(null, $username); $this->connection->expects($this->once()) ->method('writeToCache') - ->with($this->equalTo('loginName2UserName-' . $loginName), $this->equalTo($username)); + ->with($this->equalTo('getUserNameFromLoginName-' . $loginName), $this->equalTo($username)); $backend = new User_LDAP($this->access, $this->notificationManager, $this->pluginManager, $this->logger, $this->deletedUsersIndex); $user = $this->createMock(User::class); @@ -1095,11 +1095,11 @@ public function testLoginName2UserNameSuccess(): void { ->method('getAttributes') ->willReturn(['dn', 'uid', 'mail', 'displayname']); - $name = $backend->loginName2UserName($loginName); + $name = $backend->getUserNameFromLoginName($loginName); $this->assertSame($username, $name); // and once again to verify that caching works - $backend->loginName2UserName($loginName); + $backend->getUserNameFromLoginName($loginName); } public function testLoginName2UserNameNoUsersOnLDAP(): void { @@ -1116,22 +1116,22 @@ public function testLoginName2UserNameNoUsersOnLDAP(): void { $this->connection->expects($this->exactly(2)) ->method('getFromCache') - ->with($this->equalTo('loginName2UserName-' . $loginName)) + ->with($this->equalTo('getUserNameFromLoginName-' . $loginName)) ->willReturnOnConsecutiveCalls(null, false); $this->connection->expects($this->once()) ->method('writeToCache') - ->with($this->equalTo('loginName2UserName-' . $loginName), false); + ->with($this->equalTo('getUserNameFromLoginName-' . $loginName), false); $this->userManager->expects($this->any()) ->method('getAttributes') ->willReturn(['dn', 'uid', 'mail', 'displayname']); $backend = new User_LDAP($this->access, $this->notificationManager, $this->pluginManager, $this->logger, $this->deletedUsersIndex); - $name = $backend->loginName2UserName($loginName); + $name = $backend->getUserNameFromLoginName($loginName); $this->assertSame(false, $name); // and once again to verify that caching works - $backend->loginName2UserName($loginName); + $backend->getUserNameFromLoginName($loginName); } public function testLoginName2UserNameOfflineUser(): void { @@ -1149,11 +1149,11 @@ public function testLoginName2UserNameOfflineUser(): void { $this->connection->expects($this->exactly(2)) ->method('getFromCache') - ->with($this->equalTo('loginName2UserName-' . $loginName)) + ->with($this->equalTo('getUserNameFromLoginName-' . $loginName)) ->willReturnOnConsecutiveCalls(null, false); $this->connection->expects($this->once()) ->method('writeToCache') - ->with($this->equalTo('loginName2UserName-' . $loginName), $this->equalTo(false)); + ->with($this->equalTo('getUserNameFromLoginName-' . $loginName), $this->equalTo(false)); $this->userManager->expects($this->any()) ->method('get') @@ -1164,11 +1164,11 @@ public function testLoginName2UserNameOfflineUser(): void { ->willReturn(['dn', 'uid', 'mail', 'displayname']); $backend = new User_LDAP($this->access, $this->notificationManager, $this->pluginManager, $this->logger, $this->deletedUsersIndex); - $name = $backend->loginName2UserName($loginName); + $name = $backend->getUserNameFromLoginName($loginName); $this->assertSame(false, $name); // and once again to verify that caching works - $backend->loginName2UserName($loginName); + $backend->getUserNameFromLoginName($loginName); } /** diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 8e17d76491cad..d0bfedd26f332 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -19,11 +19,6 @@ - &$uid] - )]]> @@ -1303,20 +1298,6 @@ - - - &$user1] - )]]> - &$user2] - )]]> - - @@ -1343,11 +1324,6 @@ - &$shareWith] - )]]> @@ -1563,10 +1539,6 @@ - - - - @@ -2575,17 +2547,6 @@ getCode()]]> - - - - - - @@ -2959,11 +2920,6 @@ - &$user] - )]]> @@ -3057,11 +3013,6 @@ session->get(self::WEBAUTHN_LOGIN))]]> - &$uid] - )]]> diff --git a/core/Controller/LostController.php b/core/Controller/LostController.php index 57e085c89bd57..a573e75176689 100644 --- a/core/Controller/LostController.php +++ b/core/Controller/LostController.php @@ -42,7 +42,6 @@ use OCP\Security\VerificationToken\InvalidTokenException; use OCP\Security\VerificationToken\IVerificationToken; use OCP\Server; -use OCP\Util; use Psr\Log\LoggerInterface; use function array_filter; use function count; @@ -160,11 +159,7 @@ public function email(string $user): JSONResponse { return new JSONResponse($this->error($this->l10n->t('Unsupported email length (>255)'))); } - Util::emitHook( - '\OCA\Files_Sharing\API\Server2Server', - 'preLoginNameUsedAsUserName', - ['uid' => &$user] - ); + $user = $this->userManager->getUserNameFromLoginName($user); // FIXME: use HTTP error codes try { diff --git a/core/Controller/WebAuthnController.php b/core/Controller/WebAuthnController.php index b39a6fdb5e4ce..6faedc5e2057a 100644 --- a/core/Controller/WebAuthnController.php +++ b/core/Controller/WebAuthnController.php @@ -22,7 +22,6 @@ use OCP\IRequest; use OCP\ISession; use OCP\IUserManager; -use OCP\Util; use Psr\Log\LoggerInterface; use Webauthn\PublicKeyCredentialRequestOptions; @@ -51,12 +50,7 @@ public function startAuthentication(string $loginName): JSONResponse { $this->logger->debug('Starting WebAuthn login'); $this->logger->debug('Converting login name to UID'); - $uid = $loginName; - Util::emitHook( - '\OCA\Files_Sharing\API\Server2Server', - 'preLoginNameUsedAsUserName', - ['uid' => &$uid] - ); + $uid = $this->userManager->getUserNameFromLoginName($loginName); $this->logger->debug('Got UID: ' . $uid); $publicKeyCredentialRequestOptions = $this->webAuthnManger->startAuthentication($uid, $this->request->getServerHost()); diff --git a/lib/OC.php b/lib/OC.php index 251aa8b9a2bc1..f3bf884fcddb2 100644 --- a/lib/OC.php +++ b/lib/OC.php @@ -890,14 +890,6 @@ public static function initForRequest(): void { Server::get(\OCP\IUserManager::class)->registerBackend(new \OC\User\Database()); Server::get(\OCP\IGroupManager::class)->addBackend(new \OC\Group\Database()); - // Subscribe to the hook - \OCP\Util::connectHook( - '\OCA\Files_Sharing\API\Server2Server', - 'preLoginNameUsedAsUserName', - '\OC\User\Database', - 'preLoginNameUsedAsUserName' - ); - //setup extra user backends if (!\OCP\Util::needUpgrade()) { OC_User::setupBackends(); diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 52e2333085ca2..b16497b6cfbc9 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1122,6 +1122,7 @@ 'OCP\\User\\Backend\\IGetDisplayNameBackend' => $baseDir . '/lib/public/User/Backend/IGetDisplayNameBackend.php', 'OCP\\User\\Backend\\IGetHomeBackend' => $baseDir . '/lib/public/User/Backend/IGetHomeBackend.php', 'OCP\\User\\Backend\\IGetRealUIDBackend' => $baseDir . '/lib/public/User/Backend/IGetRealUIDBackend.php', + 'OCP\\User\\Backend\\IGetUserNameFromLoginNameBackend' => $baseDir . '/lib/public/User/Backend/IGetUserNameFromLoginNameBackend.php', 'OCP\\User\\Backend\\ILimitAwareCountUsersBackend' => $baseDir . '/lib/public/User/Backend/ILimitAwareCountUsersBackend.php', 'OCP\\User\\Backend\\IPasswordConfirmationBackend' => $baseDir . '/lib/public/User/Backend/IPasswordConfirmationBackend.php', 'OCP\\User\\Backend\\IPasswordHashBackend' => $baseDir . '/lib/public/User/Backend/IPasswordHashBackend.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index f29ac210c3700..d2d2bd39d09ba 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1163,6 +1163,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OCP\\User\\Backend\\IGetDisplayNameBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/IGetDisplayNameBackend.php', 'OCP\\User\\Backend\\IGetHomeBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/IGetHomeBackend.php', 'OCP\\User\\Backend\\IGetRealUIDBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/IGetRealUIDBackend.php', + 'OCP\\User\\Backend\\IGetUserNameFromLoginNameBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/IGetUserNameFromLoginNameBackend.php', 'OCP\\User\\Backend\\ILimitAwareCountUsersBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/ILimitAwareCountUsersBackend.php', 'OCP\\User\\Backend\\IPasswordConfirmationBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/IPasswordConfirmationBackend.php', 'OCP\\User\\Backend\\IPasswordHashBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/IPasswordHashBackend.php', diff --git a/lib/private/Authentication/Listeners/LoginFailedListener.php b/lib/private/Authentication/Listeners/LoginFailedListener.php index 530a6d4efc61d..830c2f442ac50 100644 --- a/lib/private/Authentication/Listeners/LoginFailedListener.php +++ b/lib/private/Authentication/Listeners/LoginFailedListener.php @@ -16,7 +16,6 @@ use OCP\EventDispatcher\IEventDispatcher; use OCP\EventDispatcher\IEventListener; use OCP\IUserManager; -use OCP\Util; /** * @template-implements IEventListener @@ -37,11 +36,7 @@ public function handle(Event $event): void { $this->dispatcher->dispatchTyped(new AnyLoginFailedEvent($event->getLoginName(), $event->getPassword())); $uid = $event->getLoginName(); - Util::emitHook( - '\OCA\Files_Sharing\API\Server2Server', - 'preLoginNameUsedAsUserName', - ['uid' => &$uid] - ); + $uid = $this->userManager->getUserNameFromLoginName($uid); if ($this->userManager->userExists($uid)) { $this->dispatcher->dispatchTyped(new LoginFailedEvent($uid)); } diff --git a/lib/private/User/Database.php b/lib/private/User/Database.php index 3395c624e5452..b371886594a7e 100644 --- a/lib/private/User/Database.php +++ b/lib/private/User/Database.php @@ -15,7 +15,6 @@ use OCP\EventDispatcher\IEventDispatcher; use OCP\IConfig; use OCP\IDBConnection; -use OCP\IUserManager; use OCP\Security\Events\ValidatePasswordPolicyEvent; use OCP\Security\IHasher; use OCP\Server; @@ -25,11 +24,13 @@ use OCP\User\Backend\IGetDisplayNameBackend; use OCP\User\Backend\IGetHomeBackend; use OCP\User\Backend\IGetRealUIDBackend; +use OCP\User\Backend\IGetUserNameFromLoginNameBackend; use OCP\User\Backend\ILimitAwareCountUsersBackend; use OCP\User\Backend\IPasswordHashBackend; use OCP\User\Backend\ISearchKnownUsersBackend; use OCP\User\Backend\ISetDisplayNameBackend; use OCP\User\Backend\ISetPasswordBackend; +use Override; /** * Class for user management in a SQL Database (e.g. MySQL, SQLite) @@ -44,8 +45,8 @@ class Database extends ABackend implements ILimitAwareCountUsersBackend, ISearchKnownUsersBackend, IGetRealUIDBackend, - IPasswordHashBackend { - + IPasswordHashBackend, + IGetUserNameFromLoginNameBackend { private CappedMemoryCache $cache; private IConfig $config; private ?IDBConnection $dbConnection; @@ -507,13 +508,8 @@ public function countUsers(int $limit = 0): int|false { return (int)$result; } - /** - * returns the username for the given login name in the correct casing - * - * @param string $loginName - * @return string|false - */ - public function loginName2UserName($loginName) { + #[Override] + public function getUserNameFromLoginName(string $loginName): string|false { if ($this->userExists($loginName)) { return $this->cache[$loginName]['uid']; } @@ -527,28 +523,10 @@ public function loginName2UserName($loginName) { * @return string the name of the backend to be shown */ #[\Override] - public function getBackendName() { + public function getBackendName(): string { return 'Database'; } - public static function preLoginNameUsedAsUserName($param) { - if (!isset($param['uid'])) { - throw new \Exception('key uid is expected to be set in $param'); - } - - $backends = Server::get(IUserManager::class)->getBackends(); - foreach ($backends as $backend) { - if ($backend instanceof Database) { - /** @var Database $backend */ - $uid = $backend->loginName2UserName($param['uid']); - if ($uid !== false) { - $param['uid'] = $uid; - return; - } - } - } - } - #[\Override] public function getRealUID(string $uid): string { if (!$this->userExists($uid)) { diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php index ae10049f6fc75..2f4c6014f58f0 100644 --- a/lib/private/User/Manager.php +++ b/lib/private/User/Manager.php @@ -33,6 +33,7 @@ use OCP\User\Backend\ICountMappedUsersBackend; use OCP\User\Backend\ICountUsersBackend; use OCP\User\Backend\IGetRealUIDBackend; +use OCP\User\Backend\IGetUserNameFromLoginNameBackend; use OCP\User\Backend\ILimitAwareCountUsersBackend; use OCP\User\Backend\IProvideEnabledStateBackend; use OCP\User\Backend\ISearchKnownUsersBackend; @@ -41,6 +42,7 @@ use OCP\User\Exceptions\UserNotFoundException; use OCP\UserInterface; use OCP\Util; +use Override; use Psr\Log\LoggerInterface; /** @@ -907,4 +909,19 @@ public function getAvatarUrlDark(string $userId, int $size): string { public function getFederatedUser(ICloudId $cloudId): IUser { return new LazyUser($cloudId->getDisplayId(), $this, $cloudId->getDisplayId()); } + + #[Override] + public function getUserNameFromLoginName(string $loginName): string { + $userName = $loginName; + foreach ($this->getBackends() as $backend) { + if ($backend instanceof IGetUserNameFromLoginNameBackend) { + $newUserName = $backend->getUserNameFromLoginName($loginName); + if ($newUserName !== false) { + $userName = $newUserName; + break; + } + } + } + return $userName; + } } diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index e5ad2dae52245..aaeb06091a1ce 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -493,11 +493,7 @@ private function isTokenAuthEnforced(): bool { } protected function isTwoFactorEnforced($username) { - Util::emitHook( - '\OCA\Files_Sharing\API\Server2Server', - 'preLoginNameUsedAsUserName', - ['uid' => &$username] - ); + $username = $this->manager->getUserNameFromLoginName($username); $user = $this->manager->get($username); if (is_null($user)) { $users = $this->manager->getByEmail($username); diff --git a/lib/private/legacy/OC_Hook.php b/lib/private/legacy/OC_Hook.php index f173679a6ad36..d51bef1ece344 100644 --- a/lib/private/legacy/OC_Hook.php +++ b/lib/private/legacy/OC_Hook.php @@ -40,7 +40,6 @@ class OC_Hook { [Filesystem::CLASSNAME, Filesystem::signal_pre_setup], [Filesystem::CLASSNAME, Filesystem::signal_post_init_mountpoints], [Filesystem::CLASSNAME, 'umount'], - ['\OCA\Files_Sharing\API\Server2Server', 'preLoginNameUsedAsUserName'], [Share::class,'share_link_access'], [Share::class,'pre_unshare'], [Share::class,'post_unshare'], diff --git a/lib/public/IUserManager.php b/lib/public/IUserManager.php index 5c3336140e890..675e924ab37f3 100644 --- a/lib/public/IUserManager.php +++ b/lib/public/IUserManager.php @@ -293,4 +293,11 @@ public function getAvatarUrlDark(string $userId, int $size): string; * @since 35.0.0 */ public function getFederatedUser(\OCP\Federation\ICloudId $cloudId): IUser; + + /** + * Get the username of a user based on its login name. + * + * @since 36.0.0 + */ + public function getUserNameFromLoginName(string $loginName): string; } diff --git a/lib/public/User/Backend/IGetUserNameFromLoginNameBackend.php b/lib/public/User/Backend/IGetUserNameFromLoginNameBackend.php new file mode 100644 index 0000000000000..fb51767de2b26 --- /dev/null +++ b/lib/public/User/Backend/IGetUserNameFromLoginNameBackend.php @@ -0,0 +1,25 @@ +defaults = $this->createMock(Defaults::class); $this->userManager = $this->createMock(IUserManager::class); + $this->userManager + ->method('getUserNameFromLoginName') + ->willReturnArgument(0); $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->mailer = $this->createMock(IMailer::class); $this->request = $this->createMock(IRequest::class); diff --git a/tests/lib/Authentication/Login/PreLoginHookCommandTest.php b/tests/lib/Authentication/Login/PreLoginHookCommandTest.php index e253d805d1c29..a89a82fba62bc 100644 --- a/tests/lib/Authentication/Login/PreLoginHookCommandTest.php +++ b/tests/lib/Authentication/Login/PreLoginHookCommandTest.php @@ -31,16 +31,13 @@ protected function setUp(): void { public function testProcess(): void { $data = $this->getBasicLoginData(); - $this->userManager->expects($this->once()) - ->method('emit') - ->with( - '\OC\User', - 'preLogin', - [ - $this->username, - $this->password, - ] - ); + $this->eventDispatcher->expects($this->once()) + ->method('dispatchTyped') + ->with($this->callback(function (BeforeUserLoggedInEvent $event): bool { + $this->assertEquals($this->username, $event->getUsername()); + $this->assertEquals($this->password, $event->getPassword()); + return true; + })); $result = $this->cmd->process($data); diff --git a/tests/lib/User/ManagerTest.php b/tests/lib/User/ManagerTest.php index 267cafc59ed67..bcc61db646665 100644 --- a/tests/lib/User/ManagerTest.php +++ b/tests/lib/User/ManagerTest.php @@ -181,7 +181,7 @@ public function testGetOneBackendExists(): void { ->with($this->equalTo('foo')) ->willReturn(true); $backend->expects($this->never()) - ->method('loginName2UserName'); + ->method('getUserNameFromLoginName'); $this->manager->registerBackend($backend); @@ -219,7 +219,7 @@ public function testGetOneBackendDoNotTranslateLoginNames(): void { ->with($this->equalTo('bLeNdEr')) ->willReturn(true); $backend->expects($this->never()) - ->method('loginName2UserName'); + ->method('getUserNameFromLoginName'); $this->manager->registerBackend($backend); @@ -233,7 +233,7 @@ public function testSearchOneBackend(): void { ->with($this->equalTo('fo')) ->willReturn(['foo', 'afoo', 'Afoo1', 'Bfoo']); $backend->expects($this->never()) - ->method('loginName2UserName'); + ->method('getUserNameFromLoginName'); $this->manager->registerBackend($backend); @@ -252,7 +252,7 @@ public function testSearchTwoBackendLimitOffset(): void { ->with($this->equalTo('fo'), $this->equalTo(3), $this->equalTo(1)) ->willReturn(['foo1', 'foo2']); $backend1->expects($this->never()) - ->method('loginName2UserName'); + ->method('getUserNameFromLoginName'); $backend2 = $this->createMock(\Test\Util\User\Dummy::class); $backend2->expects($this->once()) @@ -260,7 +260,7 @@ public function testSearchTwoBackendLimitOffset(): void { ->with($this->equalTo('fo'), $this->equalTo(3), $this->equalTo(1)) ->willReturn(['foo3']); $backend2->expects($this->never()) - ->method('loginName2UserName'); + ->method('getUserNameFromLoginName'); $this->manager->registerBackend($backend1); $this->manager->registerBackend($backend2); @@ -334,7 +334,7 @@ public function testCreateUserSingleBackendNotExists(): void { ->with($this->equalTo('foo')) ->willReturn(false); $backend->expects($this->never()) - ->method('loginName2UserName'); + ->method('getUserNameFromLoginName'); $this->manager->registerBackend($backend); diff --git a/tests/lib/User/UserTest.php b/tests/lib/User/UserTest.php index f8c3937be9db8..159d047755c3c 100644 --- a/tests/lib/User/UserTest.php +++ b/tests/lib/User/UserTest.php @@ -10,7 +10,6 @@ use OC\AllConfig; use OC\Files\Mount\ObjectHomeMountProvider; -use OC\Hooks\PublicEmitter; use OC\User\Database; use OC\User\User; use OCP\Comments\ICommentsManager; @@ -27,7 +26,6 @@ use OCP\Server; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; -use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; #[Group('DB')] diff --git a/tests/lib/Util/User/Dummy.php b/tests/lib/Util/User/Dummy.php index 0ebb045566721..655b0f310c88f 100644 --- a/tests/lib/Util/User/Dummy.php +++ b/tests/lib/Util/User/Dummy.php @@ -54,7 +54,7 @@ public function checkPassword($uid, $password): string|false { return false; } - public function loginName2UserName($loginName): string|false { + public function getUserNameFromLoginName($loginName): string|false { if (isset($this->users[strtolower($loginName)])) { return strtolower($loginName); } From 1de8eed3953068176c540782d915c5261c5988a0 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Wed, 8 Apr 2026 17:39:44 +0200 Subject: [PATCH 03/13] feat(hooks): Ignore / in front of class name Allow to use ::class as class names. Only works if the class actually exists, which in many case, isn't the case anymore. Signed-off-by: Carl Schwan --- lib/private/legacy/OC_Hook.php | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/lib/private/legacy/OC_Hook.php b/lib/private/legacy/OC_Hook.php index d51bef1ece344..ca17c4b2f2147 100644 --- a/lib/private/legacy/OC_Hook.php +++ b/lib/private/legacy/OC_Hook.php @@ -53,8 +53,8 @@ class OC_Hook { ['\OC\Files\Storage\Shared','fopen'], ['\OC\Files\Storage\Shared','file_get_contents'], ['\OC\Files\Storage\Shared','file_put_contents'], - ['\OCA\Files_Trashbin\Trashbin','post_moveToTrash'], - ['\OCA\Files_Trashbin\Trashbin','post_restore'], + [\OCA\Files_Trashbin\Trashbin::class,'post_moveToTrash'], + [\OCA\Files_Trashbin\Trashbin::class,'post_restore'], ['\OCP\Trashbin','preDeleteAll'], ['\OCP\Trashbin','deleteAll'], ['\OCP\Versions','rollback'], @@ -73,8 +73,8 @@ class OC_Hook { ['\OC\User','assignedUserId'], ['\OC\User','preUnassignedUserId'], ['\OC\User','postUnassignedUserId'], - ['\OC\Files\Cache\Scanner','scan_file'], - ['\OC\Files\Cache\Scanner','post_scan_file'], + [OC\Files\Cache\Scanner::class,'scan_file'], + [OC\Files\Cache\Scanner::class,'post_scan_file'], ['Scanner','removeFromCache'], ['Scanner','addToCache'], ['Scanner','correctFolderSize'], @@ -90,13 +90,15 @@ class OC_Hook { * @param string $signalName name of signal * @param string|object $slotClass class name of slot * @param string $slotName name of slot - * @return bool * * This function makes it very easy to connect to use hooks. * * TODO: write example */ - public static function connect($signalClass, $signalName, $slotClass, $slotName) { + public static function connect(string $signalClass, string $signalName, string|object $slotClass, string $slotName): bool { + if (str_starts_with($signalClass, '\\')) { + $signalName = substr($signalClass, 1); + } $found = array_find(self::$allowList, function ($allowed) use ($signalClass, $signalName) { [$allowedClass, $allowedSignal] = $allowed; return $allowedClass === $signalClass && $allowedSignal === $signalName; @@ -144,7 +146,10 @@ public static function connect($signalClass, $signalName, $slotClass, $slotName) * * TODO: write example */ - public static function emit($signalClass, $signalName, $params = []) { + public static function emit(string $signalClass, string $signalName, $params = []): bool { + if (str_starts_with($signalClass, '\\')) { + $signalName = substr($signalClass, 1); + } // Return false if no hook handlers are listening to this // emitting class if (!array_key_exists($signalClass, self::$registered)) { @@ -179,12 +184,13 @@ public static function emit($signalClass, $signalName, $params = []) { } /** - * clear hooks - * @param string $signalClass - * @param string $signalName + * Clear hooks */ - public static function clear($signalClass = '', $signalName = '') { + public static function clear(string $signalClass = '', string $signalName = ''): void { if ($signalClass) { + if (str_starts_with($signalClass, '\\')) { + $signalName = substr($signalClass, 1); + } if ($signalName) { self::$registered[$signalClass][$signalName] = []; } else { @@ -200,7 +206,7 @@ public static function clear($signalClass = '', $signalName = '') { * DO NOT USE! * For unit tests ONLY! */ - public static function getHooks() { + public static function getHooks(): array { return self::$registered; } } From e722745d0be2b972b03b93391012c12d7b181d47 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 9 Apr 2026 11:01:01 +0200 Subject: [PATCH 04/13] refactor(hooks): Remove all non-used user hooks And replace the few remaining by converting the event to hooks instead of the other way around. Signed-off-by: Carl Schwan --- .../dashboard/composer/composer/installed.php | 4 +- .../files_sharing/lib/AppInfo/Application.php | 2 + apps/files_sharing/lib/Helper.php | 4 +- apps/files_sharing/lib/Hooks.php | 13 -- .../lib/Listener/SharesUpdatedListener.php | 11 ++ .../tests/SharesUpdatedListenerTest.php | 2 + apps/user_ldap/lib/User/Manager.php | 14 +- apps/user_ldap/tests/AccessTest.php | 4 +- apps/user_ldap/tests/User/ManagerTest.php | 2 +- apps/user_ldap/tests/User/UserTest.php | 2 +- build/psalm-baseline.xml | 24 ---- lib/OC.php | 6 +- lib/composer/composer/autoload_classmap.php | 2 +- lib/composer/composer/autoload_static.php | 2 +- .../Login/PreLoginHookCommand.php | 20 +-- .../Authentication/LoginCredentials/Store.php | 21 +-- .../Files/Config/UserMountCacheListener.php | 28 ---- .../Listeners/UserMountCacheListener.php | 38 ++++++ lib/private/Server.php | 82 ++---------- lib/private/SubAdmin.php | 10 +- .../BackgroundJobs/CleanupDeletedUsers.php | 1 - lib/private/User/Manager.php | 18 +-- lib/private/User/Session.php | 26 ++-- lib/private/User/User.php | 21 +-- lib/private/legacy/OC_Hook.php | 49 ++++--- lib/private/legacy/OC_User.php | 32 ++--- lib/public/IUserManager.php | 6 - .../Login/ALoginTestCommand.php | 4 +- .../Login/PreLoginHookCommandTest.php | 11 +- tests/lib/Files/ViewTest.php | 14 +- tests/lib/User/UserTest.php | 122 +++++++----------- 31 files changed, 218 insertions(+), 377 deletions(-) delete mode 100644 lib/private/Files/Config/UserMountCacheListener.php create mode 100644 lib/private/Files/Listeners/UserMountCacheListener.php diff --git a/apps/dashboard/composer/composer/installed.php b/apps/dashboard/composer/composer/installed.php index 1a66c7f2416b6..304bad90929fa 100644 --- a/apps/dashboard/composer/composer/installed.php +++ b/apps/dashboard/composer/composer/installed.php @@ -3,7 +3,7 @@ 'name' => '__root__', 'pretty_version' => 'dev-master', 'version' => 'dev-master', - 'reference' => 'b1797842784b250fb01ed5e3bf130705eb94751b', + 'reference' => '707699d6351faa181d14cf50abe6be4343938400', 'type' => 'library', 'install_path' => __DIR__ . '/../', 'aliases' => array(), @@ -13,7 +13,7 @@ '__root__' => array( 'pretty_version' => 'dev-master', 'version' => 'dev-master', - 'reference' => 'b1797842784b250fb01ed5e3bf130705eb94751b', + 'reference' => '707699d6351faa181d14cf50abe6be4343938400', 'type' => 'library', 'install_path' => __DIR__ . '/../', 'aliases' => array(), diff --git a/apps/files_sharing/lib/AppInfo/Application.php b/apps/files_sharing/lib/AppInfo/Application.php index 580a3bb163d04..977228ca02459 100644 --- a/apps/files_sharing/lib/AppInfo/Application.php +++ b/apps/files_sharing/lib/AppInfo/Application.php @@ -133,6 +133,8 @@ function () use ($c) { $context->registerEventListener(ShareMovedEvent::class, SharesUpdatedListener::class); $context->registerEventListener(UserHomeSetupEvent::class, UserHomeSetupListener::class); + $context->registerEventListener(UserDeletedEvent::class, SharesUpdatedListener::class); + $context->registerConfigLexicon(ConfigLexicon::class); $context->registerEventListener(RestrictInteractionEvent::class, RestrictInteractionListener::class); diff --git a/apps/files_sharing/lib/Helper.php b/apps/files_sharing/lib/Helper.php index 593c823eabaa1..2079bbf44b566 100644 --- a/apps/files_sharing/lib/Helper.php +++ b/apps/files_sharing/lib/Helper.php @@ -16,11 +16,9 @@ use OCP\Util; class Helper { - public static function registerHooks() { + public static function registerHooks(): void { Util::connectHook('OC_Filesystem', 'post_rename', '\OCA\Files_Sharing\Updater', 'renameHook'); Util::connectHook('OC_Filesystem', 'post_delete', '\OCA\Files_Sharing\Hooks', 'unshareChildren'); - - Util::connectHook('OC_User', 'post_deleteUser', '\OCA\Files_Sharing\Hooks', 'deleteUser'); } /** diff --git a/apps/files_sharing/lib/Hooks.php b/apps/files_sharing/lib/Hooks.php index 1d2617d06aa0f..28d3b78795f2e 100644 --- a/apps/files_sharing/lib/Hooks.php +++ b/apps/files_sharing/lib/Hooks.php @@ -10,21 +10,8 @@ use OC\Files\Filesystem; use OC\Files\View; -use OCP\IUserManager; -use OCP\Server; class Hooks { - public static function deleteUser(array $params): void { - $userManager = Server::get(IUserManager::class); - $user = $userManager->get($params['uid']); - if ($user === null) { - return; - } - - $manager = Server::get(External\Manager::class); - $manager->removeUserShares($user); - } - public static function unshareChildren(array $params): void { $path = Filesystem::getView()->getAbsolutePath($params['path']); $view = new View('/'); diff --git a/apps/files_sharing/lib/Listener/SharesUpdatedListener.php b/apps/files_sharing/lib/Listener/SharesUpdatedListener.php index d509cba1c5072..23ff2eac6973b 100644 --- a/apps/files_sharing/lib/Listener/SharesUpdatedListener.php +++ b/apps/files_sharing/lib/Listener/SharesUpdatedListener.php @@ -11,6 +11,7 @@ use OCA\Files_Sharing\AppInfo\Application; use OCA\Files_Sharing\Config\ConfigLexicon; use OCA\Files_Sharing\Event\UserShareAccessUpdatedEvent; +use OCA\Files_Sharing\External\Manager as ExternalManager; use OCA\Files_Sharing\ShareRecipientUpdater; use OCP\Config\IUserConfig; use OCP\EventDispatcher\Event; @@ -26,6 +27,7 @@ use OCP\Share\Events\ShareMovedEvent; use OCP\Share\Events\ShareTransferredEvent; use OCP\Share\IManager; +use OCP\User\Events\UserDeletedEvent; use OCP\User\Exceptions\UserNotFoundException; use Psr\Clock\ClockInterface; use Psr\Log\LoggerInterface; @@ -58,6 +60,7 @@ public function __construct( private readonly LoggerInterface $logger, IAppConfig $appConfig, private readonly UserHomeSetupListener $homeSetupListener, + private readonly ExternalManager $externalManager, ) { $this->cutOffMarkTime = $appConfig->getValueFloat(Application::APP_ID, ConfigLexicon::UPDATE_CUTOFF_TIME, 3.0); } @@ -86,6 +89,10 @@ public function handle(Event $event): void { if ($event instanceof UserAddedEvent || $event instanceof UserRemovedEvent) { $this->updateOrMarkUser($event->getUser()); } + if ($event instanceof UserDeletedEvent) { + $this->deleteUser($event); + } + if ($event instanceof ShareCreatedEvent || $event instanceof ShareTransferredEvent) { $share = $event->getShare(); $shareTarget = $share->getTarget(); @@ -165,4 +172,8 @@ private function markUserForRefresh(IUser $user): void { public function setCutOffMarkTime(float|int $cutOffMarkTime): void { $this->cutOffMarkTime = (float)$cutOffMarkTime; } + + public function deleteUser(UserDeletedEvent $event): void { + $this->externalManager->removeUserShares($event->getUser()); + } } diff --git a/apps/files_sharing/tests/SharesUpdatedListenerTest.php b/apps/files_sharing/tests/SharesUpdatedListenerTest.php index 57cbafbd2527e..85357039b662a 100644 --- a/apps/files_sharing/tests/SharesUpdatedListenerTest.php +++ b/apps/files_sharing/tests/SharesUpdatedListenerTest.php @@ -9,6 +9,7 @@ use OCA\Files_Sharing\Config\ConfigLexicon; use OCA\Files_Sharing\Event\UserShareAccessUpdatedEvent; +use OCA\Files_Sharing\External\Manager as ExternalManager; use OCA\Files_Sharing\Listener\SharesUpdatedListener; use OCA\Files_Sharing\Listener\UserHomeSetupListener; use OCA\Files_Sharing\ShareRecipientUpdater; @@ -71,6 +72,7 @@ protected function setUp(): void { $this->logger, $this->appConfig, $homeSetupListener, + $this->createMock(ExternalManager::class), ); } diff --git a/apps/user_ldap/lib/User/Manager.php b/apps/user_ldap/lib/User/Manager.php index c6892312b0ae7..9fdf8a7dc5108 100644 --- a/apps/user_ldap/lib/User/Manager.php +++ b/apps/user_ldap/lib/User/Manager.php @@ -53,9 +53,8 @@ public function __construct( /** * Binds manager to an instance of Access. * It needs to be assigned first before the manager can be used. - * @param Access */ - public function setLdapAccess(Access $access) { + public function setLdapAccess(Access $access): void { $this->access = $access; } @@ -64,9 +63,8 @@ public function setLdapAccess(Access $access) { * property array * @param string $dn the DN of the user * @param string $uid the internal (owncloud) username - * @return User */ - private function createAndCache($dn, $uid) { + private function createAndCache(string $dn, string $uid): User { $this->checkAccess(); $user = new User($uid, $dn, $this->access, $this->ocConfig, $this->userConfig, $this->appConfig, clone $this->image, $this->logger, @@ -78,10 +76,9 @@ private function createAndCache($dn, $uid) { } /** - * removes a user entry from the cache - * @param $uid + * Removes a user entry from the cache. */ - public function invalidate($uid) { + public function invalidate(string $uid): void { if (!isset($this->usersByUid[$uid])) { return; } @@ -94,9 +91,8 @@ public function invalidate($uid) { * @brief checks whether the Access instance has been set * @throws \Exception if Access has not been set * @psalm-assert !null $this->access - * @return null */ - private function checkAccess() { + private function checkAccess(): void { if (is_null($this->access)) { throw new \Exception('LDAP Access instance must be set first'); } diff --git a/apps/user_ldap/tests/AccessTest.php b/apps/user_ldap/tests/AccessTest.php index 3551daf261ba5..b691137c77d2a 100644 --- a/apps/user_ldap/tests/AccessTest.php +++ b/apps/user_ldap/tests/AccessTest.php @@ -112,8 +112,8 @@ private function getConnectorAndLdapMock(): array { $this->createMock(Image::class), $this->createMock(IUserManager::class), $this->createMock(INotificationManager::class), - $this->shareManager]) - ->getMock(); + $this->shareManager, + ])->getMock(); $helper = Server::get(Helper::class); return [$lw, $connector, $um, $helper]; diff --git a/apps/user_ldap/tests/User/ManagerTest.php b/apps/user_ldap/tests/User/ManagerTest.php index b6cb9bc1bbc27..9b1be9e68ec83 100644 --- a/apps/user_ldap/tests/User/ManagerTest.php +++ b/apps/user_ldap/tests/User/ManagerTest.php @@ -80,7 +80,7 @@ protected function setUp(): void { $this->image, $this->ncUserManager, $this->notificationManager, - $this->shareManager + $this->shareManager, ); $this->manager->setLdapAccess($this->access); diff --git a/apps/user_ldap/tests/User/UserTest.php b/apps/user_ldap/tests/User/UserTest.php index ee32320e06364..75ff31a646690 100644 --- a/apps/user_ldap/tests/User/UserTest.php +++ b/apps/user_ldap/tests/User/UserTest.php @@ -80,7 +80,7 @@ protected function setUp(): void { $this->logger, $this->avatarManager, $this->userManager, - $this->notificationManager + $this->notificationManager, ); } diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index d0bfedd26f332..e7415a35ceea5 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -1643,7 +1643,6 @@ - @@ -2622,11 +2621,6 @@ - - - - - @@ -3905,12 +3899,6 @@ - - - - - - @@ -3961,18 +3949,6 @@ server]]> server]]> - - manager instanceof PublicEmitter]]> - - - - - - - - - - diff --git a/lib/OC.php b/lib/OC.php index f3bf884fcddb2..5c58aee2091a8 100644 --- a/lib/OC.php +++ b/lib/OC.php @@ -1072,9 +1072,9 @@ private static function registerAccountHooks(): void { } private static function registerAppRestrictionsHooks(): void { - /** @var \OC\Group\Manager $groupManager */ - $groupManager = Server::get(\OCP\IGroupManager::class); - $groupManager->listen('\OC\Group', 'postDelete', function (\OCP\IGroup $group) { + $eventDispatcher = Server::get(IEventDispatcher::class); + $eventDispatcher->addListener(GroupDeletedEvent::class, function (GroupDeletedEvent $event) { + $group = $event->getGroup(); $appManager = Server::get(\OCP\App\IAppManager::class); $apps = $appManager->getEnabledAppsForGroup($group); foreach ($apps as $appId) { diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index b16497b6cfbc9..4ece6c3fa77d8 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1914,11 +1914,11 @@ 'OC\\Files\\Config\\LazyStorageMountInfo' => $baseDir . '/lib/private/Files/Config/LazyStorageMountInfo.php', 'OC\\Files\\Config\\MountProviderCollection' => $baseDir . '/lib/private/Files/Config/MountProviderCollection.php', 'OC\\Files\\Config\\UserMountCache' => $baseDir . '/lib/private/Files/Config/UserMountCache.php', - 'OC\\Files\\Config\\UserMountCacheListener' => $baseDir . '/lib/private/Files/Config/UserMountCacheListener.php', 'OC\\Files\\Conversion\\ConversionManager' => $baseDir . '/lib/private/Files/Conversion/ConversionManager.php', 'OC\\Files\\FileInfo' => $baseDir . '/lib/private/Files/FileInfo.php', 'OC\\Files\\FilenameValidator' => $baseDir . '/lib/private/Files/FilenameValidator.php', 'OC\\Files\\Filesystem' => $baseDir . '/lib/private/Files/Filesystem.php', + 'OC\\Files\\Listeners\\UserMountCacheListener' => $baseDir . '/lib/private/Files/Listeners/UserMountCacheListener.php', 'OC\\Files\\Lock\\LockManager' => $baseDir . '/lib/private/Files/Lock/LockManager.php', 'OC\\Files\\Mount\\CacheMountProvider' => $baseDir . '/lib/private/Files/Mount/CacheMountProvider.php', 'OC\\Files\\Mount\\HomeMountPoint' => $baseDir . '/lib/private/Files/Mount/HomeMountPoint.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index d2d2bd39d09ba..e4e45b04a0f7b 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1955,11 +1955,11 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Files\\Config\\LazyStorageMountInfo' => __DIR__ . '/../../..' . '/lib/private/Files/Config/LazyStorageMountInfo.php', 'OC\\Files\\Config\\MountProviderCollection' => __DIR__ . '/../../..' . '/lib/private/Files/Config/MountProviderCollection.php', 'OC\\Files\\Config\\UserMountCache' => __DIR__ . '/../../..' . '/lib/private/Files/Config/UserMountCache.php', - 'OC\\Files\\Config\\UserMountCacheListener' => __DIR__ . '/../../..' . '/lib/private/Files/Config/UserMountCacheListener.php', 'OC\\Files\\Conversion\\ConversionManager' => __DIR__ . '/../../..' . '/lib/private/Files/Conversion/ConversionManager.php', 'OC\\Files\\FileInfo' => __DIR__ . '/../../..' . '/lib/private/Files/FileInfo.php', 'OC\\Files\\FilenameValidator' => __DIR__ . '/../../..' . '/lib/private/Files/FilenameValidator.php', 'OC\\Files\\Filesystem' => __DIR__ . '/../../..' . '/lib/private/Files/Filesystem.php', + 'OC\\Files\\Listeners\\UserMountCacheListener' => __DIR__ . '/../../..' . '/lib/private/Files/Listeners/UserMountCacheListener.php', 'OC\\Files\\Lock\\LockManager' => __DIR__ . '/../../..' . '/lib/private/Files/Lock/LockManager.php', 'OC\\Files\\Mount\\CacheMountProvider' => __DIR__ . '/../../..' . '/lib/private/Files/Mount/CacheMountProvider.php', 'OC\\Files\\Mount\\HomeMountPoint' => __DIR__ . '/../../..' . '/lib/private/Files/Mount/HomeMountPoint.php', diff --git a/lib/private/Authentication/Login/PreLoginHookCommand.php b/lib/private/Authentication/Login/PreLoginHookCommand.php index 8679c4f02a47f..0230a3897a36d 100644 --- a/lib/private/Authentication/Login/PreLoginHookCommand.php +++ b/lib/private/Authentication/Login/PreLoginHookCommand.php @@ -9,27 +9,21 @@ namespace OC\Authentication\Login; -use OC\Hooks\PublicEmitter; -use OCP\IUserManager; +use OCP\EventDispatcher\IEventDispatcher; +use OCP\User\Events\BeforeUserLoggedInEvent; class PreLoginHookCommand extends ALoginCommand { public function __construct( - private IUserManager $userManager, + private readonly IEventDispatcher $eventDispatcher, ) { } #[\Override] public function process(LoginData $loginData): LoginResult { - if ($this->userManager instanceof PublicEmitter) { - $this->userManager->emit( - '\OC\User', - 'preLogin', - [ - $loginData->getUsername(), - $loginData->getPassword(), - ] - ); - } + $this->eventDispatcher->dispatchTyped(new BeforeUserLoggedInEvent( + $loginData->getUsername(), + $loginData->getPassword(), + )); return $this->processNextOrFinishSuccessfully($loginData); } diff --git a/lib/private/Authentication/LoginCredentials/Store.php b/lib/private/Authentication/LoginCredentials/Store.php index c3c68774c13fe..3093a9ddd48a6 100644 --- a/lib/private/Authentication/LoginCredentials/Store.php +++ b/lib/private/Authentication/LoginCredentials/Store.php @@ -16,10 +16,13 @@ use OCP\Authentication\Exceptions\InvalidTokenException; use OCP\Authentication\LoginCredentials\ICredentials; use OCP\Authentication\LoginCredentials\IStore; +use OCP\EventDispatcher\IEventDispatcher; use OCP\ISession; use OCP\Security\ICrypto; +use OCP\Server; use OCP\Session\Exceptions\SessionNotAvailableException; -use OCP\Util; +use OCP\User\Events\UserLoggedInEvent; +use OCP\User\Events\UserLoggedInWithCookieEvent; use Psr\Log\LoggerInterface; class Store implements IStore { @@ -29,15 +32,19 @@ public function __construct( private readonly ICrypto $crypto, private ?IProvider $tokenProvider = null, ) { - Util::connectHook('OC_User', 'post_login', $this, 'authenticate'); + Server::get(IEventDispatcher::class)->addListener(UserLoggedInWithCookieEvent::class, function (UserLoggedInWithCookieEvent $event) { + $this->authenticate(['run' => true, 'uid' => $event->getUser()->getUID(), 'password' => $event->getPassword()]); + }); + + Server::get(IEventDispatcher::class)->addListener(UserLoggedInEvent::class, function (UserLoggedInEvent $event) { + $this->authenticate(['run' => true, 'uid' => $event->getUser()->getUID(), 'loginName' => $event->getLoginName(), 'password' => $event->getPassword(), 'isTokenLogin' => $event->isTokenLogin()]); + }); } /** * Hook listener on post login - * - * @param array $params */ - public function authenticate(array $params) { + public function authenticate(array $params): void { if ($params['password'] !== null) { $params['password'] = $this->crypto->encrypt((string)$params['password']); } @@ -46,10 +53,8 @@ public function authenticate(array $params) { /** * Replace the session implementation - * - * @param ISession $session */ - public function setSession(ISession $session) { + public function setSession(ISession $session): void { $this->session = $session; } diff --git a/lib/private/Files/Config/UserMountCacheListener.php b/lib/private/Files/Config/UserMountCacheListener.php deleted file mode 100644 index 319cc3113634d..0000000000000 --- a/lib/private/Files/Config/UserMountCacheListener.php +++ /dev/null @@ -1,28 +0,0 @@ -listen('\OC\User', 'postDelete', [$this->userMountCache, 'removeUserMounts']); - } -} diff --git a/lib/private/Files/Listeners/UserMountCacheListener.php b/lib/private/Files/Listeners/UserMountCacheListener.php new file mode 100644 index 0000000000000..f766098149e31 --- /dev/null +++ b/lib/private/Files/Listeners/UserMountCacheListener.php @@ -0,0 +1,38 @@ + + */ +class UserMountCacheListener implements IEventListener { + public function __construct( + private IUserMountCache $userMountCache, + ) { + } + + #[Override] + public function handle(Event $event): void { + if (!$event instanceof UserDeletedEvent) { + return; + } + + $this->userMountCache->removeUserMounts($event->getUser()); + } +} diff --git a/lib/private/Server.php b/lib/private/Server.php index f54a9568e55cf..fb2ef1e67d263 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -58,9 +58,9 @@ use OC\Files\Cache\FileAccess; use OC\Files\Config\MountProviderCollection; use OC\Files\Config\UserMountCache; -use OC\Files\Config\UserMountCacheListener; use OC\Files\Conversion\ConversionManager; use OC\Files\FilenameValidator; +use OC\Files\Listeners\UserMountCacheListener; use OC\Files\Lock\LockManager; use OC\Files\Mount\CacheMountProvider; use OC\Files\Mount\LocalHomeMountProvider; @@ -286,14 +286,11 @@ use OCP\Teams\ITeamManager; use OCP\Translation\ITranslationManager; use OCP\User\Events\BeforeUserDeletedEvent; -use OCP\User\Events\BeforeUserLoggedInEvent; -use OCP\User\Events\BeforeUserLoggedInWithCookieEvent; -use OCP\User\Events\BeforeUserLoggedOutEvent; use OCP\User\Events\PostLoginEvent; use OCP\User\Events\UserChangedEvent; +use OCP\User\Events\UserDeletedEvent; use OCP\User\Events\UserLoggedInEvent; use OCP\User\Events\UserLoggedInWithCookieEvent; -use OCP\User\Events\UserLoggedOutEvent; use OCP\User\IAvailabilityCoordinator; use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; @@ -452,77 +449,17 @@ public function __construct( $c->get(LoggerInterface::class), $c->get(IEventDispatcher::class), ); - /** @deprecated 21.0.0 use BeforeUserCreatedEvent event with the IEventDispatcher instead */ - $userSession->listen('\OC\User', 'preCreateUser', function ($uid, $password): void { - \OC_Hook::emit('OC_User', 'pre_createUser', ['run' => true, 'uid' => $uid, 'password' => $password]); - }); - /** @deprecated 21.0.0 use UserCreatedEvent event with the IEventDispatcher instead */ - $userSession->listen('\OC\User', 'postCreateUser', function ($user, $password): void { - /** @var User $user */ - \OC_Hook::emit('OC_User', 'post_createUser', ['uid' => $user->getUID(), 'password' => $password]); - }); - /** @deprecated 21.0.0 use BeforeUserDeletedEvent event with the IEventDispatcher instead */ - $userSession->listen('\OC\User', 'preDelete', function ($user): void { - /** @var User $user */ - \OC_Hook::emit('OC_User', 'pre_deleteUser', ['run' => true, 'uid' => $user->getUID()]); - }); - /** @deprecated 21.0.0 use UserDeletedEvent event with the IEventDispatcher instead */ - $userSession->listen('\OC\User', 'postDelete', function ($user): void { + $dispatcher = $this->get(IEventDispatcher::class); + $dispatcher->addListener(UserLoggedInEvent::class, function (UserLoggedInEvent $event) { /** @var User $user */ - \OC_Hook::emit('OC_User', 'post_deleteUser', ['uid' => $user->getUID()]); + \OC_Hook::emit('OC_User', 'post_login', ['run' => true, 'uid' => $event->getUser()->getUID(), 'loginName' => $event->getLoginName(), 'password' => $event->getPassword(), 'isTokenLogin' => $event->isTokenLogin()]); }); - $userSession->listen('\OC\User', 'preSetPassword', function ($user, $password, $recoveryPassword): void { - /** @var User $user */ - \OC_Hook::emit('OC_User', 'pre_setPassword', ['run' => true, 'uid' => $user->getUID(), 'password' => $password, 'recoveryPassword' => $recoveryPassword]); - }); - $userSession->listen('\OC\User', 'postSetPassword', function ($user, $password, $recoveryPassword): void { - /** @var User $user */ - \OC_Hook::emit('OC_User', 'post_setPassword', ['run' => true, 'uid' => $user->getUID(), 'password' => $password, 'recoveryPassword' => $recoveryPassword]); - }); - $userSession->listen('\OC\User', 'preLogin', function ($uid, $password): void { - \OC_Hook::emit('OC_User', 'pre_login', ['run' => true, 'uid' => $uid, 'password' => $password]); - /** @var IEventDispatcher $dispatcher */ - $dispatcher = $this->get(IEventDispatcher::class); - $dispatcher->dispatchTyped(new BeforeUserLoggedInEvent($uid, $password)); - }); - $userSession->listen('\OC\User', 'postLogin', function ($user, $loginName, $password, $isTokenLogin): void { + $dispatcher->addListener(UserLoggedInWithCookieEvent::class, function (UserLoggedInWithCookieEvent $event) { /** @var User $user */ - \OC_Hook::emit('OC_User', 'post_login', ['run' => true, 'uid' => $user->getUID(), 'loginName' => $loginName, 'password' => $password, 'isTokenLogin' => $isTokenLogin]); - - /** @var IEventDispatcher $dispatcher */ - $dispatcher = $this->get(IEventDispatcher::class); - $dispatcher->dispatchTyped(new UserLoggedInEvent($user, $loginName, $password, $isTokenLogin)); - }); - $userSession->listen('\OC\User', 'preRememberedLogin', function ($uid): void { - /** @var IEventDispatcher $dispatcher */ - $dispatcher = $this->get(IEventDispatcher::class); - $dispatcher->dispatchTyped(new BeforeUserLoggedInWithCookieEvent($uid)); - }); - $userSession->listen('\OC\User', 'postRememberedLogin', function ($user, $password): void { - /** @var User $user */ - \OC_Hook::emit('OC_User', 'post_login', ['run' => true, 'uid' => $user->getUID(), 'password' => $password]); - - /** @var IEventDispatcher $dispatcher */ - $dispatcher = $this->get(IEventDispatcher::class); - $dispatcher->dispatchTyped(new UserLoggedInWithCookieEvent($user, $password)); + \OC_Hook::emit('OC_User', 'post_login', ['run' => true, 'uid' => $event->getUser()->getUID(), 'password' => $event->getPassword()]); }); - $userSession->listen('\OC\User', 'logout', function ($user): void { - \OC_Hook::emit('OC_User', 'logout', []); - /** @var IEventDispatcher $dispatcher */ - $dispatcher = $this->get(IEventDispatcher::class); - $dispatcher->dispatchTyped(new BeforeUserLoggedOutEvent($user)); - }); - $userSession->listen('\OC\User', 'postLogout', function ($user): void { - /** @var IEventDispatcher $dispatcher */ - $dispatcher = $this->get(IEventDispatcher::class); - $dispatcher->dispatchTyped(new UserLoggedOutEvent($user)); - }); - $userSession->listen('\OC\User', 'changeUser', function ($user, $feature, $value, $oldValue): void { - /** @var User $user */ - \OC_Hook::emit('OC_User', 'changeUser', ['run' => true, 'user' => $user, 'feature' => $feature, 'value' => $value, 'old_value' => $oldValue]); - }); return $userSession; }); $this->registerAlias(IUserSession::class, Session::class); @@ -703,8 +640,9 @@ public function __construct( $this->registerService(IUserMountCache::class, static function (ContainerInterface $c): IUserMountCache { $mountCache = $c->get(UserMountCache::class); - $listener = new UserMountCacheListener($mountCache); - $listener->listen($c->get(IUserManager::class)); + /** @var IEventDispatcher $eventDispatcher */ + $eventDispatcher = $c->get(IEventDispatcher::class); + $eventDispatcher->addServiceListener(UserDeletedEvent::class, UserMountCacheListener::class); return $mountCache; }); diff --git a/lib/private/SubAdmin.php b/lib/private/SubAdmin.php index c6f224b3f7640..b0c8140906acf 100644 --- a/lib/private/SubAdmin.php +++ b/lib/private/SubAdmin.php @@ -9,6 +9,7 @@ namespace OC; use OCP\EventDispatcher\IEventDispatcher; +use OCP\Group\Events\GroupDeletedEvent; use OCP\Group\Events\SubAdminAddedEvent; use OCP\Group\Events\SubAdminRemovedEvent; use OCP\Group\ISubAdmin; @@ -17,6 +18,7 @@ use OCP\IGroupManager; use OCP\IUser; use OCP\IUserManager; +use OCP\User\Events\UserDeletedEvent; class SubAdmin implements ISubAdmin { public function __construct( @@ -25,11 +27,11 @@ public function __construct( private IDBConnection $dbConn, private IEventDispatcher $eventDispatcher, ) { - $this->userManager->listen('\OC\User', 'postDelete', function ($user): void { - $this->post_deleteUser($user); + $this->eventDispatcher->addListener(UserDeletedEvent::class, function (UserDeletedEvent $event) { + $this->post_deleteUser($event->getUser()); }); - $this->groupManager->listen('\OC\Group', 'postDelete', function ($group): void { - $this->post_deleteGroup($group); + $this->eventDispatcher->addListener(GroupDeletedEvent::class, function (GroupDeletedEvent $event) { + $this->post_deleteGroup($event->getGroup()); }); } diff --git a/lib/private/User/BackgroundJobs/CleanupDeletedUsers.php b/lib/private/User/BackgroundJobs/CleanupDeletedUsers.php index eae08736085e4..228ecb1038358 100644 --- a/lib/private/User/BackgroundJobs/CleanupDeletedUsers.php +++ b/lib/private/User/BackgroundJobs/CleanupDeletedUsers.php @@ -53,7 +53,6 @@ protected function run($argument): void { $userId, $backend, Server::get(IEventDispatcher::class), - $this->userManager, $this->config, ); $user->delete(); diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php index 2f4c6014f58f0..f4c9002286fc0 100644 --- a/lib/private/User/Manager.php +++ b/lib/private/User/Manager.php @@ -39,6 +39,7 @@ use OCP\User\Backend\ISearchKnownUsersBackend; use OCP\User\Events\BeforeUserCreatedEvent; use OCP\User\Events\UserCreatedEvent; +use OCP\User\Events\UserDeletedEvent; use OCP\User\Exceptions\UserNotFoundException; use OCP\UserInterface; use OCP\Util; @@ -49,13 +50,6 @@ * Class Manager * * Hooks available in scope \OC\User: - * - preSetPassword(\OC\User\User $user, string $password, string $recoverPassword) - * - postSetPassword(\OC\User\User $user, string $password, string $recoverPassword) - * - preDelete(\OC\User\User $user) - * - postDelete(\OC\User\User $user) - * - preCreateUser(string $uid, string $password) - * - postCreateUser(\OC\User\User $user, string $password) - * - change(\OC\User\User $user) * - assignedUserId(string $uid) * - preUnassignedUserId(string $uid) * - postUnassignedUserId(string $uid) @@ -89,8 +83,8 @@ public function __construct( private LoggerInterface $logger, ) { $this->cache = new WithLocalCache($cacheFactory->createDistributed('user_backend_map')); - $this->listen('\OC\User', 'postDelete', function (IUser $user): void { - unset($this->cachedUsers[$user->getUID()]); + $this->eventDispatcher->addListener(UserDeletedEvent::class, function (UserDeletedEvent $event) { + unset($this->cachedUsers[$event->getUser()->getUID()]); }); $this->displayNameCache = new DisplayNameCache($cacheFactory, $this); } @@ -204,7 +198,7 @@ public function getUserObject($uid, $backend, $cacheUser = true) { return $this->cachedUsers[$uid]; } - $user = new User($uid, $backend, $this->eventDispatcher, $this, $this->config); + $user = new User($uid, $backend, $this->eventDispatcher, $this->config); if ($cacheUser) { $this->cachedUsers[$uid] = $user; } @@ -461,8 +455,6 @@ public function createUserFromBackend($uid, $password, UserInterface $backend): throw new \InvalidArgumentException($l->t('The Login is already being used')); } - /** @deprecated 21.0.0 use BeforeUserCreatedEvent event with the IEventDispatcher instead */ - $this->emit('\OC\User', 'preCreateUser', [$uid, $password]); $this->eventDispatcher->dispatchTyped(new BeforeUserCreatedEvent($uid, $password)); $state = $backend->createUser($uid, $password); if ($state === false) { @@ -479,8 +471,6 @@ public function createUserFromBackend($uid, $password, UserInterface $backend): throw new \RuntimeException('Failed to get user after creation', previous: $e); } if ($user instanceof IUser) { - /** @deprecated 21.0.0 use UserCreatedEvent event with the IEventDispatcher instead */ - $this->emit('\OC\User', 'postCreateUser', [$user, $password]); $this->eventDispatcher->dispatchTyped(new UserCreatedEvent($user, $password)); return $user; } diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index aaeb06091a1ce..4abd87cc4d5ba 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -39,8 +39,13 @@ use OCP\Security\ISecureRandom; use OCP\Server; use OCP\Session\Exceptions\SessionNotAvailableException; +use OCP\User\Events\BeforeUserLoggedInEvent; +use OCP\User\Events\BeforeUserLoggedInWithCookieEvent; +use OCP\User\Events\BeforeUserLoggedOutEvent; use OCP\User\Events\PostLoginEvent; use OCP\User\Events\UserFirstTimeLoggedInEvent; +use OCP\User\Events\UserLoggedInWithCookieEvent; +use OCP\User\Events\UserLoggedOutEvent; use OCP\Util; use Psr\Log\LoggerInterface; @@ -48,18 +53,10 @@ * Class Session * * Hooks available in scope \OC\User: - * - preSetPassword(\OC\User\User $user, string $password, string $recoverPassword) - * - postSetPassword(\OC\User\User $user, string $password, string $recoverPassword) - * - preDelete(\OC\User\User $user) - * - postDelete(\OC\User\User $user) - * - preCreateUser(string $uid, string $password) - * - postCreateUser(\OC\User\User $user) * - assignedUserId(string $uid) * - preUnassignedUserId(string $uid) * - postUnassignedUserId(string $uid) - * - preLogin(string $user, string $password) * - postLogin(\OC\User\User $user, string $loginName, string $password, boolean $isTokenLogin) - * - preRememberedLogin(string $uid) * - postRememberedLogin(\OC\User\User $user) * - logout() * - postLogout() @@ -400,9 +397,7 @@ public function logClientIn($user, $remoteAddress = $request->getRemoteAddress(); $currentDelay = $throttler->sleepDelayOrThrowOnMax($remoteAddress, 'login'); - if ($this->manager instanceof PublicEmitter) { - $this->manager->emit('\OC\User', 'preLogin', [$user, $password]); - } + $this->dispatcher->dispatchTyped(new BeforeUserLoggedInEvent($user, $password)); try { $dbToken = $this->getTokenFromPassword($password); @@ -618,7 +613,7 @@ private function loginWithToken($token) { // Ignore and use empty string instead } - $this->manager->emit('\OC\User', 'preLogin', [$dbToken->getLoginName(), $password]); + $this->dispatcher->dispatchTyped(new BeforeUserLoggedInEvent($dbToken->getLoginName(), $password)); $user = $this->manager->get($uid); if (is_null($user)) { @@ -896,7 +891,7 @@ public function tryTokenLogin(IRequest $request, bool $allowOcmAccessToken = fal */ public function loginWithCookie($uid, $currentToken, $oldSessionId) { $this->session->regenerateId(); - $this->manager->emit('\OC\User', 'preRememberedLogin', [$uid]); + $this->dispatcher->dispatchTyped(new BeforeUserLoggedInWithCookieEvent($uid)); $user = $this->manager->get($uid); if (is_null($user)) { // user does not exist @@ -979,6 +974,7 @@ public function loginWithCookie($uid, $currentToken, $oldSessionId) { } catch (PasswordlessTokenException $ex) { // Ignore } + $this->dispatcher->dispatchTyped(new UserLoggedInWithCookieEvent($user, $password)); $this->manager->emit('\OC\User', 'postRememberedLogin', [$user, $password]); return true; } @@ -998,7 +994,7 @@ public function createRememberMeToken(IUser $user) { #[\Override] public function logout() { $user = $this->getUser(); - $this->manager->emit('\OC\User', 'logout', [$user]); + $this->dispatcher->dispatchTyped(new BeforeUserLoggedOutEvent($user)); if ($user !== null) { try { $token = $this->session->getId(); @@ -1017,7 +1013,7 @@ public function logout() { $this->setToken(null); $this->unsetMagicInCookie(); $this->session->clear(); - $this->manager->emit('\OC\User', 'postLogout', [$user]); + $this->dispatcher->dispatchTyped(new UserLoggedOutEvent($user)); } /** diff --git a/lib/private/User/User.php b/lib/private/User/User.php index b64d053da3045..c6e210779e3f4 100644 --- a/lib/private/User/User.php +++ b/lib/private/User/User.php @@ -11,7 +11,6 @@ use InvalidArgumentException; use OC\Accounts\AccountManager; use OC\Avatar\AvatarManager; -use OC\Hooks\Emitter; use OCP\Accounts\IAccountManager; use OCP\Comments\ICommentsManager; use OCP\Config\IUserConfig; @@ -71,7 +70,6 @@ public function __construct( private string $uid, private ?UserInterface $backend, private IEventDispatcher $dispatcher, - private Emitter|Manager|null $emitter = null, ?IConfig $config = null, ?IUserConfig $userConfig = null, ?IURLGenerator $urlGenerator = null, @@ -113,7 +111,7 @@ public function getDisplayName(): string { } /** - * Set the displayname for the user + * Set the display name for the user. * * @param string $displayName * @@ -260,10 +258,6 @@ public function delete(): bool { return false; } - if ($this->emitter) { - /** @deprecated 21.0.0 use BeforeUserDeletedEvent event with the IEventDispatcher instead */ - $this->emitter->emit('\OC\User', 'preDelete', [$this]); - } $this->dispatcher->dispatchTyped(new BeforeUserDeletedEvent($this)); // 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 @@ -324,10 +318,6 @@ public function delete(): bool { throw $e; } - if ($this->emitter !== null) { - /** @deprecated 21.0.0 use UserDeletedEvent event with the IEventDispatcher instead */ - $this->emitter->emit('\OC\User', 'postDelete', [$this]); - } $this->dispatcher->dispatchTyped(new UserDeletedEvent($this)); // Finally we can unset the delete flag and all other states @@ -345,9 +335,6 @@ public function delete(): bool { #[\Override] public function setPassword($password, $recoveryPassword = null): bool { $this->dispatcher->dispatchTyped(new BeforePasswordUpdatedEvent($this, $password, $recoveryPassword)); - if ($this->emitter) { - $this->emitter->emit('\OC\User', 'preSetPassword', [$this, $password, $recoveryPassword]); - } if ($this->backend->implementsActions(Backend::SET_PASSWORD)) { /** @var ISetPasswordBackend $backend */ $backend = $this->backend; @@ -355,9 +342,6 @@ public function setPassword($password, $recoveryPassword = null): bool { if ($result !== false) { $this->dispatcher->dispatchTyped(new PasswordUpdatedEvent($this, $password, $recoveryPassword)); - if ($this->emitter) { - $this->emitter->emit('\OC\User', 'postSetPassword', [$this, $password, $recoveryPassword]); - } } return !($result === false); @@ -685,8 +669,5 @@ private function removeProtocolFromUrl(string $url): string { public function triggerChange($feature, $value = null, $oldValue = null): void { $this->dispatcher->dispatchTyped(new UserChangedEvent($this, $feature, $value, $oldValue)); - if ($this->emitter) { - $this->emitter->emit('\OC\User', 'changeUser', [$this, $feature, $value, $oldValue]); - } } } diff --git a/lib/private/legacy/OC_Hook.php b/lib/private/legacy/OC_Hook.php index ca17c4b2f2147..173cc4261b736 100644 --- a/lib/private/legacy/OC_Hook.php +++ b/lib/private/legacy/OC_Hook.php @@ -40,6 +40,8 @@ class OC_Hook { [Filesystem::CLASSNAME, Filesystem::signal_pre_setup], [Filesystem::CLASSNAME, Filesystem::signal_post_init_mountpoints], [Filesystem::CLASSNAME, 'umount'], + [Filesystem::CLASSNAME, 'post_umount'], + [Filesystem::CLASSNAME, 'post_read'], [Share::class,'share_link_access'], [Share::class,'pre_unshare'], [Share::class,'post_unshare'], @@ -49,38 +51,31 @@ class OC_Hook { [Share::class,'post_set_expiration_date'], [Share::class,'post_update_password'], [Share::class,'post_update_permissions'], - ['\OC\Share','verifyExpirationDate'], - ['\OC\Files\Storage\Shared','fopen'], - ['\OC\Files\Storage\Shared','file_get_contents'], - ['\OC\Files\Storage\Shared','file_put_contents'], + ['OC\Share','verifyExpirationDate'], + ['OC\Files\Storage\Shared','fopen'], + ['OC\Files\Storage\Shared','file_get_contents'], + ['OC\Files\Storage\Shared','file_put_contents'], [\OCA\Files_Trashbin\Trashbin::class,'post_moveToTrash'], [\OCA\Files_Trashbin\Trashbin::class,'post_restore'], - ['\OCP\Trashbin','preDeleteAll'], - ['\OCP\Trashbin','deleteAll'], - ['\OCP\Versions','rollback'], - ['\OCP\Versions','preDelete'], - ['\OCP\Versions','delete'], - [OC_User::class,'pre_createUser'], - [OC_User::class,'post_createUser'], - [OC_User::class,'pre_deleteUser'], - [OC_User::class,'post_deleteUser'], - [OC_User::class,'pre_setPassword'], - [OC_User::class,'post_setPassword'], - [OC_User::class,'pre_login'], + ['OCP\Trashbin','preDeleteAll'], + ['OCP\Trashbin','deleteAll'], + ['OCP\Versions','rollback'], + ['OCP\Versions','preDelete'], + ['OCP\Versions','delete'], [OC_User::class,'post_login'], [OC_User::class,'logout'], [OC_User::class,'changeUser'], - ['\OC\User','assignedUserId'], - ['\OC\User','preUnassignedUserId'], - ['\OC\User','postUnassignedUserId'], - [OC\Files\Cache\Scanner::class,'scan_file'], - [OC\Files\Cache\Scanner::class,'post_scan_file'], + ['OC\User','assignedUserId'], + ['OC\User','preUnassignedUserId'], + ['OC\User','postUnassignedUserId'], + [\OC\Files\Cache\Scanner::class,'scan_file'], + [\OC\Files\Cache\Scanner::class,'post_scan_file'], ['Scanner','removeFromCache'], ['Scanner','addToCache'], ['Scanner','correctFolderSize'], - ['\OCP\Config','js'], - ['\OC\Core\LostPassword\Controller\LostController','post_passwordReset'], - ['\OC\Core\LostPassword\Controller\LostController','pre_passwordReset'], + ['OCP\Config','js'], + ['OC\Core\LostPassword\Controller\LostController','post_passwordReset'], + ['OC\Core\LostPassword\Controller\LostController','pre_passwordReset'], ]; /** @@ -97,7 +92,7 @@ class OC_Hook { */ public static function connect(string $signalClass, string $signalName, string|object $slotClass, string $slotName): bool { if (str_starts_with($signalClass, '\\')) { - $signalName = substr($signalClass, 1); + $signalClass = substr($signalClass, 1); } $found = array_find(self::$allowList, function ($allowed) use ($signalClass, $signalName) { [$allowedClass, $allowedSignal] = $allowed; @@ -148,7 +143,7 @@ public static function connect(string $signalClass, string $signalName, string|o */ public static function emit(string $signalClass, string $signalName, $params = []): bool { if (str_starts_with($signalClass, '\\')) { - $signalName = substr($signalClass, 1); + $signalClass = substr($signalClass, 1); } // Return false if no hook handlers are listening to this // emitting class @@ -189,7 +184,7 @@ public static function emit(string $signalClass, string $signalName, $params = [ public static function clear(string $signalClass = '', string $signalName = ''): void { if ($signalClass) { if (str_starts_with($signalClass, '\\')) { - $signalName = substr($signalClass, 1); + $signalClass = substr($signalClass, 1); } if ($signalName) { self::$registered[$signalClass][$signalName] = []; diff --git a/lib/private/legacy/OC_User.php b/lib/private/legacy/OC_User.php index c4531f062c2d6..39c9d40837d97 100644 --- a/lib/private/legacy/OC_User.php +++ b/lib/private/legacy/OC_User.php @@ -40,13 +40,6 @@ * Note that &run is deprecated and won't work anymore. * * Hooks provided: - * pre_createUser(&run, uid, password) - * post_createUser(uid, password) - * pre_deleteUser(&run, uid) - * post_deleteUser(uid) - * pre_setPassword(&run, uid, password, recoveryPassword) - * post_setPassword(uid, password, recoveryPassword) - * pre_login(&run, uid, password) * post_login(uid) * logout() */ @@ -148,7 +141,6 @@ public static function setupBackends() { public static function loginWithApache(IApacheBackend $backend): bool { $uid = $backend->getCurrentUserId(); $run = true; - Util::emitHook('OC_User', 'pre_login', ['run' => &$run, 'uid' => $uid, 'backend' => $backend]); if ($uid) { if (self::getUser() !== $uid) { @@ -191,29 +183,23 @@ public static function loginWithApache(IApacheBackend $backend): bool { } } - // setup the filesystem - OC_Util::setupFS($uid); - // first call the post_login hooks, the login-process needs to be - // completed before we can safely create the users folder. + $user = Server::get(IUserManager::class)->get($uid); + + // set up the filesystem + Server::get(\OCP\Files\ISetupManager::class)->setupForUser($user); + + // first call the UserLoggedIn event, the login-process needs to be + // completed before we can safely create the user's folder. // For example encryption needs to initialize the users keys first // before we can create the user folder with the skeleton files - Util::emitHook( - 'OC_User', - 'post_login', - [ - 'uid' => $uid, - 'password' => $password, - 'isTokenLogin' => false, - ] - ); $dispatcher->dispatchTyped(new UserLoggedInEvent( - Server::get(IUserManager::class)->get($uid), + $user, $uid, null, false) ); - //trigger creation of user home and /files folder + // trigger creation of user home and /files folder Server::get(IRootFolder::class)->getUserFolder($uid); } return true; diff --git a/lib/public/IUserManager.php b/lib/public/IUserManager.php index 675e924ab37f3..97c4223c7fe7d 100644 --- a/lib/public/IUserManager.php +++ b/lib/public/IUserManager.php @@ -12,12 +12,6 @@ * Class Manager * * Hooks available in scope \OC\User: - * - preSetPassword(\OC\User\User $user, string $password, string $recoverPassword) - * - postSetPassword(\OC\User\User $user, string $password, string $recoverPassword) - * - preDelete(\OC\User\User $user) - * - postDelete(\OC\User\User $user) - * - preCreateUser(string $uid, string $password) - * - postCreateUser(\OC\User\User $user, string $password) * - assignedUserId(string $uid) * - preUnassignedUserId(string $uid) * - postUnassignedUserId(string $uid) diff --git a/tests/lib/Authentication/Login/ALoginTestCommand.php b/tests/lib/Authentication/Login/ALoginTestCommand.php index 95194dfaa8526..ca5a29d9187a0 100644 --- a/tests/lib/Authentication/Login/ALoginTestCommand.php +++ b/tests/lib/Authentication/Login/ALoginTestCommand.php @@ -9,6 +9,7 @@ namespace Test\Authentication\Login; +use OC\Authentication\Login\ALoginCommand; use OC\Authentication\Login\LoginData; use OCP\IRequest; use OCP\IUser; @@ -36,8 +37,7 @@ abstract class ALoginTestCommand extends TestCase { /** @var IUser|MockObject */ protected $user; - /** @var ALoginTestCommand */ - protected $cmd; + protected ALoginCommand $cmd; #[\Override] protected function setUp(): void { diff --git a/tests/lib/Authentication/Login/PreLoginHookCommandTest.php b/tests/lib/Authentication/Login/PreLoginHookCommandTest.php index a89a82fba62bc..91c1187da7d47 100644 --- a/tests/lib/Authentication/Login/PreLoginHookCommandTest.php +++ b/tests/lib/Authentication/Login/PreLoginHookCommandTest.php @@ -10,22 +10,21 @@ namespace Test\Authentication\Login; use OC\Authentication\Login\PreLoginHookCommand; -use OC\User\Manager; -use OCP\IUserManager; +use OCP\EventDispatcher\IEventDispatcher; +use OCP\User\Events\BeforeUserLoggedInEvent; use PHPUnit\Framework\MockObject\MockObject; class PreLoginHookCommandTest extends ALoginTestCommand { - /** @var IUserManager|MockObject */ - private $userManager; + private IEventDispatcher&MockObject $eventDispatcher; #[\Override] protected function setUp(): void { parent::setUp(); - $this->userManager = $this->createMock(Manager::class); + $this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->cmd = new PreLoginHookCommand( - $this->userManager + $this->eventDispatcher, ); } diff --git a/tests/lib/Files/ViewTest.php b/tests/lib/Files/ViewTest.php index 8a8ea6d64d566..9fac960c09f57 100644 --- a/tests/lib/Files/ViewTest.php +++ b/tests/lib/Files/ViewTest.php @@ -2188,12 +2188,14 @@ public function testLockBasicOperationUnlocksAfterCancelledHook( Filesystem::mount($storage, [], self::$user . '/'); $storage->mkdir('files'); - Util::connectHook( - Filesystem::CLASSNAME, - $hookType, - HookHelper::class, - 'cancellingCallback' - ); + if ($hookType !== '') { + Util::connectHook( + Filesystem::CLASSNAME, + $hookType, + HookHelper::class, + 'cancellingCallback' + ); + } call_user_func_array([$view, $operation], $operationArgs); diff --git a/tests/lib/User/UserTest.php b/tests/lib/User/UserTest.php index 159d047755c3c..8019dd6213d2c 100644 --- a/tests/lib/User/UserTest.php +++ b/tests/lib/User/UserTest.php @@ -18,12 +18,17 @@ use OCP\Files\FileInfo; use OCP\Files\IRootFolder; use OCP\Files\Storage\IStorageFactory; +use OCP\Group\Events\UserRemovedEvent; use OCP\IConfig; use OCP\IURLGenerator; use OCP\IUser; use OCP\Notification\IManager as INotificationManager; use OCP\Notification\INotification; use OCP\Server; +use OCP\User\Events\BeforePasswordUpdatedEvent; +use OCP\User\Events\BeforeUserDeletedEvent; +use OCP\User\Events\PasswordUpdatedEvent; +use OCP\User\Events\UserChangedEvent; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; use Test\TestCase; @@ -260,7 +265,7 @@ public function testGetHomeNotSupported(): void { ->method('getValueBool') ->willReturn(true); - $user = new User('foo', $backend, $this->dispatcher, null, $allConfig, $userConfig); + $user = new User('foo', $backend, $this->dispatcher, $allConfig, $userConfig); $this->assertEquals('arbitrary/path/foo', $user->getHome()); } @@ -298,7 +303,7 @@ public function testCanChangeDisplayName(): void { ->with('allow_user_to_change_display_name') ->willReturn(true); - $user = new User('foo', $backend, $this->dispatcher, null, $config); + $user = new User('foo', $backend, $this->dispatcher, $config); $this->assertTrue($user->canChangeDisplayName()); } @@ -370,37 +375,38 @@ public function testSetPasswordHooks(): void { ->method('setPassword') ->willReturn(true); - $hook = function (IUser $user, string $password) use ($test, &$hooksCalled): void { + $this->dispatcher->addListener(BeforePasswordUpdatedEvent::class, function (BeforePasswordUpdatedEvent $event) use ($test, &$hooksCalled): void { $hooksCalled++; - $test->assertEquals('foo', $user->getUID()); - $test->assertEquals('bar', $password); - }; + $test->assertEquals('foo', $event->getUser()->getUID()); + $test->assertEquals('bar', $event->getPassword()); + }); - $emitter = new PublicEmitter(); - $emitter->listen('\OC\User', 'preSetPassword', $hook); - $emitter->listen('\OC\User', 'postSetPassword', $hook); + $this->dispatcher->addListener(PasswordUpdatedEvent::class, function (PasswordUpdatedEvent $event) use ($test, &$hooksCalled): void { + $hooksCalled++; + $test->assertEquals('foo', $event->getUser()->getUID()); + $test->assertEquals('bar', $event->getPassword()); + }); $backend->expects($this->any()) ->method('implementsActions') ->willReturnCallback(static fn (int $actions): bool => $actions === \OC\User\Backend::SET_PASSWORD); - $user = new User('foo', $backend, $this->dispatcher, $emitter); + $user = new User('foo', $backend, $this->dispatcher); $user->setPassword('bar', ''); $this->assertEquals(2, $hooksCalled); } - public static function dataDeleteHooks(): array { + public static function dataDeleteEvent(): array { return [ [true, 2], [false, 1], ]; } - #[DataProvider('dataDeleteHooks')] - public function testDeleteHooks(bool $result, int $expectedHooks): void { + #[DataProvider('dataDeleteEvent')] + public function testDeleteEvent(bool $result, int $expectedHooks): void { $hooksCalled = 0; - $test = $this; $backend = $this->createMock(\Test\Util\User\Dummy::class); $backend->method('getBackendName')->willReturn('foo'); @@ -420,16 +426,17 @@ public function testDeleteHooks(bool $result, int $expectedHooks): void { $userConfig = $this->createMock(IUserConfig::class); - $emitter = new PublicEmitter(); - $user = new User('foo', $backend, $this->dispatcher, $emitter, $config, $userConfig); + $user = new User('foo', $backend, $this->dispatcher, $config, $userConfig); - $hook = function (IUser $user) use ($test, &$hooksCalled): void { + $this->dispatcher->addListener(BeforeUserDeletedEvent::class, function (BeforeUserDeletedEvent $event) use (&$hooksCalled) { $hooksCalled++; - $test->assertEquals('foo', $user->getUID()); - }; + $this->assertEquals('foo', $event->getUser()->getUID()); + }); - $emitter->listen('\OC\User', 'preDelete', $hook); - $emitter->listen('\OC\User', 'postDelete', $hook); + $this->dispatcher->addListener(UserRemovedEvent::class, function (UserRemovedEvent $event) use (&$hooksCalled) { + $hooksCalled++; + $this->assertEquals('foo', $event->getUser()->getUID()); + }); $commentsManager = $this->createMock(ICommentsManager::class); $notificationManager = $this->createMock(INotificationManager::class); @@ -547,7 +554,7 @@ public function testGetCloudId(string $absoluteUrl, string $cloudId): void { $urlGenerator->method('getAbsoluteURL') ->withAnyParameters() ->willReturn($absoluteUrl); - $user = new User('foo', $backend, $this->dispatcher, null, null, null, $urlGenerator); + $user = new User('foo', $backend, $this->dispatcher, null, null, $urlGenerator); $this->assertEquals($cloudId, $user->getCloudId()); } @@ -555,17 +562,13 @@ public function testSetEMailAddressEmpty(): void { $backend = $this->createMock(\Test\Util\User\Dummy::class); $backend->method('getBackendName')->willReturn('foo'); - $test = $this; $hooksCalled = 0; - $hook = function (IUser $user, string $feature, string $value) use ($test, &$hooksCalled): void { + $this->dispatcher->addListener(UserChangedEvent::class, function (UserChangedEvent $event) use (&$hooksCalled): void { $hooksCalled++; - $test->assertEquals('eMailAddress', $feature); - $test->assertEquals('', $value); - }; - - $emitter = new PublicEmitter(); - $emitter->listen('\OC\User', 'changeUser', $hook); + $this->assertEquals('eMailAddress', $event->getFeature()); + $this->assertEquals('', $event->getValue()); + }); $userConfig = $this->createMock(IUserConfig::class); $userConfig->expects($this->once()) @@ -576,7 +579,7 @@ public function testSetEMailAddressEmpty(): void { 'email' ); - $user = new User('foo', $backend, $this->dispatcher, $emitter, null, $userConfig); + $user = new User('foo', $backend, $this->dispatcher, null, $userConfig); $user->setSystemEMailAddress(''); } @@ -584,17 +587,13 @@ public function testSetEMailAddress(): void { $backend = $this->createMock(\Test\Util\User\Dummy::class); $backend->method('getBackendName')->willReturn('foo'); - $test = $this; $hooksCalled = 0; - $hook = function (IUser $user, string $feature, string $value) use ($test, &$hooksCalled): void { + $this->dispatcher->addListener(UserChangedEvent::class, function (UserChangedEvent $event) use (&$hooksCalled): void { $hooksCalled++; - $test->assertEquals('eMailAddress', $feature); - $test->assertEquals('foo@bar.com', $value); - }; - - $emitter = new PublicEmitter(); - $emitter->listen('\OC\User', 'changeUser', $hook); + $this->assertEquals('eMailAddress', $event->getFeature()); + $this->assertEquals('foo@bar.com', $event->getValue()); + }); $userConfig = $this->createMock(IUserConfig::class); $userConfig->expects($this->once()) @@ -606,7 +605,7 @@ public function testSetEMailAddress(): void { 'foo@bar.com' ); - $user = new User('foo', $backend, $this->dispatcher, $emitter, null, $userConfig); + $user = new User('foo', $backend, $this->dispatcher, null, $userConfig); $user->setSystemEMailAddress('foo@bar.com'); } @@ -614,13 +613,11 @@ public function testSetEMailAddressNoChange(): void { $backend = $this->createMock(\Test\Util\User\Dummy::class); $backend->method('getBackendName')->willReturn('foo'); - $emitter = $this->createMock(PublicEmitter::class); - $emitter->expects($this->never()) - ->method('emit'); - $dispatcher = $this->createMock(IEventDispatcher::class); $dispatcher->expects($this->never()) ->method('dispatch'); + $dispatcher->expects($this->never()) + ->method('dispatchTyped'); $userConfig = $this->createMock(IUserConfig::class); $userConfig->expects($this->any()) @@ -629,7 +626,7 @@ public function testSetEMailAddressNoChange(): void { $userConfig->expects($this->any()) ->method('setValueString'); - $user = new User('foo', $backend, $dispatcher, $emitter, null, $userConfig); + $user = new User('foo', $backend, $dispatcher, null, $userConfig); $user->setSystemEMailAddress('foo@bar.com'); } @@ -637,17 +634,13 @@ public function testSetQuota(): void { $backend = $this->createMock(\Test\Util\User\Dummy::class); $backend->method('getBackendName')->willReturn('foo'); - $test = $this; $hooksCalled = 0; - $hook = function (IUser $user, string $feature, string $value) use ($test, &$hooksCalled): void { + $this->dispatcher->addListener(UserChangedEvent::class, function (UserChangedEvent $event) use (&$hooksCalled): void { $hooksCalled++; - $test->assertEquals('quota', $feature); - $test->assertEquals('23 TB', $value); - }; - - $emitter = new PublicEmitter(); - $emitter->listen('\OC\User', 'changeUser', $hook); + $this->assertEquals('quota', $event->getFeature()); + $this->assertEquals('23 TB', $event->getValue()); + }); $userConfig = $this->createMock(IUserConfig::class); $userConfig->expects($this->once()) @@ -662,7 +655,7 @@ public function testSetQuota(): void { /* Overwrite IRootFolder to avoid crash about unknown user */ $this->overwriteService(IRootFolder::class, $this->createMock(IRootFolder::class)); - $user = new User('foo', $backend, $this->dispatcher, $emitter, null, $userConfig); + $user = new User('foo', $backend, $this->dispatcher, null, $userConfig); $user->setQuota('23 TB'); $this->restoreService(IRootFolder::class); @@ -672,14 +665,9 @@ public function testGetDefaultUnlimitedQuota(): void { $backend = $this->createMock(\Test\Util\User\Dummy::class); $backend->method('getBackendName')->willReturn('foo'); - /** @var PublicEmitter|MockObject $emitter */ - $emitter = $this->createMock(PublicEmitter::class); - $emitter->expects($this->never()) - ->method('emit'); - $config = $this->createMock(IConfig::class); $userConfig = $this->createMock(IUserConfig::class); - $user = new User('foo', $backend, $this->dispatcher, $emitter, $config, $userConfig); + $user = new User('foo', $backend, $this->dispatcher, $config, $userConfig); $userValueMap = [ ['foo', 'files', 'quota', 'default', 'default'], @@ -701,14 +689,9 @@ public function testGetDefaultUnlimitedQuota(): void { public function testGetDefaultUnlimitedQuotaForbidden(): void { $backend = $this->createMock(\Test\Util\User\Dummy::class); - /** @var PublicEmitter|MockObject $emitter */ - $emitter = $this->createMock(PublicEmitter::class); - $emitter->expects($this->never()) - ->method('emit'); - $config = $this->createMock(IConfig::class); $userConfig = $this->createMock(IUserConfig::class); - $user = new User('foo', $backend, $this->dispatcher, $emitter, $config, $userConfig); + $user = new User('foo', $backend, $this->dispatcher, $config, $userConfig); $userValueMap = [ ['foo', 'files', 'quota', 'default', 'default'], @@ -733,11 +716,6 @@ public function testGetDefaultUnlimitedQuotaForbidden(): void { public function testSetQuotaAddressNoChange(): void { $backend = $this->createMock(\Test\Util\User\Dummy::class); - /** @var PublicEmitter|MockObject $emitter */ - $emitter = $this->createMock(PublicEmitter::class); - $emitter->expects($this->never()) - ->method('emit'); - $userConfig = $this->createMock(IUserConfig::class); $userConfig->expects($this->any()) ->method('getValueString') @@ -748,7 +726,7 @@ public function testSetQuotaAddressNoChange(): void { /* Overwrite IRootFolder to avoid crash about unknown user */ $this->overwriteService(IRootFolder::class, $this->createMock(IRootFolder::class)); - $user = new User('foo', $backend, $this->dispatcher, $emitter, null, $userConfig); + $user = new User('foo', $backend, $this->dispatcher, null, $userConfig); $user->setQuota('23 TB'); $this->restoreService(IRootFolder::class); From b21545239a4245c7a14033646f191e380e02c7f8 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 21 Apr 2026 15:51:32 +0200 Subject: [PATCH 05/13] fix: Move event listener outside of controllers Signed-off-by: Carl Schwan --- .../Authentication/LoginCredentials/Store.php | 33 ++++---- lib/private/Server.php | 5 ++ lib/private/SubAdmin.php | 84 +++++++------------ .../LoginCredentials/IStore.php | 8 +- 4 files changed, 54 insertions(+), 76 deletions(-) diff --git a/lib/private/Authentication/LoginCredentials/Store.php b/lib/private/Authentication/LoginCredentials/Store.php index 3093a9ddd48a6..f2fa895b6334f 100644 --- a/lib/private/Authentication/LoginCredentials/Store.php +++ b/lib/private/Authentication/LoginCredentials/Store.php @@ -16,29 +16,35 @@ use OCP\Authentication\Exceptions\InvalidTokenException; use OCP\Authentication\LoginCredentials\ICredentials; use OCP\Authentication\LoginCredentials\IStore; -use OCP\EventDispatcher\IEventDispatcher; +use OCP\EventDispatcher\Event; +use OCP\EventDispatcher\IEventListener; use OCP\ISession; use OCP\Security\ICrypto; -use OCP\Server; use OCP\Session\Exceptions\SessionNotAvailableException; use OCP\User\Events\UserLoggedInEvent; use OCP\User\Events\UserLoggedInWithCookieEvent; +use Override; use Psr\Log\LoggerInterface; -class Store implements IStore { +/** + * @template-implements IEventListener + */ +class Store implements IStore, IEventListener { public function __construct( private ISession $session, private LoggerInterface $logger, private readonly ICrypto $crypto, private ?IProvider $tokenProvider = null, ) { - Server::get(IEventDispatcher::class)->addListener(UserLoggedInWithCookieEvent::class, function (UserLoggedInWithCookieEvent $event) { - $this->authenticate(['run' => true, 'uid' => $event->getUser()->getUID(), 'password' => $event->getPassword()]); - }); + } - Server::get(IEventDispatcher::class)->addListener(UserLoggedInEvent::class, function (UserLoggedInEvent $event) { + #[Override] + public function handle(Event $event): void { + if ($event instanceof UserLoggedInWithCookieEvent) { + $this->authenticate(['run' => true, 'uid' => $event->getUser()->getUID(), 'password' => $event->getPassword()]); + } elseif ($event instanceof UserLoggedInEvent) { $this->authenticate(['run' => true, 'uid' => $event->getUser()->getUID(), 'loginName' => $event->getLoginName(), 'password' => $event->getPassword(), 'isTokenLogin' => $event->isTokenLogin()]); - }); + } } /** @@ -58,13 +64,7 @@ public function setSession(ISession $session): void { $this->session = $session; } - /** - * @since 12 - * - * @return ICredentials the login credentials of the current user - * @throws CredentialsUnavailableException - */ - #[\Override] + #[Override] public function getLoginCredentials(): ICredentials { if ($this->tokenProvider === null) { throw new CredentialsUnavailableException(); @@ -91,8 +91,7 @@ public function getLoginCredentials(): ICredentials { } if ($trySession && $this->session->exists('login_credentials')) { - /** @var array $creds */ - $creds = json_decode($this->session->get('login_credentials'), true); + $creds = json_decode($this->session->get('login_credentials'), true, flags: JSON_THROW_ON_ERROR); if ($creds['password'] !== null) { try { $creds['password'] = $this->crypto->decrypt($creds['password']); diff --git a/lib/private/Server.php b/lib/private/Server.php index fb2ef1e67d263..802bbad74b7b3 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -209,6 +209,7 @@ use OCP\FilesMetadata\IFilesMetadataManager; use OCP\FullTextSearch\IFullTextSearchManager; use OCP\GlobalScale\IGlobalScaleService; +use OCP\Group\Events\GroupDeletedEvent; use OCP\Group\ISubAdmin; use OCP\Http\Client\IClientService; use OCP\IAppConfig; @@ -1116,8 +1117,12 @@ private function connectDispatcher(): void { $eventDispatcher = $this->get(IEventDispatcher::class); $eventDispatcher->addServiceListener(LoginFailed::class, LoginFailedListener::class); $eventDispatcher->addServiceListener(PostLoginEvent::class, UserLoggedInListener::class); + $eventDispatcher->addServiceListener(UserLoggedInEvent::class, Store::class); + $eventDispatcher->addServiceListener(UserLoggedInWithCookieEvent::class, Store::class); $eventDispatcher->addServiceListener(UserChangedEvent::class, UserChangedListener::class); $eventDispatcher->addServiceListener(BeforeUserDeletedEvent::class, BeforeUserDeletedListener::class); + $eventDispatcher->addServiceListener(UserDeletedEvent::class, SubAdmin::class); + $eventDispatcher->addServiceListener(GroupDeletedEvent::class, SubAdmin::class); FilesMetadataManager::loadListeners($eventDispatcher); GenerateBlurhashMetadata::loadListeners($eventDispatcher); diff --git a/lib/private/SubAdmin.php b/lib/private/SubAdmin.php index b0c8140906acf..514360dc8628c 100644 --- a/lib/private/SubAdmin.php +++ b/lib/private/SubAdmin.php @@ -8,7 +8,9 @@ namespace OC; +use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventDispatcher; +use OCP\EventDispatcher\IEventListener; use OCP\Group\Events\GroupDeletedEvent; use OCP\Group\Events\SubAdminAddedEvent; use OCP\Group\Events\SubAdminRemovedEvent; @@ -19,28 +21,32 @@ use OCP\IUser; use OCP\IUserManager; use OCP\User\Events\UserDeletedEvent; +use Override; -class SubAdmin implements ISubAdmin { +/** + * @template-implements IEventListener + */ +class SubAdmin implements ISubAdmin, IEventListener { public function __construct( private IUserManager $userManager, private IGroupManager $groupManager, private IDBConnection $dbConn, private IEventDispatcher $eventDispatcher, ) { - $this->eventDispatcher->addListener(UserDeletedEvent::class, function (UserDeletedEvent $event) { - $this->post_deleteUser($event->getUser()); - }); - $this->eventDispatcher->addListener(GroupDeletedEvent::class, function (GroupDeletedEvent $event) { + } + + #[Override] + public function handle(Event $event): void { + if ($event instanceof GroupDeletedEvent) { $this->post_deleteGroup($event->getGroup()); - }); + } + + if ($event instanceof UserDeletedEvent) { + $this->post_deleteUser($event->getUser()); + } } - /** - * add a SubAdmin - * @param IUser $user user to be SubAdmin - * @param IGroup $group group $user becomes subadmin of - */ - #[\Override] + #[Override] public function createSubAdmin(IUser $user, IGroup $group): void { $qb = $this->dbConn->getQueryBuilder(); @@ -55,12 +61,7 @@ public function createSubAdmin(IUser $user, IGroup $group): void { $this->eventDispatcher->dispatchTyped($event); } - /** - * delete a SubAdmin - * @param IUser $user the user that is the SubAdmin - * @param IGroup $group the group - */ - #[\Override] + #[Override] public function deleteSubAdmin(IUser $user, IGroup $group): void { $qb = $this->dbConn->getQueryBuilder(); @@ -73,12 +74,7 @@ public function deleteSubAdmin(IUser $user, IGroup $group): void { $this->eventDispatcher->dispatchTyped($event); } - /** - * get groups of a SubAdmin - * @param IUser $user the SubAdmin - * @return IGroup[] - */ - #[\Override] + #[Override] public function getSubAdminsGroups(IUser $user): array { $groupIds = $this->getSubAdminsGroupIds($user); @@ -126,12 +122,7 @@ public function getSubAdminsGroupsName(IUser $user): array { }, $this->getSubAdminsGroups($user)); } - /** - * get SubAdmins of a group - * @param IGroup $group the group - * @return IUser[] - */ - #[\Override] + #[Override] public function getGroupsSubAdmins(IGroup $group): array { $qb = $this->dbConn->getQueryBuilder(); @@ -179,13 +170,7 @@ public function getAllSubAdmins(): array { return $subadmins; } - /** - * checks if a user is a SubAdmin of a group - * @param IUser $user - * @param IGroup $group - * @return bool - */ - #[\Override] + #[Override] public function isSubAdminOfGroup(IUser $user, IGroup $group): bool { $qb = $this->dbConn->getQueryBuilder(); @@ -205,12 +190,7 @@ public function isSubAdminOfGroup(IUser $user, IGroup $group): bool { return $result; } - /** - * checks if a user is a SubAdmin - * @param IUser $user - * @return bool - */ - #[\Override] + #[Override] public function isSubAdmin(IUser $user): bool { // Check if the user is already an admin if ($this->groupManager->isAdmin($user->getUID())) { @@ -236,13 +216,7 @@ public function isSubAdmin(IUser $user): bool { return $isSubAdmin !== false; } - /** - * checks if a user is a accessible by a subadmin - * @param IUser $subadmin - * @param IUser $user - * @return bool - */ - #[\Override] + #[Override] public function isUserAccessible(IUser $subadmin, IUser $user): bool { if ($subadmin->getUID() === $user->getUID()) { return true; @@ -264,10 +238,9 @@ public function isUserAccessible(IUser $subadmin, IUser $user): bool { } /** - * delete all SubAdmins by $user - * @param IUser $user + * Delete all SubAdmins by $user */ - private function post_deleteUser(IUser $user) { + private function post_deleteUser(IUser $user): void { $qb = $this->dbConn->getQueryBuilder(); $qb->delete('group_admin') @@ -276,10 +249,9 @@ private function post_deleteUser(IUser $user) { } /** - * delete all SubAdmins by $group - * @param IGroup $group + * Delete all SubAdmins by $group */ - private function post_deleteGroup(IGroup $group) { + private function post_deleteGroup(IGroup $group): void { $qb = $this->dbConn->getQueryBuilder(); $qb->delete('group_admin') diff --git a/lib/public/Authentication/LoginCredentials/IStore.php b/lib/public/Authentication/LoginCredentials/IStore.php index 30ee562da3aab..9cae3e1ee65a4 100644 --- a/lib/public/Authentication/LoginCredentials/IStore.php +++ b/lib/public/Authentication/LoginCredentials/IStore.php @@ -9,16 +9,18 @@ namespace OCP\Authentication\LoginCredentials; +use OCP\AppFramework\Attribute\Consumable; use OCP\Authentication\Exceptions\CredentialsUnavailableException; /** - * @since 12 + * @since 12.0.0 */ +#[Consumable(since: '12.0.0')] interface IStore { /** - * Get login credentials of the currently logged in user + * Get login credentials of the currently logged-in user. * - * @since 12 + * @since 12.0.0 * * @throws CredentialsUnavailableException * @return ICredentials the login credentials of the current user From 73d1c05f2e1d64a492372210a945175342a167eb Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 21 Apr 2026 16:10:58 +0200 Subject: [PATCH 06/13] refactor: Use UserSession::completeLogin in OC_User Allow to get rid of more legacy code and make sure UserSessionTest is also in psalm Signed-off-by: Carl Schwan --- build/psalm-baseline.xml | 10 -- lib/private/Session/Memory.php | 2 +- lib/private/User/Session.php | 7 +- lib/private/legacy/OC_User.php | 16 +-- lib/public/Lockdown/ILockdownManager.php | 4 +- psalm.xml | 3 + tests/lib/User/SessionTest.php | 153 +++++++++++++---------- tests/lib/User/UserTest.php | 12 +- 8 files changed, 107 insertions(+), 100 deletions(-) diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index e7415a35ceea5..bd9017888e346 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -3862,11 +3862,6 @@ - - - - - @@ -3960,11 +3955,6 @@ - - - - - headers)]]> diff --git a/lib/private/Session/Memory.php b/lib/private/Session/Memory.php index 81bec82ed275c..5dfdd6b7d97a6 100644 --- a/lib/private/Session/Memory.php +++ b/lib/private/Session/Memory.php @@ -23,7 +23,7 @@ class Memory extends Session { /** * @param string $key - * @param integer $value + * @param mixed $value */ #[\Override] public function set(string $key, $value) { diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index 4abd87cc4d5ba..639b50ccd9026 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -66,8 +66,7 @@ class Session implements IUserSession, Emitter { use TTransactional; - /** @var User $activeUser */ - protected $activeUser; + protected ?IUser $activeUser = null; public function __construct( private Manager $manager, @@ -531,8 +530,8 @@ protected function prepareUserLogin($firstTimeLogin, $refreshCsrfToken = true) { if ($firstTimeLogin) { // trigger any other initialization - Server::get(IEventDispatcher::class)->dispatch(IUser::class . '::firstLogin', new GenericEvent($this->getUser())); - Server::get(IEventDispatcher::class)->dispatchTyped(new UserFirstTimeLoggedInEvent($this->getUser())); + $this->dispatcher->dispatch(IUser::class . '::firstLogin', new GenericEvent($this->getUser())); + $this->dispatcher->dispatchTyped(new UserFirstTimeLoggedInEvent($this->getUser())); } } diff --git a/lib/private/legacy/OC_User.php b/lib/private/legacy/OC_User.php index 39c9d40837d97..67a4a388ff8ad 100644 --- a/lib/private/legacy/OC_User.php +++ b/lib/private/legacy/OC_User.php @@ -17,7 +17,6 @@ use OCP\Authentication\IProvideUserSecretBackend; use OCP\Authentication\Token\IToken; use OCP\EventDispatcher\IEventDispatcher; -use OCP\Files\IRootFolder; use OCP\IGroupManager; use OCP\IRequest; use OCP\ISession; @@ -147,8 +146,6 @@ public static function loginWithApache(IApacheBackend $backend): bool { self::setUserId($uid); /** @var Session $userSession */ $userSession = Server::get(IUserSession::class); - - /** @var IEventDispatcher $dispatcher */ $dispatcher = Server::get(IEventDispatcher::class); if ($userSession->getUser() && !$userSession->getUser()->isEnabled()) { @@ -162,11 +159,12 @@ public static function loginWithApache(IApacheBackend $backend): bool { $password = $backend->getCurrentUserSecret(); } - /** @var IEventDispatcher $dispatcher */ $dispatcher->dispatchTyped(new BeforeUserLoggedInEvent($uid, $password, $backend)); + $user = $userSession->getUser(); + $userSession->completeLogin($user, ['loginName' => $uid, 'password' => $password]); $userSession->createSessionToken($request, $uid, $uid, $password); - $userSession->createRememberMeToken($userSession->getUser()); + $userSession->createRememberMeToken($user); if (empty($password)) { $tokenProvider = Server::get(IProvider::class); @@ -183,11 +181,6 @@ public static function loginWithApache(IApacheBackend $backend): bool { } } - $user = Server::get(IUserManager::class)->get($uid); - - // set up the filesystem - Server::get(\OCP\Files\ISetupManager::class)->setupForUser($user); - // first call the UserLoggedIn event, the login-process needs to be // completed before we can safely create the user's folder. // For example encryption needs to initialize the users keys first @@ -198,9 +191,6 @@ public static function loginWithApache(IApacheBackend $backend): bool { null, false) ); - - // trigger creation of user home and /files folder - Server::get(IRootFolder::class)->getUserFolder($uid); } return true; } diff --git a/lib/public/Lockdown/ILockdownManager.php b/lib/public/Lockdown/ILockdownManager.php index 2d64e7125e06d..c18c58777f676 100644 --- a/lib/public/Lockdown/ILockdownManager.php +++ b/lib/public/Lockdown/ILockdownManager.php @@ -7,7 +7,7 @@ namespace OCP\Lockdown; -use OC\Authentication\Token\IToken; +use OCP\Authentication\Token\IToken; /** * @since 9.2 @@ -25,6 +25,8 @@ public function enable(); * * @param IToken $token * @since 9.2 + * @since 36 Use OCP\Authentication\Token\IToken instead of OC namespaced deprecated one + * @return void */ public function setToken(IToken $token); diff --git a/psalm.xml b/psalm.xml index 3787cb9f0acad..bb6d7b60d52aa 100644 --- a/psalm.xml +++ b/psalm.xml @@ -72,6 +72,8 @@ + + @@ -213,6 +215,7 @@ + diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php index f466569eb9b3c..aae0ee7a44d9e 100644 --- a/tests/lib/User/SessionTest.php +++ b/tests/lib/User/SessionTest.php @@ -10,11 +10,9 @@ use OC\AppFramework\Http\Request; use OC\Authentication\Events\LoginFailed; -use OC\Authentication\Exceptions\InvalidTokenException; 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\Security\CSRF\CsrfTokenManager; use OC\Session\Memory; @@ -24,6 +22,9 @@ use OC\User\User; use OCA\DAV\Connector\Sabre\Auth; use OCP\AppFramework\Utility\ITimeFactory; +use OCP\Authentication\Exceptions\InvalidTokenException; +use OCP\Authentication\Token\IToken; +use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventDispatcher; use OCP\ICacheFactory; use OCP\IConfig; @@ -34,40 +35,30 @@ use OCP\Lockdown\ILockdownManager; use OCP\Security\Bruteforce\IThrottler; use OCP\Security\ISecureRandom; +use OCP\User\Events\BeforeUserLoggedInEvent; use OCP\User\Events\PostLoginEvent; +use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\ExpectationFailedException; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; +use Test\TestCase; use function array_diff; use function get_class_methods; -/** - * @package Test\User - */ -#[\PHPUnit\Framework\Attributes\Group('DB')] -class SessionTest extends \Test\TestCase { - /** @var ITimeFactory|MockObject */ - private $timeFactory; - /** @var IProvider|MockObject */ - private $tokenProvider; - /** @var IConfig|MockObject */ - private $config; - /** @var IThrottler|MockObject */ - private $throttler; - /** @var ISecureRandom|MockObject */ - private $random; - /** @var Manager|MockObject */ - private $manager; - /** @var ISession|MockObject */ - private $session; - /** @var Session|MockObject */ - private $userSession; - /** @var ILockdownManager|MockObject */ - private $lockdownManager; - /** @var LoggerInterface|MockObject */ - private $logger; - /** @var IEventDispatcher|MockObject */ - private $dispatcher; +#[Group(name: 'DB')] +class SessionTest extends TestCase { + private ITimeFactory&MockObject $timeFactory; + private IProvider&MockObject $tokenProvider; + private IConfig&MockObject $config; + private IThrottler&MockObject $throttler; + private ISecureRandom&MockObject $random; + private Manager&MockObject $manager; + private ISession&MockObject $session; + private Session&MockObject $userSession; + private ILockdownManager&MockObject $lockdownManager; + private LoggerInterface&MockObject $logger; + private IEventDispatcher&MockObject $dispatcher; #[\Override] protected function setUp(): void { @@ -82,6 +73,9 @@ protected function setUp(): void { $this->throttler = $this->createMock(IThrottler::class); $this->random = $this->createMock(ISecureRandom::class); $this->manager = $this->createMock(Manager::class); + $this->manager + ->method('getUserNameFromLoginName') + ->willReturnArgument(0); $this->session = $this->createMock(ISession::class); $this->lockdownManager = $this->createMock(ILockdownManager::class); $this->logger = $this->createMock(LoggerInterface::class); @@ -96,7 +90,7 @@ protected function setUp(): void { $this->random, $this->lockdownManager, $this->logger, - $this->dispatcher + $this->dispatcher, ]) ->onlyMethods([ 'setMagicInCookie', @@ -113,7 +107,7 @@ public static function isLoggedInData(): array { ]; } - #[\PHPUnit\Framework\Attributes\DataProvider('isLoggedInData')] + #[DataProvider(methodName: 'isLoggedInData')] public function testIsLoggedIn($isLoggedIn): void { $session = $this->createMock(Memory::class); @@ -140,8 +134,6 @@ public function testSetUser(): void { $manager = $this->createMock(Manager::class); - $backend = $this->createMock(\Test\Util\User\Dummy::class); - $user = $this->createMock(IUser::class); $user->expects($this->once()) ->method('getUID') @@ -161,17 +153,9 @@ public function testLoginValidPasswordEnabled(): void { ->willThrowException(new InvalidTokenException()); $session->expects($this->exactly(2)) ->method('set') - ->with($this->callback(function ($key) { - switch ($key) { - case 'user_id': - case 'loginname': - return true; - break; - default: - return false; - break; - } - }, 'foo')); + ->with($this->callback(function (string $key): bool { + return $key === 'user_id' || $key === 'loginname'; + })); $managerMethods = get_class_methods(Manager::class); //keep following methods intact in order to ensure hooks are working @@ -186,8 +170,6 @@ public function testLoginValidPasswordEnabled(): void { ]) ->getMock(); - $backend = $this->createMock(\Test\Util\User\Dummy::class); - $user = $this->createMock(IUser::class); $user->expects($this->any()) ->method('isEnabled') @@ -215,7 +197,7 @@ public function testLoginValidPasswordEnabled(): void { $this->dispatcher->expects($this->once()) ->method('dispatchTyped') ->with( - $this->callback(function (PostLoginEvent $e) { + $this->callback(function (PostLoginEvent $e): bool { return $e->getUser()->getUID() === 'foo' && $e->getPassword() === 'bar' && $e->isTokenLogin() === false; @@ -285,7 +267,6 @@ public function testLoginInvalidPassword(): void { $this->createMock(LoggerInterface::class), ]) ->getMock(); - $backend = $this->createMock(\Test\Util\User\Dummy::class); $userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher); $user = $this->createMock(IUser::class); @@ -432,7 +413,6 @@ public function testLogClientInNoTokenPasswordWith2fa(): void { $session = $this->createMock(ISession::class); $request = $this->createMock(IRequest::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]) ->onlyMethods(['login', 'supportsCookies', 'createSessionToken', 'getUser']) @@ -465,15 +445,28 @@ public function testLogClientInNoTokenPasswordWith2fa(): void { ->method('registerAttempt') ->with('login', '192.168.0.1', ['user' => 'john']); $this->dispatcher - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('dispatchTyped') - ->with(new LoginFailed('john', 'doe')); + ->willReturnCallback( + function (Event $event) { + if ($event instanceof LoginFailed) { + $this->assertEquals($event, new LoginFailed('john', 'doe')); + } elseif ($event instanceof BeforeUserLoggedInEvent) { + $this->assertEquals($event, new BeforeUserLoggedInEvent('john', 'doe')); + } else { + $this->fail('Unexpected event'); + } + } + ); $userSession->logClientIn('john', 'doe', $request, $this->throttler); } public function testLogClientInUnexist(): void { $manager = $this->createMock(Manager::class); + $manager + ->method('getUserNameFromLoginName') + ->willReturnArgument(0); $session = $this->createMock(ISession::class); $request = $this->createMock(IRequest::class); @@ -503,7 +496,6 @@ public function testLogClientInWithTokenPassword(): void { $session = $this->createMock(ISession::class); $request = $this->createMock(IRequest::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]) ->onlyMethods(['login', 'supportsCookies', 'createSessionToken', 'getUser']) @@ -545,7 +537,6 @@ public function testLogClientInNoTokenPasswordNo2fa(): void { $session = $this->createMock(ISession::class); $request = $this->createMock(IRequest::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]) ->onlyMethods(['login', 'isTwoFactorEnforced']) @@ -583,10 +574,21 @@ public function testLogClientInNoTokenPasswordNo2fa(): void { ->expects($this->once()) ->method('registerAttempt') ->with('login', '192.168.0.1', ['user' => 'john']); + $this->dispatcher - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('dispatchTyped') - ->with(new LoginFailed('john', 'doe')); + ->willReturnCallback( + function (Event $event) { + if ($event instanceof LoginFailed) { + $this->assertEquals($event, new LoginFailed('john', 'doe')); + } elseif ($event instanceof BeforeUserLoggedInEvent) { + $this->assertEquals($event, new BeforeUserLoggedInEvent('john', 'doe')); + } else { + $this->fail('Unexpected event'); + } + } + ); $userSession->logClientIn('john', 'doe', $request, $this->throttler); } @@ -1116,7 +1118,7 @@ public function testCreateRememberedSessionToken(): void { ->method('generateToken') ->with($sessionId, $uid, $loginName, $password, 'Firefox', IToken::TEMPORARY_TOKEN, IToken::REMEMBER); - $this->assertTrue($userSession->createSessionToken($request, $uid, $loginName, $password, true)); + $this->assertTrue($userSession->createSessionToken($request, $uid, $loginName, $password, IToken::REMEMBER)); } public function testCreateSessionTokenWithTokenPassword(): void { @@ -1226,7 +1228,7 @@ public function testTryBasicAuthLoginValid(): void { $this->session ->method('set') - ->willReturnCallback(function ($k, $v) use (&$davAuthenticatedSet, &$lastPasswordConfirmSet): void { + ->willReturnCallback(function (string $k, $v) use (&$davAuthenticatedSet): void { switch ($k) { case Auth::DAV_AUTHENTICATED: $davAuthenticatedSet = $v; @@ -1236,6 +1238,7 @@ public function testTryBasicAuthLoginValid(): void { } }); + /** @var Session&MockObject $userSession */ $userSession = $this->getMockBuilder(Session::class) ->setConstructorArgs([ $this->manager, @@ -1254,7 +1257,6 @@ public function testTryBasicAuthLoginValid(): void { ]) ->getMock(); - /** @var Session|MockObject */ $userSession->expects($this->once()) ->method('logClientIn') ->with( @@ -1273,6 +1275,7 @@ public function testTryBasicAuthLoginValid(): void { $this->assertTrue($userSession->tryBasicAuthLogin($request, $this->throttler)); + /** @var string|false $davAuthenticatedSet */ $this->assertSame('username', $davAuthenticatedSet); } @@ -1304,7 +1307,6 @@ public function testTryBasicAuthLoginNoLogin(): void { ]) ->getMock(); - /** @var Session|MockObject */ $userSession->expects($this->never()) ->method('logClientIn'); @@ -1324,7 +1326,6 @@ public function testLogClientInThrottlerUsername(): void { $session = $this->createMock(ISession::class); $request = $this->createMock(IRequest::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]) ->onlyMethods(['login', 'supportsCookies', 'createSessionToken', 'getUser']) @@ -1359,19 +1360,31 @@ public function testLogClientInThrottlerUsername(): void { ->method('registerAttempt') ->with('login', '192.168.0.1', ['user' => 'john']); $this->dispatcher - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('dispatchTyped') - ->with(new LoginFailed('john', 'I-AM-A-PASSWORD')); + ->willReturnCallback( + function (Event $event) { + if ($event instanceof LoginFailed) { + $this->assertEquals($event, new LoginFailed('john', 'I-AM-A-PASSWORD')); + } elseif ($event instanceof BeforeUserLoggedInEvent) { + $this->assertEquals($event, new BeforeUserLoggedInEvent('john', 'I-AM-A-PASSWORD')); + } else { + $this->fail('Unexpected event'); + } + } + ); $this->assertFalse($userSession->logClientIn('john', 'I-AM-A-PASSWORD', $request, $this->throttler)); } public function testLogClientInThrottlerEmail(): void { $manager = $this->createMock(Manager::class); + $manager + ->method('getUserNameFromLoginName') + ->willReturnArgument(0); $session = $this->createMock(ISession::class); $request = $this->createMock(IRequest::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]) ->onlyMethods(['login', 'supportsCookies', 'createSessionToken', 'getUser']) @@ -1410,9 +1423,19 @@ public function testLogClientInThrottlerEmail(): void { ->method('registerAttempt') ->with('login', '192.168.0.1', ['user' => 'john@foo.bar']); $this->dispatcher - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('dispatchTyped') - ->with(new LoginFailed('john@foo.bar', 'I-AM-A-PASSWORD')); + ->willReturnCallback( + function (Event $event) { + if ($event instanceof LoginFailed) { + $this->assertEquals($event, new LoginFailed('john@foo.bar', 'I-AM-A-PASSWORD')); + } elseif ($event instanceof BeforeUserLoggedInEvent) { + $this->assertEquals($event, new BeforeUserLoggedInEvent('john@foo.bar', 'I-AM-A-PASSWORD')); + } else { + $this->fail('Unexpected event'); + } + } + ); $this->assertFalse($userSession->logClientIn('john@foo.bar', 'I-AM-A-PASSWORD', $request, $this->throttler)); } diff --git a/tests/lib/User/UserTest.php b/tests/lib/User/UserTest.php index 8019dd6213d2c..eb691d2433d7d 100644 --- a/tests/lib/User/UserTest.php +++ b/tests/lib/User/UserTest.php @@ -33,7 +33,7 @@ use PHPUnit\Framework\Attributes\Group; use Test\TestCase; -#[Group('DB')] +#[Group(name: 'DB')] class UserTest extends TestCase { protected IEventDispatcher $dispatcher; @@ -404,7 +404,7 @@ public static function dataDeleteEvent(): array { ]; } - #[DataProvider('dataDeleteEvent')] + #[DataProvider(methodName: 'dataDeleteEvent')] public function testDeleteEvent(bool $result, int $expectedHooks): void { $hooksCalled = 0; @@ -547,7 +547,7 @@ public static function dataGetCloudId(): array { ]; } - #[DataProvider('dataGetCloudId')] + #[DataProvider(methodName: 'dataGetCloudId')] public function testGetCloudId(string $absoluteUrl, string $cloudId): void { $backend = $this->createMock(\Test\Util\User\Dummy::class); $urlGenerator = $this->createMock(IURLGenerator::class); @@ -745,7 +745,7 @@ public function testGetLastLogin(): void { } }); - $user = new User('foo', $backend, $this->dispatcher, null, null, $userConfig); + $user = new User('foo', $backend, $this->dispatcher, null, $userConfig); $this->assertSame(42, $user->getLastLogin()); } @@ -770,7 +770,7 @@ public function testSetEnabled(): void { fn ($user, $app, $key, $default) => ($key === 'enabled' ? false : $default) ); - $user = new User('foo', $backend, $this->dispatcher, null, $config, $userConfig); + $user = new User('foo', $backend, $this->dispatcher, $config, $userConfig); $user->setEnabled(true); } @@ -855,7 +855,7 @@ public function testGetEMailAddress(): void { } }); - $user = new User('foo', $backend, $this->dispatcher, null, null, $userConfig); + $user = new User('foo', $backend, $this->dispatcher, null, $userConfig); $this->assertSame('foo@bar.com', $user->getEMailAddress()); } } From 6aa6e0eaa2502cbeb16f0ff9fc310795faf2e3cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 17 Sep 2026 14:04:56 +0200 Subject: [PATCH 07/13] chore: Fix warning in PHPUnit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- tests/Core/Controller/LoginControllerTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Core/Controller/LoginControllerTest.php b/tests/Core/Controller/LoginControllerTest.php index aab340ca2d7ce..ea986861dd110 100644 --- a/tests/Core/Controller/LoginControllerTest.php +++ b/tests/Core/Controller/LoginControllerTest.php @@ -46,6 +46,7 @@ class LoginControllerTest extends TestCase { private ISession&MockObject $session; private Session&MockObject $userSession; private IURLGenerator&MockObject $urlGenerator; + private Manager&MockObject $twoFactorManager; private Defaults&MockObject $defaults; private IThrottler&MockObject $throttler; private IInitialState&MockObject $initialState; From dbb596af79fe9a22f1f0f548deac3d3dbfb2f47e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 17 Sep 2026 14:07:00 +0200 Subject: [PATCH 08/13] chore: Allow the hooks used by tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/legacy/OC_Hook.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/private/legacy/OC_Hook.php b/lib/private/legacy/OC_Hook.php index 173cc4261b736..1c529331709da 100644 --- a/lib/private/legacy/OC_Hook.php +++ b/lib/private/legacy/OC_Hook.php @@ -76,6 +76,9 @@ class OC_Hook { ['OCP\Config','js'], ['OC\Core\LostPassword\Controller\LostController','post_passwordReset'], ['OC\Core\LostPassword\Controller\LostController','pre_passwordReset'], + /* Only used by tests */ + ['LegacyHookTest', 'error'], + ['LegacyHookTest', 'hint'], ]; /** From 2216faf6f43bb39bb5ee13f3f8574adfa6f28cef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 17 Sep 2026 14:10:25 +0200 Subject: [PATCH 09/13] chore: Fix federatedfilesharing tests following hook removal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../tests/OCM/CloudFederationProviderFilesTest.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/federatedfilesharing/tests/OCM/CloudFederationProviderFilesTest.php b/apps/federatedfilesharing/tests/OCM/CloudFederationProviderFilesTest.php index d2e540849ce4e..890060211bbbc 100644 --- a/apps/federatedfilesharing/tests/OCM/CloudFederationProviderFilesTest.php +++ b/apps/federatedfilesharing/tests/OCM/CloudFederationProviderFilesTest.php @@ -189,6 +189,7 @@ public function testShareReceivedAcceptsMultiProtocolEnvelope(): void { $this->discoveryService->method('discover') ->willThrowException(new \Exception('network error')); + $this->userManager->method('getUserNameFromLoginName')->with('localuser')->willReturn('localuser'); $this->userManager->method('get')->with('localuser')->willReturn(null); $this->filenameValidator->method('isFilenameValid')->willReturn(true); @@ -268,6 +269,7 @@ public function testShareReceivedMustExchangeTokenStoresAccessToken(): void { // Exchange succeeds → share creation continues; we stop it at the user // lookup stage to avoid a full integration setup. + $this->userManager->method('getUserNameFromLoginName')->with('localuser')->willReturn('localuser'); $this->userManager->method('get')->with('localuser')->willReturn(null); $this->filenameValidator->method('isFilenameValid')->willReturn(true); @@ -296,6 +298,7 @@ public function testShareReceivedOptionalExchangeGracefulOnDiscoveryFailure(): v // Discovery failure is caught and logged; share creation continues. // We stop it at the user lookup stage. + $this->userManager->method('getUserNameFromLoginName')->with('localuser')->willReturn('localuser'); $this->userManager->method('get')->with('localuser')->willReturn(null); $this->filenameValidator->method('isFilenameValid')->willReturn(true); @@ -348,6 +351,7 @@ public function testShareReceivedOptionalExchangeStoresAccessTokenOnSuccess(): v $httpClient->method('post')->willReturn($response); $this->clientService->method('newClient')->willReturn($httpClient); + $this->userManager->method('getUserNameFromLoginName')->with('localuser')->willReturn('localuser'); $this->userManager->method('get')->with('localuser')->willReturn(null); $this->filenameValidator->method('isFilenameValid')->willReturn(true); From 295701980424e312993cb3fbec81c9c59ca79023 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 17 Sep 2026 14:57:16 +0200 Subject: [PATCH 10/13] fix(tests): Fix UserTest. Mock event dispatcher instead of listening. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Registering listeners was messing with other tests, use a mocked event dispatcher instead. Also fixed a few constructor calls. Signed-off-by: Côme Chilliet --- tests/lib/User/UserTest.php | 66 ++++++++++++++++++++++--------------- 1 file changed, 39 insertions(+), 27 deletions(-) diff --git a/tests/lib/User/UserTest.php b/tests/lib/User/UserTest.php index eb691d2433d7d..00f3d4997c656 100644 --- a/tests/lib/User/UserTest.php +++ b/tests/lib/User/UserTest.php @@ -14,11 +14,11 @@ use OC\User\User; use OCP\Comments\ICommentsManager; use OCP\Config\IUserConfig; +use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\FileInfo; use OCP\Files\IRootFolder; use OCP\Files\Storage\IStorageFactory; -use OCP\Group\Events\UserRemovedEvent; use OCP\IConfig; use OCP\IURLGenerator; use OCP\IUser; @@ -29,18 +29,20 @@ use OCP\User\Events\BeforeUserDeletedEvent; use OCP\User\Events\PasswordUpdatedEvent; use OCP\User\Events\UserChangedEvent; +use OCP\User\Events\UserDeletedEvent; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; +use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; #[Group(name: 'DB')] class UserTest extends TestCase { - protected IEventDispatcher $dispatcher; + protected IEventDispatcher&MockObject $dispatcher; #[\Override] protected function setUp(): void { parent::setUp(); - $this->dispatcher = Server::get(IEventDispatcher::class); + $this->dispatcher = $this->createMock(IEventDispatcher::class); } public function testDisplayName(): void { @@ -367,7 +369,6 @@ public function testSetDisplayNameNotSupported(): void { public function testSetPasswordHooks(): void { $hooksCalled = 0; - $test = $this; $backend = $this->createMock(\Test\Util\User\Dummy::class); $backend->method('getBackendName')->willReturn('foo'); @@ -375,17 +376,23 @@ public function testSetPasswordHooks(): void { ->method('setPassword') ->willReturn(true); - $this->dispatcher->addListener(BeforePasswordUpdatedEvent::class, function (BeforePasswordUpdatedEvent $event) use ($test, &$hooksCalled): void { - $hooksCalled++; - $test->assertEquals('foo', $event->getUser()->getUID()); - $test->assertEquals('bar', $event->getPassword()); - }); - - $this->dispatcher->addListener(PasswordUpdatedEvent::class, function (PasswordUpdatedEvent $event) use ($test, &$hooksCalled): void { - $hooksCalled++; - $test->assertEquals('foo', $event->getUser()->getUID()); - $test->assertEquals('bar', $event->getPassword()); - }); + $this->dispatcher->expects(self::atLeastOnce()) + ->method('dispatchTyped') + ->willReturnCallback( + function (Event $event) use (&$hooksCalled) { + if ($event instanceof BeforePasswordUpdatedEvent) { + $hooksCalled++; + $this->assertEquals('foo', $event->getUser()->getUID()); + $this->assertEquals('bar', $event->getPassword()); + } elseif ($event instanceof PasswordUpdatedEvent) { + $hooksCalled++; + $this->assertEquals('foo', $event->getUser()->getUID()); + $this->assertEquals('bar', $event->getPassword()); + } else { + $this->fail('was not expecting any more events'); + } + } + ); $backend->expects($this->any()) ->method('implementsActions') @@ -428,15 +435,22 @@ public function testDeleteEvent(bool $result, int $expectedHooks): void { $user = new User('foo', $backend, $this->dispatcher, $config, $userConfig); - $this->dispatcher->addListener(BeforeUserDeletedEvent::class, function (BeforeUserDeletedEvent $event) use (&$hooksCalled) { - $hooksCalled++; - $this->assertEquals('foo', $event->getUser()->getUID()); - }); - - $this->dispatcher->addListener(UserRemovedEvent::class, function (UserRemovedEvent $event) use (&$hooksCalled) { - $hooksCalled++; - $this->assertEquals('foo', $event->getUser()->getUID()); - }); + $this->dispatcher->expects(self::atLeastOnce()) + ->method('dispatchTyped') + ->willReturnCallback( + function (Event $event) use (&$hooksCalled) { + if ($event instanceof BeforeUserDeletedEvent) { + $hooksCalled++; + $this->assertEquals('foo', $event->getUser()->getUID()); + } elseif ($event instanceof UserDeletedEvent) { + $hooksCalled++; + $this->assertEquals('foo', $event->getUser()->getUID()); + } else { + var_dump(get_class($event)); + $this->fail('was not expecting any more events'); + } + } + ); $commentsManager = $this->createMock(ICommentsManager::class); $notificationManager = $this->createMock(INotificationManager::class); @@ -528,7 +542,7 @@ public function testDeleteRecoverState() { $user = $this->getMockBuilder(User::class) ->onlyMethods(['getHome']) - ->setConstructorArgs(['foo', $backend, $this->dispatcher, null, $config, $userConfig]) + ->setConstructorArgs(['foo', $backend, $this->dispatcher, $config, $userConfig]) ->getMock(); $user->expects(self::atLeastOnce()) @@ -792,7 +806,6 @@ public function testSetDisabled(): void { 'foo', $backend, $this->dispatcher, - null, $config, ]) ->onlyMethods(['isEnabled', 'triggerChange']) @@ -826,7 +839,6 @@ public function testSetDisabledAlreadyDisabled(): void { 'foo', $backend, $this->dispatcher, - null, $config, $userConfig, ]) From 4c5efb916b7a15a18cc951a534884343950be97c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 17 Sep 2026 17:01:34 +0200 Subject: [PATCH 11/13] fix: Simplify the UserMountCacheListener build. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The listener can simply be added with the other ones. For the tests we have to call the method manually as the listener does not target the same instance. Signed-off-by: Côme Chilliet --- lib/private/Server.php | 9 ++------- tests/lib/Files/Config/UserMountCacheTest.php | 4 ++++ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/lib/private/Server.php b/lib/private/Server.php index 802bbad74b7b3..c05d7656c2608 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -639,13 +639,7 @@ public function __construct( ); }); - $this->registerService(IUserMountCache::class, static function (ContainerInterface $c): IUserMountCache { - $mountCache = $c->get(UserMountCache::class); - /** @var IEventDispatcher $eventDispatcher */ - $eventDispatcher = $c->get(IEventDispatcher::class); - $eventDispatcher->addServiceListener(UserDeletedEvent::class, UserMountCacheListener::class); - return $mountCache; - }); + $this->registerAlias(IUserMountCache::class, UserMountCache::class); $this->registerService(IMountProviderCollection::class, static function (ContainerInterface $c): IMountProviderCollection { $loader = $c->get(IStorageFactory::class); @@ -1123,6 +1117,7 @@ private function connectDispatcher(): void { $eventDispatcher->addServiceListener(BeforeUserDeletedEvent::class, BeforeUserDeletedListener::class); $eventDispatcher->addServiceListener(UserDeletedEvent::class, SubAdmin::class); $eventDispatcher->addServiceListener(GroupDeletedEvent::class, SubAdmin::class); + $eventDispatcher->addServiceListener(UserDeletedEvent::class, UserMountCacheListener::class); FilesMetadataManager::loadListeners($eventDispatcher); GenerateBlurhashMetadata::loadListeners($eventDispatcher); diff --git a/tests/lib/Files/Config/UserMountCacheTest.php b/tests/lib/Files/Config/UserMountCacheTest.php index d0550e3c3471e..ce5f318aa71aa 100644 --- a/tests/lib/Files/Config/UserMountCacheTest.php +++ b/tests/lib/Files/Config/UserMountCacheTest.php @@ -306,6 +306,8 @@ public function testGetMountsForUser(): void { $this->clearCache(); $user3->delete(); + // We have to call this manually as the listener is not connected to our test instance + $this->cache->removeUserMounts($user3); $cachedMounts = $this->cache->getMountsForUser($user1); @@ -543,6 +545,8 @@ public function testGetMountsForFileIdDeletedUser(): void { $this->cache->registerMounts($user1, [$mount1]); $user1->delete(); + // We have to call this manually as the listener is not connected to our test instance + $this->cache->removeUserMounts($user1); $this->clearCache(); $cachedMounts = $this->cache->getMountsForFileId($rootId); From 2a2f89c15d40b7b7c56cb889a677fcdf3e4044dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 17 Sep 2026 17:14:36 +0200 Subject: [PATCH 12/13] fix(tests): Fix user manager deletion test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We have to call the event handling manually when the event dispatcher is mocked. Signed-off-by: Côme Chilliet --- lib/private/User/Manager.php | 8 +++++--- tests/lib/User/ManagerTest.php | 6 +++++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php index f4c9002286fc0..dc890d6764095 100644 --- a/lib/private/User/Manager.php +++ b/lib/private/User/Manager.php @@ -83,12 +83,14 @@ public function __construct( private LoggerInterface $logger, ) { $this->cache = new WithLocalCache($cacheFactory->createDistributed('user_backend_map')); - $this->eventDispatcher->addListener(UserDeletedEvent::class, function (UserDeletedEvent $event) { - unset($this->cachedUsers[$event->getUser()->getUID()]); - }); + $this->eventDispatcher->addListener(UserDeletedEvent::class, $this->handleUserDeletedEvent(...)); $this->displayNameCache = new DisplayNameCache($cacheFactory, $this); } + private function handleUserDeletedEvent(UserDeletedEvent $event): void { + unset($this->cachedUsers[$event->getUser()->getUID()]); + } + private function getKnownUserService(): KnownUserService { return $this->knownUserService ??= Server::get(KnownUserService::class); } diff --git a/tests/lib/User/ManagerTest.php b/tests/lib/User/ManagerTest.php index bcc61db646665..9530baf2fd7d1 100644 --- a/tests/lib/User/ManagerTest.php +++ b/tests/lib/User/ManagerTest.php @@ -22,6 +22,7 @@ use OCP\IUser; use OCP\IUserManager; use OCP\Server; +use OCP\User\Events\UserDeletedEvent; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; @@ -672,7 +673,10 @@ public function testDeleteUser(): void { $this->manager->registerBackend($backend); $backend->createUser('foo', 'bar'); $this->assertTrue($this->manager->userExists('foo')); - $this->manager->get('foo')->delete(); + $fooUser = $this->manager->get('foo'); + $fooUser->delete(); + // Call manually as event dispatcher is a mock + self::invokePrivate($this->manager, 'handleUserDeletedEvent', [new UserDeletedEvent($fooUser)]); $this->assertFalse($this->manager->userExists('foo')); } From e69d06d8ad967fc8dd71fedcb6cfa150a17576e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 17 Sep 2026 17:28:51 +0200 Subject: [PATCH 13/13] fix: Make sure OC_User does not set null as password when calling completeLogin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/User/Session.php | 2 +- lib/private/legacy/OC_User.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index 639b50ccd9026..e154dad813879 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -322,7 +322,7 @@ public function login($uid, $password) { /** * @param IUser $user - * @param array $loginDetails + * @param array{loginName:string,password:string,token?:IToken} $loginDetails * @param bool $regenerateSessionId * @return true returns true if login successful or an exception otherwise * @throws LoginException diff --git a/lib/private/legacy/OC_User.php b/lib/private/legacy/OC_User.php index 67a4a388ff8ad..3dff9c857a8f1 100644 --- a/lib/private/legacy/OC_User.php +++ b/lib/private/legacy/OC_User.php @@ -162,7 +162,7 @@ public static function loginWithApache(IApacheBackend $backend): bool { $dispatcher->dispatchTyped(new BeforeUserLoggedInEvent($uid, $password, $backend)); $user = $userSession->getUser(); - $userSession->completeLogin($user, ['loginName' => $uid, 'password' => $password]); + $userSession->completeLogin($user, ['loginName' => $uid, 'password' => $password ?? '']); $userSession->createSessionToken($request, $uid, $uid, $password); $userSession->createRememberMeToken($user);