From 8078ea6dbf8657a925d37567476bc732f2947014 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philipp=20Sch=C3=BCle?= Date: Wed, 10 Jan 2024 16:26:51 +0100 Subject: [PATCH] tweak(Filemanager/Frontend/Download): add/improve logging ... and some code cleanup --- .../Controller/DownloadLinkTests.php | 2 +- .../Filemanager/Controller/DownloadLink.php | 19 +++++++++++-------- tine20/Filemanager/Controller/Node.php | 1 - tine20/Filemanager/Frontend/Download.php | 9 +++++---- tine20/Tinebase/Frontend/Http/Abstract.php | 2 ++ 5 files changed, 19 insertions(+), 14 deletions(-) diff --git a/tests/tine20/Filemanager/Controller/DownloadLinkTests.php b/tests/tine20/Filemanager/Controller/DownloadLinkTests.php index 51a40f120ba..92cb6950c38 100644 --- a/tests/tine20/Filemanager/Controller/DownloadLinkTests.php +++ b/tests/tine20/Filemanager/Controller/DownloadLinkTests.php @@ -140,7 +140,7 @@ public function testDownloadLinkExpiry() $resultNode = $this->_getUit()->getNode($downloadLink, array()); $this->fail('user should not be able to access expired download link node'); } catch (Tinebase_Exception_AccessDenied $tead) { - $this->assertEquals('Download link has expired', $tead->getMessage()); + $this->assertStringContainsString('Download link has expired', $tead->getMessage()); } } diff --git a/tine20/Filemanager/Controller/DownloadLink.php b/tine20/Filemanager/Controller/DownloadLink.php index 11851d4037b..e8d05a71933 100644 --- a/tine20/Filemanager/Controller/DownloadLink.php +++ b/tine20/Filemanager/Controller/DownloadLink.php @@ -176,11 +176,16 @@ public function hasPassword(Filemanager_Model_DownloadLink $download) */ protected function _checkExpiryDate(Filemanager_Model_DownloadLink $download) { - if (Tinebase_Core::isLogLevel(Zend_Log::DEBUG)) Tinebase_Core::getLogger()->debug(__METHOD__ . '::' . __LINE__ - . ' Checking download link expiry time: ' . $download->expiry_time); + if (Tinebase_Core::isLogLevel(Zend_Log::DEBUG)) Tinebase_Core::getLogger()->debug( + __METHOD__ . '::' . __LINE__ . ' Checking download link expiry time: ' . $download->expiry_time); - if ($download->expiry_time instanceof Tinebase_DateTime && $download->expiry_time->isEarlier(Tinebase_DateTime::now())) { - throw new Tinebase_Exception_AccessDenied('Download link has expired'); + if ( $download->expiry_time instanceof Tinebase_DateTime + && $download->expiry_time->isEarlier(Tinebase_DateTime::now()) + ) { + $message = 'Download link has expired: ' . $download->expiry_time; + if (Tinebase_Core::isLogLevel(Zend_Log::NOTICE)) Tinebase_Core::getLogger()->notice( + __METHOD__ . '::' . __LINE__ . ' ' . $message); + throw new Tinebase_Exception_AccessDenied($message); } } @@ -205,12 +210,10 @@ public function validatePassword(Filemanager_Model_DownloadLink $download, $pass * @param Filemanager_Model_DownloadLink $download * @return Tinebase_Model_Tree_Node */ - protected function _getRootNode(Filemanager_Model_DownloadLink $download) + protected function _getRootNode(Filemanager_Model_DownloadLink $download): Tinebase_Model_Tree_Node { // ACL is checked here, download link user should be already set by frontend - $node = Filemanager_Controller_Node::getInstance()->get($download->node_id); - - return $node; + return Filemanager_Controller_Node::getInstance()->get($download->node_id); } /** diff --git a/tine20/Filemanager/Controller/Node.php b/tine20/Filemanager/Controller/Node.php index c4ca5252070..96d26b624bd 100644 --- a/tine20/Filemanager/Controller/Node.php +++ b/tine20/Filemanager/Controller/Node.php @@ -352,7 +352,6 @@ public function get($_id, $_containerId = NULL, $_getRelatedData = true, $_getDe $record->notes = Tinebase_Notes::getInstance()->getNotesOfRecord(Tinebase_Model_Tree_Node::class, $record->getId()); - $record->path = Tinebase_Model_Tree_Node_Path::removeAppIdFromPath($nodePath->flatpath, $this->_applicationName); $this->resolveGrants($record); diff --git a/tine20/Filemanager/Frontend/Download.php b/tine20/Filemanager/Frontend/Download.php index 78697ff8812..721fa141d9b 100644 --- a/tine20/Filemanager/Frontend/Download.php +++ b/tine20/Filemanager/Frontend/Download.php @@ -159,6 +159,9 @@ public function downloadNode($path) exit; } + if (Tinebase_Core::isLogLevel(Zend_Log::INFO)) Tinebase_Core::getLogger()->info( + __METHOD__ . '::' . __LINE__ . ' Download path: ' . $$path); + $this->_setDownloadLinkOwnerAsUser($download); $node = Filemanager_Controller_DownloadLink::getInstance()->getNode($download, $splittedPath); @@ -185,11 +188,9 @@ public function downloadNode($path) * @param string $id * @return Filemanager_Model_DownloadLink */ - protected function _getDownloadLink($id) + protected function _getDownloadLink(string $id): Filemanager_Model_DownloadLink { - $download = Filemanager_Controller_DownloadLink::getInstance()->get($id); - - return $download; + return Filemanager_Controller_DownloadLink::getInstance()->get($id); } /** diff --git a/tine20/Tinebase/Frontend/Http/Abstract.php b/tine20/Tinebase/Frontend/Http/Abstract.php index c92fc105a19..a46302e9bdd 100644 --- a/tine20/Tinebase/Frontend/Http/Abstract.php +++ b/tine20/Tinebase/Frontend/Http/Abstract.php @@ -324,6 +324,8 @@ protected function _downloadTempFile(Tinebase_Model_TempFile $tempFile, $filesys protected function _downloadFileNode(Tinebase_Model_Tree_Node $node, $filesystemPath, $revision = null, $ignoreAcl = false) { if (! $ignoreAcl && ! Tinebase_Core::getUser()->hasGrant($node, Tinebase_Model_Grants::GRANT_DOWNLOAD)) { + if (Tinebase_Core::isLogLevel(Zend_Log::DEBUG)) Tinebase_Core::getLogger()->debug( + __METHOD__ . '::' . __LINE__ . ' User has no download grant for node ' . $node->getId()); $this->_handleFailure(403); }