Skip to content

Commit

Permalink
fix: Add missing null checks
Browse files Browse the repository at this point in the history
Signed-off-by: provokateurin <[email protected]>
  • Loading branch information
provokateurin committed Sep 20, 2024
1 parent de0e4b5 commit 2e3acb5
Show file tree
Hide file tree
Showing 13 changed files with 139 additions and 58 deletions.
9 changes: 8 additions & 1 deletion lib/Command/FolderCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -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('<error>storage id not found ' . $rootStorageId . '</error>');
return null;
}


$folder = $this->folderManager->getFolder($folderId, $rootStorageId);
if ($folder === null) {
$output->writeln('<error>Folder not found: ' . $folderId . '</error>');
return null;
Expand Down
5 changes: 5 additions & 0 deletions lib/Command/ListCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int
}

$rootStorageId = $this->rootFolder->getMountPoint()->getNumericStorageId();
if ($rootStorageId === null) {
$output->writeln('<error>storage id not found ' . $rootStorageId . '</error>');
return 1;
}

if ($userId) {
$user = $this->userManager->get($userId);
if (!$user) {
Expand Down
6 changes: 3 additions & 3 deletions lib/Command/Trashbin/Cleanup.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
10 changes: 6 additions & 4 deletions lib/Controller/DelegationController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
40 changes: 36 additions & 4 deletions lib/Controller/FolderController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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']) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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();
}
}

Expand Down Expand Up @@ -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();
}
Expand All @@ -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);

Expand Down Expand Up @@ -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);
Expand Down
12 changes: 12 additions & 0 deletions lib/DAV/ACLPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
}

Expand All @@ -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);
}

Expand Down Expand Up @@ -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;
Expand Down
22 changes: 19 additions & 3 deletions lib/DAV/GroupFoldersHome.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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 {
Expand All @@ -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);
}

Expand Down
1 change: 1 addition & 0 deletions lib/DAV/PropFindPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
62 changes: 22 additions & 40 deletions lib/Folder/FolderManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
*/
Expand Down
2 changes: 1 addition & 1 deletion lib/Mount/GroupMountPoint.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

class GroupMountPoint extends MountPoint implements ISystemMountPoint {
/**
* @param ?IStorage $storage
* @param IStorage $storage
*/
public function __construct(
private int $folderId,
Expand Down
14 changes: 12 additions & 2 deletions lib/Service/DelegationService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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) {
Expand Down
3 changes: 3 additions & 0 deletions lib/Service/FoldersFilter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
11 changes: 11 additions & 0 deletions psalm.xml
Original file line number Diff line number Diff line change
Expand Up @@ -134,5 +134,16 @@
<DeprecatedMethod errorLevel="error"/>
<DeprecatedProperty errorLevel="error"/>
<DeprecatedTrait errorLevel="error"/>
<PossiblyNullArgument errorLevel="error"/>
<PossiblyNullArrayAccess errorLevel="error"/>
<PossiblyNullArrayAssignment errorLevel="error"/>
<PossiblyNullArrayOffset errorLevel="error"/>
<PossiblyNullFunctionCall errorLevel="error"/>
<PossiblyNullIterator errorLevel="error"/>
<PossiblyNullOperand errorLevel="error"/>
<PossiblyNullPropertyAssignment errorLevel="error"/>
<PossiblyNullPropertyAssignmentValue errorLevel="error"/>
<PossiblyNullPropertyFetch errorLevel="error"/>
<PossiblyNullReference errorLevel="error"/>
</issueHandlers>
</psalm>

0 comments on commit 2e3acb5

Please sign in to comment.