diff --git a/src/libsync/account.cpp b/src/libsync/account.cpp index bc09419b66674..d55b7951eae09 100644 --- a/src/libsync/account.cpp +++ b/src/libsync/account.cpp @@ -9,6 +9,7 @@ #include "accountfwd.h" #include "capabilities.h" #include "clientsideencryptionjobs.h" +#include "common/remotepermissions.h" #include "common/utility.h" #include "configfile.h" #include "cookiejar.h" @@ -1289,6 +1290,11 @@ void Account::listRemoteFolder(QPromise *promise, co newEntry.size = 0; } + if (!newEntry.remotePerm.isNull() && !newEntry.remotePerm.hasPermission(RemotePermissions::CanRead)) { + qCWarning(lcAccount()) << "skip non-readable item" << absoluteItemPathName; + return; + } + promise->emplaceResult(itemFileName, itemFileName.toStdWString(), absoluteItemPathName, newEntry); }); diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index e1e2a3b4a623e..394a7b22fd2bb 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -843,7 +843,7 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo(const SyncFileItemPtr &it item->_direction = SyncFileItem::Down; item->_instruction = CSYNC_INSTRUCTION_SYNC; item->_type = ItemTypeVirtualFileDownload; - } else if (serverEntry.isValid() && !serverEntry.isDirectory && !serverEntry.remotePerm.isNull() && !serverEntry.remotePerm.hasPermission(RemotePermissions::CanRead)) { + } else if (serverEntry.isValid() && !serverEntry.remotePerm.isNull() && !serverEntry.remotePerm.hasPermission(RemotePermissions::CanRead)) { item->_instruction = CSYNC_INSTRUCTION_REMOVE; item->_direction = SyncFileItem::Down; } else if (dbEntry._etag != serverEntry.etag) { @@ -938,7 +938,7 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo(const SyncFileItemPtr &it return; } - if (serverEntry.isValid() && !serverEntry.isDirectory && !serverEntry.remotePerm.isNull() && !serverEntry.remotePerm.hasPermission(RemotePermissions::CanRead)) { + if (serverEntry.isValid() && !serverEntry.remotePerm.isNull() && !serverEntry.remotePerm.hasPermission(RemotePermissions::CanRead)) { item->_instruction = CSYNC_INSTRUCTION_IGNORE; emit _discoveryData->itemDiscovered(item); diff --git a/test/testaccount.cpp b/test/testaccount.cpp index e216250cc5e2b..9f032c6ebe298 100644 --- a/test/testaccount.cpp +++ b/test/testaccount.cpp @@ -18,6 +18,7 @@ #include "syncenginetestutils.h" using namespace OCC; +using namespace Qt::StringLiterals; class TestAccount: public QObject { @@ -108,9 +109,12 @@ private slots: QTest::newRow("root = /; requesting A/") << "" << "A/" << QStringList{ "A/a1", "A/a2", "A/sub1" }; QTest::newRow("root = /; requesting A/sub1/") << "" << "A/sub1/" << QStringList{ "A/sub1/sub2" }; QTest::newRow("root = /; requesting A/sub1/sub2") << "" << "A/sub1/sub2/" << QStringList{}; + QTest::newRow("root = /; requesting A/sub_nodownload/") << "" << "A/sub_nodownload/" << QStringList{}; QTest::newRow("root = /A; requesting A/") << "/A" << "A/" << QStringList{ "a1", "a2", "sub1" }; QTest::newRow("root = /A; requesting A/sub1") << "/A" << "A/sub1/" << QStringList{ "sub1/sub2" }; QTest::newRow("root = /A; requesting A/sub1/sub2") << "/A" << "A/sub1/sub2/" << QStringList{}; + QTest::newRow("root = /A; requesting A/sub_nodownload/") << "/A" << "A/sub_nodownload/" << QStringList{}; + QTest::newRow("root = /shared_nodownload; requesting shared_nodownload/") << "/shared_nodownload" << "shared_nodownload/" << QStringList{}; } void testAccount_listRemoteFolder() @@ -122,6 +126,26 @@ private slots: FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; fakeFolder.remoteModifier().mkdir("A/sub1"); fakeFolder.remoteModifier().mkdir("A/sub1/sub2"); + fakeFolder.remoteModifier().mkdir("A/sub1/sub2"); + + // prepare shares that are not allowed to be downloaded + std::function makeSharedNoDownload; + makeSharedNoDownload = [&makeSharedNoDownload](FileInfo &fileInfo) -> void { + fileInfo.isShared = true; + fileInfo.downloadForbidden = true; + for (auto &subItem : fileInfo.children) { + makeSharedNoDownload(subItem); + } + }; + FileInfo sharedNoDownload { "shared_nodownload"_L1, { { "file1"_L1, 32 }, { "file2"_L1, 32 } } }; + FileInfo sharedNoDownloadSub { "subdir"_L1, { { "file3"_L1, 32 }, { "file4"_L1, 32 } } }; + FileInfo aSubSharedNoDownload { "sub_nodownload"_L1, { { "file4"_L1, 32 }, { "file5"_L1, 32 } } }; + sharedNoDownload.children.insert(sharedNoDownloadSub.name, std::move(sharedNoDownloadSub)); + makeSharedNoDownload(sharedNoDownload); + makeSharedNoDownload(aSubSharedNoDownload); + fakeFolder.remoteModifier().children.insert(sharedNoDownload.name, std::move(sharedNoDownload)); + fakeFolder.remoteModifier().find("A")->children.insert(aSubSharedNoDownload.name, std::move(aSubSharedNoDownload)); + const auto account = fakeFolder.account(); QEventLoop localEventLoop; diff --git a/test/testpermissions.cpp b/test/testpermissions.cpp index 170b1a0b47f6c..6a00402ef73fe 100644 --- a/test/testpermissions.cpp +++ b/test/testpermissions.cpp @@ -876,11 +876,19 @@ private slots: }); fakeFolder.remoteModifier().insert("file"); + fakeFolder.remoteModifier().mkdir("folder"); + fakeFolder.remoteModifier().insert("folder/file"); + + const auto setDownloadForbiddenShare = [&fakeFolder](const QString &relativePath) -> void { + auto fileInfo = fakeFolder.remoteModifier().find(relativePath); + Q_ASSERT(fileInfo); + fileInfo->isShared = true; + fileInfo->downloadForbidden = true; + }; - auto fileItem = fakeFolder.remoteModifier().find("file"); - Q_ASSERT(fileItem); - fileItem->isShared = true; - fileItem->downloadForbidden = true; + setDownloadForbiddenShare("file"); + setDownloadForbiddenShare("folder"); + setDownloadForbiddenShare("folder/file"); // also hook into discovery!! SyncFileItemVector discovery; @@ -890,6 +898,8 @@ private slots: QVERIFY(itemInstruction(completeSpy, "file", CSYNC_INSTRUCTION_IGNORE)); QVERIFY(discoveryInstruction(discovery, "file", CSYNC_INSTRUCTION_IGNORE)); + QVERIFY(itemInstruction(completeSpy, "folder", CSYNC_INSTRUCTION_IGNORE)); + QVERIFY(discoveryInstruction(discovery, "folder", CSYNC_INSTRUCTION_IGNORE)); } void testExistingFileBecomeForbiddenDownload() @@ -898,9 +908,18 @@ private slots: QObject parent; fakeFolder.remoteModifier().insert("file"); - auto fileInfo = fakeFolder.remoteModifier().find("file"); - Q_ASSERT(fileInfo); - fileInfo->isShared = true; + fakeFolder.remoteModifier().mkdir("folder"); + fakeFolder.remoteModifier().insert("folder/file"); + + const auto setShared = [&fakeFolder](const QString &relativePath) -> void { + auto fileInfo = fakeFolder.remoteModifier().find(relativePath); + Q_ASSERT(fileInfo); + fileInfo->isShared = true; + }; + + setShared("file"); + setShared("folder"); + setShared("folder/file"); QVERIFY(fakeFolder.syncOnce()); @@ -914,10 +933,16 @@ private slots: return nullptr; }); - auto fileItem = fakeFolder.remoteModifier().find("file", FileInfo::Invalidate); - Q_ASSERT(fileItem); - fileItem->isShared = true; - fileItem->downloadForbidden = true; + const auto setDownloadForbidden = [&fakeFolder](const QString &relativePath) -> void { + auto fileInfo = fakeFolder.remoteModifier().find(relativePath, FileInfo::Invalidate); + Q_ASSERT(fileInfo); + fileInfo->isShared = true; + fileInfo->downloadForbidden = true; + }; + + setDownloadForbidden("file"); + setDownloadForbidden("folder"); + setDownloadForbidden("folder/file"); // also hook into discovery!! SyncFileItemVector discovery; @@ -927,6 +952,10 @@ private slots: QVERIFY(itemInstruction(completeSpy, "file", CSYNC_INSTRUCTION_REMOVE)); QVERIFY(discoveryInstruction(discovery, "file", CSYNC_INSTRUCTION_REMOVE)); + QVERIFY(itemInstruction(completeSpy, "folder", CSYNC_INSTRUCTION_REMOVE)); + QVERIFY(discoveryInstruction(discovery, "folder", CSYNC_INSTRUCTION_REMOVE)); + QVERIFY(itemInstruction(completeSpy, "folder/file", CSYNC_INSTRUCTION_NONE)); + QVERIFY(discoveryInstruction(discovery, "folder/file", CSYNC_INSTRUCTION_REMOVE)); } void testChangingPermissionsWithoutEtagChange()