diff --git a/src/csync/csync.h b/src/csync/csync.h index 5c8fbc0978b6b..9da7497f75c7f 100644 --- a/src/csync/csync.h +++ b/src/csync/csync.h @@ -217,6 +217,7 @@ struct OCSYNC_EXPORT csync_file_stat_s { bool is_hidden BITFIELD(1); // Not saved in the DB, only used during discovery for local files. bool isE2eEncrypted BITFIELD(1); bool is_metadata_missing BITFIELD(1); // Indicates the file has missing metadata, f.ex. the file is not a placeholder in case of vfs. + bool isPermissionsInvalid BITFIELD(1); QByteArray path; QByteArray rename_path; @@ -244,6 +245,7 @@ struct OCSYNC_EXPORT csync_file_stat_s { , is_hidden(false) , isE2eEncrypted(false) , is_metadata_missing(false) + , isPermissionsInvalid(false) { } }; diff --git a/src/csync/vio/csync_vio_local_unix.cpp b/src/csync/vio/csync_vio_local_unix.cpp index c5e22abb3c8b0..ec47ab3c7c994 100644 --- a/src/csync/vio/csync_vio_local_unix.cpp +++ b/src/csync/vio/csync_vio_local_unix.cpp @@ -170,5 +170,7 @@ static int _csync_vio_local_stat_mb(const mbchar_t *wuri, csync_file_stat_t *buf buf->inode = sb.st_ino; buf->modtime = sb.st_mtime; buf->size = sb.st_size; + buf->isPermissionsInvalid = (sb.st_mode & S_IWOTH) == S_IWOTH; + return 0; } diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 05bdf1554bc0b..7300a4c5f9a1e 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -1070,6 +1070,10 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( if (_queryLocal != NormalQuery && _queryServer != NormalQuery) recurse = false; + if (localEntry.isPermissionsInvalid) { + recurse = true; + } + if ((item->_direction == SyncFileItem::Down || item->_instruction == CSYNC_INSTRUCTION_CONFLICT || item->_instruction == CSYNC_INSTRUCTION_NEW || item->_instruction == CSYNC_INSTRUCTION_SYNC) && (item->_modtime <= 0 || item->_modtime >= 0xFFFFFFFF)) { item->_instruction = CSYNC_INSTRUCTION_ERROR; @@ -1097,6 +1101,13 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( } } + if (localEntry.isPermissionsInvalid && item->_instruction == CSyncEnums::CSYNC_INSTRUCTION_NONE) { + item->_instruction = CSYNC_INSTRUCTION_UPDATE_METADATA; + item->_direction = SyncFileItem::Down; + } + + item->isPermissionsInvalid = localEntry.isPermissionsInvalid; + auto recurseQueryLocal = _queryLocal == ParentNotChanged ? ParentNotChanged : localEntry.isDirectory || item->_instruction == CSYNC_INSTRUCTION_RENAME ? NormalQuery : ParentDontExist; processFileFinalize(item, path, recurse, recurseQueryLocal, recurseQueryServer); }; diff --git a/src/libsync/discoveryphase.cpp b/src/libsync/discoveryphase.cpp index 3ca34e94f50fb..6cd226f2ee1a5 100644 --- a/src/libsync/discoveryphase.cpp +++ b/src/libsync/discoveryphase.cpp @@ -348,6 +348,7 @@ void DiscoverySingleLocalDirectoryJob::run() { i.isSymLink = dirent->type == ItemTypeSoftLink; i.isVirtualFile = dirent->type == ItemTypeVirtualFile || dirent->type == ItemTypeVirtualFileDownload; i.isMetadataMissing = dirent->is_metadata_missing; + i.isPermissionsInvalid = dirent->isPermissionsInvalid; i.type = dirent->type; results.push_back(i); } diff --git a/src/libsync/discoveryphase.h b/src/libsync/discoveryphase.h index 2e801de34cd53..bb932f568b4db 100644 --- a/src/libsync/discoveryphase.h +++ b/src/libsync/discoveryphase.h @@ -106,6 +106,7 @@ struct LocalInfo bool isVirtualFile = false; bool isSymLink = false; bool isMetadataMissing = false; + bool isPermissionsInvalid = false; [[nodiscard]] bool isValid() const { return !name.isNull(); } }; diff --git a/src/libsync/filesystem.cpp b/src/libsync/filesystem.cpp index 2931a3d877c7e..9dbdde6a7474c 100644 --- a/src/libsync/filesystem.cpp +++ b/src/libsync/filesystem.cpp @@ -468,6 +468,7 @@ bool FileSystem::setFolderPermissions(const QString &path, case OCC::FileSystem::FolderPermissions::ReadOnly: break; case OCC::FileSystem::FolderPermissions::ReadWrite: + std::filesystem::permissions(stdStrPath, std::filesystem::perms::others_write, std::filesystem::perm_options::remove); std::filesystem::permissions(stdStrPath, std::filesystem::perms::owner_write, std::filesystem::perm_options::add); break; } diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index c8b85657bb0c8..ceb41fd84d021 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -1457,21 +1457,13 @@ 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))) { FileSystem::setFolderPermissions(propagator()->fullLocalPath(_item->_file), FileSystem::FolderPermissions::ReadOnly); - qCDebug(lcDirectory) << "old permissions" << static_cast(std::filesystem::status(propagator()->fullLocalPath(_item->_file).toStdWString()).permissions()); - std::filesystem::permissions(propagator()->fullLocalPath(_item->_file).toStdWString(), std::filesystem::perms::owner_write | std::filesystem::perms::group_write | std::filesystem::perms::others_write, std::filesystem::perm_options::remove); - qCDebug(lcDirectory) << "new permissions" << static_cast(std::filesystem::status(propagator()->fullLocalPath(_item->_file).toStdWString()).permissions()); } if (!_item->_renameTarget.isEmpty() && FileSystem::fileExists(propagator()->fullLocalPath(_item->_renameTarget))) { FileSystem::setFolderPermissions(propagator()->fullLocalPath(_item->_renameTarget), FileSystem::FolderPermissions::ReadOnly); - qCDebug(lcDirectory) << "old permissions" << static_cast(std::filesystem::status(propagator()->fullLocalPath(_item->_renameTarget).toStdWString()).permissions()); - std::filesystem::permissions(propagator()->fullLocalPath(_item->_renameTarget).toStdWString(), std::filesystem::perms::owner_write | std::filesystem::perms::group_write | std::filesystem::perms::others_write, std::filesystem::perm_options::remove); - qCDebug(lcDirectory) << "new permissions" << static_cast(std::filesystem::status(propagator()->fullLocalPath(_item->_renameTarget).toStdWString()).permissions()); } } catch (const std::filesystem::filesystem_error &e) @@ -1480,23 +1472,20 @@ 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); - qCDebug(lcDirectory) << "old permissions" << static_cast(std::filesystem::status(propagator()->fullLocalPath(_item->_file).toStdWString()).permissions()); - std::filesystem::permissions(propagator()->fullLocalPath(_item->_file).toStdWString(), std::filesystem::perms::owner_write, std::filesystem::perm_options::add); - qCDebug(lcDirectory) << "new permissions" << static_cast(std::filesystem::status(propagator()->fullLocalPath(_item->_file).toStdWString()).permissions()); + const auto permissionsChangeHelper = [] (const auto fileName) + { + qCDebug(lcDirectory) << fileName << "permissions changed: old permissions" << static_cast(std::filesystem::status(fileName.toStdWString()).permissions()); + FileSystem::setFolderPermissions(fileName, FileSystem::FolderPermissions::ReadWrite); + qCDebug(lcDirectory) << fileName << "applied new permissions" << static_cast(std::filesystem::status(fileName.toStdWString()).permissions()); + }; + + if (const auto fileName = propagator()->fullLocalPath(_item->_file); FileSystem::fileExists(fileName)) { + permissionsChangeHelper(fileName); } - if (!_item->_renameTarget.isEmpty() && FileSystem::fileExists(propagator()->fullLocalPath(_item->_renameTarget))) { - FileSystem::setFolderPermissions(propagator()->fullLocalPath(_item->_renameTarget), FileSystem::FolderPermissions::ReadWrite); - qCDebug(lcDirectory) << "old permissions" << static_cast(std::filesystem::status(propagator()->fullLocalPath(_item->_renameTarget).toStdWString()).permissions()); - std::filesystem::permissions(propagator()->fullLocalPath(_item->_renameTarget).toStdWString(), std::filesystem::perms::owner_write, std::filesystem::perm_options::add); - qCDebug(lcDirectory) << "new permissions" << static_cast(std::filesystem::status(propagator()->fullLocalPath(_item->_renameTarget).toStdWString()).permissions()); + if (const auto fileName = propagator()->fullLocalPath(_item->_renameTarget); !_item->_renameTarget.isEmpty() && FileSystem::fileExists(fileName)) { + permissionsChangeHelper(fileName); } } catch (const std::filesystem::filesystem_error &e) diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index ac48c0be2c2cf..0f8691cd05960 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -363,6 +363,10 @@ void OCC::SyncEngine::slotItemDiscovered(const OCC::SyncFileItemPtr &item) const bool isReadOnly = !item->_remotePerm.isNull() && !item->_remotePerm.hasPermission(RemotePermissions::CanWrite); modificationHappened = FileSystem::setFileReadOnlyWeak(filePath, isReadOnly); } + if (item->isPermissionsInvalid) { + const auto isReadOnly = !item->_remotePerm.isNull() && !item->_remotePerm.hasPermission(RemotePermissions::CanWrite); + FileSystem::setFileReadOnly(filePath, isReadOnly); + } modificationHappened |= item->_size != prev._fileSize; diff --git a/src/libsync/syncfileitem.h b/src/libsync/syncfileitem.h index d90348af4ebda..46ee49621c688 100644 --- a/src/libsync/syncfileitem.h +++ b/src/libsync/syncfileitem.h @@ -343,6 +343,8 @@ class OWNCLOUDSYNC_EXPORT SyncFileItem bool _isLivePhoto = false; QString _livePhotoFile; + bool isPermissionsInvalid = false; + QString _discoveryResult; }; diff --git a/test/testpermissions.cpp b/test/testpermissions.cpp index f334b22de5abc..a090d3b6fe141 100644 --- 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 &folders, const QString &localPath, std::function 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(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(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');