From b7ff9b078048cac6919b6d50ff3d5202c3f6ca58 Mon Sep 17 00:00:00 2001 From: provokateurin Date: Thu, 19 Sep 2024 23:48:50 +0200 Subject: [PATCH] fix: Add missing null checks Signed-off-by: provokateurin --- lib/Command/FolderCommand.php | 9 +++- lib/Command/ListCommand.php | 5 ++ lib/Command/Trashbin/Cleanup.php | 6 +-- lib/Controller/DelegationController.php | 10 ++-- lib/Controller/FolderController.php | 40 ++++++++++++++-- lib/DAV/ACLPlugin.php | 12 +++++ lib/DAV/GroupFoldersHome.php | 22 +++++++-- lib/DAV/PropFindPlugin.php | 1 + lib/Folder/FolderManager.php | 62 +++++++++---------------- lib/Mount/GroupMountPoint.php | 2 +- lib/Service/DelegationService.php | 14 +++++- lib/Service/FoldersFilter.php | 3 ++ psalm.xml | 11 +++++ 13 files changed, 139 insertions(+), 58 deletions(-) diff --git a/lib/Command/FolderCommand.php b/lib/Command/FolderCommand.php index 8ebb8a3fa..05cde733d 100644 --- a/lib/Command/FolderCommand.php +++ b/lib/Command/FolderCommand.php @@ -40,7 +40,14 @@ protected function getFolder(InputInterface $input, OutputInterface $output): ?a return null; } - $folder = $this->folderManager->getFolder($folderId, $this->rootFolder->getMountPoint()->getNumericStorageId()); + $rootStorageId = $this->rootFolder->getMountPoint()->getNumericStorageId(); + if ($rootStorageId === null) { + $output->writeln('Root storage id not found'); + return null; + } + + + $folder = $this->folderManager->getFolder($folderId, $rootStorageId); if ($folder === null) { $output->writeln('Folder not found: ' . $folderId . ''); return null; diff --git a/lib/Command/ListCommand.php b/lib/Command/ListCommand.php index 8381e3142..e1473e21b 100644 --- a/lib/Command/ListCommand.php +++ b/lib/Command/ListCommand.php @@ -55,6 +55,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int } $rootStorageId = $this->rootFolder->getMountPoint()->getNumericStorageId(); + if ($rootStorageId === null) { + $output->writeln('Root storage id not found'); + return 1; + } + if ($userId) { $user = $this->userManager->get($userId); if (!$user) { diff --git a/lib/Command/Trashbin/Cleanup.php b/lib/Command/Trashbin/Cleanup.php index b6340c907..71e71f3f3 100644 --- a/lib/Command/Trashbin/Cleanup.php +++ b/lib/Command/Trashbin/Cleanup.php @@ -21,13 +21,13 @@ class Cleanup extends Base { private ?TrashBackend $trashBackend = null; - private ?FolderManager $folderManager = null; - public function __construct(FolderManager $folderManager) { + public function __construct( + private FolderManager $folderManager, + ) { parent::__construct(); if (Server::get(IAppManager::class)->isEnabledForUser('files_trashbin')) { $this->trashBackend = \OCP\Server::get(TrashBackend::class); - $this->folderManager = $folderManager; } } diff --git a/lib/Controller/DelegationController.php b/lib/Controller/DelegationController.php index 642daeb5f..ead39573e 100644 --- a/lib/Controller/DelegationController.php +++ b/lib/Controller/DelegationController.php @@ -111,10 +111,12 @@ public function getAuthorizedGroups(string $classname = ''): DataResponse { foreach ($authorizedGroups as $authorizedGroup) { $group = $this->groupManager->get($authorizedGroup->getGroupId()); - $data[] = [ - 'gid' => $group->getGID(), - 'displayName' => $group->getDisplayName(), - ]; + if ($group !== null) { + $data[] = [ + 'gid' => $group->getGID(), + 'displayName' => $group->getDisplayName(), + ]; + } } return new DataResponse($data); diff --git a/lib/Controller/FolderController.php b/lib/Controller/FolderController.php index 3139705f2..8e3d5b648 100644 --- a/lib/Controller/FolderController.php +++ b/lib/Controller/FolderController.php @@ -17,6 +17,7 @@ use OCP\AppFramework\Http\Attribute\ApiRoute; use OCP\AppFramework\Http\Attribute\NoAdminRequired; use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\OCS\OCSForbiddenException; use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\AppFramework\OCSController; use OCP\Files\IRootFolder; @@ -49,6 +50,10 @@ public function __construct( * Regular users can access their own folders, but they only get to see the permission for their own groups */ private function filterNonAdminFolder(array $folder): ?array { + if ($this->user === null) { + return null; + } + $userGroups = $this->groupManager->getUserGroupIds($this->user); $folder['groups'] = array_filter($folder['groups'], fn (string $group): bool => in_array($group, $userGroups), ARRAY_FILTER_USE_KEY); if ($folder['groups']) { @@ -73,7 +78,12 @@ private function formatFolder(array $folder): array { #[NoAdminRequired] #[ApiRoute(verb: 'GET', url: '/folders')] public function getFolders(bool $applicable = false): DataResponse { - $folders = $this->manager->getAllFoldersWithSize($this->getRootFolderStorageId()); + $storageId = $this->getRootFolderStorageId(); + if ($storageId === null) { + throw new OCSNotFoundException(); + } + + $folders = $this->manager->getAllFoldersWithSize($storageId); $folders = array_map($this->formatFolder(...), $folders); $isAdmin = $this->delegationService->isAdminNextcloud() || $this->delegationService->isDelegatedAdmin(); if ($isAdmin && !$applicable) { @@ -101,11 +111,19 @@ public function getFolder(int $id): DataResponse { } $storageId = $this->getRootFolderStorageId(); + if ($storageId === null) { + throw new OCSNotFoundException(); + } + $folder = $this->manager->getFolder($id, $storageId); + if ($folder === null) { + throw new OCSNotFoundException(); + } + if (!$this->delegationService->hasApiAccess()) { $folder = $this->filterNonAdminFolder($folder); - if (!$folder) { - return new DataResponse([], Http::STATUS_NOT_FOUND); + if ($folder === null) { + throw new OCSNotFoundException(); } } @@ -137,8 +155,14 @@ private function getRootFolderStorageId(): ?int { #[NoAdminRequired] #[ApiRoute(verb: 'POST', url: '/folders')] public function addFolder(string $mountpoint): DataResponse { + + $storageId = $this->rootFolder->getMountPoint()->getNumericStorageId(); + if ($storageId === null) { + throw new OCSNotFoundException(); + } + $id = $this->manager->createFolder(trim($mountpoint)); - $folder = $this->manager->getFolder($id, $this->rootFolder->getMountPoint()->getNumericStorageId()); + $folder = $this->manager->getFolder($id, $storageId); if ($folder === null) { throw new OCSNotFoundException(); } @@ -156,6 +180,10 @@ public function removeFolder(int $id): DataResponse { } $folder = $this->mountProvider->getFolder($id); + if ($folder === null) { + throw new OCSNotFoundException(); + } + $folder->delete(); $this->manager->removeFolder($id); @@ -312,6 +340,10 @@ public function aclMappingSearch(int $id, ?int $fileId, string $search = ''): Da $users = []; $groups = []; + if ($this->user === null) { + throw new OCSForbiddenException(); + } + if ($this->manager->canManageACL($id, $this->user) === true) { $groups = $this->manager->searchGroups($id, $search); $users = $this->manager->searchUsers($id, $search); diff --git a/lib/DAV/ACLPlugin.php b/lib/DAV/ACLPlugin.php index 95479173b..c5afd9051 100644 --- a/lib/DAV/ACLPlugin.php +++ b/lib/DAV/ACLPlugin.php @@ -98,6 +98,10 @@ public function propFind(PropFind $propFind, INode $node): void { if ($this->isAdmin($fileInfo->getPath())) { $rules = $this->ruleManager->getAllRulesForPaths($mount->getNumericStorageId(), [$path]); } else { + if ($this->user === null) { + return []; + } + $rules = $this->ruleManager->getRulesForFilesByPath($this->user, $mount->getNumericStorageId(), [$path]); } @@ -110,6 +114,10 @@ public function propFind(PropFind $propFind, INode $node): void { if ($this->isAdmin($fileInfo->getPath())) { $rulesByPath = $this->ruleManager->getAllRulesForPaths($mount->getNumericStorageId(), $parentPaths); } else { + if ($this->user === null) { + return []; + } + $rulesByPath = $this->ruleManager->getRulesForFilesByPath($this->user, $mount->getNumericStorageId(), $parentPaths); } @@ -156,6 +164,10 @@ public function propFind(PropFind $propFind, INode $node): void { } public function propPatch(string $path, PropPatch $propPatch): void { + if ($this->server === null) { + return; + } + $node = $this->server->tree->getNodeForPath($path); if (!$node instanceof Node) { return; diff --git a/lib/DAV/GroupFoldersHome.php b/lib/DAV/GroupFoldersHome.php index fde1e7cdc..d7c894d8a 100644 --- a/lib/DAV/GroupFoldersHome.php +++ b/lib/DAV/GroupFoldersHome.php @@ -13,6 +13,7 @@ use OCP\Files\Cache\ICacheEntry; use OCP\Files\IRootFolder; use OCP\IUser; +use RuntimeException; use Sabre\DAV\Exception\Forbidden; use Sabre\DAV\Exception\NotFound; use Sabre\DAV\ICollection; @@ -51,7 +52,12 @@ public function createDirectory($name): never { * @return array{folder_id: int, mount_point: string, permissions: int, quota: int, acl: bool, rootCacheEntry: ?ICacheEntry}|null */ private function getFolder(string $name): ?array { - $folders = $this->folderManager->getFoldersForUser($this->user, $this->rootFolder->getMountPoint()->getNumericStorageId()); + $storageId = $this->rootFolder->getMountPoint()->getNumericStorageId(); + if ($storageId === null) { + return null; + } + + $folders = $this->folderManager->getFoldersForUser($this->user, $storageId); foreach ($folders as $folder) { if (basename($folder['mount_point']) === $name) { return $folder; @@ -68,7 +74,12 @@ private function getDirectoryForFolder(array $folder): GroupFolderNode { $userHome = '/' . $this->user->getUID() . '/files'; $node = $this->rootFolder->get($userHome . '/' . $folder['mount_point']); - return new GroupFolderNode(Filesystem::getView(), $node, $folder['folder_id']); + $view = Filesystem::getView(); + if ($view === null) { + throw new RuntimeException('Unable to create view.'); + } + + return new GroupFolderNode($view, $node, $folder['folder_id']); } public function getChild($name): GroupFolderNode { @@ -84,7 +95,12 @@ public function getChild($name): GroupFolderNode { * @return GroupFolderNode[] */ public function getChildren(): array { - $folders = $this->folderManager->getFoldersForUser($this->user, $this->rootFolder->getMountPoint()->getNumericStorageId()); + $storageId = $this->rootFolder->getMountPoint()->getNumericStorageId(); + if ($storageId === null) { + return []; + } + + $folders = $this->folderManager->getFoldersForUser($this->user, $storageId); return array_map($this->getDirectoryForFolder(...), $folders); } diff --git a/lib/DAV/PropFindPlugin.php b/lib/DAV/PropFindPlugin.php index 5556a57de..630a848ac 100644 --- a/lib/DAV/PropFindPlugin.php +++ b/lib/DAV/PropFindPlugin.php @@ -48,6 +48,7 @@ public function propFind(PropFind $propFind, INode $node): void { if ($node instanceof GroupFolderNode) { $propFind->handle( self::MOUNT_POINT_PROPERTYNAME, + /** @psalm-suppress PossiblyNullReference Null already checked above */ fn () => $this->userFolder->getRelativePath($node->getFileInfo()->getMountPoint()->getMountPoint()) ); $propFind->handle( diff --git a/lib/Folder/FolderManager.php b/lib/Folder/FolderManager.php index 6213efaea..8eb0f9d98 100644 --- a/lib/Folder/FolderManager.php +++ b/lib/Folder/FolderManager.php @@ -10,7 +10,6 @@ use OC\Files\Cache\CacheEntry; use OC\Files\Node\Node; use OCA\Circles\CirclesManager; -use OCA\Circles\CirclesQueryHelper; use OCA\Circles\Exceptions\CircleNotFoundException; use OCA\Circles\Model\Probes\CircleProbe; use OCA\GroupFolders\Mount\GroupMountPoint; @@ -324,52 +323,35 @@ private function getAllApplicable(): array { $applicableMap[$id] = []; } - $entry = $this->generateApplicableMapEntry($row, $queryHelper, $entityId); - $applicableMap[$id][$entityId] = $entry; - } + if (!$row['circle_id']) { + $entityId = $row['group_id']; - return $applicableMap; - } - - - /** - * @param array $row the row from database - * @param string|null $entityId the type of the entity - * - * @return array{displayName: string, permissions: int, type: 'circle'|'group'} - */ - private function generateApplicableMapEntry( - array $row, - ?CirclesQueryHelper $queryHelper = null, - ?string &$entityId = null, - ): array { - if (!$row['circle_id']) { - $entityId = $row['group_id']; + $entry = [ + 'displayName' => $row['group_id'], + 'permissions' => (int)$row['permissions'], + 'type' => 'group' + ]; + } else { + $entityId = $row['circle_id']; + try { + $circle = $queryHelper?->extractCircle($row); + } catch (CircleNotFoundException) { + $circle = null; + } - return [ - 'displayName' => $row['group_id'], - 'permissions' => (int)$row['permissions'], - 'type' => 'group' - ]; - } + $entry = [ + 'displayName' => $circle?->getDisplayName() ?? $row['circle_id'], + 'permissions' => (int)$row['permissions'], + 'type' => 'circle' + ]; + } - $entityId = $row['circle_id']; - try { - $circle = $queryHelper?->extractCircle($row); - } catch (CircleNotFoundException) { - $circle = null; + $applicableMap[$id][$entityId] = $entry; } - $displayName = $circle?->getDisplayName() ?? $row['circle_id']; - - return [ - 'displayName' => $displayName, - 'permissions' => (int)$row['permissions'], - 'type' => 'circle' - ]; + return $applicableMap; } - /** * @throws Exception */ diff --git a/lib/Mount/GroupMountPoint.php b/lib/Mount/GroupMountPoint.php index aace3a17d..53c2e5c54 100644 --- a/lib/Mount/GroupMountPoint.php +++ b/lib/Mount/GroupMountPoint.php @@ -13,7 +13,7 @@ class GroupMountPoint extends MountPoint implements ISystemMountPoint { /** - * @param ?IStorage $storage + * @param IStorage $storage */ public function __construct( private int $folderId, diff --git a/lib/Service/DelegationService.php b/lib/Service/DelegationService.php index 2c7ace146..3d642aa62 100644 --- a/lib/Service/DelegationService.php +++ b/lib/Service/DelegationService.php @@ -33,7 +33,12 @@ public function __construct( } public function isAdminNextcloud(): bool { - return $this->groupManager->isAdmin($this->userSession->getUser()->getUID()); + $user = $this->userSession->getUser(); + if ($user === null) { + return false; + } + + return $this->groupManager->isAdmin($user->getUID()); } public function isDelegatedAdmin(): bool { @@ -60,8 +65,13 @@ public function hasOnlyApiAccess(): bool { } private function getAccessLevel(array $settingClasses): bool { + $user = $this->userSession->getUser(); + if ($user === null) { + return false; + } + $authorized = false; - $authorizedClasses = $this->groupAuthorizationMapper->findAllClassesForUser($this->userSession->getUser()); + $authorizedClasses = $this->groupAuthorizationMapper->findAllClassesForUser($user); foreach ($settingClasses as $settingClass) { $authorized = in_array($settingClass, $authorizedClasses, true); if ($authorized) { diff --git a/lib/Service/FoldersFilter.php b/lib/Service/FoldersFilter.php index 22324b36f..ee587942a 100644 --- a/lib/Service/FoldersFilter.php +++ b/lib/Service/FoldersFilter.php @@ -23,6 +23,9 @@ public function __construct( */ public function getForApiUser(array $folders): array { $user = $this->userSession->getUser(); + if ($user === null) { + return []; + } return array_filter($folders, function (array $folder) use ($user): bool { foreach ($folder['manage'] as $manager) { diff --git a/psalm.xml b/psalm.xml index 20f52ff37..8915fab3b 100644 --- a/psalm.xml +++ b/psalm.xml @@ -134,5 +134,16 @@ + + + + + + + + + + +