From 233c6915edd43116dbf1630f2bfac0746912d0a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philipp=20Sch=C3=BCle?= Date: Tue, 23 May 2023 11:01:20 +0200 Subject: [PATCH] fix(Felamimail/Folder): folder unselectable if another user deletes it --- .../Admin/Frontend/Json/EmailAccountTest.php | 2 +- tests/tine20/Felamimail/Frontend/JsonTest.php | 67 ++++++++++++++++++- tests/tine20/Felamimail/TestCase.php | 7 +- tine20/Felamimail/Backend/ImapProxy.php | 1 + tine20/Felamimail/Controller/Folder.php | 41 +++++++++--- 5 files changed, 102 insertions(+), 16 deletions(-) diff --git a/tests/tine20/Admin/Frontend/Json/EmailAccountTest.php b/tests/tine20/Admin/Frontend/Json/EmailAccountTest.php index 92ceb3a047d..ba56fb2d015 100644 --- a/tests/tine20/Admin/Frontend/Json/EmailAccountTest.php +++ b/tests/tine20/Admin/Frontend/Json/EmailAccountTest.php @@ -94,7 +94,7 @@ public function testEmailAccountApi() * @param array $data * @return array */ - public static function getSharedAccountData($sendgrant = true, $data = []) + public static function getSharedAccountData(bool $sendgrant = true, array $data = []) { return array_merge([ 'name' => 'unittest shared account', diff --git a/tests/tine20/Felamimail/Frontend/JsonTest.php b/tests/tine20/Felamimail/Frontend/JsonTest.php index c0602534676..48cc90dc9b0 100644 --- a/tests/tine20/Felamimail/Frontend/JsonTest.php +++ b/tests/tine20/Felamimail/Frontend/JsonTest.php @@ -1203,7 +1203,7 @@ public function testForwardAttachmentCachePdf() 'attachments' => $messageToSend['attachments'] )); Felamimail_Controller_Message_Send::getInstance()->sendMessage($forwardMessage); - + $forwardMessage = $this->_searchForMessageBySubject('test forward with attachmnets'); $this->_foldersToClear = array('INBOX', $this->_account->sent_folder); $fullMessage = Felamimail_Controller_Message::getInstance()->getCompleteMessage($forwardMessage['id']); @@ -3092,7 +3092,70 @@ public function testMoveFolder() $testFolder = Felamimail_Controller_Folder::getInstance()->getByBackendAndGlobalName($this->_account, $this->_testFolderName); self::assertEquals(1, $testFolder->has_children, 'has_children should be 1'); } - + + /** + * 1. Folder "A" unter der Inbox erstellen. + * 2. Bei beiden Usern die Ordnerliste aktualisieren. + * 3. Bei UserA den Folder "A" löschen. + * 4. Bei UserB den Folder "B" unter den Folder "A" erstellen. + * 5. Bei beiden Usern die Ordnerliste aktualisieren. + * + * @return void + * @throws Tinebase_Exception_AccessDenied + * @throws Tinebase_Exception_InvalidArgument + * @throws Tinebase_Exception_NotFound + * @throws Tinebase_Exception_Record_DefinitionFailure + * @throws Tinebase_Exception_Record_Validation + */ + public function testDeleteFolderWithAnotherUserInSharedAccount() + { + $parent = 'INBOX'; + $testFolder = 'subfolder'; + $jsmith = $this->_personas['jsmith']; + $account = $this->_createSharedAccount(true, [ + 'grants' => [ + [ + 'readGrant' => true, + 'editGrant' => true, + 'addGrant' => true, + 'account_type' => 'user', + 'account_id' => Tinebase_Core::getUser()->getId(), + ], [ + 'readGrant' => true, + 'editGrant' => true, + 'addGrant' => true, + 'account_type' => 'user', + 'account_id' => $jsmith->getId(), + ] + ] + ]); + + try { + $this->_json->addFolder($parent, '', $account->getId()); + } catch (Tinebase_Exception_SystemGeneric $tesg) { + // already exists + } + $folder = $this->_json->addFolder($testFolder, $parent, $account->getId()); + $testFolderGlobalname = $folder['globalname']; + + $this->_json->updateFolderCache($account->getId(), $parent); + Tinebase_Core::setUser($jsmith); + $this->_json->updateFolderCache($account->getId(), $parent); + Tinebase_Core::setUser($this->_originalTestUser); + $this->_json->deleteFolder($testFolderGlobalname, $account->getId()); + Tinebase_Core::setUser($jsmith); + try { + $this->_json->addFolder('subfolder2', $testFolderGlobalname, $account->getId()); + $result = $this->_json->updateFolderCache($account->getId(), $parent); + $folder = $result[0]; + self::assertEquals(1, $folder['is_selectable'], 'folder should be selectable (or removed!): ' + . print_r($folder, true)); + } catch (Tinebase_Exception_NotFound $tenf) { + self::assertEquals('Could not create folder: parent folder INBOX.subfolder not found', + $tenf->getMessage()); + } + } + public function testRefreshFolder() { $folder = $this->_getFolder($this->_testFolderName); diff --git a/tests/tine20/Felamimail/TestCase.php b/tests/tine20/Felamimail/TestCase.php index 98f8f69a759..db2e4ed2ff4 100644 --- a/tests/tine20/Felamimail/TestCase.php +++ b/tests/tine20/Felamimail/TestCase.php @@ -509,17 +509,18 @@ protected function _sieveTestHelper(array $_sieveData, bool $_isMime = false, ?s /** * @param bool $sendgrant - * @return Tinebase_Record_Interface + * @return Felamimail_Model_Account * @throws Tinebase_Exception_AccessDenied * @throws Tinebase_Exception_InvalidArgument * @throws Tinebase_Exception_NotFound * @throws Tinebase_Exception_Record_DefinitionFailure * @throws Tinebase_Exception_Record_Validation */ - protected function _createSharedAccount($sendgrant = true) + protected function _createSharedAccount(bool $sendgrant = true, array $data = []) { Tinebase_EmailUser::clearCaches(); - $sharedAccountData = Admin_Frontend_Json_EmailAccountTest::getSharedAccountData($sendgrant); + $sharedAccountData = Admin_Frontend_Json_EmailAccountTest::getSharedAccountData($sendgrant, $data); + /* @var Felamimail_Model_Account $sharedAccount */ $sharedAccount = Admin_Controller_EmailAccount::getInstance()->create(new Felamimail_Model_Account($sharedAccountData)); // we need to commit so imap user is in imap db Tinebase_TransactionManager::getInstance()->commitTransaction($this->_transactionId); diff --git a/tine20/Felamimail/Backend/ImapProxy.php b/tine20/Felamimail/Backend/ImapProxy.php index f1553dfcae0..c4110dad628 100644 --- a/tine20/Felamimail/Backend/ImapProxy.php +++ b/tine20/Felamimail/Backend/ImapProxy.php @@ -23,6 +23,7 @@ * @method examineFolder($globalName) * @method removeMessage($id) * @method getChangedFlags($modseq) + * @method createFolder($name, $parentFolder = null, $_delimiter = '/') */ class Felamimail_Backend_ImapProxy { diff --git a/tine20/Felamimail/Controller/Folder.php b/tine20/Felamimail/Controller/Folder.php index 57676289b32..8d9078213c7 100644 --- a/tine20/Felamimail/Controller/Folder.php +++ b/tine20/Felamimail/Controller/Folder.php @@ -215,27 +215,46 @@ public function getByBackendAndGlobalName($_accountId, $_globalName) * create folder * * @param string|Felamimail_Model_Account $_accountId - * @param string $_folderName to create - * @param string $_parentFolder parent folder globalname + * @param string $_folderName + * @param string $_parentFolder * @return Felamimail_Model_Folder + * @throws Felamimail_Exception + * @throws Felamimail_Exception_IMAPInvalidCredentials * @throws Felamimail_Exception_IMAPServiceUnavailable + * @throws Tinebase_Exception_NotFound + * @throws Tinebase_Exception_Record_DefinitionFailure + * @throws Tinebase_Exception_Record_Validation * @throws Tinebase_Exception_SystemGeneric */ - public function create($_accountId, $_folderName, $_parentFolder = '') + public function create($_accountId, string $_folderName, string $_parentFolder = ''): Felamimail_Model_Folder { - $account = ($_accountId instanceof Felamimail_Model_Account) ? $_accountId : Felamimail_Controller_Account::getInstance()->get($_accountId); + $account = ($_accountId instanceof Felamimail_Model_Account) + ? $_accountId + : Felamimail_Controller_Account::getInstance()->get($_accountId); $this->_delimiter = $account->delimiter; $foldername = $this->_prepareFolderName($_folderName); $globalname = (empty($_parentFolder)) ? $foldername : $_parentFolder . $this->_delimiter . $foldername; - if (Tinebase_Core::isLogLevel(Zend_Log::DEBUG)) Tinebase_Core::getLogger()->debug(__METHOD__ . '::' . __LINE__ . ' Trying to create new folder: ' . $globalname . ' (parent: ' . $_parentFolder . ')'); + if (Tinebase_Core::isLogLevel(Zend_Log::DEBUG)) Tinebase_Core::getLogger()->debug( + __METHOD__ . '::' . __LINE__ . ' Trying to create new folder: ' . $globalname + . ' (parent: ' . $_parentFolder . ')'); $imap = Felamimail_Backend_ImapFactory::factory($account); - + + // check if parent folder exists + if (! empty($_parentFolder)) { + try { + $imap->examineFolder($_parentFolder); + } catch (Zend_Mail_Storage_Exception $zmse) { + throw new Tinebase_Exception_NotFound('Could not create folder: parent folder ' + . $_parentFolder . ' not found'); + } + } + try { $imap->createFolder( Felamimail_Model_Folder::encodeFolderName($foldername), - (empty($_parentFolder)) ? NULL : Felamimail_Model_Folder::encodeFolderName($_parentFolder), + (empty($_parentFolder)) ? null : Felamimail_Model_Folder::encodeFolderName($_parentFolder), $this->_delimiter ); @@ -247,7 +266,8 @@ public function create($_accountId, $_folderName, $_parentFolder = '') 'parent' => $_parentFolder, )); $folder->supports_condstore = $this->supportsCondStore($folder, $account, $imap); - + + /* @var Felamimail_Model_Folder $folder */ $folder = $this->_backend->create($folder); if (Tinebase_Core::isLogLevel(Zend_Log::INFO)) Tinebase_Core::getLogger()->info( @@ -256,12 +276,13 @@ public function create($_accountId, $_folderName, $_parentFolder = '') } catch (Zend_Mail_Storage_Exception $zmse) { // perhaps the folder already exists if (Tinebase_Core::isLogLevel(Zend_Log::INFO)) Tinebase_Core::getLogger()->info( - __METHOD__ . '::' . __LINE__ . ' Could not create new folder: ' . $globalname . ' (' . $zmse->getMessage() . ')'); + __METHOD__ . '::' . __LINE__ . ' Could not create new folder: ' . $globalname + . ' (' . $zmse->getMessage() . ')'); // reload folder cache of parent $parentSubs = $this->_cacheController->update($account, $_parentFolder); $folder = $parentSubs->filter('globalname', $globalname)->getFirstRecord(); - if ($folder === NULL) { + if ($folder === null) { throw new Felamimail_Exception_IMAPServiceUnavailable($zmse->getMessage()); }