From c016c6854dd0159f7e797bf1ec754b04f7c1c334 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 16 Dec 2021 12:11:01 +0100 Subject: [PATCH 1/3] Check if folderId is an int Prevents accidentaly selecting the first groupfolder when for example typing the folder name of another groupfolder. Signed-off-by: Carl Schwan --- lib/Command/ACL.php | 5 +++++ lib/Command/Delete.php | 7 ++++++- lib/Command/Group.php | 5 +++++ lib/Command/Quota.php | 5 +++++ lib/Command/Rename.php | 5 +++++ 5 files changed, 26 insertions(+), 1 deletion(-) diff --git a/lib/Command/ACL.php b/lib/Command/ACL.php index 2bba8e298..520368f66 100644 --- a/lib/Command/ACL.php +++ b/lib/Command/ACL.php @@ -89,6 +89,11 @@ protected function configure() { protected function execute(InputInterface $input, OutputInterface $output) { $folderId = (int)$input->getArgument('folder_id'); + if ((string)$folderId !== $input->getArgument('folder_id')) { + // Protect against removing folderId === 0 when typing a string (e.g. folder name instead of folder id) + $output->writeln('Folder id argument is not an integer. Got ' . $input->getArgument('folder_id') . ''); + return; + } $folder = $this->folderManager->getFolder($folderId, $this->rootFolder->getMountPoint()->getNumericStorageId()); if ($folder) { if ($input->getOption('enable')) { diff --git a/lib/Command/Delete.php b/lib/Command/Delete.php index 0eb205529..896e986a0 100644 --- a/lib/Command/Delete.php +++ b/lib/Command/Delete.php @@ -54,10 +54,15 @@ protected function configure() { protected function execute(InputInterface $input, OutputInterface $output) { $folderId = (int)$input->getArgument('folder_id'); + if ((string)$folderId !== $input->getArgument('folder_id')) { + // Protect against removing folderId === 0 when typing a string (e.g. folder name instead of folder id) + $output->writeln('Folder id argument is not an integer. Got ' . $input->getArgument('folder_id') . ''); + return; + } $folder = $this->folderManager->getFolder($folderId, $this->rootFolder->getMountPoint()->getNumericStorageId()); if ($folder) { $helper = $this->getHelper('question'); - $question = new ConfirmationQuestion('Are you sure you want to delete the group folder' . $folder['mount_point'] . ' and all files within, this cannot be undone (y/N).', false); + $question = new ConfirmationQuestion('Are you sure you want to delete the group folder ' . $folder['mount_point'] . ' and all files within, this cannot be undone (y/N).', false); if ($input->getOption('force') || $helper->ask($input, $output, $question)) { $folder = $this->mountProvider->getFolder($folderId); $this->folderManager->removeFolder($folderId); diff --git a/lib/Command/Group.php b/lib/Command/Group.php index 93ae8e7d6..6f7d8f62a 100644 --- a/lib/Command/Group.php +++ b/lib/Command/Group.php @@ -65,6 +65,11 @@ protected function configure() { protected function execute(InputInterface $input, OutputInterface $output) { $folderId = (int)$input->getArgument('folder_id'); + if ((string)$folderId !== $input->getArgument('folder_id')) { + // Protect against removing folderId === 0 when typing a string (e.g. folder name instead of folder id) + $output->writeln('Folder id argument is not an integer. Got ' . $input->getArgument('folder_id') . ''); + return; + } $folder = $this->folderManager->getFolder($folderId, $this->rootFolder->getMountPoint()->getNumericStorageId()); if ($folder) { $groupString = $input->getArgument('group'); diff --git a/lib/Command/Quota.php b/lib/Command/Quota.php index 78ce3ff44..cf0102fa7 100644 --- a/lib/Command/Quota.php +++ b/lib/Command/Quota.php @@ -50,6 +50,11 @@ protected function configure() { protected function execute(InputInterface $input, OutputInterface $output) { $folderId = (int)$input->getArgument('folder_id'); + if ((string)$folderId !== $input->getArgument('folder_id')) { + // Protect against removing folderId === 0 when typing a string (e.g. folder name instead of folder id) + $output->writeln('Folder id argument is not an integer. Got ' . $input->getArgument('folder_id') . ''); + return; + } $folder = $this->folderManager->getFolder($folderId, $this->rootFolder->getMountPoint()->getNumericStorageId()); if ($folder) { $quotaString = strtolower($input->getArgument('quota')); diff --git a/lib/Command/Rename.php b/lib/Command/Rename.php index 69fa1a173..0ddfc6b03 100644 --- a/lib/Command/Rename.php +++ b/lib/Command/Rename.php @@ -49,6 +49,11 @@ protected function configure() { protected function execute(InputInterface $input, OutputInterface $output) { $folderId = (int)$input->getArgument('folder_id'); + if ((string)$folderId !== $input->getArgument('folder_id')) { + // Protect against removing folderId === 0 when typing a string (e.g. folder name instead of folder id) + $output->writeln('Folder id argument is not an integer. Got ' . $input->getArgument('folder_id') . ''); + return; + } $folder = $this->folderManager->getFolder($folderId, $this->rootFolder->getMountPoint()->getNumericStorageId()); if ($folder) { $this->folderManager->renameFolder($folderId, $input->getArgument('name')); From 17aff8dfc6a439d87c3d52720922d6b2b47df099 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 16 Dec 2021 13:01:46 +0100 Subject: [PATCH 2/3] More refactoring Signed-off-by: Carl Schwan --- lib/Command/ACL.php | 217 ++++++++++++++++------------------ lib/Command/Delete.php | 45 ++----- lib/Command/FolderCommand.php | 66 +++++++++++ lib/Command/Quota.php | 41 ++----- lib/Command/Rename.php | 25 +--- lib/Command/Scan.php | 86 ++++++-------- 6 files changed, 235 insertions(+), 245 deletions(-) create mode 100644 lib/Command/FolderCommand.php diff --git a/lib/Command/ACL.php b/lib/Command/ACL.php index 520368f66..bfa4d42f5 100644 --- a/lib/Command/ACL.php +++ b/lib/Command/ACL.php @@ -22,6 +22,7 @@ namespace OCA\GroupFolders\Command; use OC\Core\Command\Base; +use OCA\GroupFolders\Command\FolderCommand; use OCA\GroupFolders\ACL\ACLManagerFactory; use OCA\GroupFolders\ACL\Rule; use OCA\GroupFolders\ACL\RuleManager; @@ -37,7 +38,7 @@ use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; -class ACL extends Base { +class ACL extends FolderCommand { const PERMISSIONS_MAP = [ 'read' => Constants::PERMISSION_READ, 'write' => Constants::PERMISSION_UPDATE, @@ -46,10 +47,7 @@ class ACL extends Base { 'share' => Constants::PERMISSION_SHARE, ]; - private $folderManager; - private $rootFolder; private $ruleManager; - private $mountProvider; private $aclManagerFactory; private $userManager; @@ -61,11 +59,8 @@ public function __construct( ACLManagerFactory $aclManagerFactory, IUserManager $userManager ) { - parent::__construct(); - $this->folderManager = $folderManager; - $this->rootFolder = $rootFolder; + parent::__construct($folderManager, $rootFolder, $mountProvider); $this->ruleManager = $ruleManager; - $this->mountProvider = $mountProvider; $this->aclManagerFactory = $aclManagerFactory; $this->userManager = $userManager; } @@ -88,122 +83,114 @@ protected function configure() { } protected function execute(InputInterface $input, OutputInterface $output) { - $folderId = (int)$input->getArgument('folder_id'); - if ((string)$folderId !== $input->getArgument('folder_id')) { - // Protect against removing folderId === 0 when typing a string (e.g. folder name instead of folder id) - $output->writeln('Folder id argument is not an integer. Got ' . $input->getArgument('folder_id') . ''); - return; + $folder = $this->getFolder($input, $output); + if ($folder === false) { + return -1; } - $folder = $this->folderManager->getFolder($folderId, $this->rootFolder->getMountPoint()->getNumericStorageId()); - if ($folder) { - if ($input->getOption('enable')) { - $this->folderManager->setFolderACL($folderId, true); - } elseif ($input->getOption('disable')) { - $this->folderManager->setFolderACL($folderId, false); - } elseif ($input->getOption('test')) { - if ($input->getOption('user') && ($input->getArgument('path'))) { - $mappingId = $input->getOption('user'); - $user = $this->userManager->get($mappingId); - if (!$user) { - $output->writeln('User not found: ' . $mappingId . ''); - return -1; - } - $jailPath = $this->mountProvider->getJailPath((int)$folder['id']); - $path = $input->getArgument('path'); - $aclManager = $this->aclManagerFactory->getACLManager($user); - $permissions = $aclManager->getACLPermissionsForPath($jailPath . rtrim('/' . $path, '/')); - $permissionString = $this->formatRulePermissions(Constants::PERMISSION_ALL, $permissions); - $output->writeln($permissionString); - return; - } else { - $output->writeln('--user and options needs to be set for permissions testing'); - return -3; + if ($input->getOption('enable')) { + $this->folderManager->setFolderACL($folderId, true); + } elseif ($input->getOption('disable')) { + $this->folderManager->setFolderACL($folderId, false); + } elseif ($input->getOption('test')) { + if ($input->getOption('user') && ($input->getArgument('path'))) { + $mappingId = $input->getOption('user'); + $user = $this->userManager->get($mappingId); + if (!$user) { + $output->writeln('User not found: ' . $mappingId . ''); + return -1; } - } elseif (!$folder['acl']) { - $output->writeln('Advanced permissions not enabled for folder: ' . $folderId . ''); - return -2; - } elseif ( - !$input->getArgument('path') && - !$input->getArgument('permissions') && - !$input->getOption('user') && - !$input->getOption('group') - ) { - $this->printPermissions($input, $output, $folder); - } elseif ($input->getOption('manage-add') && ($input->getOption('user') || $input->getOption('group'))) { - $mappingType = $input->getOption('user') ? 'user' : 'group'; - $mappingId = $input->getOption('user') ? $input->getOption('user') : $input->getOption('group'); - $this->folderManager->setManageACL($folderId, $mappingType, $mappingId, true); - } elseif ($input->getOption('manage-remove') && ($input->getOption('user') || $input->getOption('group'))) { - $mappingType = $input->getOption('user') ? 'user' : 'group'; - $mappingId = $input->getOption('user') ? $input->getOption('user') : $input->getOption('group'); - $this->folderManager->setManageACL($folderId, $mappingType, $mappingId, false); - } elseif (!$input->getArgument('path')) { - $output->writeln(' argument has to be set when not using --enable or --disable'); - return -3; - } elseif (!$input->getArgument('permissions')) { - $output->writeln(' argument has to be set when not using --enable or --disable'); - return -3; - } elseif ($input->getOption('user') && $input->getOption('group')) { - $output->writeln('--user and --group can not be used at the same time'); - return -3; - } elseif (!$input->getOption('user') && !$input->getOption('group')) { - $output->writeln('either --user or --group has to be used when not using --enable or --disable'); - return -3; - } else { - $mappingType = $input->getOption('user') ? 'user' : 'group'; - $mappingId = $input->getOption('user') ? $input->getOption('user') : $input->getOption('group'); + $jailPath = $this->mountProvider->getJailPath((int)$folder['id']); $path = $input->getArgument('path'); - $path = trim($path, '/'); - $permissionStrings = $input->getArgument('permissions'); + $aclManager = $this->aclManagerFactory->getACLManager($user); + $permissions = $aclManager->getACLPermissionsForPath($jailPath . rtrim('/' . $path, '/')); + $permissionString = $this->formatRulePermissions(Constants::PERMISSION_ALL, $permissions); + $output->writeln($permissionString); + return; + } else { + $output->writeln('--user and options needs to be set for permissions testing'); + return -3; + } + } elseif (!$folder['acl']) { + $output->writeln('Advanced permissions not enabled for folder: ' . $folderId . ''); + return -2; + } elseif ( + !$input->getArgument('path') && + !$input->getArgument('permissions') && + !$input->getOption('user') && + !$input->getOption('group') + ) { + $this->printPermissions($input, $output, $folder); + } elseif ($input->getOption('manage-add') && ($input->getOption('user') || $input->getOption('group'))) { + $mappingType = $input->getOption('user') ? 'user' : 'group'; + $mappingId = $input->getOption('user') ? $input->getOption('user') : $input->getOption('group'); + $this->folderManager->setManageACL($folderId, $mappingType, $mappingId, true); + } elseif ($input->getOption('manage-remove') && ($input->getOption('user') || $input->getOption('group'))) { + $mappingType = $input->getOption('user') ? 'user' : 'group'; + $mappingId = $input->getOption('user') ? $input->getOption('user') : $input->getOption('group'); + $this->folderManager->setManageACL($folderId, $mappingType, $mappingId, false); + } elseif (!$input->getArgument('path')) { + $output->writeln(' argument has to be set when not using --enable or --disable'); + return -3; + } elseif (!$input->getArgument('permissions')) { + $output->writeln(' argument has to be set when not using --enable or --disable'); + return -3; + } elseif ($input->getOption('user') && $input->getOption('group')) { + $output->writeln('--user and --group can not be used at the same time'); + return -3; + } elseif (!$input->getOption('user') && !$input->getOption('group')) { + $output->writeln('either --user or --group has to be used when not using --enable or --disable'); + return -3; + } else { + $mappingType = $input->getOption('user') ? 'user' : 'group'; + $mappingId = $input->getOption('user') ? $input->getOption('user') : $input->getOption('group'); + $path = $input->getArgument('path'); + $path = trim($path, '/'); + $permissionStrings = $input->getArgument('permissions'); - $mount = $this->mountProvider->getMount( - $folder['id'], - '/dummy/files/' . $folder['mount_point'], - $folder['permissions'], - $folder['quota'], - $folder['rootCacheEntry'], - null, - $folder['acl'] - ); - $id = $mount->getStorage()->getCache()->getId($path); - if ($id === -1) { - $output->writeln('Path not found in folder: ' . $path . ''); - return -1; - } + $mount = $this->mountProvider->getMount( + $folder['id'], + '/dummy/files/' . $folder['mount_point'], + $folder['permissions'], + $folder['quota'], + $folder['rootCacheEntry'], + null, + $folder['acl'] + ); + $id = $mount->getStorage()->getCache()->getId($path); + if ($id === -1) { + $output->writeln('Path not found in folder: ' . $path . ''); + return -1; + } - if ($permissionStrings === ['clear']) { - $this->ruleManager->deleteRule(new Rule( - new UserMapping($mappingType, $mappingId), - $id, - 0, - 0 - )); - } else { - foreach ($permissionStrings as $permission) { - if ($permission[0] !== '+' && $permission[0] !== '-') { - $output->writeln('incorrect format for permissions "' . $permission . '"'); - return -3; - } - $name = substr($permission, 1); - if (!isset(self::PERMISSIONS_MAP[$name])) { - $output->writeln('incorrect format for permissions2 "' . $permission . '"'); - return -3; - } + if ($permissionStrings === ['clear']) { + $this->ruleManager->deleteRule(new Rule( + new UserMapping($mappingType, $mappingId), + $id, + 0, + 0 + )); + } else { + foreach ($permissionStrings as $permission) { + if ($permission[0] !== '+' && $permission[0] !== '-') { + $output->writeln('incorrect format for permissions "' . $permission . '"'); + return -3; } + $name = substr($permission, 1); + if (!isset(self::PERMISSIONS_MAP[$name])) { + $output->writeln('incorrect format for permissions2 "' . $permission . '"'); + return -3; + } + } - [$mask, $permissions] = $this->parsePermissions($permissionStrings); + [$mask, $permissions] = $this->parsePermissions($permissionStrings); - $this->ruleManager->saveRule(new Rule( - new UserMapping($mappingType, $mappingId), - $id, - $mask, - $permissions - )); - } + $this->ruleManager->saveRule(new Rule( + new UserMapping($mappingType, $mappingId), + $id, + $mask, + $permissions + )); } - } else { - $output->writeln('Folder not found: ' . $folderId . ''); - return -1; } return 0; } diff --git a/lib/Command/Delete.php b/lib/Command/Delete.php index 896e986a0..7e3db5977 100644 --- a/lib/Command/Delete.php +++ b/lib/Command/Delete.php @@ -22,27 +22,14 @@ namespace OCA\GroupFolders\Command; use OC\Core\Command\Base; -use OCA\GroupFolders\Folder\FolderManager; -use OCA\GroupFolders\Mount\MountProvider; -use OCP\Files\IRootFolder; +use OCA\GroupFolders\Command\FolderCommand; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Question\ConfirmationQuestion; -class Delete extends Base { - private $folderManager; - private $rootFolder; - private $mountProvider; - - public function __construct(FolderManager $folderManager, IRootFolder $rootFolder, MountProvider $mountProvider) { - parent::__construct(); - $this->folderManager = $folderManager; - $this->rootFolder = $rootFolder; - $this->mountProvider = $mountProvider; - } - +class FolderCommand extends Base { protected function configure() { $this ->setName('groupfolders:delete') @@ -53,25 +40,17 @@ protected function configure() { } protected function execute(InputInterface $input, OutputInterface $output) { - $folderId = (int)$input->getArgument('folder_id'); - if ((string)$folderId !== $input->getArgument('folder_id')) { - // Protect against removing folderId === 0 when typing a string (e.g. folder name instead of folder id) - $output->writeln('Folder id argument is not an integer. Got ' . $input->getArgument('folder_id') . ''); - return; - } - $folder = $this->folderManager->getFolder($folderId, $this->rootFolder->getMountPoint()->getNumericStorageId()); - if ($folder) { - $helper = $this->getHelper('question'); - $question = new ConfirmationQuestion('Are you sure you want to delete the group folder ' . $folder['mount_point'] . ' and all files within, this cannot be undone (y/N).', false); - if ($input->getOption('force') || $helper->ask($input, $output, $question)) { - $folder = $this->mountProvider->getFolder($folderId); - $this->folderManager->removeFolder($folderId); - $folder->delete(); - } - return 0; - } else { - $output->writeln('Folder not found: ' . $folderId . ''); + $folder = $this->getFolder($input, $output); + if ($folder === false) { return -1; } + $helper = $this->getHelper('question'); + $question = new ConfirmationQuestion('Are you sure you want to delete the group folder ' . $folder['mount_point'] . ' and all files within, this cannot be undone (y/N).', false); + if ($input->getOption('force') || $helper->ask($input, $output, $question)) { + $folder = $this->mountProvider->getFolder($folderId); + $this->folderManager->removeFolder($folderId); + $folder->delete(); + } + return 0; } } diff --git a/lib/Command/FolderCommand.php b/lib/Command/FolderCommand.php new file mode 100644 index 000000000..b47f8e46f --- /dev/null +++ b/lib/Command/FolderCommand.php @@ -0,0 +1,66 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\GroupFolders\Command; + +use OC\Core\Command\Base; +use OCA\GroupFolders\Folder\FolderManager; +use OCA\GroupFolders\Mount\MountProvider; +use OCP\Files\IRootFolder; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; +use Symfony\Component\Console\Output\OutputInterface; + +/** + * Base command for commands asking the user for a folder id. + */ +abstract class FolderCommand extends Base { + /** @var FolderManager */ + protected $folderManager; + /** @var IRootFolder */ + protected $rootFolder; + /** @var MountProvider */ + protected $mountProvider; + + public function __construct(FolderManager $folderManager, IRootFolder $rootFolder, MountProvider $mountProvider) { + parent::__construct(); + $this->folderManager = $folderManager; + $this->rootFolder = $rootFolder; + $this->mountProvider = $mountProvider; + } + + /** + * @psalm-return array{id: mixed, mount_point: mixed, groups: array|mixed, quota: int, size: int|mixed, acl: bool}|false + */ + protected function getFolder(InputInterface $input, OutputInterface $output) { + $folderId = (int)$input->getArgument('folder_id'); + if ((string)$folderId !== $input->getArgument('folder_id')) { + // Protect against removing folderId === 0 when typing a string (e.g. folder name instead of folder id) + $output->writeln('Folder id argument is not an integer. Got ' . $input->getArgument('folder_id') . ''); + return false; + } + $folder = $this->folderManager->getFolder($folderId, $this->rootFolder->getMountPoint()->getNumericStorageId()); + if ($folder === false) { + $output->writeln('Folder not found: ' . $folderId . ''); + return false; + } + } +} diff --git a/lib/Command/Quota.php b/lib/Command/Quota.php index cf0102fa7..11521c292 100644 --- a/lib/Command/Quota.php +++ b/lib/Command/Quota.php @@ -22,6 +22,7 @@ namespace OCA\GroupFolders\Command; use OC\Core\Command\Base; +use OCA\GroupFolders\Command\FolderCommand; use OCA\GroupFolders\Folder\FolderManager; use OCP\Files\FileInfo; use OCP\Files\IRootFolder; @@ -29,16 +30,7 @@ use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; -class Quota extends Base { - private $folderManager; - private $rootFolder; - - public function __construct(FolderManager $folderManager, IRootFolder $rootFolder) { - parent::__construct(); - $this->folderManager = $folderManager; - $this->rootFolder = $rootFolder; - } - +class Quota extends FolderCommand { protected function configure() { $this ->setName('groupfolders:quota') @@ -49,26 +41,17 @@ protected function configure() { } protected function execute(InputInterface $input, OutputInterface $output) { - $folderId = (int)$input->getArgument('folder_id'); - if ((string)$folderId !== $input->getArgument('folder_id')) { - // Protect against removing folderId === 0 when typing a string (e.g. folder name instead of folder id) - $output->writeln('Folder id argument is not an integer. Got ' . $input->getArgument('folder_id') . ''); - return; - } - $folder = $this->folderManager->getFolder($folderId, $this->rootFolder->getMountPoint()->getNumericStorageId()); - if ($folder) { - $quotaString = strtolower($input->getArgument('quota')); - $quota = ($quotaString === 'unlimited') ? FileInfo::SPACE_UNLIMITED : \OCP\Util::computerFileSize($quotaString); - if ($quota) { - $this->folderManager->setFolderQuota($folderId, $quota); - return 0; - } else { - $output->writeln('Unable to parse quota input: ' . $quotaString . ''); - return -1; - } - } else { - $output->writeln('Folder not found: ' . $folderId . ''); + $folder = $this->getFolder($input, $output); + if ($folder === false) { return -1; } + $quotaString = strtolower($input->getArgument('quota')); + $quota = ($quotaString === 'unlimited') ? FileInfo::SPACE_UNLIMITED : \OCP\Util::computerFileSize($quotaString); + if ($quota) { + $this->folderManager->setFolderQuota($folderId, $quota); + return 0; + } + $output->writeln('Unable to parse quota input: ' . $quotaString . ''); + return -1; } } diff --git a/lib/Command/Rename.php b/lib/Command/Rename.php index 0ddfc6b03..3ca895522 100644 --- a/lib/Command/Rename.php +++ b/lib/Command/Rename.php @@ -23,21 +23,16 @@ use OC\Core\Command\Base; use OCA\GroupFolders\Folder\FolderManager; +use OCA\GroupFolders\Command\FolderCommand; use OCP\Files\IRootFolder; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; -class Rename extends Base { +class Rename extends FolderCommand { private $folderManager; private $rootFolder; - public function __construct(FolderManager $folderManager, IRootFolder $rootFolder) { - parent::__construct(); - $this->folderManager = $folderManager; - $this->rootFolder = $rootFolder; - } - protected function configure() { $this ->setName('groupfolders:rename') @@ -48,19 +43,11 @@ protected function configure() { } protected function execute(InputInterface $input, OutputInterface $output) { - $folderId = (int)$input->getArgument('folder_id'); - if ((string)$folderId !== $input->getArgument('folder_id')) { - // Protect against removing folderId === 0 when typing a string (e.g. folder name instead of folder id) - $output->writeln('Folder id argument is not an integer. Got ' . $input->getArgument('folder_id') . ''); - return; - } - $folder = $this->folderManager->getFolder($folderId, $this->rootFolder->getMountPoint()->getNumericStorageId()); - if ($folder) { - $this->folderManager->renameFolder($folderId, $input->getArgument('name')); - return 0; - } else { - $output->writeln('Folder not found: ' . $folderId . ''); + $folder = $this->getFolder($input, $output); + if ($folder === false) { return -1; } + $this->folderManager->renameFolder($folderId, $input->getArgument('name')); + return 0; } } diff --git a/lib/Command/Scan.php b/lib/Command/Scan.php index 8fc1de070..c6d51f550 100644 --- a/lib/Command/Scan.php +++ b/lib/Command/Scan.php @@ -23,6 +23,7 @@ use OC\Core\Command\Base; use OC\Files\ObjectStore\NoopScanner; +use OCA\GroupFolders\Command\FolderCommand; use OCA\GroupFolders\Folder\FolderManager; use OCA\GroupFolders\Mount\MountProvider; use OCP\Constants; @@ -32,20 +33,10 @@ use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; -class Scan extends Base { - private $folderManager; - private $rootFolder; - private $mountProvider; +class Scan extends FolderCommand { private $foldersCounter = 0; private $filesCounter = 0; - public function __construct(FolderManager $folderManager, IRootFolder $rootFolder, MountProvider $mountProvider) { - parent::__construct(); - $this->folderManager = $folderManager; - $this->rootFolder = $rootFolder; - $this->mountProvider = $mountProvider; - } - protected function configure() { $this ->setName('groupfolders:scan') @@ -55,52 +46,49 @@ protected function configure() { } protected function execute(InputInterface $input, OutputInterface $output) { - $folderId = (int)$input->getArgument('folder_id'); - $folder = $this->folderManager->getFolder($folderId, $this->rootFolder->getMountPoint()->getNumericStorageId()); - if ($folder) { - $mount = $this->mountProvider->getMount($folder['id'], '/' . $folder['mount_point'], Constants::PERMISSION_ALL, $folder['quota']); - $scanner = $mount->getStorage()->getScanner(); + $folder = $this->getFolder($input, $output); + if ($folder === false) { + return -1; + } + $mount = $this->mountProvider->getMount($folder['id'], '/' . $folder['mount_point'], Constants::PERMISSION_ALL, $folder['quota']); + $scanner = $mount->getStorage()->getScanner(); - if ($scanner instanceof NoopScanner) { - $output->writeln("Scanning group folders using an object store as primary storage is not supported."); - return -1; - } + if ($scanner instanceof NoopScanner) { + $output->writeln("Scanning group folders using an object store as primary storage is not supported."); + return -1; + } - $scanner->listen('\OC\Files\Cache\Scanner', 'scanFile', function ($path) use ($output) { - $output->writeln("\tFile\t$path", OutputInterface::VERBOSITY_VERBOSE); - ++$this->filesCounter; - // abortIfInterrupted doesn't exist in nc14 - if (method_exists($this, 'abortIfInterrupted')) { - $this->abortIfInterrupted(); - } - }); + $scanner->listen('\OC\Files\Cache\Scanner', 'scanFile', function ($path) use ($output) { + $output->writeln("\tFile\t$path", OutputInterface::VERBOSITY_VERBOSE); + ++$this->filesCounter; + // abortIfInterrupted doesn't exist in nc14 + if (method_exists($this, 'abortIfInterrupted')) { + $this->abortIfInterrupted(); + } + }); - $scanner->listen('\OC\Files\Cache\Scanner', 'scanFolder', function ($path) use ($output) { - $output->writeln("\tFolder\t$path", OutputInterface::VERBOSITY_VERBOSE); - ++$this->foldersCounter; - // abortIfInterrupted doesn't exist in nc14 - if (method_exists($this, 'abortIfInterrupted')) { - $this->abortIfInterrupted(); - } - }); + $scanner->listen('\OC\Files\Cache\Scanner', 'scanFolder', function ($path) use ($output) { + $output->writeln("\tFolder\t$path", OutputInterface::VERBOSITY_VERBOSE); + ++$this->foldersCounter; + // abortIfInterrupted doesn't exist in nc14 + if (method_exists($this, 'abortIfInterrupted')) { + $this->abortIfInterrupted(); + } + }); - $start = microtime(true); + $start = microtime(true); - $scanner->setUseTransactions(false); - $scanner->scan(''); + $scanner->setUseTransactions(false); + $scanner->scan(''); - $end = microtime(true); + $end = microtime(true); - $headers = [ - 'Folders', 'Files', 'Elapsed time' - ]; + $headers = [ + 'Folders', 'Files', 'Elapsed time' + ]; - $this->showSummary($headers, null, $output, $end - $start); - return 0; - } else { - $output->writeln('Folder not found: ' . $folderId . ''); - return -1; - } + $this->showSummary($headers, null, $output, $end - $start); + return 0; } protected function showSummary($headers, $rows, OutputInterface $output, float $duration) { From fed94f0e357fc2606e9403e06b3bc4afa250d363 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 16 Dec 2021 13:16:45 +0100 Subject: [PATCH 3/3] Fixes Signed-off-by: Carl Schwan --- lib/Command/ACL.php | 50 ++++++++++++++-------------- lib/Command/Create.php | 5 ++- lib/Command/Delete.php | 8 ++--- lib/Command/FolderCommand.php | 8 ++--- lib/Command/Group.php | 62 ++++++++++++++--------------------- lib/Command/Quota.php | 2 +- lib/Command/Rename.php | 5 +-- tests/psalm-baseline.xml | 43 +++++++++++++++++++++++- 8 files changed, 103 insertions(+), 80 deletions(-) diff --git a/lib/Command/ACL.php b/lib/Command/ACL.php index bfa4d42f5..a4c2c4907 100644 --- a/lib/Command/ACL.php +++ b/lib/Command/ACL.php @@ -88,9 +88,9 @@ protected function execute(InputInterface $input, OutputInterface $output) { return -1; } if ($input->getOption('enable')) { - $this->folderManager->setFolderACL($folderId, true); + $this->folderManager->setFolderACL($folder['id'], true); } elseif ($input->getOption('disable')) { - $this->folderManager->setFolderACL($folderId, false); + $this->folderManager->setFolderACL($folder['id'], false); } elseif ($input->getOption('test')) { if ($input->getOption('user') && ($input->getArgument('path'))) { $mappingId = $input->getOption('user'); @@ -105,13 +105,13 @@ protected function execute(InputInterface $input, OutputInterface $output) { $permissions = $aclManager->getACLPermissionsForPath($jailPath . rtrim('/' . $path, '/')); $permissionString = $this->formatRulePermissions(Constants::PERMISSION_ALL, $permissions); $output->writeln($permissionString); - return; + return 0; } else { $output->writeln('--user and options needs to be set for permissions testing'); return -3; } } elseif (!$folder['acl']) { - $output->writeln('Advanced permissions not enabled for folder: ' . $folderId . ''); + $output->writeln('Advanced permissions not enabled for folder: ' . $folder['id'] . ''); return -2; } elseif ( !$input->getArgument('path') && @@ -123,11 +123,11 @@ protected function execute(InputInterface $input, OutputInterface $output) { } elseif ($input->getOption('manage-add') && ($input->getOption('user') || $input->getOption('group'))) { $mappingType = $input->getOption('user') ? 'user' : 'group'; $mappingId = $input->getOption('user') ? $input->getOption('user') : $input->getOption('group'); - $this->folderManager->setManageACL($folderId, $mappingType, $mappingId, true); + $this->folderManager->setManageACL($folder['id'], $mappingType, $mappingId, true); } elseif ($input->getOption('manage-remove') && ($input->getOption('user') || $input->getOption('group'))) { $mappingType = $input->getOption('user') ? 'user' : 'group'; $mappingId = $input->getOption('user') ? $input->getOption('user') : $input->getOption('group'); - $this->folderManager->setManageACL($folderId, $mappingType, $mappingId, false); + $this->folderManager->setManageACL($folder['id'], $mappingType, $mappingId, false); } elseif (!$input->getArgument('path')) { $output->writeln(' argument has to be set when not using --enable or --disable'); return -3; @@ -169,28 +169,28 @@ protected function execute(InputInterface $input, OutputInterface $output) { 0, 0 )); - } else { - foreach ($permissionStrings as $permission) { - if ($permission[0] !== '+' && $permission[0] !== '-') { - $output->writeln('incorrect format for permissions "' . $permission . '"'); - return -3; - } - $name = substr($permission, 1); - if (!isset(self::PERMISSIONS_MAP[$name])) { - $output->writeln('incorrect format for permissions2 "' . $permission . '"'); - return -3; - } + return 0; + } + foreach ($permissionStrings as $permission) { + if ($permission[0] !== '+' && $permission[0] !== '-') { + $output->writeln('incorrect format for permissions "' . $permission . '"'); + return -3; } + $name = substr($permission, 1); + if (!isset(self::PERMISSIONS_MAP[$name])) { + $output->writeln('incorrect format for permissions2 "' . $permission . '"'); + return -3; + } + } - [$mask, $permissions] = $this->parsePermissions($permissionStrings); + [$mask, $permissions] = $this->parsePermissions($permissionStrings); - $this->ruleManager->saveRule(new Rule( - new UserMapping($mappingType, $mappingId), - $id, - $mask, - $permissions - )); - } + $this->ruleManager->saveRule(new Rule( + new UserMapping($mappingType, $mappingId), + $id, + $mask, + $permissions + )); } return 0; } diff --git a/lib/Command/Create.php b/lib/Command/Create.php index 6ba92570a..1455711ae 100644 --- a/lib/Command/Create.php +++ b/lib/Command/Create.php @@ -29,13 +29,12 @@ use Symfony\Component\Console\Output\OutputInterface; class Create extends Base { + /** @var FolderManager $folderManager */ private $folderManager; - private $rootFolder; - public function __construct(FolderManager $folderManager, IRootFolder $rootFolder) { + public function __construct(FolderManager $folderManager) { parent::__construct(); $this->folderManager = $folderManager; - $this->rootFolder = $rootFolder; } protected function configure() { diff --git a/lib/Command/Delete.php b/lib/Command/Delete.php index 7e3db5977..3b33f42d2 100644 --- a/lib/Command/Delete.php +++ b/lib/Command/Delete.php @@ -29,7 +29,7 @@ use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Question\ConfirmationQuestion; -class FolderCommand extends Base { +class Delete extends FolderCommand { protected function configure() { $this ->setName('groupfolders:delete') @@ -47,9 +47,9 @@ protected function execute(InputInterface $input, OutputInterface $output) { $helper = $this->getHelper('question'); $question = new ConfirmationQuestion('Are you sure you want to delete the group folder ' . $folder['mount_point'] . ' and all files within, this cannot be undone (y/N).', false); if ($input->getOption('force') || $helper->ask($input, $output, $question)) { - $folder = $this->mountProvider->getFolder($folderId); - $this->folderManager->removeFolder($folderId); - $folder->delete(); + $folderMount = $this->mountProvider->getFolder($folder['id']); + $this->folderManager->removeFolder($folder['id']); + $folderMount->delete(); } return 0; } diff --git a/lib/Command/FolderCommand.php b/lib/Command/FolderCommand.php index b47f8e46f..ca5968df9 100644 --- a/lib/Command/FolderCommand.php +++ b/lib/Command/FolderCommand.php @@ -26,18 +26,17 @@ use OCA\GroupFolders\Mount\MountProvider; use OCP\Files\IRootFolder; use Symfony\Component\Console\Input\InputInterface; -use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; /** * Base command for commands asking the user for a folder id. */ abstract class FolderCommand extends Base { - /** @var FolderManager */ + /** @var FolderManager $folderManager */ protected $folderManager; - /** @var IRootFolder */ + /** @var IRootFolder $rootFolder */ protected $rootFolder; - /** @var MountProvider */ + /** @var MountProvider $mountProvider */ protected $mountProvider; public function __construct(FolderManager $folderManager, IRootFolder $rootFolder, MountProvider $mountProvider) { @@ -62,5 +61,6 @@ protected function getFolder(InputInterface $input, OutputInterface $output) { $output->writeln('Folder not found: ' . $folderId . ''); return false; } + return $folder; } } diff --git a/lib/Command/Group.php b/lib/Command/Group.php index 6f7d8f62a..69611a18c 100644 --- a/lib/Command/Group.php +++ b/lib/Command/Group.php @@ -23,6 +23,7 @@ use OC\Core\Command\Base; use OCA\GroupFolders\Folder\FolderManager; +use OCA\GroupFolders\Mount\MountProvider; use OCP\Constants; use OCP\Files\IRootFolder; use OCP\IGroupManager; @@ -31,23 +32,18 @@ use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; -class Group extends Base { +class Group extends FolderCommand { const PERMISSION_VALUES = [ 'read' => Constants::PERMISSION_READ, 'write' => Constants::PERMISSION_UPDATE | Constants::PERMISSION_CREATE, 'share' => Constants::PERMISSION_SHARE, 'delete' => Constants::PERMISSION_DELETE, ]; - - - private $folderManager; - private $rootFolder; + /** @var IGroupManager $groupManager */ private $groupManager; - public function __construct(FolderManager $folderManager, IRootFolder $rootFolder, IGroupManager $groupManager) { - parent::__construct(); - $this->folderManager = $folderManager; - $this->rootFolder = $rootFolder; + public function __construct(FolderManager $folderManager, IRootFolder $rootFolder, IGroupManager $groupManager, MountProvider $mountProvider) { + parent::__construct($folderManager, $rootFolder, $mountProvider); $this->groupManager = $groupManager; } @@ -64,40 +60,30 @@ protected function configure() { } protected function execute(InputInterface $input, OutputInterface $output) { - $folderId = (int)$input->getArgument('folder_id'); - if ((string)$folderId !== $input->getArgument('folder_id')) { - // Protect against removing folderId === 0 when typing a string (e.g. folder name instead of folder id) - $output->writeln('Folder id argument is not an integer. Got ' . $input->getArgument('folder_id') . ''); - return; + $folder = $this->getFolder($input, $output); + if ($folder === false) { + return -1; } - $folder = $this->folderManager->getFolder($folderId, $this->rootFolder->getMountPoint()->getNumericStorageId()); - if ($folder) { - $groupString = $input->getArgument('group'); - $group = $this->groupManager->get($groupString); - if ($input->getOption('delete')) { - $this->folderManager->removeApplicableGroup($folderId, $groupString); - return 0; - } elseif ($group) { - $permissionsString = $input->getArgument('permissions'); - $permissions = $this->getNewPermissions($permissionsString); - if ($permissions) { - if (!isset($folder['groups'][$groupString])) { - $this->folderManager->addApplicableGroup($folderId, $groupString); - } - $this->folderManager->setGroupPermissions($folderId, $groupString, $permissions); - return 0; - } else { - $output->writeln('Unable to parse permissions input: ' . implode(' ', $permissionsString) . ''); - return -1; + $groupString = $input->getArgument('group'); + $group = $this->groupManager->get($groupString); + if ($input->getOption('delete')) { + $this->folderManager->removeApplicableGroup($folder['id'], $groupString); + return 0; + } elseif ($group) { + $permissionsString = $input->getArgument('permissions'); + $permissions = $this->getNewPermissions($permissionsString); + if ($permissions) { + if (!isset($folder['groups'][$groupString])) { + $this->folderManager->addApplicableGroup($folder['id'], $groupString); } - } else { - $output->writeln('group not found: ' . $groupString . ''); - return -1; + $this->folderManager->setGroupPermissions($folder['id'], $groupString, $permissions); + return 0; } - } else { - $output->writeln('Folder not found: ' . $folderId . ''); + $output->writeln('Unable to parse permissions input: ' . implode(' ', $permissionsString) . ''); return -1; } + $output->writeln('group not found: ' . $groupString . ''); + return -1; } private function getNewPermissions(array $input) { diff --git a/lib/Command/Quota.php b/lib/Command/Quota.php index 11521c292..1087c4c16 100644 --- a/lib/Command/Quota.php +++ b/lib/Command/Quota.php @@ -48,7 +48,7 @@ protected function execute(InputInterface $input, OutputInterface $output) { $quotaString = strtolower($input->getArgument('quota')); $quota = ($quotaString === 'unlimited') ? FileInfo::SPACE_UNLIMITED : \OCP\Util::computerFileSize($quotaString); if ($quota) { - $this->folderManager->setFolderQuota($folderId, $quota); + $this->folderManager->setFolderQuota($folder['id'], $quota); return 0; } $output->writeln('Unable to parse quota input: ' . $quotaString . ''); diff --git a/lib/Command/Rename.php b/lib/Command/Rename.php index 3ca895522..1c2a9d4cd 100644 --- a/lib/Command/Rename.php +++ b/lib/Command/Rename.php @@ -30,9 +30,6 @@ use Symfony\Component\Console\Output\OutputInterface; class Rename extends FolderCommand { - private $folderManager; - private $rootFolder; - protected function configure() { $this ->setName('groupfolders:rename') @@ -47,7 +44,7 @@ protected function execute(InputInterface $input, OutputInterface $output) { if ($folder === false) { return -1; } - $this->folderManager->renameFolder($folderId, $input->getArgument('name')); + $this->folderManager->renameFolder($folder['id'], $input->getArgument('name')); return 0; } } diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index 786745366..772c58ca3 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -10,26 +10,67 @@ + + registerEventListener + $c->getServer()->getRootFolder() + $c->getServer()->getRootFolder() + ExpireGroupVersions ExpireGroupVersions ExpireGroupVersionsPlaceholder GroupVersionsExpireManager + GroupVersionsExpireManager + TrashBackend TrashBackend + TrashBackend + TrashBackend + VersionsBackend VersionsBackend + VersionsBackend + \OCA\GroupFolders\BackgroundJob\ExpireGroupVersions \OCA\GroupFolders\BackgroundJob\ExpireGroupVersions \OCA\GroupFolders\BackgroundJob\ExpireGroupVersionsPlaceholder BeforeTemplateRenderedEvent - GroupFolderStorage + + + FolderCommand + + + + + FolderCommand + + + + + FolderCommand + + + + + FolderCommand + + + + + FolderCommand + + + + + FolderCommand + + $this->rootFolder