From 23e2baeb95c901bfb0e715c03a6a8d6f76cf6ae8 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 10 Feb 2020 16:36:03 +0100 Subject: [PATCH 1/2] Add option to check share ACL's when listing directories If a file or folder in a directory doesn't have read permissions they will not be shown Note that enabling this option incurs a performance penalty additional requests need to be made to get all the acl. Additionally the acl resolving logic is fairly primitive at the moment and might not work correctly in all setups (it should error to showing the entry) Signed-off-by: Robin Appelman --- apps/files_external/lib/Lib/Backend/SMB.php | 3 ++ apps/files_external/lib/Lib/Storage/SMB.php | 34 +++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/apps/files_external/lib/Lib/Backend/SMB.php b/apps/files_external/lib/Lib/Backend/SMB.php index eefeb137bf..7cb692ad9a 100644 --- a/apps/files_external/lib/Lib/Backend/SMB.php +++ b/apps/files_external/lib/Lib/Backend/SMB.php @@ -56,6 +56,9 @@ class SMB extends Backend { (new DefinitionParameter('show_hidden', $l->t('Show hidden files'))) ->setType(DefinitionParameter::VALUE_BOOLEAN) ->setFlag(DefinitionParameter::FLAG_OPTIONAL), + (new DefinitionParameter('check_acl', $l->t('Verify ACL access when listing files'))) + ->setType(DefinitionParameter::VALUE_BOOLEAN) + ->setFlag(DefinitionParameter::FLAG_OPTIONAL), (new DefinitionParameter('timeout', $l->t('Timeout'))) ->setType(DefinitionParameter::VALUE_HIDDEN) ->setFlag(DefinitionParameter::FLAG_OPTIONAL), diff --git a/apps/files_external/lib/Lib/Storage/SMB.php b/apps/files_external/lib/Lib/Storage/SMB.php index 3ff8179c7b..ce37d55612 100644 --- a/apps/files_external/lib/Lib/Storage/SMB.php +++ b/apps/files_external/lib/Lib/Storage/SMB.php @@ -36,6 +36,7 @@ namespace OCA\Files_External\Lib\Storage; +use Icewind\SMB\ACL; use Icewind\SMB\BasicAuth; use Icewind\SMB\Exception\AlreadyExistsException; use Icewind\SMB\Exception\ConnectException; @@ -90,6 +91,9 @@ class SMB extends Common implements INotifyStorage { /** @var bool */ protected $showHidden; + /** @var bool */ + protected $checkAcl; + public function __construct($params) { if (!isset($params['host'])) { throw new \Exception('Invalid configuration, no host provided'); @@ -126,6 +130,7 @@ class SMB extends Common implements INotifyStorage { $this->root = rtrim($this->root, '/') . '/'; $this->showHidden = isset($params['show_hidden']) && $params['show_hidden']; + $this->checkAcl = isset($params['check_acl']) && $params['check_acl']; $this->statCache = new CappedMemoryCache(); parent::__construct($params); @@ -202,6 +207,24 @@ class SMB extends Common implements INotifyStorage { throw new StorageAuthException($e->getMessage(), $e); } + /** + * get the acl from fileinfo that is relevant for the configured user + * + * @param IFileInfo $file + * @return ACL|null + */ + private function getACL(IFileInfo $file): ?ACL { + $acls = $file->getAcls(); + foreach ($acls as $user => $acl) { + [, $user] = explode('\\', $user); // strip domain + if ($user === $this->server->getAuth()->getUsername()) { + return $acl; + } + } + + return null; + } + /** * @param string $path * @return \Icewind\SMB\IFileInfo[] @@ -220,6 +243,17 @@ class SMB extends Common implements INotifyStorage { // the isHidden check is done before checking the config boolean to ensure that the metadata is always fetch // so we trigger the below exceptions where applicable $hide = $file->isHidden() && !$this->showHidden; + + if ($this->checkAcl && $acl = $this->getACL($file)) { + // if there is no explicit deny, we assume it's allowed + // this doesn't take inheritance fully into account but if read permissions is denied for a parent we wouldn't be in this folder + // additionally, it's better to have false negatives here then false positives + if ($acl->denies(ACL::MASK_READ) || $acl->denies(ACL::MASK_EXECUTE)) { + $this->logger->debug('Hiding non readable entry ' . $file->getName()); + return false; + } + } + if ($hide) { $this->logger->debug('hiding hidden file ' . $file->getName()); } From 7f32fe6d506474943ca9be0ad23c84a6fe3c35b0 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 10 Apr 2020 13:20:20 +0200 Subject: [PATCH 2/2] add tooltip to option Signed-off-by: Robin Appelman --- apps/files_external/lib/Lib/Backend/SMB.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/files_external/lib/Lib/Backend/SMB.php b/apps/files_external/lib/Lib/Backend/SMB.php index 7cb692ad9a..5344bf5f78 100644 --- a/apps/files_external/lib/Lib/Backend/SMB.php +++ b/apps/files_external/lib/Lib/Backend/SMB.php @@ -58,7 +58,8 @@ class SMB extends Backend { ->setFlag(DefinitionParameter::FLAG_OPTIONAL), (new DefinitionParameter('check_acl', $l->t('Verify ACL access when listing files'))) ->setType(DefinitionParameter::VALUE_BOOLEAN) - ->setFlag(DefinitionParameter::FLAG_OPTIONAL), + ->setFlag(DefinitionParameter::FLAG_OPTIONAL) + ->setTooltip($l->t("Check the ACL's of each file or folder inside a directory to filter out items where the user has no read permissions, comes with a performance penalty")), (new DefinitionParameter('timeout', $l->t('Timeout'))) ->setType(DefinitionParameter::VALUE_HIDDEN) ->setFlag(DefinitionParameter::FLAG_OPTIONAL),