diff --git a/include/multipass/platform.h b/include/multipass/platform.h index c5e42e4191..6600bdc361 100644 --- a/include/multipass/platform.h +++ b/include/multipass/platform.h @@ -58,7 +58,9 @@ class Platform : public Singleton virtual bool is_remote_supported(const std::string& remote) const; virtual bool is_backend_supported(const QString& backend) const; // temporary (?) virtual int chown(const char* path, unsigned int uid, unsigned int gid) const; - virtual bool set_permissions(const Path& path, const Perms permissions) const; + virtual bool set_permissions(const Path& path, const Perms permissions, bool try_inherit = false) const; + virtual bool take_ownership(const Path& path) const; + virtual void setup_permission_inheritance(bool restricted = true) const; virtual bool link(const char* target, const char* link) const; virtual bool symlink(const char* target, const char* link, bool is_dir) const; virtual int utime(const char* path, int atime, int mtime) const; diff --git a/include/multipass/utils.h b/include/multipass/utils.h index df65dc31d9..2ffa4d5368 100644 --- a/include/multipass/utils.h +++ b/include/multipass/utils.h @@ -207,9 +207,8 @@ class Utils : public Singleton virtual std::string contents_of(const multipass::Path& file_path) const; virtual void make_file_with_content(const std::string& file_name, const std::string& content, const bool& overwrite = false); - virtual Path make_dir(const QDir& a_dir, const QString& name, - QFileDevice::Permissions permissions = QFileDevice::Permissions(0)); - virtual Path make_dir(const QDir& dir, QFileDevice::Permissions permissions = QFileDevice::Permissions(0)); + virtual Path make_dir(const QDir& a_dir, const QString& name, QFileDevice::Permissions permissions = {0}); + virtual Path make_dir(const QDir& dir, QFileDevice::Permissions permissions = {0}); // command and process helpers virtual std::string run_cmd_for_output(const QString& cmd, const QStringList& args, diff --git a/include/multipass/utils/permission_utils.h b/include/multipass/utils/permission_utils.h new file mode 100644 index 0000000000..95b0d47a4c --- /dev/null +++ b/include/multipass/utils/permission_utils.h @@ -0,0 +1,45 @@ +/* + * Copyright (C) Canonical, Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 3. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +#ifndef MULTIPASS_PERMISSION_UTILS_H +#define MULTIPASS_PERMISSION_UTILS_H + +#include + +#include +#include + +#define MP_PERMISSIONS multipass::PermissionUtils::instance() + +namespace multipass +{ +namespace fs = std::filesystem; + +class PermissionUtils : public Singleton +{ +public: + PermissionUtils(const PrivatePass&) noexcept; + + virtual void set_permissions(const fs::path& path, const QFileDevice::Permissions& permissions) const; + virtual void take_ownership(const fs::path& path) const; + + // sets owner to root and sets permissions such that only owner has access. + virtual void restrict_permissions(const fs::path& path) const; +}; +} // namespace multipass + +#endif // MULTIPASS_PERMISSION_UTILS_H diff --git a/src/cert/client_cert_store.cpp b/src/cert/client_cert_store.cpp index 52dade2a2b..1b9591f32d 100644 --- a/src/cert/client_cert_store.cpp +++ b/src/cert/client_cert_store.cpp @@ -76,8 +76,6 @@ void mp::ClientCertStore::add_cert(const std::string& pem_cert) if (!MP_FILEOPS.open(file, QIODevice::WriteOnly)) throw std::runtime_error("failed to create file to store certificate"); - file.setPermissions(QFile::ReadOwner | QFile::WriteOwner); - // QIODevice::Append is not supported in QSaveFile, so must write out all of the // existing clients certs each time. for (const auto& saved_cert : authenticated_client_certs) diff --git a/src/daemon/daemon_config.cpp b/src/daemon/daemon_config.cpp index 1df1e5bcba..93679577fe 100644 --- a/src/daemon/daemon_config.cpp +++ b/src/daemon/daemon_config.cpp @@ -31,6 +31,7 @@ #include #include #include +#include #include #include @@ -108,9 +109,11 @@ std::unique_ptr mp::DaemonConfigBuilder::build() auto multiplexing_logger = std::make_shared(std::move(logger)); mpl::set_logger(multiplexing_logger); + MP_PLATFORM.setup_permission_inheritance(); + auto storage_path = MP_PLATFORM.multipass_storage_location(); if (!storage_path.isEmpty()) - MP_UTILS.make_dir(storage_path, QFileDevice::ReadOwner | QFileDevice::WriteOwner | QFileDevice::ExeOwner); + MP_UTILS.make_dir(storage_path); if (cache_directory.isEmpty()) { @@ -189,6 +192,17 @@ std::unique_ptr mp::DaemonConfigBuilder::build() std::make_unique(url_downloader.get(), cache_directory, manifest_ttl); } + // restrict permissions for all existing files and folders + if (!storage_path.isEmpty()) + { + MP_PERMISSIONS.restrict_permissions(storage_path.toStdU16String()); + } + else + { + MP_PERMISSIONS.restrict_permissions(data_directory.toStdU16String()); + MP_PERMISSIONS.restrict_permissions(cache_directory.toStdU16String()); + } + return std::unique_ptr(new DaemonConfig{ std::move(url_downloader), std::move(factory), std::move(image_hosts), std::move(vault), std::move(name_generator), std::move(ssh_key_provider), std::move(cert_provider), std::move(client_cert_store), diff --git a/src/platform/platform_unix.cpp b/src/platform/platform_unix.cpp index c0c87e74ba..761932f9f4 100644 --- a/src/platform/platform_unix.cpp +++ b/src/platform/platform_unix.cpp @@ -58,11 +58,31 @@ int mp::platform::Platform::chown(const char* path, unsigned int uid, unsigned i return ::lchown(path, uid, gid); } -bool mp::platform::Platform::set_permissions(const Path& path, const Perms permissions) const +bool mp::platform::Platform::set_permissions(const Path& path, const Perms permissions, bool try_inherit) const { + // try_inherit is ignored on unix since inheritance is global return QFile::setPermissions(path, permissions); } +bool mp::platform::Platform::take_ownership(const Path& path) const +{ + return this->chown(path.toStdString().c_str(), 0, 0) == 0; +} + +void mp::platform::Platform::setup_permission_inheritance(bool restricted) const +{ + if (restricted) + { + // only user can read/write/execute + ::umask(~(S_IRUSR | S_IWUSR | S_IXUSR)); + } + else + { + // typical default umask permissions + ::umask(S_IWGRP | S_IWOTH); + } +} + bool mp::platform::Platform::symlink(const char* target, const char* link, bool is_dir) const { return ::symlink(target, link) == 0; diff --git a/src/sshfs_mount/sshfs_server.cpp b/src/sshfs_mount/sshfs_server.cpp index 8eb92a5478..46518bf91c 100644 --- a/src/sshfs_mount/sshfs_server.cpp +++ b/src/sshfs_mount/sshfs_server.cpp @@ -103,6 +103,8 @@ int main(int argc, char* argv[]) auto standard_logger = std::make_shared(std::move(logger)); mpl::set_logger(standard_logger); + MP_PLATFORM.setup_permission_inheritance(false); + try { auto watchdog = mpp::make_quit_watchdog(); // called while there is only one thread diff --git a/src/utils/CMakeLists.txt b/src/utils/CMakeLists.txt index f0b501c22b..006c198678 100644 --- a/src/utils/CMakeLists.txt +++ b/src/utils/CMakeLists.txt @@ -16,6 +16,7 @@ function(add_target TARGET_NAME) add_library(${TARGET_NAME} STATIC file_ops.cpp memory_size.cpp + permission_utils.cpp json_utils.cpp snap_utils.cpp standard_paths.cpp diff --git a/src/utils/permission_utils.cpp b/src/utils/permission_utils.cpp new file mode 100644 index 0000000000..c868991c6e --- /dev/null +++ b/src/utils/permission_utils.cpp @@ -0,0 +1,106 @@ +/* + * Copyright (C) Canonical, Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 3. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +#include + +#include +#include + +namespace mp = multipass; +namespace fs = mp::fs; + +namespace +{ +void set_single_permissions(const fs::path& path, const QFileDevice::Permissions& permissions, bool try_inherit) +{ + QString qpath = QString::fromUtf8(path.u8string()); + + if (!MP_PLATFORM.set_permissions(qpath, permissions, try_inherit)) + throw std::runtime_error(fmt::format("Cannot set permissions for '{}'", path.string())); +} + +void set_single_owner(const fs::path& path) +{ + QString qpath = QString::fromUtf8(path.u8string()); + + if (!MP_PLATFORM.take_ownership(qpath)) + throw std::runtime_error(fmt::format("Cannot set owner for '{}'", path.string())); +} + +// only exists because MP_FILEOPS doesn't overload the throwing variaions of std::filesystem functions +void throw_if_error(const fs::path& path, const std::error_code& ec) +{ + if (ec) + throw std::system_error( + ec, + fmt::format("System error occurred while handling permissions for '{}'", path.string())); +} + +// recursively iterates over all files in folder and applies a function that takes a path +template +void apply_on_files(const fs::path& path, Func&& func) +{ + std::error_code ec{}; + if (!MP_FILEOPS.exists(path, ec) || ec) + throw std::runtime_error(fmt::format("Cannot handle permissions for nonexistent file '{}'", path.string())); + + func(path, true); + + // iterate over children of directory + if (MP_FILEOPS.is_directory(path, ec)) + { + auto dir_iterator = MP_FILEOPS.recursive_dir_iterator(path, ec); + throw_if_error(path, ec); + + if (!dir_iterator) [[unlikely]] + throw std::runtime_error(fmt::format("Cannot iterate over directory '{}'", path.string())); + + while (dir_iterator->hasNext()) + { + const auto& entry = dir_iterator->next(); + + func(entry.path(), false); + } + } + + throw_if_error(path, ec); +} +} // namespace + +mp::PermissionUtils::PermissionUtils(const PrivatePass& pass) noexcept : Singleton{pass} +{ +} + +void mp::PermissionUtils::set_permissions(const fs::path& path, const QFileDevice::Permissions& permissions) const +{ + apply_on_files(path, [&](const fs::path& apply_path, bool root_dir) { + set_single_permissions(apply_path, permissions, !root_dir); + }); +} + +void mp::PermissionUtils::take_ownership(const fs::path& path) const +{ + apply_on_files(path, [&](const fs::path& apply_path, bool) { set_single_owner(apply_path); }); +} + +void mp::PermissionUtils::restrict_permissions(const fs::path& path) const +{ + apply_on_files(path, [&](const fs::path& apply_path, bool root_dir) { + set_single_owner(apply_path); + set_single_permissions(apply_path, QFile::ReadOwner | QFile::WriteOwner | QFile::ExeOwner, !root_dir); + }); +} diff --git a/src/utils/utils.cpp b/src/utils/utils.cpp index 5dacfd8dc7..2b73926517 100644 --- a/src/utils/utils.cpp +++ b/src/utils/utils.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -303,7 +304,7 @@ mp::Path mp::Utils::make_dir(const QDir& a_dir, const QString& name, QFileDevice if (permissions) { - MP_PLATFORM.set_permissions(dir_path, permissions); + MP_PERMISSIONS.set_permissions(dir_path.toStdU16String(), permissions); } return dir_path; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index b201a931ae..a1e24a761d 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -124,6 +124,7 @@ add_executable(multipass_tests test_sftp_utils.cpp test_file_ops.cpp test_recursive_dir_iter.cpp + test_permission_utils.cpp ) target_include_directories(multipass_tests diff --git a/tests/linux/test_platform_linux.cpp b/tests/linux/test_platform_linux.cpp index b277ac5c48..3de730a06e 100644 --- a/tests/linux/test_platform_linux.cpp +++ b/tests/linux/test_platform_linux.cpp @@ -632,7 +632,7 @@ TEST_F(PlatformLinux, create_alias_script_overwrites) EXPECT_CALL(*mock_utils, make_file_with_content(_, _, true)).Times(1); EXPECT_CALL(*mock_file_ops, permissions(_)).WillOnce(Return(QFileDevice::ReadOwner | QFileDevice::WriteOwner)); - EXPECT_CALL(*mock_platform, set_permissions(_, _)).WillOnce(Return(true)); + EXPECT_CALL(*mock_platform, set_permissions(_, _, _)).WillOnce(Return(true)); // Calls the platform function directly since MP_PLATFORM is mocked. EXPECT_NO_THROW( @@ -672,7 +672,7 @@ TEST_F(PlatformLinux, create_alias_script_throws_if_cannot_set_permissions) EXPECT_CALL(*mock_utils, make_file_with_content(_, _, true)).Times(1); EXPECT_CALL(*mock_file_ops, permissions(_)).WillOnce(Return(QFileDevice::ReadOwner | QFileDevice::WriteOwner)); - EXPECT_CALL(*mock_platform, set_permissions(_, _)).WillOnce(Return(false)); + EXPECT_CALL(*mock_platform, set_permissions(_, _, _)).WillOnce(Return(false)); MP_EXPECT_THROW_THAT( mock_platform->Platform::create_alias_script("alias_name", mp::AliasDefinition{"instance", "command", "map"}), diff --git a/tests/mock_permission_utils.h b/tests/mock_permission_utils.h new file mode 100644 index 0000000000..ec95c568d3 --- /dev/null +++ b/tests/mock_permission_utils.h @@ -0,0 +1,47 @@ +/* + * Copyright (C) Canonical, Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 3. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +#ifndef MULTIPASS_MOCK_PERMISSION_UTILS_H +#define MULTIPASS_MOCK_PERMISSION_UTILS_H + +#include "common.h" +#include "mock_singleton_helpers.h" + +#include + +namespace multipass::test +{ + +class MockPermissionUtils : public PermissionUtils +{ +public: + MockPermissionUtils(const PrivatePass& pass) : PermissionUtils(pass) + { + } + + MOCK_METHOD(void, + set_permissions, + (const fs::path& path, const QFileDevice::Permissions& permissions), + (const, override)); + MOCK_METHOD(void, take_ownership, (const fs::path& path), (const, override)); + MOCK_METHOD(void, restrict_permissions, (const fs::path& path), (const, override)); + + MP_MOCK_SINGLETON_BOILERPLATE(MockPermissionUtils, PermissionUtils); +}; +} // namespace multipass::test + +#endif // MULTIPASS_MOCK_PERMISSION_UTILS_H diff --git a/tests/mock_platform.h b/tests/mock_platform.h index 30baa9fe92..902cff84ad 100644 --- a/tests/mock_platform.h +++ b/tests/mock_platform.h @@ -40,8 +40,10 @@ class MockPlatform : public platform::Platform MOCK_METHOD(bool, is_remote_supported, (const std::string&), (const, override)); MOCK_METHOD(bool, is_backend_supported, (const QString&), (const, override)); MOCK_METHOD(bool, is_alias_supported, (const std::string&, const std::string&), (const, override)); - MOCK_METHOD(bool, set_permissions, (const multipass::Path&, const QFileDevice::Permissions), (const, override)); MOCK_METHOD(int, chown, (const char*, unsigned int, unsigned int), (const, override)); + MOCK_METHOD(bool, set_permissions, (const Path&, const Perms, bool), (const, override)); + MOCK_METHOD(bool, take_ownership, (const Path&), (const, override)); + MOCK_METHOD(void, setup_permission_inheritance, (bool), (const, override)); MOCK_METHOD(bool, link, (const char*, const char*), (const, override)); MOCK_METHOD(bool, symlink, (const char*, const char*, bool), (const, override)); MOCK_METHOD(int, utime, (const char*, int, int), (const, override)); diff --git a/tests/test_alias_dict.cpp b/tests/test_alias_dict.cpp index b8cb38559d..1a049d0792 100644 --- a/tests/test_alias_dict.cpp +++ b/tests/test_alias_dict.cpp @@ -28,6 +28,7 @@ #include "file_operations.h" #include "json_test_utils.h" #include "mock_file_ops.h" +#include "mock_permission_utils.h" #include "mock_platform.h" #include "mock_settings.h" #include "mock_vm_image_vault.h" @@ -607,6 +608,9 @@ struct DaemonAliasTestsuite mpt::MockSettings::GuardedMock mock_settings_injection = mpt::MockSettings::inject(); mpt::MockSettings& mock_settings = *mock_settings_injection.first; + + const mpt::MockPermissionUtils::GuardedMock mock_permission_utils_injection = + mpt::MockPermissionUtils::inject(); }; TEST_P(DaemonAliasTestsuite, purge_removes_purged_instance_aliases_and_scripts) diff --git a/tests/test_client_common.cpp b/tests/test_client_common.cpp index c02ff554bb..c7eb110a9c 100644 --- a/tests/test_client_common.cpp +++ b/tests/test_client_common.cpp @@ -22,6 +22,7 @@ #include "mock_cert_store.h" #include "mock_client_rpc.h" #include "mock_daemon.h" +#include "mock_permission_utils.h" #include "mock_standard_paths.h" #include "mock_utils.h" #include "stub_terminal.h" @@ -60,6 +61,9 @@ struct TestClientCommon : public mpt::DaemonTestFixture std::make_unique>()}; std::unique_ptr mock_cert_store{std::make_unique()}; + const mpt::MockPermissionUtils::GuardedMock mock_permission_utils_injection = + mpt::MockPermissionUtils::inject(); + const std::string server_address{"localhost:50052"}; mpt::TempDir temp_dir; }; diff --git a/tests/test_daemon.cpp b/tests/test_daemon.cpp index 8f5479ff95..c6d4e8ffbf 100644 --- a/tests/test_daemon.cpp +++ b/tests/test_daemon.cpp @@ -27,6 +27,7 @@ #include "mock_image_host.h" #include "mock_json_utils.h" #include "mock_logger.h" +#include "mock_permission_utils.h" #include "mock_platform.h" #include "mock_server_reader_writer.h" #include "mock_settings.h" @@ -139,6 +140,10 @@ struct Daemon : public mpt::DaemonTestFixture mpt::MockSettings::GuardedMock mock_settings_injection = mpt::MockSettings::inject(); mpt::MockSettings& mock_settings = *mock_settings_injection.first; + const mpt::MockPermissionUtils::GuardedMock mock_permission_utils_injection = + mpt::MockPermissionUtils::inject(); + mpt::MockPermissionUtils& mock_permission_utils = *mock_permission_utils_injection.first; + const mpt::MockJsonUtils::GuardedMock mock_json_utils_injection = mpt::MockJsonUtils::inject(); mpt::MockJsonUtils& mock_json_utils = *mock_json_utils_injection.first; }; @@ -273,18 +278,6 @@ TEST_F(Daemon, proxy_contains_valid_info) EXPECT_THAT(config->network_proxy->port(), port); } -TEST_F(Daemon, daemonAppliesPermissionsToStorageDirectory) -{ - QTemporaryDir storage_dir; - - mpt::SetEnvScope storage(mp::multipass_storage_env_var, storage_dir.path().toUtf8()); - - EXPECT_CALL(mock_platform, multipass_storage_location()).WillOnce(Return(mp::utils::get_multipass_storage())); - EXPECT_CALL(mock_utils, make_dir(_, QFileDevice::ReadOwner | QFileDevice::WriteOwner | QFileDevice::ExeOwner)); - - auto config = config_builder.build(); -} - TEST_F(Daemon, data_path_valid) { QTemporaryDir data_dir, cache_dir; @@ -2519,4 +2512,38 @@ TEST_F(Daemon, info_all_returns_all_instances) mp::Daemon daemon{config_builder.build()}; call_daemon_slot(daemon, &mp::Daemon::info, mp::InfoRequest{}, mock_server); } + +TEST_F(Daemon, sets_permissions_on_provided_storage_path) +{ + const QString path{"Where all the secrets go"}; + const std::filesystem::path std_path{path.toStdU16String()}; + + EXPECT_CALL(mock_platform, multipass_storage_location()).WillOnce(Return(path)); + EXPECT_CALL(mock_permission_utils, restrict_permissions(std_path)); + + config_builder.build(); +} + +TEST_F(Daemon, sets_permissions_on_storage_dirs) +{ + + config_builder.data_directory = "Sensitive data location"; + const std::filesystem::path std_data_path{config_builder.data_directory.toStdU16String()}; + + config_builder.cache_directory = "Pirate's secret cache"; + const std::filesystem::path std_cache_path{config_builder.cache_directory.toStdU16String()}; + + EXPECT_CALL(mock_permission_utils, restrict_permissions(std_data_path)); + EXPECT_CALL(mock_permission_utils, restrict_permissions(std_cache_path)); + + config_builder.build(); +} + +TEST_F(Daemon, sets_up_permission_inheritance) +{ + EXPECT_CALL(mock_platform, setup_permission_inheritance(true)); + + config_builder.build(); +} + } // namespace diff --git a/tests/test_daemon_authenticate.cpp b/tests/test_daemon_authenticate.cpp index 60fd37e6ed..fbbc172fe7 100644 --- a/tests/test_daemon_authenticate.cpp +++ b/tests/test_daemon_authenticate.cpp @@ -17,6 +17,7 @@ #include "common.h" #include "daemon_test_fixture.h" +#include "mock_permission_utils.h" #include "mock_platform.h" #include "mock_server_reader_writer.h" #include "mock_settings.h" @@ -50,6 +51,10 @@ struct TestDaemonAuthenticate : public mpt::DaemonTestFixture mpt::MockSettings::GuardedMock mock_settings_injection = mpt::MockSettings::inject(); mpt::MockSettings& mock_settings = *mock_settings_injection.first; + + const mpt::MockPermissionUtils::GuardedMock mock_permission_utils_injection = + mpt::MockPermissionUtils::inject(); + mpt::MockPermissionUtils& mock_permission_utils = *mock_permission_utils_injection.first; }; } // namespace diff --git a/tests/test_daemon_clone.cpp b/tests/test_daemon_clone.cpp index 55026ece62..f77f0fd615 100644 --- a/tests/test_daemon_clone.cpp +++ b/tests/test_daemon_clone.cpp @@ -17,6 +17,7 @@ #include "common.h" #include "daemon_test_fixture.h" +#include "mock_permission_utils.h" #include "mock_platform.h" #include "mock_server_reader_writer.h" #include "mock_virtual_machine.h" @@ -57,6 +58,10 @@ struct TestDaemonClone : public mpt::DaemonTestFixture const mpt::MockPlatform::GuardedMock attr{mpt::MockPlatform::inject()}; const mpt::MockPlatform& mock_platform = *attr.first; + + const mpt::MockPermissionUtils::GuardedMock mock_permission_utils_injection = + mpt::MockPermissionUtils::inject(); + mpt::MockPermissionUtils& mock_permission_utils = *mock_permission_utils_injection.first; }; TEST_F(TestDaemonClone, missingOnSrcInstance) diff --git a/tests/test_daemon_find.cpp b/tests/test_daemon_find.cpp index b8077f8829..f6f54262fd 100644 --- a/tests/test_daemon_find.cpp +++ b/tests/test_daemon_find.cpp @@ -18,6 +18,7 @@ #include "common.h" #include "daemon_test_fixture.h" #include "mock_image_host.h" +#include "mock_permission_utils.h" #include "mock_platform.h" #include "mock_settings.h" #include "mock_vm_blueprint_provider.h" @@ -55,6 +56,10 @@ struct DaemonFind : public mpt::DaemonTestFixture mpt::MockSettings::GuardedMock mock_settings_injection = mpt::MockSettings::inject(); mpt::MockSettings& mock_settings = *mock_settings_injection.first; + + const mpt::MockPermissionUtils::GuardedMock mock_permission_utils_injection = + mpt::MockPermissionUtils::inject(); + mpt::MockPermissionUtils& mock_permission_utils = *mock_permission_utils_injection.first; }; TEST_F(DaemonFind, blankQueryReturnsAllData) diff --git a/tests/test_daemon_launch.cpp b/tests/test_daemon_launch.cpp index 566229ed4d..9a0c66a799 100644 --- a/tests/test_daemon_launch.cpp +++ b/tests/test_daemon_launch.cpp @@ -20,6 +20,7 @@ #include "daemon_test_fixture.h" #include "mock_image_host.h" #include "mock_json_utils.h" +#include "mock_permission_utils.h" #include "mock_platform.h" #include "mock_server_reader_writer.h" #include "mock_settings.h" @@ -54,6 +55,10 @@ struct TestDaemonLaunch : public mpt::DaemonTestFixture mpt::MockSettings& mock_settings = *mock_settings_injection.first; const mpt::MockJsonUtils::GuardedMock mock_json_utils_injection = mpt::MockJsonUtils::inject(); + + const mpt::MockPermissionUtils::GuardedMock mock_permission_utils_injection = + mpt::MockPermissionUtils::inject(); + mpt::MockPermissionUtils& mock_permission_utils = *mock_permission_utils_injection.first; }; TEST_F(TestDaemonLaunch, blueprintFoundMountsWorkspaceWithNameOverride) diff --git a/tests/test_daemon_mount.cpp b/tests/test_daemon_mount.cpp index 2972fc3149..cc392d644c 100644 --- a/tests/test_daemon_mount.cpp +++ b/tests/test_daemon_mount.cpp @@ -20,6 +20,7 @@ #include "mock_file_ops.h" #include "mock_logger.h" #include "mock_mount_handler.h" +#include "mock_permission_utils.h" #include "mock_platform.h" #include "mock_server_reader_writer.h" #include "mock_settings.h" @@ -68,6 +69,10 @@ struct TestDaemonMount : public mpt::DaemonTestFixture mpt::MockSettings::GuardedMock mock_settings_injection = mpt::MockSettings::inject(); mpt::MockSettings& mock_settings = *mock_settings_injection.first; + + const mpt::MockPermissionUtils::GuardedMock mock_permission_utils_injection = + mpt::MockPermissionUtils::inject(); + mpt::MockPermissionUtils& mock_permission_utils = *mock_permission_utils_injection.first; }; } // namespace diff --git a/tests/test_daemon_snapshot_restore.cpp b/tests/test_daemon_snapshot_restore.cpp index 7474b65148..6b83119e08 100644 --- a/tests/test_daemon_snapshot_restore.cpp +++ b/tests/test_daemon_snapshot_restore.cpp @@ -17,6 +17,7 @@ #include "common.h" #include "daemon_test_fixture.h" +#include "mock_permission_utils.h" #include "mock_platform.h" #include "mock_server_reader_writer.h" #include "mock_settings.h" @@ -65,6 +66,10 @@ struct TestDaemonSnapshotRestoreBase : public mpt::DaemonTestFixture mpt::MockSettings::GuardedMock mock_settings_injection = mpt::MockSettings::inject(); mpt::MockSettings& mock_settings = *mock_settings_injection.first; + const mpt::MockPermissionUtils::GuardedMock mock_permission_utils_injection = + mpt::MockPermissionUtils::inject(); + mpt::MockPermissionUtils& mock_permission_utils = *mock_permission_utils_injection.first; + mpt::MockVirtualMachineFactory& mock_factory = *use_a_mock_vm_factory(); std::vector extra_interfaces; diff --git a/tests/test_daemon_start.cpp b/tests/test_daemon_start.cpp index dc2fced146..fff214a31d 100644 --- a/tests/test_daemon_start.cpp +++ b/tests/test_daemon_start.cpp @@ -21,6 +21,7 @@ #include "common.h" #include "mock_image_host.h" #include "mock_mount_handler.h" +#include "mock_permission_utils.h" #include "mock_platform.h" #include "mock_server_reader_writer.h" #include "mock_settings.h" @@ -54,6 +55,10 @@ struct TestDaemonStart : public mpt::DaemonTestFixture mpt::MockSettings::GuardedMock mock_settings_injection = mpt::MockSettings::inject(); mpt::MockSettings& mock_settings = *mock_settings_injection.first; + + const mpt::MockPermissionUtils::GuardedMock mock_permission_utils_injection = + mpt::MockPermissionUtils::inject(); + mpt::MockPermissionUtils& mock_permission_utils = *mock_permission_utils_injection.first; }; TEST_F(TestDaemonStart, successfulStartOkStatus) diff --git a/tests/test_daemon_suspend.cpp b/tests/test_daemon_suspend.cpp index 98e8a0fe71..b0dc27672a 100644 --- a/tests/test_daemon_suspend.cpp +++ b/tests/test_daemon_suspend.cpp @@ -18,6 +18,7 @@ #include "common.h" #include "daemon_test_fixture.h" #include "mock_mount_handler.h" +#include "mock_permission_utils.h" #include "mock_platform.h" #include "mock_server_reader_writer.h" #include "mock_settings.h" @@ -54,6 +55,10 @@ struct TestDaemonSuspend : public mpt::DaemonTestFixture mpt::MockSettings::GuardedMock mock_settings_injection = mpt::MockSettings::inject(); mpt::MockSettings& mock_settings = *mock_settings_injection.first; + + const mpt::MockPermissionUtils::GuardedMock mock_permission_utils_injection = + mpt::MockPermissionUtils::inject(); + mpt::MockPermissionUtils& mock_permission_utils = *mock_permission_utils_injection.first; }; } // namespace diff --git a/tests/test_daemon_umount.cpp b/tests/test_daemon_umount.cpp index 69705432be..dbd7cfdfeb 100644 --- a/tests/test_daemon_umount.cpp +++ b/tests/test_daemon_umount.cpp @@ -19,6 +19,7 @@ #include "daemon_test_fixture.h" #include "mock_logger.h" #include "mock_mount_handler.h" +#include "mock_permission_utils.h" #include "mock_platform.h" #include "mock_server_reader_writer.h" #include "mock_settings.h" @@ -57,6 +58,10 @@ struct TestDaemonUmount : public mpt::DaemonTestFixture mpt::MockSettings::GuardedMock mock_settings_injection = mpt::MockSettings::inject(); mpt::MockSettings& mock_settings = *mock_settings_injection.first; + + const mpt::MockPermissionUtils::GuardedMock mock_permission_utils_injection = + mpt::MockPermissionUtils::inject(); + mpt::MockPermissionUtils& mock_permission_utils = *mock_permission_utils_injection.first; }; } // namespace diff --git a/tests/test_permission_utils.cpp b/tests/test_permission_utils.cpp new file mode 100644 index 0000000000..1204e205b3 --- /dev/null +++ b/tests/test_permission_utils.cpp @@ -0,0 +1,218 @@ +/* + * Copyright (C) Canonical, Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 3. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +#include "mock_file_ops.h" +#include "mock_permission_utils.h" +#include "mock_platform.h" +#include "mock_recursive_dir_iterator.h" + +namespace mp = multipass; +namespace mpt = mp::test; +using namespace testing; + +namespace +{ +struct TestPermissionUtils : public Test +{ + const mpt::MockFileOps::GuardedMock guarded_mock_file_ops = mpt::MockFileOps::inject(); + mpt::MockFileOps& mock_file_ops = *guarded_mock_file_ops.first; + const mpt::MockPlatform::GuardedMock guarded_mock_platform = mpt::MockPlatform::inject(); + mpt::MockPlatform& mock_platform = *guarded_mock_platform.first; + + const mp::fs::path test_path{"test_path"}; + const QString test_qpath{QString::fromUtf8(test_path.u8string())}; + static constexpr QFileDevice::Permissions test_permissions{QFileDevice::Permission::ReadOwner}; + static constexpr QFileDevice::Permissions restricted_permissions{ + QFileDevice::Permission::ReadOwner | QFileDevice::Permission::WriteOwner | QFile::ExeOwner}; +}; + +struct TestPermissionUtilsNoFile : public TestPermissionUtils +{ + TestPermissionUtilsNoFile() + { + EXPECT_CALL(mock_file_ops, exists(test_path, _)).WillOnce(Return(false)); + } +}; + +TEST_F(TestPermissionUtilsNoFile, set_permissions_throws_when_file_non_existant) +{ + MP_EXPECT_THROW_THAT(MP_PERMISSIONS.set_permissions(test_path, test_permissions), + std::runtime_error, + mpt::match_what(HasSubstr(test_path.string()))); +} + +TEST_F(TestPermissionUtilsNoFile, take_ownership_throws_when_file_non_existant) +{ + MP_EXPECT_THROW_THAT(MP_PERMISSIONS.take_ownership(test_path), + std::runtime_error, + mpt::match_what(HasSubstr(test_path.string()))); +} + +TEST_F(TestPermissionUtilsNoFile, restrict_permissions_throws_when_file_non_existant) +{ + MP_EXPECT_THROW_THAT(MP_PERMISSIONS.restrict_permissions(test_path), + std::runtime_error, + mpt::match_what(AllOf(HasSubstr("nonexistent file"), HasSubstr(test_path.string())))); +} + +struct TestPermissionUtilsFile : public TestPermissionUtils +{ + TestPermissionUtilsFile() + { + EXPECT_CALL(mock_file_ops, exists(test_path, _)).WillOnce(Return(true)); + EXPECT_CALL(mock_file_ops, is_directory(test_path, _)).WillRepeatedly(Return(false)); + } +}; + +TEST_F(TestPermissionUtilsFile, set_permissions_sets_permissions) +{ + EXPECT_CALL(mock_platform, set_permissions(test_qpath, test_permissions, false)).WillOnce(Return(true)); + + MP_PERMISSIONS.set_permissions(test_path, test_permissions); +} + +TEST_F(TestPermissionUtilsFile, set_permissions_throws_on_failure) +{ + EXPECT_CALL(mock_platform, set_permissions(test_qpath, test_permissions, _)).WillOnce(Return(false)); + + MP_EXPECT_THROW_THAT(MP_PERMISSIONS.set_permissions(test_path, test_permissions), + std::runtime_error, + mpt::match_what(AllOf(HasSubstr("Cannot set permissions"), HasSubstr(test_path.string())))); +} + +TEST_F(TestPermissionUtilsFile, take_ownership_takes_ownership) +{ + EXPECT_CALL(mock_platform, take_ownership(test_qpath)).WillOnce(Return(true)); + + MP_PERMISSIONS.take_ownership(test_path); +} + +TEST_F(TestPermissionUtilsFile, take_ownership_throws_on_failure) +{ + EXPECT_CALL(mock_platform, take_ownership(test_qpath)).WillOnce(Return(false)); + + MP_EXPECT_THROW_THAT(MP_PERMISSIONS.take_ownership(test_path), + std::runtime_error, + mpt::match_what(AllOf(HasSubstr("Cannot set owner"), HasSubstr(test_path.string())))); +} + +TEST_F(TestPermissionUtilsFile, restrict_permissions_restricts_permissions) +{ + EXPECT_CALL(mock_platform, take_ownership(test_qpath)).WillOnce(Return(true)); + EXPECT_CALL(mock_platform, set_permissions(test_qpath, restricted_permissions, false)).WillOnce(Return(true)); + + MP_PERMISSIONS.restrict_permissions(test_path); +} + +struct TestPermissionUtilsDir : public TestPermissionUtils +{ + TestPermissionUtilsDir() + { + EXPECT_CALL(mock_file_ops, exists(test_path, _)).WillRepeatedly(Return(true)); + EXPECT_CALL(mock_file_ops, is_directory(test_path, _)).WillOnce(Return(true)); + + EXPECT_CALL(entry1, path).WillRepeatedly(ReturnRef(path1)); + EXPECT_CALL(entry2, path).WillRepeatedly(ReturnRef(path2)); + + auto iter = std::make_unique(); + auto iter_p = iter.get(); + + EXPECT_CALL(*iter_p, hasNext).WillOnce(Return(true)).WillOnce(Return(true)).WillRepeatedly(Return(false)); + EXPECT_CALL(*iter_p, next).WillOnce(ReturnRef(entry1)).WillOnce(ReturnRef(entry2)); + EXPECT_CALL(mock_file_ops, recursive_dir_iterator(test_path, _)).WillOnce(Return(std::move(iter))); + } + + const mp::fs::path path1{"Hello.txt"}; + const QString qpath1{QString::fromUtf8(path1.u8string())}; + mpt::MockDirectoryEntry entry1; + + const mp::fs::path path2{"World.txt"}; + const QString qpath2{QString::fromUtf8(path2.u8string())}; + mpt::MockDirectoryEntry entry2; +}; + +TEST_F(TestPermissionUtilsDir, set_permissions_iterates_dir) +{ + EXPECT_CALL(mock_platform, set_permissions(test_qpath, test_permissions, false)).WillOnce(Return(true)); + EXPECT_CALL(mock_platform, set_permissions(qpath1, test_permissions, true)).WillOnce(Return(true)); + EXPECT_CALL(mock_platform, set_permissions(qpath2, test_permissions, true)).WillOnce(Return(true)); + + MP_PERMISSIONS.set_permissions(test_path, test_permissions); +} + +TEST_F(TestPermissionUtilsDir, take_ownership_iterates_dir) +{ + EXPECT_CALL(mock_platform, take_ownership(test_qpath)).WillOnce(Return(true)); + EXPECT_CALL(mock_platform, take_ownership(qpath1)).WillOnce(Return(true)); + EXPECT_CALL(mock_platform, take_ownership(qpath2)).WillOnce(Return(true)); + + MP_PERMISSIONS.take_ownership(test_path); +} + +TEST_F(TestPermissionUtilsDir, restrict_permissions_iterates_dir) +{ + EXPECT_CALL(mock_platform, take_ownership(test_qpath)).WillOnce(Return(true)); + EXPECT_CALL(mock_platform, take_ownership(qpath1)).WillOnce(Return(true)); + EXPECT_CALL(mock_platform, take_ownership(qpath2)).WillOnce(Return(true)); + + EXPECT_CALL(mock_platform, set_permissions(test_qpath, restricted_permissions, false)).WillOnce(Return(true)); + EXPECT_CALL(mock_platform, set_permissions(qpath1, restricted_permissions, true)).WillOnce(Return(true)); + EXPECT_CALL(mock_platform, set_permissions(qpath2, restricted_permissions, true)).WillOnce(Return(true)); + + MP_PERMISSIONS.restrict_permissions(test_path); +} + +struct TestPermissionUtilsBadDir : public TestPermissionUtils +{ + TestPermissionUtilsBadDir() + { + EXPECT_CALL(mock_file_ops, exists(test_path, _)).WillRepeatedly(Return(true)); + EXPECT_CALL(mock_file_ops, is_directory(test_path, _)).WillOnce(Return(true)); + + EXPECT_CALL(mock_file_ops, recursive_dir_iterator(test_path, _)).WillOnce(Return(nullptr)); + } +}; + +TEST_F(TestPermissionUtilsBadDir, set_permissions_throws_on_broken_iterator) +{ + EXPECT_CALL(mock_platform, set_permissions(test_qpath, test_permissions, false)).WillOnce(Return(true)); + + MP_EXPECT_THROW_THAT(MP_PERMISSIONS.set_permissions(test_path, test_permissions), + std::runtime_error, + mpt::match_what(AllOf(HasSubstr("Cannot iterate"), HasSubstr(test_path.string())))); +} + +TEST_F(TestPermissionUtilsBadDir, take_ownership_throws_on_broken_iterator) +{ + EXPECT_CALL(mock_platform, take_ownership(test_qpath)).WillOnce(Return(true)); + + MP_EXPECT_THROW_THAT(MP_PERMISSIONS.take_ownership(test_path), + std::runtime_error, + mpt::match_what(AllOf(HasSubstr("Cannot iterate"), HasSubstr(test_path.string())))); +} + +TEST_F(TestPermissionUtilsBadDir, restrict_permissions_throws_on_broken_iterator) +{ + EXPECT_CALL(mock_platform, set_permissions(test_qpath, restricted_permissions, false)).WillOnce(Return(true)); + EXPECT_CALL(mock_platform, take_ownership(test_qpath)).WillOnce(Return(true)); + + MP_EXPECT_THROW_THAT(MP_PERMISSIONS.restrict_permissions(test_path), + std::runtime_error, + mpt::match_what(AllOf(HasSubstr("Cannot iterate"), HasSubstr(test_path.string())))); +} + +} // namespace diff --git a/tests/test_sftp_client.cpp b/tests/test_sftp_client.cpp index 2a799b19ce..87ad0e526e 100644 --- a/tests/test_sftp_client.cpp +++ b/tests/test_sftp_client.cpp @@ -293,8 +293,8 @@ TEST_F(SFTPClient, pull_file_success) REPLACE(sftp_stat, [&](auto...) { return get_dummy_sftp_attr(SSH_FILEXFER_TYPE_REGULAR, "", perms); }); mp::Perms written_perms; EXPECT_CALL(mock_platform, - set_permissions(QString::fromStdString(target_path.u8string()), static_cast(perms))) - .WillOnce([&](auto, auto perms) { + set_permissions(QString::fromStdString(target_path.u8string()), static_cast(perms), _)) + .WillOnce([&](auto, auto perms, auto) { written_perms = perms; return true; }); @@ -361,7 +361,7 @@ TEST_F(SFTPClient, pull_file_cannot_write_target) }; REPLACE(sftp_read, mocked_sftp_read); REPLACE(sftp_stat, [&](auto...) { return get_dummy_sftp_attr(); }); - EXPECT_CALL(mock_platform, set_permissions(QString::fromStdString(target_path.u8string()), _)) + EXPECT_CALL(mock_platform, set_permissions(QString::fromStdString(target_path.u8string()), _, _)) .WillOnce(Return(true)); REPLACE(sftp_setstat, [](auto...) { return SSH_FX_OK; }); @@ -402,7 +402,7 @@ TEST_F(SFTPClient, pull_file_cannot_set_perms) REPLACE(sftp_stat, [&](auto...) { return get_dummy_sftp_attr(SSH_FILEXFER_TYPE_REGULAR, "", perms); }); EXPECT_CALL(mock_platform, - set_permissions(QString::fromStdString(target_path.u8string()), static_cast(perms))) + set_permissions(QString::fromStdString(target_path.u8string()), static_cast(perms), _)) .WillOnce(Return(false)); auto sftp_client = make_sftp_client(); @@ -751,14 +751,14 @@ TEST_F(SFTPClient, pull_dir_success_regular) mp::Perms dir_written_perms; EXPECT_CALL( mock_platform, - set_permissions(QString::fromStdString((target_path / "file").u8string()), static_cast(perms))) - .WillOnce([&](auto, auto perms) { + set_permissions(QString::fromStdString((target_path / "file").u8string()), static_cast(perms), _)) + .WillOnce([&](auto, auto perms, auto) { file_written_perms = perms; return true; }); EXPECT_CALL(mock_platform, - set_permissions(QString::fromStdString(target_path.u8string()), static_cast(perms))) - .WillOnce([&](auto, auto perms) { + set_permissions(QString::fromStdString(target_path.u8string()), static_cast(perms), _)) + .WillOnce([&](auto, auto perms, auto) { dir_written_perms = perms; return true; }); @@ -785,7 +785,7 @@ TEST_F(SFTPClient, pull_dir_success_dir) .WillOnce(Return(std::unique_ptr( get_dummy_sftp_attr(SSH_FILEXFER_TYPE_DIRECTORY, "source/path/dir")))); EXPECT_CALL(*mock_file_ops, create_directory(target_path / "dir", _)); - EXPECT_CALL(mock_platform, set_permissions(_, _)).Times(2).WillRepeatedly(Return(true)); + EXPECT_CALL(mock_platform, set_permissions(_, _, _)).Times(2).WillRepeatedly(Return(true)); auto sftp_client = make_sftp_client(); @@ -811,7 +811,7 @@ TEST_F(SFTPClient, pull_dir_fail_dir) e = err; return false; }); - EXPECT_CALL(mock_platform, set_permissions(_, _)).WillOnce(Return(false)); + EXPECT_CALL(mock_platform, set_permissions(_, _, _)).WillOnce(Return(false)); auto sftp_client = make_sftp_client(); @@ -840,7 +840,7 @@ TEST_F(SFTPClient, pull_dir_success_symlink) EXPECT_CALL(*mock_file_ops, is_directory).WillOnce(Return(false)); EXPECT_CALL(*mock_file_ops, remove(_, _)).WillOnce(Return(true)); EXPECT_CALL(*mock_file_ops, create_symlink(_, target_path / "symlink", _)); - EXPECT_CALL(mock_platform, set_permissions(_, _)).WillRepeatedly(Return(true)); + EXPECT_CALL(mock_platform, set_permissions(_, _, _)).WillRepeatedly(Return(true)); auto sftp_client = make_sftp_client(); @@ -864,7 +864,7 @@ TEST_F(SFTPClient, pull_dir_cannot_read_symlink) REPLACE(sftp_readlink, [](auto...) { return nullptr; }); auto err = "SFTP server: Permission denied"; REPLACE(ssh_get_error, [&](auto...) { return err; }); - EXPECT_CALL(mock_platform, set_permissions(_, _)).WillRepeatedly(Return(true)); + EXPECT_CALL(mock_platform, set_permissions(_, _, _)).WillRepeatedly(Return(true)); auto sftp_client = make_sftp_client(); @@ -893,7 +893,7 @@ TEST_F(SFTPClient, pull_dir_cannot_create_symlink) auto err = std::make_error_code(std::errc::permission_denied); EXPECT_CALL(*mock_file_ops, create_symlink(_, target_path / "symlink", _)) .WillOnce([&](auto, auto, std::error_code& e) { e = err; }); - EXPECT_CALL(mock_platform, set_permissions(_, _)).WillRepeatedly(Return(true)); + EXPECT_CALL(mock_platform, set_permissions(_, _, _)).WillRepeatedly(Return(true)); auto sftp_client = make_sftp_client(); @@ -918,7 +918,7 @@ TEST_F(SFTPClient, pull_dir_symlink_over_dir) REPLACE(sftp_readlink, [](auto...) { return (char*)malloc(10); }); EXPECT_CALL(*mock_file_ops, is_directory).WillOnce(Return(true)); - EXPECT_CALL(mock_platform, set_permissions(_, _)).WillRepeatedly(Return(true)); + EXPECT_CALL(mock_platform, set_permissions(_, _, _)).WillRepeatedly(Return(true)); auto sftp_client = make_sftp_client(); @@ -940,7 +940,7 @@ TEST_F(SFTPClient, pull_dir_unknown_file_type) EXPECT_CALL(*iter_p, next) .WillOnce(Return(std::unique_ptr( get_dummy_sftp_attr(SSH_FILEXFER_TYPE_UNKNOWN, source_path.u8string() + "/unknown")))); - EXPECT_CALL(mock_platform, set_permissions(_, _)).WillRepeatedly(Return(true)); + EXPECT_CALL(mock_platform, set_permissions(_, _, _)).WillRepeatedly(Return(true)); auto sftp_client = make_sftp_client(); diff --git a/tests/test_sftpserver.cpp b/tests/test_sftpserver.cpp index 0d74d13e7c..2a4a1e145c 100644 --- a/tests/test_sftpserver.cpp +++ b/tests/test_sftpserver.cpp @@ -546,7 +546,7 @@ TEST_F(SftpServer, handles_mkdir) const auto [file_ops, mock_file_ops_guard] = mpt::MockFileOps::inject(); const auto [platform, mock_platform_guard] = mpt::MockPlatform::inject(); - EXPECT_CALL(*platform, set_permissions(A(), _)).WillOnce(Return(true)); + EXPECT_CALL(*platform, set_permissions(A(), _, _)).WillOnce(Return(true)); EXPECT_CALL(*file_ops, ownerId(_)).WillRepeatedly([](const QFileInfo& file) { return file.ownerId(); }); EXPECT_CALL(*file_ops, groupId(_)).WillRepeatedly([](const QFileInfo& file) { return file.groupId(); }); @@ -601,7 +601,7 @@ TEST_F(SftpServer, mkdir_set_permissions_fails) const auto [file_ops, mock_file_ops_guard] = mpt::MockFileOps::inject(); const auto [platform, mock_platform_guard] = mpt::MockPlatform::inject(); - EXPECT_CALL(*platform, set_permissions(_, _)).WillOnce(Return(false)); + EXPECT_CALL(*platform, set_permissions(_, _, _)).WillOnce(Return(false)); EXPECT_CALL(*file_ops, ownerId(_)).WillRepeatedly([](const QFileInfo& file) { return file.ownerId(); }); EXPECT_CALL(*file_ops, groupId(_)).WillRepeatedly([](const QFileInfo& file) { return file.groupId(); }); @@ -636,7 +636,7 @@ TEST_F(SftpServer, mkdir_chown_failure_fails) const auto [mock_platform, guard] = mpt::MockPlatform::inject(); EXPECT_CALL(*mock_platform, chown(_, _, _)).WillOnce(Return(-1)); - EXPECT_CALL(*mock_platform, set_permissions(_, _)).WillRepeatedly(Return(true)); + EXPECT_CALL(*mock_platform, set_permissions(_, _, _)).WillRepeatedly(Return(true)); auto sftp = make_sftpserver(temp_dir.path().toStdString()); auto msg = make_msg(SFTP_MKDIR); @@ -1933,7 +1933,7 @@ TEST_F(SftpServer, setstat_set_permissions_failure_fails) const auto [file_ops, mock_file_ops_guard] = mpt::MockFileOps::inject(); const auto [platform, mock_platform_guard] = mpt::MockPlatform::inject(); EXPECT_CALL(*file_ops, resize).WillOnce(Return(true)); - EXPECT_CALL(*platform, set_permissions(_, _)).WillOnce(Return(false)); + EXPECT_CALL(*platform, set_permissions(_, _, _)).WillOnce(Return(false)); EXPECT_CALL(*file_ops, ownerId(_)).WillRepeatedly([](const QFileInfo& file) { return file.ownerId(); }); EXPECT_CALL(*file_ops, groupId(_)).WillRepeatedly([](const QFileInfo& file) { return file.groupId(); }); EXPECT_CALL(*file_ops, exists(A())).WillRepeatedly([](const QFileInfo& file) { @@ -2705,7 +2705,7 @@ TEST_F(SftpServer, DISABLE_ON_WINDOWS(mkdir_chown_honors_maps_in_the_host)) EXPECT_CALL(*mock_platform, chown(_, host_uid, host_gid)).Times(1); EXPECT_CALL(*mock_platform, chown(_, sftp_uid, sftp_gid)).Times(0); - EXPECT_CALL(*mock_platform, set_permissions(_, _)).WillRepeatedly(Return(true)); + EXPECT_CALL(*mock_platform, set_permissions(_, _, _)).WillRepeatedly(Return(true)); sftp.run(); } @@ -2732,7 +2732,7 @@ TEST_F(SftpServer, DISABLE_ON_WINDOWS(mkdir_chown_works_when_ids_are_not_mapped) QFileInfo parent_dir(temp_dir.path()); EXPECT_CALL(*mock_platform, chown(_, parent_dir.ownerId(), parent_dir.groupId())).Times(1); - EXPECT_CALL(*mock_platform, set_permissions(_, _)).WillRepeatedly(Return(true)); + EXPECT_CALL(*mock_platform, set_permissions(_, _, _)).WillRepeatedly(Return(true)); sftp.run(); } diff --git a/tests/unix/test_daemon_rpc.cpp b/tests/unix/test_daemon_rpc.cpp index 77e66a7420..7ec3b86af5 100644 --- a/tests/unix/test_daemon_rpc.cpp +++ b/tests/unix/test_daemon_rpc.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -98,6 +99,10 @@ struct TestDaemonRpc : public mpt::DaemonTestFixture mpt::MockUtils::GuardedMock attr{mpt::MockUtils::inject()}; mpt::MockUtils* mock_utils = attr.first; + + const mpt::MockPermissionUtils::GuardedMock mock_permission_utils_injection = + mpt::MockPermissionUtils::inject(); + mpt::MockPermissionUtils& mock_permission_utils = *mock_permission_utils_injection.first; }; } // namespace diff --git a/tests/unix/test_platform_unix.cpp b/tests/unix/test_platform_unix.cpp index c783d2d109..0f76ec0d47 100644 --- a/tests/unix/test_platform_unix.cpp +++ b/tests/unix/test_platform_unix.cpp @@ -49,7 +49,7 @@ TEST_F(TestPlatformUnix, setServerSocketRestrictionsNotRestrictedIsCorrect) auto [mock_platform, guard] = mpt::MockPlatform::inject(); EXPECT_CALL(*mock_platform, chown(_, 0, 0)).WillOnce(Return(0)); - EXPECT_CALL(*mock_platform, set_permissions(_, relaxed_permissions)).WillOnce(Return(true)); + EXPECT_CALL(*mock_platform, set_permissions(_, relaxed_permissions, false)).WillOnce(Return(true)); EXPECT_NO_THROW(MP_PLATFORM.Platform::set_server_socket_restrictions(fmt::format("unix:{}", file.name()), false)); } @@ -62,7 +62,7 @@ TEST_F(TestPlatformUnix, setServerSocketRestrictionsRestrictedIsCorrect) group.gr_gid = gid; EXPECT_CALL(*mock_platform, chown(_, 0, gid)).WillOnce(Return(0)); - EXPECT_CALL(*mock_platform, set_permissions(_, restricted_permissions)).WillOnce(Return(true)); + EXPECT_CALL(*mock_platform, set_permissions(_, restricted_permissions, false)).WillOnce(Return(true)); REPLACE(getgrnam, [&group](auto) { return &group; }); @@ -106,7 +106,7 @@ TEST_F(TestPlatformUnix, setServerSocketRestrictionsChmodFailsThrows) auto [mock_platform, guard] = mpt::MockPlatform::inject(); EXPECT_CALL(*mock_platform, chown(_, 0, 0)).WillOnce(Return(0)); - EXPECT_CALL(*mock_platform, set_permissions(_, relaxed_permissions)).WillOnce([](auto...) { + EXPECT_CALL(*mock_platform, set_permissions(_, relaxed_permissions, false)).WillOnce([](auto...) { errno = EPERM; return false; });