Skip to content

Commit

Permalink
ensure folder permissions are read-only when needed
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@mgallien
mgallien committed Nov 21, 2024
1 parent 0208ec6 commit 4349b4a
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 67 deletions.
8 changes: 1 addition & 7 deletions src/libsync/owncloudpropagator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))) {
Expand All @@ -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);
Expand Down
63 changes: 3 additions & 60 deletions test/testpermissions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{}};
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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');
Expand Down

0 comments on commit 4349b4a

Please sign in to comment.