diff --git a/apps/provisioning_api/composer/composer/autoload_classmap.php b/apps/provisioning_api/composer/composer/autoload_classmap.php index 7a007f4577d9e..1c1f47b75f015 100644 --- a/apps/provisioning_api/composer/composer/autoload_classmap.php +++ b/apps/provisioning_api/composer/composer/autoload_classmap.php @@ -17,6 +17,7 @@ 'OCA\\Provisioning_API\\Controller\\UsersController' => $baseDir . '/../lib/Controller/UsersController.php', 'OCA\\Provisioning_API\\Controller\\VerificationController' => $baseDir . '/../lib/Controller/VerificationController.php', 'OCA\\Provisioning_API\\FederatedShareProviderFactory' => $baseDir . '/../lib/FederatedShareProviderFactory.php', + 'OCA\\Provisioning_API\\Listener\\UserDataCacheListener' => $baseDir . '/../lib/Listener/UserDataCacheListener.php', 'OCA\\Provisioning_API\\Listener\\UserDeletedListener' => $baseDir . '/../lib/Listener/UserDeletedListener.php', 'OCA\\Provisioning_API\\Middleware\\Exceptions\\NotSubAdminException' => $baseDir . '/../lib/Middleware/Exceptions/NotSubAdminException.php', 'OCA\\Provisioning_API\\Middleware\\ProvisioningApiMiddleware' => $baseDir . '/../lib/Middleware/ProvisioningApiMiddleware.php', diff --git a/apps/provisioning_api/composer/composer/autoload_static.php b/apps/provisioning_api/composer/composer/autoload_static.php index 537203fb53d3a..bd5ce0ad63042 100644 --- a/apps/provisioning_api/composer/composer/autoload_static.php +++ b/apps/provisioning_api/composer/composer/autoload_static.php @@ -32,6 +32,7 @@ class ComposerStaticInitProvisioning_API 'OCA\\Provisioning_API\\Controller\\UsersController' => __DIR__ . '/..' . '/../lib/Controller/UsersController.php', 'OCA\\Provisioning_API\\Controller\\VerificationController' => __DIR__ . '/..' . '/../lib/Controller/VerificationController.php', 'OCA\\Provisioning_API\\FederatedShareProviderFactory' => __DIR__ . '/..' . '/../lib/FederatedShareProviderFactory.php', + 'OCA\\Provisioning_API\\Listener\\UserDataCacheListener' => __DIR__ . '/..' . '/../lib/Listener/UserDataCacheListener.php', 'OCA\\Provisioning_API\\Listener\\UserDeletedListener' => __DIR__ . '/..' . '/../lib/Listener/UserDeletedListener.php', 'OCA\\Provisioning_API\\Middleware\\Exceptions\\NotSubAdminException' => __DIR__ . '/..' . '/../lib/Middleware/Exceptions/NotSubAdminException.php', 'OCA\\Provisioning_API\\Middleware\\ProvisioningApiMiddleware' => __DIR__ . '/..' . '/../lib/Middleware/ProvisioningApiMiddleware.php', diff --git a/apps/provisioning_api/lib/AppInfo/Application.php b/apps/provisioning_api/lib/AppInfo/Application.php index 9595dd4a60671..e18cb4d29ebc4 100644 --- a/apps/provisioning_api/lib/AppInfo/Application.php +++ b/apps/provisioning_api/lib/AppInfo/Application.php @@ -8,6 +8,7 @@ use OC\Group\Manager as GroupManager; use OCA\Provisioning_API\Capabilities; +use OCA\Provisioning_API\Listener\UserDataCacheListener; use OCA\Provisioning_API\Listener\UserDeletedListener; use OCA\Provisioning_API\Middleware\ProvisioningApiMiddleware; use OCA\Settings\Mailer\NewUserMailHelper; @@ -27,6 +28,8 @@ use OCP\Mail\IMailer; use OCP\Security\ICrypto; use OCP\Security\ISecureRandom; +use OCP\User\Events\PasswordUpdatedEvent; +use OCP\User\Events\UserChangedEvent; use OCP\User\Events\UserDeletedEvent; use OCP\Util; use Psr\Container\ContainerInterface; @@ -37,7 +40,10 @@ public function __construct(array $urlParams = []) { } public function register(IRegistrationContext $context): void { + $context->registerEventListener(UserChangedEvent::class, UserDataCacheListener::class); + $context->registerEventListener(UserDeletedEvent::class, UserDataCacheListener::class); $context->registerEventListener(UserDeletedEvent::class, UserDeletedListener::class); + $context->registerEventListener(PasswordUpdatedEvent::class, UserDataCacheListener::class); $context->registerService(NewUserMailHelper::class, function (ContainerInterface $c) { return new NewUserMailHelper( diff --git a/apps/provisioning_api/lib/Controller/AUserDataOCSController.php b/apps/provisioning_api/lib/Controller/AUserDataOCSController.php index 5f3f474ec7b7b..d94eaf94ca601 100644 --- a/apps/provisioning_api/lib/Controller/AUserDataOCSController.php +++ b/apps/provisioning_api/lib/Controller/AUserDataOCSController.php @@ -22,6 +22,8 @@ use OCP\Files\IRootFolder; use OCP\Files\NotFoundException; use OCP\Group\ISubAdmin; +use OCP\ICache; +use OCP\ICacheFactory; use OCP\IConfig; use OCP\IRequest; use OCP\IUser; @@ -50,6 +52,10 @@ abstract class AUserDataOCSController extends OCSController { public const USER_FIELD_MANAGER = 'manager'; public const USER_FIELD_NOTIFICATION_EMAIL = 'notify_email'; + private const CACHE_TTL = 300; // 5 minutes + + private ICache $cache; + public function __construct( string $appName, IRequest $request, @@ -61,8 +67,10 @@ public function __construct( protected ISubAdmin $subAdminManager, protected IFactory $l10nFactory, protected IRootFolder $rootFolder, + ICacheFactory $cacheFactory, ) { parent::__construct($appName, $request); + $this->cache = $cacheFactory->createDistributed('provisioning_api'); } /** @@ -79,8 +87,6 @@ protected function getUserData(string $userId, bool $includeScopes = false): ?ar $currentLoggedInUser = $this->userSession->getUser(); assert($currentLoggedInUser !== null, 'No user logged in'); - $data = []; - // Check if the target user exists $targetUserObject = $this->userManager->get($userId); if ($targetUserObject === null) { @@ -89,6 +95,15 @@ protected function getUserData(string $userId, bool $includeScopes = false): ?ar $isAdmin = $this->groupManager->isAdmin($currentLoggedInUser->getUID()); $isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($currentLoggedInUser->getUID()); + + $cacheKey = 'user_data_' . $userId . '_' . ($isAdmin || $isDelegatedAdmin ? 'admin' : 'noadmin') . ($includeScopes ? '_scoped' : ''); + /** @var Provisioning_APIUserDetails|null $cached */ + $cached = $this->cache->get($cacheKey); + if ($cached !== null) { + return $cached; + } + + $data = []; if ($isAdmin || $isDelegatedAdmin || $this->groupManager->getSubAdmin()->isUserAccessible($currentLoggedInUser, $targetUserObject)) { @@ -197,6 +212,7 @@ protected function getUserData(string $userId, bool $includeScopes = false): ?ar 'setPassword' => $backend instanceof ISetPasswordBackend || $backend->implementsActions(Backend::SET_PASSWORD), ]; + $this->cache->set($cacheKey, $data, self::CACHE_TTL); return $data; } diff --git a/apps/provisioning_api/lib/Controller/GroupsController.php b/apps/provisioning_api/lib/Controller/GroupsController.php index 62f639b48e4e4..b1a2196ada4e1 100644 --- a/apps/provisioning_api/lib/Controller/GroupsController.php +++ b/apps/provisioning_api/lib/Controller/GroupsController.php @@ -23,6 +23,7 @@ use OCP\AppFramework\OCSController; use OCP\Files\IRootFolder; use OCP\Group\ISubAdmin; +use OCP\ICacheFactory; use OCP\IConfig; use OCP\IGroup; use OCP\IGroupManager; @@ -51,6 +52,7 @@ public function __construct( IFactory $l10nFactory, IRootFolder $rootFolder, private LoggerInterface $logger, + ICacheFactory $cacheFactory, ) { parent::__construct($appName, $request, @@ -62,6 +64,7 @@ public function __construct( $subAdminManager, $l10nFactory, $rootFolder, + $cacheFactory, ); } diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index ef88985fdafeb..61c7e949e87b5 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -36,6 +36,8 @@ use OCP\Files\IRootFolder; use OCP\Group\ISubAdmin; use OCP\HintException; +use OCP\ICache; +use OCP\ICacheFactory; use OCP\IConfig; use OCP\IGroup; use OCP\IGroupManager; @@ -60,6 +62,9 @@ class UsersController extends AUserDataOCSController { private IL10N $l10n; + private ICache $cache; + + private const CACHE_PREFIX = 'provisioning_usercache'; public function __construct( string $appName, @@ -81,6 +86,7 @@ public function __construct( private IEventDispatcher $eventDispatcher, private IPhoneNumberUtil $phoneNumberUtil, private IAppManager $appManager, + private ICacheFactory $cacheFactory, ) { parent::__construct( $appName, @@ -93,9 +99,11 @@ public function __construct( $subAdminManager, $l10nFactory, $rootFolder, + $cacheFactory, ); $this->l10n = $l10nFactory->get($appName); + $this->cache = $this->cacheFactory->createDistributed(self::CACHE_PREFIX); } /** @@ -111,30 +119,46 @@ public function __construct( #[NoAdminRequired] public function getUsers(string $search = '', ?int $limit = null, int $offset = 0): DataResponse { $user = $this->userSession->getUser(); - $users = []; - - // Admin? Or SubAdmin? $uid = $user->getUID(); - $subAdminManager = $this->groupManager->getSubAdmin(); + + $users = $this->cache->get($uid . '_' . $search); + if (!empty($users)) { + return new DataResponse([ + 'users' => $users + ]); + } + $isAdmin = $this->groupManager->isAdmin($uid); $isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($uid); if ($isAdmin || $isDelegatedAdmin) { $users = $this->userManager->search($search, $limit, $offset); - } elseif ($subAdminManager->isSubAdmin($user)) { - $subAdminOfGroups = $subAdminManager->getSubAdminsGroups($user); - foreach ($subAdminOfGroups as $key => $group) { - $subAdminOfGroups[$key] = $group->getGID(); - } + $users = array_keys($users); + $this->cache->set($uid . '_' . $search, $users, 10); + return new DataResponse([ + 'users' => $users + ]); + } - $users = []; - foreach ($subAdminOfGroups as $group) { - $users = array_merge($users, $this->groupManager->displayNamesInGroup($group, $search, $limit, $offset)); - } + $users = []; + $subAdminManager = $this->groupManager->getSubAdmin(); + if (!$subAdminManager->isSubAdmin($user)) { + return new DataResponse([ + 'users' => $users + ]); } - /** @var list $users */ - $users = array_keys($users); + $subAdminOfGroups = $subAdminManager->getSubAdminsGroups($user); + foreach ($subAdminOfGroups as $key => $group) { + $subAdminOfGroups[$key] = $group->getGID(); + } + + $users = []; + foreach ($subAdminOfGroups as $group) { + $users += $this->groupManager->displayNamesInGroup($group, $search, $limit, $offset); + } + $users = array_keys($users); + $this->cache->set($uid . '_' . $search, $users, 10); return new DataResponse([ 'users' => $users ]); @@ -152,29 +176,7 @@ public function getUsers(string $search = '', ?int $limit = null, int $offset = */ #[NoAdminRequired] public function getUsersDetails(string $search = '', ?int $limit = null, int $offset = 0): DataResponse { - $currentUser = $this->userSession->getUser(); - $users = []; - - // Admin? Or SubAdmin? - $uid = $currentUser->getUID(); - $subAdminManager = $this->groupManager->getSubAdmin(); - $isAdmin = $this->groupManager->isAdmin($uid); - $isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($uid); - if ($isAdmin || $isDelegatedAdmin) { - $users = $this->userManager->search($search, $limit, $offset); - $users = array_keys($users); - } elseif ($subAdminManager->isSubAdmin($currentUser)) { - $subAdminOfGroups = $subAdminManager->getSubAdminsGroups($currentUser); - foreach ($subAdminOfGroups as $key => $group) { - $subAdminOfGroups[$key] = $group->getGID(); - } - - $users = []; - foreach ($subAdminOfGroups as $group) { - $users[] = array_keys($this->groupManager->displayNamesInGroup($group, $search, $limit, $offset)); - } - $users = array_merge(...$users); - } + $users = $this->getUsers($search, $limit, $offset)->getData()['users']; $usersDetails = []; foreach ($users as $userId) { @@ -187,14 +189,9 @@ public function getUsersDetails(string $search = '', ?int $limit = null, int $of $userData = null; $this->logger->warning('Found one enabled account that is removed from its backend, but still exists in Nextcloud database', ['accountId' => $userId]); } - // Do not insert empty entry - if ($userData !== null) { - $usersDetails[$userId] = $userData; - } else { - // Logged user does not have permissions to see this user - // only showing its id - $usersDetails[$userId] = ['id' => $userId]; - } + + // $userdata === null means logged user does not have permissions to see this user + $usersDetails[$userId] = $userData ?? ['id' => $userId]; } return new DataResponse([ @@ -238,25 +235,21 @@ public function getDisabledUsersDetails(string $search = '', ?int $limit = null, } elseif ($subAdminManager->isSubAdmin($currentUser)) { $subAdminOfGroups = $subAdminManager->getSubAdminsGroups($currentUser); - $users = []; + $usersSet = []; /* We have to handle offset ourselve for correctness */ $tempLimit = ($limit === null ? null : $limit + $offset); foreach ($subAdminOfGroups as $group) { - $users = array_unique(array_merge( - $users, - array_map( - fn (IUser $user): string => $user->getUID(), - array_filter( - $group->searchUsers($search), - fn (IUser $user): bool => !$user->isEnabled() - ) - ) - )); - if (($tempLimit !== null) && (count($users) >= $tempLimit)) { + foreach (array_filter( + $group->searchUsers($search), + fn (IUser $user): bool => !$user->isEnabled() + ) as $user) { + $usersSet[$user->getUID()] = true; + } + if (($tempLimit !== null) && (count($usersSet) >= $tempLimit)) { break; } } - $users = array_slice($users, $offset, $limit); + $users = array_slice(array_keys($usersSet), $offset, $limit); } $usersDetails = []; @@ -820,6 +813,7 @@ public function editUserMultiValue( throw new OCSException('', OCSController::RESPOND_NOT_FOUND); } + $this->cache->clear(self::CACHE_PREFIX); $subAdminManager = $this->groupManager->getSubAdmin(); $isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($currentLoggedInUser->getUID()); $isAdminOrSubadmin = $this->groupManager->isAdmin($currentLoggedInUser->getUID()) @@ -920,6 +914,7 @@ public function editUser(string $userId, string $key, string $value): DataRespon throw new OCSException('', OCSController::RESPOND_NOT_FOUND); } + $this->cache->clear(self::CACHE_PREFIX); $permittedFields = []; if ($targetUser->getUID() === $currentLoggedInUser->getUID()) { if ($targetUser->canChangeDisplayName()) { @@ -1295,6 +1290,7 @@ public function deleteUser(string $userId): DataResponse { throw new OCSException('', OCSController::RESPOND_NOT_FOUND); } + $this->cache->clear(self::CACHE_PREFIX); // Go ahead with the delete if ($targetUser->delete()) { return new DataResponse(); @@ -1341,7 +1337,6 @@ public function enableUser(string $userId): DataResponse { */ private function setEnabled(string $userId, bool $value): DataResponse { $currentLoggedInUser = $this->userSession->getUser(); - $targetUser = $this->userManager->get($userId); if ($targetUser === null || $targetUser->getUID() === $currentLoggedInUser->getUID()) { throw new OCSException('', 101); @@ -1355,6 +1350,7 @@ private function setEnabled(string $userId, bool $value): DataResponse { throw new OCSException('', OCSController::RESPOND_NOT_FOUND); } + $this->cache->clear(self::CACHE_PREFIX); // enable/disable the user now $targetUser->setEnabled($value); return new DataResponse(); @@ -1387,22 +1383,21 @@ public function getUsersGroups(string $userId): DataResponse { return new DataResponse([ 'groups' => $this->groupManager->getUserGroupIds($targetUser) ]); - } else { - $subAdminManager = $this->groupManager->getSubAdmin(); + } - // Looking up someone else - if ($subAdminManager->isUserAccessible($loggedInUser, $targetUser)) { - // Return the group that the method caller is subadmin of for the user in question - $groups = array_values(array_intersect( - array_map(static fn (IGroup $group) => $group->getGID(), $subAdminManager->getSubAdminsGroups($loggedInUser)), - $this->groupManager->getUserGroupIds($targetUser) - )); - return new DataResponse(['groups' => $groups]); - } else { - // Not permitted - throw new OCSException('', OCSController::RESPOND_NOT_FOUND); - } + $subAdminManager = $this->groupManager->getSubAdmin(); + + // Looking up someone else + if (!$subAdminManager->isUserAccessible($loggedInUser, $targetUser)) { + throw new OCSException('', OCSController::RESPOND_NOT_FOUND); } + + // Return the group that the method caller is subadmin of for the user in question + $groups = array_values(array_intersect( + array_map(static fn (IGroup $group) => $group->getGID(), $subAdminManager->getSubAdminsGroups($loggedInUser)), + $this->groupManager->getUserGroupIds($targetUser) + )); + return new DataResponse(['groups' => $groups]); } /** @@ -1445,41 +1440,41 @@ function (Group $group) { return new DataResponse([ 'groups' => $groups, ]); - } else { - $subAdminManager = $this->groupManager->getSubAdmin(); + } - // Looking up someone else - if ($subAdminManager->isUserAccessible($loggedInUser, $targetUser)) { - // Return the group that the method caller is subadmin of for the user in question - $gids = array_values(array_intersect( - array_map( - static fn (IGroup $group) => $group->getGID(), - $subAdminManager->getSubAdminsGroups($loggedInUser), - ), - $this->groupManager->getUserGroupIds($targetUser) - )); - $groups = array_map( - function (string $gid) { - $group = $this->groupManager->get($gid); - return [ - 'id' => $group->getGID(), - 'displayname' => $group->getDisplayName(), - 'usercount' => $group->count(), - 'disabled' => $group->countDisabled(), - 'canAdd' => $group->canAddUser(), - 'canRemove' => $group->canRemoveUser(), - ]; - }, - $gids, - ); - return new DataResponse([ - 'groups' => $groups, - ]); - } else { - // Not permitted - throw new OCSException('', OCSController::RESPOND_NOT_FOUND); - } + $subAdminManager = $this->groupManager->getSubAdmin(); + + // Looking up someone else + if (!$subAdminManager->isUserAccessible($loggedInUser, $targetUser)) { + // Not permitted + throw new OCSException('', OCSController::RESPOND_NOT_FOUND); } + + // Return the group that the method caller is subadmin of for the user in question + $gids = array_values(array_intersect( + array_map( + static fn (IGroup $group) => $group->getGID(), + $subAdminManager->getSubAdminsGroups($loggedInUser), + ), + $this->groupManager->getUserGroupIds($targetUser) + )); + $groups = array_map( + function (string $gid) { + $group = $this->groupManager->get($gid); + return [ + 'id' => $group->getGID(), + 'displayname' => $group->getDisplayName(), + 'usercount' => $group->count(), + 'disabled' => $group->countDisabled(), + 'canAdd' => $group->canAddUser(), + 'canRemove' => $group->canRemoveUser(), + ]; + }, + $gids, + ); + return new DataResponse([ + 'groups' => $groups, + ]); } /** @@ -1581,7 +1576,7 @@ public function addToGroup(string $userId, string $groupid = ''): DataResponse { public function removeFromGroup(string $userId, string $groupid): DataResponse { $loggedInUser = $this->userSession->getUser(); - if ($groupid === null || trim($groupid) === '') { + if (trim($groupid) === '') { throw new OCSException('', 101); } diff --git a/apps/provisioning_api/lib/Listener/UserDataCacheListener.php b/apps/provisioning_api/lib/Listener/UserDataCacheListener.php new file mode 100644 index 0000000000000..a7071eb9f26e2 --- /dev/null +++ b/apps/provisioning_api/lib/Listener/UserDataCacheListener.php @@ -0,0 +1,46 @@ + + */ +class UserDataCacheListener implements IEventListener { + + private ICache $cache; + + public function __construct(ICacheFactory $cacheFactory) { + $this->cache = $cacheFactory->createDistributed('provisioning_api'); + } + + public function handle(Event $event): void { + if ($event instanceof UserChangedEvent) { + $uid = $event->getUser()->getUID(); + } elseif ($event instanceof UserDeletedEvent) { + $uid = $event->getUser()->getUID(); + } elseif ($event instanceof PasswordUpdatedEvent) { + $uid = $event->getUser()->getUID(); + } else { + return; + } + + // Clear all cached variants for this user (admin, noadmin, scoped, etc.) + $this->cache->clear('user_data_' . $uid); + } +} diff --git a/apps/provisioning_api/tests/Controller/GroupsControllerTest.php b/apps/provisioning_api/tests/Controller/GroupsControllerTest.php index 129853d0c9bd2..848e33e0b868a 100644 --- a/apps/provisioning_api/tests/Controller/GroupsControllerTest.php +++ b/apps/provisioning_api/tests/Controller/GroupsControllerTest.php @@ -14,6 +14,8 @@ use OCP\AppFramework\OCS\OCSException; use OCP\Files\IRootFolder; use OCP\Group\ISubAdmin; +use OCP\ICache; +use OCP\ICacheFactory; use OCP\IConfig; use OCP\IGroup; use OCP\IRequest; @@ -38,6 +40,8 @@ class GroupsControllerTest extends \Test\TestCase { protected GroupsController&MockObject $api; private IRootFolder $rootFolder; + private ICache&MockObject $cache; + private ICacheFactory&MockObject $cacheFactory; protected function setUp(): void { @@ -53,6 +57,9 @@ protected function setUp(): void { $this->l10nFactory = $this->createMock(IFactory::class); $this->logger = $this->createMock(LoggerInterface::class); $this->rootFolder = $this->createMock(IRootFolder::class); + $this->cache = $this->createMock(ICache::class); + $this->cacheFactory = $this->createMock(ICacheFactory::class); + $this->cacheFactory->method('createDistributed')->willReturn($this->cache); $this->groupManager ->method('getSubAdmin') @@ -70,7 +77,8 @@ protected function setUp(): void { $this->subAdminManager, $this->l10nFactory, $this->rootFolder, - $this->logger + $this->logger, + $this->cacheFactory, ]) ->onlyMethods(['fillStorageInfo']) ->getMock(); diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index c135360b8f19e..5c36cc94632ca 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -26,6 +26,8 @@ use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\IRootFolder; use OCP\Group\ISubAdmin; +use OCP\ICache; +use OCP\ICacheFactory; use OCP\IConfig; use OCP\IGroup; use OCP\IL10N; @@ -65,6 +67,8 @@ class UsersControllerTest extends TestCase { private IRootFolder $rootFolder; private IPhoneNumberUtil $phoneNumberUtil; private IAppManager $appManager; + private ICache&MockObject $cache; + private ICacheFactory&MockObject $cacheFactory; protected function setUp(): void { parent::setUp(); @@ -87,6 +91,9 @@ protected function setUp(): void { $this->phoneNumberUtil = new PhoneNumberUtil(); $this->appManager = $this->createMock(IAppManager::class); $this->rootFolder = $this->createMock(IRootFolder::class); + $this->cache = $this->createMock(ICache::class); + $this->cacheFactory = $this->createMock(ICacheFactory::class); + $this->cacheFactory->method('createDistributed')->willReturn($this->cache); $l10n = $this->createMock(IL10N::class); $l10n->method('t')->willReturnCallback(fn (string $txt, array $replacement = []) => sprintf($txt, ...$replacement)); @@ -113,6 +120,7 @@ protected function setUp(): void { $this->eventDispatcher, $this->phoneNumberUtil, $this->appManager, + $this->cacheFactory, ]) ->onlyMethods(['fillStorageInfo']) ->getMock(); @@ -502,6 +510,7 @@ public function testAddUserSuccessfulWithDisplayName(): void { $this->eventDispatcher, $this->phoneNumberUtil, $this->appManager, + $this->cacheFactory, ]) ->onlyMethods(['editUser']) ->getMock(); @@ -3842,6 +3851,7 @@ public function testGetCurrentUserLoggedIn(): void { $this->eventDispatcher, $this->phoneNumberUtil, $this->appManager, + $this->cacheFactory, ]) ->onlyMethods(['getUserData']) ->getMock(); @@ -3936,6 +3946,7 @@ public function testGetUser(): void { $this->eventDispatcher, $this->phoneNumberUtil, $this->appManager, + $this->cacheFactory, ]) ->onlyMethods(['getUserData']) ->getMock(); @@ -4383,6 +4394,137 @@ public function testGetEditableFields(bool $allowedToChangeDisplayName, bool $al $this->assertEquals($expectedResp, $this->api->getEditableFields('userId')); } + public function testGetUserDataReturnsCachedDataOnCacheHit(): void { + $loggedInUser = $this->createMock(IUser::class); + $loggedInUser->method('getUID')->willReturn('admin'); + $this->userSession->method('getUser')->willReturn($loggedInUser); + $this->groupManager->method('isAdmin')->with('admin')->willReturn(true); + $this->groupManager->method('isDelegatedAdmin')->with('admin')->willReturn(false); + + $cachedData = ['id' => 'UID', 'displayname' => 'Test User', 'email' => 'test@example.com']; + $this->cache + ->method('get') + ->with('user_data_UID_admin') + ->willReturn($cachedData); + + // With a cache hit the database must not be consulted at all + $this->userManager->expects($this->never())->method('get'); + $this->accountManager->expects($this->never())->method('getAccount'); + + $result = $this->api->getUser('UID'); + $this->assertSame($cachedData, $result->getData()); + } + + public function testGetUserDataPopulatesCacheOnMiss(): void { + $loggedInUser = $this->createMock(IUser::class); + $loggedInUser->method('getUID')->willReturn('admin'); + $this->userSession->method('getUser')->willReturn($loggedInUser); + $this->groupManager->method('isAdmin')->with('admin')->willReturn(true); + $this->groupManager->method('isDelegatedAdmin')->with('admin')->willReturn(false); + + // Cache miss + $this->cache->method('get')->willReturn(null); + + $targetUser = $this->createMock(IUser::class); + $targetUser->method('getUID')->willReturn('UID'); + $targetUser->method('getSystemEMailAddress')->willReturn('test@example.com'); + $targetUser->method('getPrimaryEMailAddress')->willReturn('test@example.com'); + $targetUser->method('getDisplayName')->willReturn('Test User'); + $targetUser->method('getLastLogin')->willReturn(0); + $targetUser->method('getFirstLogin')->willReturn(0); + $targetUser->method('getBackendClassName')->willReturn('Database'); + $targetUser->method('getBackend')->willReturn($this->createMock(\OCP\UserInterface::class)); + $targetUser->method('getHome')->willReturn('/home/UID'); + $targetUser->method('getManagerUids')->willReturn([]); + $targetUser->method('getQuota')->willReturn('none'); + $this->userManager->method('get')->with('UID')->willReturn($targetUser); + + $subAdminManager = $this->createMock(\OC\SubAdmin::class); + $subAdminManager->method('getSubAdminsGroups')->willReturn([]); + $this->groupManager->method('getSubAdmin')->willReturn($subAdminManager); + $this->groupManager->method('getUserGroups')->willReturn([]); + + $this->mockAccount($targetUser, [ + IAccountManager::PROPERTY_ADDRESS => ['value' => ''], + IAccountManager::PROPERTY_PHONE => ['value' => ''], + IAccountManager::PROPERTY_TWITTER => ['value' => ''], + IAccountManager::PROPERTY_BLUESKY => ['value' => ''], + IAccountManager::PROPERTY_FEDIVERSE => ['value' => ''], + IAccountManager::PROPERTY_WEBSITE => ['value' => ''], + IAccountManager::PROPERTY_ORGANISATION => ['value' => ''], + IAccountManager::PROPERTY_ROLE => ['value' => ''], + IAccountManager::PROPERTY_HEADLINE => ['value' => ''], + IAccountManager::PROPERTY_BIOGRAPHY => ['value' => ''], + IAccountManager::PROPERTY_PROFILE_ENABLED => ['value' => '0'], + IAccountManager::PROPERTY_PRONOUNS => ['value' => ''], + ]); + $emailCollection = $this->createMock(\OCP\Accounts\IAccountPropertyCollection::class); + $emailCollection->method('getProperties')->willReturn([]); + $account = $this->accountManager->getAccount($targetUser); + $this->accountManager->method('getAccount')->with($targetUser)->willReturnCallback(function () use ($targetUser, $emailCollection) { + $account = $this->createMock(\OCP\Accounts\IAccount::class); + $account->method('getPropertyCollection')->willReturn($emailCollection); + $account->method('getProperty')->willReturnCallback(function (string $name) { + $prop = $this->createMock(IAccountProperty::class); + $prop->method('getValue')->willReturn(''); + $prop->method('getScope')->willReturn(''); + return $prop; + }); + return $account; + }); + $this->config->method('getUserValue')->willReturn('true'); + $this->l10nFactory->method('getUserLanguage')->willReturn('en'); + $this->api->method('fillStorageInfo')->willReturn(['used' => 0, 'quota' => -3, 'free' => -3, 'total' => -3, 'relative' => 0]); + + // The computed data must be stored in the cache with the 5-minute TTL + $this->cache + ->expects($this->once()) + ->method('set') + ->with('user_data_UID_admin', $this->isArray(), 300); + + $this->api->getUser('UID'); + } + + public function testGetUserDataUsesAdminCacheKey(): void { + $loggedInUser = $this->createMock(IUser::class); + $loggedInUser->method('getUID')->willReturn('admin'); + $this->userSession->method('getUser')->willReturn($loggedInUser); + $this->groupManager->method('isAdmin')->with('admin')->willReturn(true); + $this->groupManager->method('isDelegatedAdmin')->with('admin')->willReturn(false); + + $this->cache + ->expects($this->once()) + ->method('get') + ->with($this->stringContains('_admin')) + ->willReturn(['id' => 'UID']); + + $this->api->getUser('UID'); + } + + public function testGetUserDataUsesNonAdminCacheKey(): void { + $loggedInUser = $this->createMock(IUser::class); + $loggedInUser->method('getUID')->willReturn('subadmin'); + $this->userSession->method('getUser')->willReturn($loggedInUser); + $this->groupManager->method('isAdmin')->with('subadmin')->willReturn(false); + $this->groupManager->method('isDelegatedAdmin')->with('subadmin')->willReturn(false); + + $targetUser = $this->createMock(IUser::class); + $targetUser->method('getUID')->willReturn('UID'); + $this->userManager->method('get')->with('UID')->willReturn($targetUser); + + $subAdminManager = $this->createMock(\OC\SubAdmin::class); + $subAdminManager->method('isUserAccessible')->with($loggedInUser, $targetUser)->willReturn(true); + $this->groupManager->method('getSubAdmin')->willReturn($subAdminManager); + + $this->cache + ->expects($this->once()) + ->method('get') + ->with($this->stringContains('_noadmin')) + ->willReturn(['id' => 'UID']); + + $this->api->getUser('UID'); + } + private function mockAccount($targetUser, $accountProperties) { $mockedProperties = []; diff --git a/apps/provisioning_api/tests/Listener/UserDataCacheListenerTest.php b/apps/provisioning_api/tests/Listener/UserDataCacheListenerTest.php new file mode 100644 index 0000000000000..2a870c56212f8 --- /dev/null +++ b/apps/provisioning_api/tests/Listener/UserDataCacheListenerTest.php @@ -0,0 +1,87 @@ +cache = $this->createMock(ICache::class); + $cacheFactory = $this->createMock(ICacheFactory::class); + $cacheFactory->method('createDistributed') + ->with('provisioning_api') + ->willReturn($this->cache); + + $this->listener = new UserDataCacheListener($cacheFactory); + } + + private function makeUser(string $uid): IUser&MockObject { + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn($uid); + return $user; + } + + public function testHandleUserChangedEventClearsCache(): void { + $user = $this->makeUser('alice'); + $event = new UserChangedEvent($user, 'displayName', 'New Name', 'Old Name'); + + $this->cache + ->expects($this->once()) + ->method('clear') + ->with('user_data_alice'); + + $this->listener->handle($event); + } + + public function testHandleUserDeletedEventClearsCache(): void { + $user = $this->makeUser('bob'); + $event = new UserDeletedEvent($user); + + $this->cache + ->expects($this->once()) + ->method('clear') + ->with('user_data_bob'); + + $this->listener->handle($event); + } + + public function testHandlePasswordUpdatedEventClearsCache(): void { + $user = $this->makeUser('carol'); + $event = new PasswordUpdatedEvent($user, 'newpassword'); + + $this->cache + ->expects($this->once()) + ->method('clear') + ->with('user_data_carol'); + + $this->listener->handle($event); + } + + public function testHandleUnrelatedEventDoesNothing(): void { + $this->cache->expects($this->never())->method('clear'); + $this->cache->expects($this->never())->method('remove'); + + $this->listener->handle(new Event()); + } +} diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index fc0e97dcaba9e..a0d838ee37452 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -2074,10 +2074,6 @@ - - - -