ensure folder permissions are read-only when needed

a folder item must be set read-only if files or sub-folders cannot be
created

a folder item must be set read-write in all other situations

Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>

diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp
index c8b85657bb0c87cae475f92b5f623b424a32f7f8..193da94df3e9a75fc100c48187c79253cc45cc0f 100644 (file)
--- a/src/libsync/owncloudpropagator.cpp
+++ b/src/libsync/owncloudpropagator.cpp
@@ -1457,8 +1457,6 @@ void PropagateDirectory::slotSubJobsFinished(SyncFileItem::Status status)
 #if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15
             if (!_item->_remotePerm.isNull() &&
                 !_item->_remotePerm.hasPermission(RemotePermissions::CanAddFile) &&
-                !_item->_remotePerm.hasPermission(RemotePermissions::CanRename) &&
-                !_item->_remotePerm.hasPermission(RemotePermissions::CanMove) &&
                 !_item->_remotePerm.hasPermission(RemotePermissions::CanAddSubDirectories)) {
                 try {
                     if (FileSystem::fileExists(propagator()->fullLocalPath(_item->_file))) {
@@ -1480,11 +1478,7 @@ void PropagateDirectory::slotSubJobsFinished(SyncFileItem::Status status)
                     _item->_status = SyncFileItem::NormalError;
                     _item->_errorString = tr("The folder %1 cannot be made read-only: %2").arg(_item->_file, e.what());
                 }
-            } else if (!_item->_remotePerm.isNull() &&
-                       (_item->_remotePerm.hasPermission(RemotePermissions::CanAddFile) ||
-                        !_item->_remotePerm.hasPermission(RemotePermissions::CanRename) ||
-                        !_item->_remotePerm.hasPermission(RemotePermissions::CanMove) ||
-                        !_item->_remotePerm.hasPermission(RemotePermissions::CanAddSubDirectories))) {
+            } else {
                 try {
                     if (FileSystem::fileExists(propagator()->fullLocalPath(_item->_file))) {
                         FileSystem::setFolderPermissions(propagator()->fullLocalPath(_item->_file), FileSystem::FolderPermissions::ReadWrite);

diff --git a/test/testpermissions.cpp b/test/testpermissions.cpp
index f334b22de5abc7b952315bd4a8ec83316ee03238..a090d3b6fe14139374e4fa096d97fa681e818e25 100644 (file)
--- a/test/testpermissions.cpp
+++ b/test/testpermissions.cpp
@@ -564,63 +564,6 @@ private slots:
         QVERIFY(cls.find("zallowed/sub2/file"));
     }
 
-    // Test for issue #7293
-    void testAllowedMoveForbiddenDelete() {
-         FakeFolder fakeFolder{FileInfo{}};
-
-        QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveRemnantsReadOnlyFolders,
-                         [&](const QList<SyncFileItemPtr> &folders, const QString &localPath, std::function<void(bool)> callback) {
-                             for(const auto &oneFolder : folders) {
-                                 FileSystem::removeRecursively(localPath + oneFolder->_file);
-                             }
-                             callback(false);
-                         });
-
-        // Some of this test depends on the order of discovery. With threading
-        // that order becomes effectively random, but we want to make sure to test
-        // all cases and thus disable threading.
-        auto syncOpts = fakeFolder.syncEngine().syncOptions();
-        syncOpts._parallelNetworkJobs = 1;
-        fakeFolder.syncEngine().setSyncOptions(syncOpts);
-
-        auto &lm = fakeFolder.localModifier();
-        auto &rm = fakeFolder.remoteModifier();
-        rm.mkdir("changeonly");
-        rm.mkdir("changeonly/sub1");
-        rm.insert("changeonly/sub1/file1");
-        rm.insert("changeonly/sub1/filetorname1a");
-        rm.insert("changeonly/sub1/filetorname1z");
-        rm.mkdir("changeonly/sub2");
-        rm.insert("changeonly/sub2/file2");
-        rm.insert("changeonly/sub2/filetorname2a");
-        rm.insert("changeonly/sub2/filetorname2z");
-
-        setAllPerm(rm.find("changeonly"), RemotePermissions::fromServerString("NSV"));
-
-        QVERIFY(fakeFolder.syncOnce());
-
-        lm.rename("changeonly/sub1/filetorname1a", "changeonly/sub1/aaa1_renamed");
-        lm.rename("changeonly/sub1/filetorname1z", "changeonly/sub1/zzz1_renamed");
-
-        lm.rename("changeonly/sub2/filetorname2a", "changeonly/sub2/aaa2_renamed");
-        lm.rename("changeonly/sub2/filetorname2z", "changeonly/sub2/zzz2_renamed");
-
-        auto expectedState = fakeFolder.currentLocalState();
-
-        QVERIFY(fakeFolder.syncOnce());
-        QCOMPARE(fakeFolder.currentLocalState(), expectedState);
-        QCOMPARE(fakeFolder.currentRemoteState(), expectedState);
-
-        lm.rename("changeonly/sub1", "changeonly/aaa");
-        lm.rename("changeonly/sub2", "changeonly/zzz");
-
-        expectedState = fakeFolder.currentLocalState();
-
-        QVERIFY(fakeFolder.syncOnce());
-        QCOMPARE(fakeFolder.currentLocalState(), expectedState);
-        QCOMPARE(fakeFolder.currentRemoteState(), expectedState);
-    }
-
     void testParentMoveNotAllowedChildrenRestored()
     {
         FakeFolder fakeFolder{FileInfo{}};
@@ -722,7 +665,7 @@ private slots:
 
         remote.mkdir("readWriteFolder");
 
-        remote.find("readWriteFolder")->permissions = RemotePermissions::fromServerString("WDNVRSM");
+        remote.find("readWriteFolder")->permissions = RemotePermissions::fromServerString("CKWDNVRSM");
 
         QVERIFY(fakeFolder.syncOnce());
         QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
@@ -757,7 +700,7 @@ private slots:
         QVERIFY(folderStatus.permissions() & std::filesystem::perms::owner_read);
         QVERIFY(!static_cast<bool>(folderStatus.permissions() & std::filesystem::perms::owner_write));
 
-        remote.find("testFolder")->permissions = RemotePermissions::fromServerString("WDNVRSM");
+        remote.find("testFolder")->permissions = RemotePermissions::fromServerString("CKWDNVRSM");
 
         QVERIFY(fakeFolder.syncOnce());
         QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
@@ -815,7 +758,7 @@ private slots:
         QVERIFY(subFolderReadOnlyStatus.permissions() & std::filesystem::perms::owner_read);
         QVERIFY(!static_cast<bool>(subFolderReadOnlyStatus.permissions() & std::filesystem::perms::owner_write));
 
-        remote.find("testFolder/subFolderReadOnly")->permissions = RemotePermissions::fromServerString("WDNVRSm");
+        remote.find("testFolder/subFolderReadOnly")->permissions = RemotePermissions::fromServerString("CKWDNVRSm");
         remote.find("testFolder/subFolderReadWrite")->permissions = RemotePermissions::fromServerString("m");
         remote.mkdir("testFolder/newSubFolder");
         remote.create("testFolder/testFile", 12, '9');