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 b8ea137428f44..fc284f951bd00 100644 --- a/apps/files_external/lib/Service/UserGlobalStoragesService.php +++ b/apps/files_external/lib/Service/UserGlobalStoragesService.php @@ -76,7 +76,7 @@ protected function readDBConfig() { $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 3ef816f503e41..7395be703d353 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -234,6 +234,8 @@ public function __construct($appName, $urlParams = [], ServerContainer $server = ) ); + + $securityMiddleware = new SecurityMiddleware( $c->get(IRequest::class), $c->get(IControllerMethodReflector::class), @@ -242,7 +244,7 @@ public function __construct($appName, $urlParams = [], ServerContainer $server = $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 4c731a9f48833..145b363120643 100644 --- a/lib/private/Group/Manager.php +++ b/lib/private/Group/Manager.php @@ -276,7 +276,7 @@ public function getUserGroups(IUser $user = null) { * @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 @@ public function isInGroup($userId, $group) { * 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 b228cae6ffb33..cd4665756010b 100644 --- a/lib/private/Template/JSConfigHelper.php +++ b/lib/private/Template/JSConfigHelper.php @@ -167,7 +167,7 @@ public function getConfig() { $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 @@ public function getConfig() { $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 f8f0a5c47cdc3..eec36f3de791b 100644 --- a/lib/public/IGroupManager.php +++ b/lib/public/IGroupManager.php @@ -112,10 +112,10 @@ public function getUserGroups(IUser $user = null); /** * @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 8b6be03615a8a..dfe21b825f7cb 100644 --- a/tests/lib/Group/ManagerTest.php +++ b/tests/lib/Group/ManagerTest.php @@ -395,6 +395,7 @@ public function testGetUserGroupIds() { */ $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 @@ public function testGetUserGroupIds() { /** @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);