From 68ec18323d07e7293fd59c82d51300a6d9d68176 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 2 Mar 2021 19:34:20 +0100 Subject: [PATCH] Fix types in the Group Manager Psalm found an issue. However the issue found was because of lying docblocks. Fixed those and did some typing to make it all better. For #25839 Signed-off-by: Roeland Jago Douma --- .../lib/Service/UserGlobalStoragesService.php | 2 +- .../AppFramework/DependencyInjection/DIContainer.php | 4 +++- lib/private/Group/Manager.php | 10 +++++----- lib/private/Template/JSConfigHelper.php | 4 ++-- lib/public/IGroupManager.php | 4 ++-- tests/lib/Group/ManagerTest.php | 3 +++ 6 files changed, 16 insertions(+), 11 deletions(-) diff --git a/apps/files_external/lib/Service/UserGlobalStoragesService.php b/apps/files_external/lib/Service/UserGlobalStoragesService.php index b8ea137428..fc284f951b 100644 --- a/apps/files_external/lib/Service/UserGlobalStoragesService.php +++ b/apps/files_external/lib/Service/UserGlobalStoragesService.php @@ -76,7 +76,7 @@ class UserGlobalStoragesService extends GlobalStoragesService { $userMounts = $this->dbConfig->getAdminMountsFor(DBConfigService::APPLICABLE_TYPE_USER, $this->getUser()->getUID()); $globalMounts = $this->dbConfig->getAdminMountsFor(DBConfigService::APPLICABLE_TYPE_GLOBAL, null); $groups = $this->groupManager->getUserGroupIds($this->getUser()); - if (is_array($groups) && count($groups) !== 0) { + if (count($groups) !== 0) { $groupMounts = $this->dbConfig->getAdminMountsForMultiple(DBConfigService::APPLICABLE_TYPE_GROUP, $groups); } else { $groupMounts = []; diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php index 3ef816f503..7395be703d 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -234,6 +234,8 @@ class DIContainer extends SimpleContainer implements IAppContainer { ) ); + + $securityMiddleware = new SecurityMiddleware( $c->get(IRequest::class), $c->get(IControllerMethodReflector::class), @@ -242,7 +244,7 @@ class DIContainer extends SimpleContainer implements IAppContainer { $server->query(ILogger::class), $c->get('AppName'), $server->getUserSession()->isLoggedIn(), - $server->getGroupManager()->isAdmin($this->getUserId()), + $this->getUserId() !== null && $server->getGroupManager()->isAdmin($this->getUserId()), $server->getUserSession()->getUser() !== null && $server->query(ISubAdmin::class)->isSubAdmin($server->getUserSession()->getUser()), $server->getAppManager(), $server->getL10N('lib') diff --git a/lib/private/Group/Manager.php b/lib/private/Group/Manager.php index 4c731a9f48..145b363120 100644 --- a/lib/private/Group/Manager.php +++ b/lib/private/Group/Manager.php @@ -276,7 +276,7 @@ class Manager extends PublicEmitter implements IGroupManager { * @param string $uid the user id * @return \OC\Group\Group[] */ - public function getUserIdGroups($uid) { + public function getUserIdGroups(string $uid): array { $groups = []; foreach ($this->getUserIdGroupIds($uid) as $groupId) { @@ -321,17 +321,17 @@ class Manager extends PublicEmitter implements IGroupManager { * get a list of group ids for a user * * @param IUser $user - * @return array with group ids + * @return string[] with group ids */ - public function getUserGroupIds(IUser $user) { + public function getUserGroupIds(IUser $user): array { return $this->getUserIdGroupIds($user->getUID()); } /** * @param string $uid the user id - * @return GroupInterface[] + * @return string[] */ - private function getUserIdGroupIds($uid) { + private function getUserIdGroupIds(string $uid): array { if (!isset($this->cachedUserGroups[$uid])) { $groups = []; foreach ($this->backends as $backend) { diff --git a/lib/private/Template/JSConfigHelper.php b/lib/private/Template/JSConfigHelper.php index b228cae6ff..cd46657560 100644 --- a/lib/private/Template/JSConfigHelper.php +++ b/lib/private/Template/JSConfigHelper.php @@ -167,7 +167,7 @@ class JSConfigHelper { $countOfDataLocation = 0; $dataLocation = str_replace(\OC::$SERVERROOT . '/', '', $this->config->getSystemValue('datadirectory', ''), $countOfDataLocation); - if ($countOfDataLocation !== 1 || !$this->groupManager->isAdmin($uid)) { + if ($countOfDataLocation !== 1 || $uid === null || !$this->groupManager->isAdmin($uid)) { $dataLocation = false; } @@ -198,7 +198,7 @@ class JSConfigHelper { $array = [ "_oc_debug" => $this->config->getSystemValue('debug', false) ? 'true' : 'false', - "_oc_isadmin" => $this->groupManager->isAdmin($uid) ? 'true' : 'false', + "_oc_isadmin" => $uid !== null && $this->groupManager->isAdmin($uid) ? 'true' : 'false', "backendAllowsPasswordConfirmation" => $userBackendAllowsPasswordConfirmation ? 'true' : 'false', "oc_dataURL" => is_string($dataLocation) ? "\"" . $dataLocation . "\"" : 'false', "_oc_webroot" => "\"" . \OC::$WEBROOT . "\"", diff --git a/lib/public/IGroupManager.php b/lib/public/IGroupManager.php index f8f0a5c47c..eec36f3de7 100644 --- a/lib/public/IGroupManager.php +++ b/lib/public/IGroupManager.php @@ -112,10 +112,10 @@ interface IGroupManager { /** * @param \OCP\IUser $user - * @return array with group names + * @return string[] with group names * @since 8.0.0 */ - public function getUserGroupIds(IUser $user); + public function getUserGroupIds(IUser $user): array; /** * get a list of all display names in a group diff --git a/tests/lib/Group/ManagerTest.php b/tests/lib/Group/ManagerTest.php index 8b6be03615..dfe21b825f 100644 --- a/tests/lib/Group/ManagerTest.php +++ b/tests/lib/Group/ManagerTest.php @@ -395,6 +395,7 @@ class ManagerTest extends TestCase { */ $backend = $this->getTestBackend(); $backend->method('getUserGroups') + ->with('myUID') ->willReturn(['123', 'abc']); $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger); @@ -402,6 +403,8 @@ class ManagerTest extends TestCase { /** @var \OC\User\User|\PHPUnit\Framework\MockObject\MockObject $user */ $user = $this->createMock(IUser::class); + $user->method('getUID') + ->willReturn('myUID'); $groups = $manager->getUserGroupIds($user); $this->assertCount(2, $groups);