From 5360fb3c895abfeb2363ff398a364ab79954e012 Mon Sep 17 00:00:00 2001 From: sithlord48 Date: Thu, 6 Mar 2025 19:42:16 -0500 Subject: [PATCH] refactor: move requirepeerchecking to Settings newkey: security/checkpeerfingerprints <= General/requireClientCerts remove requireClientCerts from appconfig --- src/apps/deskflow-gui/MainWindow.cpp | 2 +- .../deskflow-gui/dialogs/SettingsDialog.cpp | 4 ++-- src/lib/common/Settings.cpp | 6 +++--- src/lib/common/Settings.h | 19 ++++++++++--------- src/lib/gui/config/AppConfig.cpp | 16 +--------------- src/lib/gui/config/AppConfig.h | 5 +---- src/lib/gui/config/IAppConfig.h | 2 -- src/lib/gui/core/CoreProcess.cpp | 2 +- src/lib/gui/core/ServerConnection.cpp | 4 ++-- src/test/shared/gui/mocks/AppConfigMock.h | 2 -- .../gui/core/ServerConnectionTests.cpp | 2 +- 11 files changed, 22 insertions(+), 42 deletions(-) diff --git a/src/apps/deskflow-gui/MainWindow.cpp b/src/apps/deskflow-gui/MainWindow.cpp index 9d7b8df32..8a9e2b302 100644 --- a/src/apps/deskflow-gui/MainWindow.cpp +++ b/src/apps/deskflow-gui/MainWindow.cpp @@ -385,7 +385,7 @@ void MainWindow::firstShown() void MainWindow::settingsChanged(const QString &key) { if ((key == Settings::Security::Certificate) || (key == Settings::Security::KeySize) || - (key == Settings::Security::TlsEnabled)) { + (key == Settings::Security::TlsEnabled) || (key == Settings::Security::CheckPeers)) { const auto certificate = Settings::value(Settings::Security::Certificate).toString(); if (m_tlsUtility.isEnabled() && !QFile::exists(certificate)) { m_tlsUtility.generateCertificate(); diff --git a/src/apps/deskflow-gui/dialogs/SettingsDialog.cpp b/src/apps/deskflow-gui/dialogs/SettingsDialog.cpp index 895150139..d9d8ce5e3 100644 --- a/src/apps/deskflow-gui/dialogs/SettingsDialog.cpp +++ b/src/apps/deskflow-gui/dialogs/SettingsDialog.cpp @@ -160,7 +160,7 @@ void SettingsDialog::accept() m_appConfig.setEnableService(ui->cbServiceEnabled->isChecked()); Settings::setValue(Settings::Gui::CloseToTray, ui->cbCloseToTray->isChecked()); Settings::setValue(Settings::Gui::SymbolicTrayIcon, ui->rbIconMono->isChecked()); - m_appConfig.setRequireClientCerts(ui->cbRequireClientCert->isChecked()); + Settings::setValue(Settings::Security::CheckPeers, ui->cbRequireClientCert->isChecked()); QDialog::accept(); } @@ -223,7 +223,7 @@ void SettingsDialog::updateTlsControls() const auto enabled = writable && tlsEnabled; ui->lineTlsCertPath->setText(certificate); - ui->cbRequireClientCert->setChecked(m_appConfig.requireClientCerts()); + ui->cbRequireClientCert->setChecked(Settings::value(Settings::Security::CheckPeers).toBool()); ui->groupSecurity->setChecked(tlsEnabled); ui->groupSecurity->setEnabled(writable); diff --git a/src/lib/common/Settings.cpp b/src/lib/common/Settings.cpp index c129d34d0..8963ac776 100644 --- a/src/lib/common/Settings.cpp +++ b/src/lib/common/Settings.cpp @@ -74,8 +74,8 @@ QVariant Settings::defaultValue(const QString &key) return false; } - if ((key == Gui::CloseToTray) || (key == Gui::LogExpanded) || (key == Gui::SymbolicTrayIcon) || - (key == Gui::CloseReminder) || (key == Security::TlsEnabled)) { + if ((key == Gui::CloseToTray) || (key == Gui::LogExpanded) || (key == Gui::SymbolicTrayIcon) + || (key == Gui::CloseReminder) || (key == Security::TlsEnabled) || (key == Security::CheckPeers)) { return true; } @@ -85,7 +85,7 @@ QVariant Settings::defaultValue(const QString &key) if (key == Security::KeySize) return 2048; - if (key == Settings::Security::Certificate) + if (key == Security::Certificate) return QStringLiteral("%1/%2/%3").arg(instance()->settingsPath(), kTlsDirName, kTlsCertificateFilename); return QVariant(); diff --git a/src/lib/common/Settings.h b/src/lib/common/Settings.h index 975d45781..162858344 100644 --- a/src/lib/common/Settings.h +++ b/src/lib/common/Settings.h @@ -17,18 +17,17 @@ class Settings : public QObject Q_OBJECT public: #if defined(Q_OS_WIN) - inline const static auto UserSettingFile = - QStringLiteral("%1/AppData/Local/%2/%3.conf").arg(QDir::homePath(), kAppName, kAppName); - inline const static auto SystemSettingFile = QStringLiteral("C:/ProgramData/%2/%3.conf").arg(kAppName, kAppName); + inline const static auto UserDir = QStringLiteral("%1/AppData/Local/%2").arg(QDir::homePath(), kAppName); + inline const static auto SystemDir = QStringLiteral("C:/ProgramData/%1").arg(kAppName); #elif defined(Q_OS_MAC) - inline const static auto UserSettingFile = - QStringLiteral("%1/Library/%2/%3.conf").arg(QDir::homePath(), kAppName, kAppName); - inline const static auto SystemSettingFile = QStringLiteral("/Libaray/%2/%3.conf").arg(kAppName, kAppName); + inline const static auto UserDir = QStringLiteral("%1/Library/%2").arg(QDir::homePath(), kAppName); + inline const static auto SystemDir = QStringLiteral("/Library/%1").arg(kAppName); #else - inline const static auto UserSettingFile = - QStringLiteral("%1/.config/%2/%3.conf").arg(QDir::homePath(), kAppName, kAppName); - inline const static auto SystemSettingFile = QStringLiteral("/etc/%2/%3.conf").arg(kAppName, kAppName); + inline const static auto UserDir = QStringLiteral("%1/.config/%2").arg(QDir::homePath(), kAppName); + inline const static auto SystemDir = QStringLiteral("/etc/%1").arg(kAppName); #endif + inline const static auto UserSettingFile = QStringLiteral("%1/%2.conf").arg(UserDir, kAppName); + inline const static auto SystemSettingFile = QStringLiteral("%1/%2.conf").arg(SystemDir, kAppName); struct Core { @@ -47,6 +46,7 @@ public: }; struct Security { + inline static const auto CheckPeers = QStringLiteral("security/checkpeerfingerprints"); inline static const auto Certificate = QStringLiteral("security/certificate"); inline static const auto KeySize = QStringLiteral("security/keySize"); inline static const auto TlsEnabled = QStringLiteral("security/tlsEnabled"); @@ -92,6 +92,7 @@ private: , Gui::SymbolicTrayIcon , Gui::WindowGeometry , Security::Certificate + , Security::CheckPeers , Security::KeySize , Security::TlsEnabled }; diff --git a/src/lib/gui/config/AppConfig.cpp b/src/lib/gui/config/AppConfig.cpp index 123d2a1a3..52924aa69 100644 --- a/src/lib/gui/config/AppConfig.cpp +++ b/src/lib/gui/config/AppConfig.cpp @@ -79,7 +79,7 @@ const char *const AppConfig::m_SettingsName[] = { "", // 43 Moved to deskflow settings "", // 44, Moved to deskflow settings. "", // 45 Moved to deskflow settings - "requireClientCerts", + "", // 46 require peer certs, Moved to deskflow settings }; AppConfig::AppConfig(deskflow::gui::IConfigScopes &scopes, std::shared_ptr deps) @@ -130,7 +130,6 @@ void AppConfig::recallFromCurrentScope() m_LanguageSync = getFromCurrentScope(kLanguageSync, m_LanguageSync).toBool(); m_InvertScrollDirection = getFromCurrentScope(kInvertScrollDirection, m_InvertScrollDirection).toBool(); m_EnableService = getFromCurrentScope(kEnableService, m_EnableService).toBool(); - m_RequireClientCert = getFromCurrentScope(kRequireClientCert, m_RequireClientCert).toBool(); } void AppConfig::recallScreenName() @@ -178,7 +177,6 @@ void AppConfig::commit() setInCurrentScope(kLanguageSync, m_LanguageSync); setInCurrentScope(kInvertScrollDirection, m_InvertScrollDirection); setInCurrentScope(kEnableService, m_EnableService); - setInCurrentScope(kRequireClientCert, m_RequireClientCert); } } @@ -485,11 +483,6 @@ bool AppConfig::clientGroupChecked() const return m_ClientGroupChecked; } -bool AppConfig::requireClientCerts() const -{ - return m_RequireClientCert; -} - const QString &AppConfig::serverHostname() const { return m_ServerHostname; @@ -596,13 +589,6 @@ void AppConfig::setEnableService(bool enabled) m_EnableService = enabled; } -void AppConfig::setRequireClientCerts(bool requireClientCerts) -{ - if (requireClientCerts == m_RequireClientCert) - return; - m_RequireClientCert = requireClientCerts; -} - /////////////////////////////////////////////////////////////////////////////// // End setters /////////////////////////////////////////////////////////////////////////////// diff --git a/src/lib/gui/config/AppConfig.h b/src/lib/gui/config/AppConfig.h index f04d0fb3a..3932cd124 100644 --- a/src/lib/gui/config/AppConfig.h +++ b/src/lib/gui/config/AppConfig.h @@ -100,7 +100,7 @@ private: // 43 = Enable Update Check, // 44 = LogExpanded, Moved to deskflow settings // 45 = Colorful Icon, Moved to deskflow settings - kRequireClientCert = 46 + // kRequireClientCert = 46 Moved to deskflow settings }; public: @@ -150,7 +150,6 @@ public: int logLevel() const override; bool enableService() const override; bool clientGroupChecked() const override; - bool requireClientCerts() const override; // // Getters (new methods) @@ -176,7 +175,6 @@ public: void setLanguageSync(bool b) override; void setPreventSleep(bool b) override; void setEnableService(bool enabled) override; - void setRequireClientCerts(bool requireClientCerts) override; // // Setters (new methods) @@ -278,7 +276,6 @@ private: QString m_ServerHostname = ""; bool m_EnableService = deskflow::gui::kDefaultProcessMode == ProcessMode::kService; bool m_LoadFromSystemScope = false; - bool m_RequireClientCert = true; deskflow::gui::IConfigScopes &m_Scopes; std::shared_ptr m_pDeps; diff --git a/src/lib/gui/config/IAppConfig.h b/src/lib/gui/config/IAppConfig.h index 2367d32fc..0f6712213 100644 --- a/src/lib/gui/config/IAppConfig.h +++ b/src/lib/gui/config/IAppConfig.h @@ -54,7 +54,6 @@ public: virtual bool isActiveScopeSystem() const = 0; virtual bool isActiveScopeWritable() const = 0; virtual bool clientGroupChecked() const = 0; - virtual bool requireClientCerts() const = 0; // // Setters @@ -72,7 +71,6 @@ public: virtual void setLanguageSync(bool languageSync) = 0; virtual void setInvertScrollDirection(bool invertScrollDirection) = 0; virtual void setEnableService(bool enableService) = 0; - virtual void setRequireClientCerts(bool requireClientCerts) = 0; }; } // namespace deskflow::gui diff --git a/src/lib/gui/core/CoreProcess.cpp b/src/lib/gui/core/CoreProcess.cpp index b04c571c3..5093d3368 100644 --- a/src/lib/gui/core/CoreProcess.cpp +++ b/src/lib/gui/core/CoreProcess.cpp @@ -534,7 +534,7 @@ bool CoreProcess::addServerArgs(QStringList &args, QString &app) args << "--log" << m_appConfig.logFilename(); } - if (!m_appConfig.requireClientCerts()) { + if (!Settings::value(Settings::Security::CheckPeers).toBool()) { args << "--disable-client-cert-check"; } diff --git a/src/lib/gui/core/ServerConnection.cpp b/src/lib/gui/core/ServerConnection.cpp index e09758b7e..752dd61c4 100644 --- a/src/lib/gui/core/ServerConnection.cpp +++ b/src/lib/gui/core/ServerConnection.cpp @@ -96,8 +96,8 @@ void ServerConnection::handleNewClient(const QString &clientName) m_messageShowing = true; const bool tlsEnabled = Settings::value(Settings::Security::TlsEnabled).toBool(); - const auto result = - m_pDeps->showNewClientPrompt(m_pParent, clientName, tlsEnabled && m_appConfig.requireClientCerts()); + const bool requireCerts = Settings::value(Settings::Security::CheckPeers).toBool(); + const auto result = m_pDeps->showNewClientPrompt(m_pParent, clientName, tlsEnabled && requireCerts); m_messageShowing = false; if (result == Add) { diff --git a/src/test/shared/gui/mocks/AppConfigMock.h b/src/test/shared/gui/mocks/AppConfigMock.h index c5257e03d..b8a95e158 100644 --- a/src/test/shared/gui/mocks/AppConfigMock.h +++ b/src/test/shared/gui/mocks/AppConfigMock.h @@ -55,7 +55,6 @@ public: MOCK_METHOD(bool, isActiveScopeSystem, (), (const, override)); MOCK_METHOD(bool, isActiveScopeWritable, (), (const, override)); MOCK_METHOD(bool, clientGroupChecked, (), (const, override)); - MOCK_METHOD(bool, requireClientCerts, (), (const, override)); // // Setters @@ -73,7 +72,6 @@ public: MOCK_METHOD(void, setLanguageSync, (bool languageSync), (override)); MOCK_METHOD(void, setInvertScrollDirection, (bool invertScrollDirection), (override)); MOCK_METHOD(void, setEnableService, (bool enableService), (override)); - MOCK_METHOD(void, setRequireClientCerts, (bool requireClientCerts), (override)); private: const QString m_stub = "stub"; diff --git a/src/test/unittests/gui/core/ServerConnectionTests.cpp b/src/test/unittests/gui/core/ServerConnectionTests.cpp index d39c68390..8057fc48c 100644 --- a/src/test/unittests/gui/core/ServerConnectionTests.cpp +++ b/src/test/unittests/gui/core/ServerConnectionTests.cpp @@ -46,7 +46,7 @@ TEST_F(ServerConnectionTests, handleLogLine_newClient_shouldShowPrompt) ServerConnection serverConnection(nullptr, m_appConfig, m_serverConfig, m_serverConfigDialogState, m_pDeps); QString clientName = "test client"; - EXPECT_CALL(*m_pDeps, showNewClientPrompt(_, clientName, false)); + EXPECT_CALL(*m_pDeps, showNewClientPrompt(_, clientName, true)); serverConnection.handleLogLine(R"(unrecognised client name "test client")"); }