Merge branch 'pu/ps/rt/227060' into '2023.11'

tweak(Filemanager/Frontend/Download): add/improve logging See merge request tine20/tine20!4731
tine-groupware · Jan 10, 2024 · 6be7fb8 · 6be7fb8
2 parents d667053 + 8078ea6
commit 6be7fb8
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 14 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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);
         }
Original file line number	Diff line number	Diff line change
Expand Up		@@ -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);

Expand Down