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 <roeland@famdouma.nl>

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 b8ea137428f..fc284f951bd 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 3ef816f503e..7395be703d3 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 4c731a9f488..145b3631206 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 b228cae6ffb..cd466575601 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 f8f0a5c47cd..eec36f3de79 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 8b6be03615a..dfe21b825f7 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);