diff --git a/src/libsync/filesystem.cpp b/src/libsync/filesystem.cpp index b8eb2d2f8b..7fa9acb75b 100644 --- a/src/libsync/filesystem.cpp +++ b/src/libsync/filesystem.cpp @@ -345,8 +345,15 @@ bool FileSystem::getInode(const QString &filename, quint64 *inode) } bool FileSystem::setFolderPermissions(const QString &path, - FileSystem::FolderPermissions permissions) noexcept + FileSystem::FolderPermissions permissions, + bool * const permissionsChanged) noexcept { + bool permissionsDidChange = false; + + if (permissionsChanged) { + *permissionsChanged = false; + } + #ifdef Q_OS_WIN SECURITY_INFORMATION info = DACL_SECURITY_INFORMATION; std::unique_ptr securityDescriptor; @@ -476,18 +483,47 @@ bool FileSystem::setFolderPermissions(const QString &path, qCWarning(lcFileSystem) << "error when calling SetFileSecurityW" << QDir::toNativeSeparators(path) << GetLastError(); return false; } + + permissionsDidChange = true; #else static constexpr auto writePerms = std::filesystem::perms::owner_write | std::filesystem::perms::group_write | std::filesystem::perms::others_write; const auto stdStrPath = path.toStdWString(); + + const auto currentPermissions = std::filesystem::status(stdStrPath).permissions(); + qCDebug(lcFileSystem()).nospace() << "current permissions path=" << path << " perms=" << Qt::showbase << Qt::oct << static_cast(currentPermissions); + try { switch (permissions) { - case OCC::FileSystem::FolderPermissions::ReadOnly: - std::filesystem::permissions(stdStrPath, writePerms, std::filesystem::perm_options::remove); + case OCC::FileSystem::FolderPermissions::ReadOnly: { + qCDebug(lcFileSystem()).nospace() << "ensuring folder is read only path=" << path; + + if ((currentPermissions & writePerms) != std::filesystem::perms::none) { + qCDebug(lcFileSystem()).nospace() << "removing owner/group/others write permissions path=" << path; + std::filesystem::permissions(stdStrPath, writePerms, std::filesystem::perm_options::remove); + permissionsDidChange = true; + } + break; - case OCC::FileSystem::FolderPermissions::ReadWrite: + } + case OCC::FileSystem::FolderPermissions::ReadWrite: { + qCDebug(lcFileSystem()).nospace() << "ensuring folder is read/writable path=" << path; + + if ((currentPermissions & std::filesystem::perms::others_write) != std::filesystem::perms::none) { + qCDebug(lcFileSystem()).nospace() << "removing others write permissions path=" << path; + std::filesystem::permissions(stdStrPath, std::filesystem::perms::others_write, std::filesystem::perm_options::remove); + permissionsDidChange = true; + } + + if ((currentPermissions & std::filesystem::perms::owner_write) == std::filesystem::perms::none) { + qCDebug(lcFileSystem()).nospace() << "adding owner write permissions path=" << path; + std::filesystem::permissions(stdStrPath, std::filesystem::perms::owner_write, std::filesystem::perm_options::add); + permissionsDidChange = true; + } + break; } + } } catch (const std::filesystem::filesystem_error &e) { @@ -505,34 +541,16 @@ bool FileSystem::setFolderPermissions(const QString &path, return false; } - try - { - switch (permissions) { - 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; - } - } - catch (const std::filesystem::filesystem_error &e) - { - qCWarning(lcFileSystem()) << "exception when modifying folder permissions" << e.what() << e.path1().c_str() << e.path2().c_str(); - return false; - } - catch (const std::system_error &e) - { - qCWarning(lcFileSystem()) << "exception when modifying folder permissions" << e.what() << "- path:" << stdStrPath; - return false; - } - catch (...) - { - qCWarning(lcFileSystem()) << "exception when modifying folder permissions - path:" << stdStrPath; - return false; + if (permissionsDidChange) { + const auto newPermissions = std::filesystem::status(stdStrPath).permissions(); + qCDebug(lcFileSystem()).nospace() << "updated permissions path=" << path << " perms=" << Qt::showbase << Qt::oct << static_cast(newPermissions); } #endif + if (permissionsChanged) { + *permissionsChanged = permissionsDidChange; + } + return true; } diff --git a/src/libsync/filesystem.h b/src/libsync/filesystem.h index 0f04635692..7edc62c8b3 100644 --- a/src/libsync/filesystem.h +++ b/src/libsync/filesystem.h @@ -119,7 +119,8 @@ namespace FileSystem { const std::function &onError = nullptr); bool OWNCLOUDSYNC_EXPORT setFolderPermissions(const QString &path, - FileSystem::FolderPermissions permissions) noexcept; + FileSystem::FolderPermissions permissions, + bool *permissionsChanged = nullptr) noexcept; bool OWNCLOUDSYNC_EXPORT isFolderReadOnly(const std::filesystem::path &path) noexcept; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index e4e9027a67..92f7365806 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -99,6 +99,8 @@ nextcloud_add_test(SyncConflictsModel) nextcloud_add_test(DateFieldBackend) nextcloud_add_test(ClientStatusReporting) +nextcloud_add_test(FileSystem) + nextcloud_add_test(FolderStatusModel) target_link_libraries(SecureFileDropTest PRIVATE Nextcloud::sync) diff --git a/test/testfilesystem.cpp b/test/testfilesystem.cpp new file mode 100644 index 0000000000..50ba31984c --- /dev/null +++ b/test/testfilesystem.cpp @@ -0,0 +1,133 @@ +/* + * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: CC0-1.0 + * + * This software is in the public domain, furnished "as is", without technical + * support, and with no warranty, express or implied, as to its usefulness for + * any purpose. + */ + +#include +#include + +#include "common/filesystembase.h" +#include "logger.h" + +#include "libsync/filesystem.h" + +using namespace OCC; +namespace std_fs = std::filesystem; + +class TestFileSystem : public QObject +{ + Q_OBJECT + +private: + QTemporaryDir testDir; + +private Q_SLOTS: + void initTestCase() + { + OCC::Logger::instance()->setLogFlush(true); + OCC::Logger::instance()->setLogDebug(true); + + QStandardPaths::setTestModeEnabled(true); + + QDir dir(testDir.path()); + dir.mkdir("existingDirectory"); + } + +#ifndef Q_OS_WIN + void testSetFolderPermissionsExistingDirectory_data() + { + constexpr auto perms_0555 = + std_fs::perms::owner_read | std_fs::perms::owner_exec + | std_fs::perms::group_read | std_fs::perms::group_exec + | std_fs::perms::others_read | std_fs::perms::others_exec; + constexpr auto perms_0755 = perms_0555 | std_fs::perms::owner_write; + constexpr auto perms_0775 = perms_0755 | std_fs::perms::group_write; + + QTest::addColumn("originalPermissions"); + QTest::addColumn("folderPermissions"); + QTest::addColumn("expectedResult"); + QTest::addColumn("expectedPermissionsChanged"); + QTest::addColumn("expectedPermissions"); + + QTest::newRow("0777, readonly -> 0555, changed") + << std_fs::perms::all + << FileSystem::FolderPermissions::ReadOnly + << true + << true + << perms_0555; + + QTest::newRow("0555, readonly -> 0555, not changed") + << perms_0555 + << FileSystem::FolderPermissions::ReadOnly + << true + << false + << perms_0555; + + QTest::newRow("0777, readwrite -> 0775, changed") + << std_fs::perms::all + << FileSystem::FolderPermissions::ReadWrite + << true + << true + << perms_0775; + + QTest::newRow("0775, readwrite -> 0775, not changed") + << perms_0775 + << FileSystem::FolderPermissions::ReadWrite + << true + << false + << perms_0775; + + QTest::newRow("0755, readwrite -> 0755, not changed") + << perms_0755 + << FileSystem::FolderPermissions::ReadWrite + << true + << false + << perms_0755; + + QTest::newRow("0555, readwrite -> 0755, changed") + << perms_0555 + << FileSystem::FolderPermissions::ReadWrite + << true + << true + << perms_0755; + } + + void testSetFolderPermissionsExistingDirectory() + { + QFETCH(std_fs::perms, originalPermissions); + QFETCH(FileSystem::FolderPermissions, folderPermissions); + QFETCH(bool, expectedResult); + QFETCH(bool, expectedPermissionsChanged); + QFETCH(std_fs::perms, expectedPermissions); + + bool permissionsDidChange = false; + QString fullPath = testDir.filePath("existingDirectory"); + const auto stdStrPath = fullPath.toStdWString(); + + std_fs::permissions(stdStrPath, originalPermissions); + + QCOMPARE(FileSystem::setFolderPermissions(fullPath, folderPermissions, &permissionsDidChange), expectedResult); + + const auto newPermissions = std_fs::status(stdStrPath).permissions(); + QCOMPARE(newPermissions, expectedPermissions); + QCOMPARE(permissionsDidChange, expectedPermissionsChanged); + } + + void testSetFolderPermissionsNonexistentDirectory() + { + bool permissionsDidChange = false; + + QString fullPath = testDir.filePath("nonexistentDirectory"); + + QCOMPARE(FileSystem::setFolderPermissions("nonexistentDirectory", FileSystem::FolderPermissions::ReadOnly, &permissionsDidChange), false); + QCOMPARE(permissionsDidChange, false); + } +#endif +}; + +QTEST_GUILESS_MAIN(TestFileSystem) +#include "testfilesystem.moc"