From fe6011820eee2be4dfbe2236614d274574bfd5bd Mon Sep 17 00:00:00 2001 From: Nicolas Fella Date: Thu, 14 Oct 2021 15:42:32 +0200 Subject: [PATCH] Port to QtKeychain Currently we optionally use KWallet to store passwords and fall back to storing it in a settings file. Instead use QtKeychain to store the password in the platforms respective native API (which happens to be KWallet on Plasma). This should improve the cross-platformness of the app and matches the general direction KDE is taking wrt KWallet. It also considerably simplifies the implifies the implementation since we no longer need to handle the optionalness of KWallet --- CMakeLists.txt | 3 +- resources/CMakeLists.txt | 11 -- resources/config-kwallet.h.cmake | 1 - resources/sugarcrm/CMakeLists.txt | 7 +- resources/sugarcrm/passwordhandler.cpp | 142 +++++++++---------------- resources/sugarcrm/passwordhandler.h | 12 +-- 6 files changed, 59 insertions(+), 117 deletions(-) delete mode 100644 resources/config-kwallet.h.cmake diff --git a/CMakeLists.txt b/CMakeLists.txt index 6334681c..1747e395 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -64,7 +64,6 @@ find_package(KF5IconThemes CONFIG REQUIRED) find_package(KF5TextWidgets CONFIG REQUIRED) find_package(KF5WidgetsAddons CONFIG REQUIRED) find_package(KF5WindowSystem CONFIG REQUIRED) -find_package(KF5Wallet CONFIG REQUIRED) find_package(KF5ConfigWidgets CONFIG REQUIRED) # KDChart is potentially a dependency of KDReports @@ -96,6 +95,8 @@ set_package_properties(ICU PROPERTIES PURPOSE "Required to localize country names" ) +find_package(Qt5Keychain REQUIRED) + if(PhoneNumber_FOUND AND ICU_FOUND) set(USE_PHONENUMBER 1) else() diff --git a/resources/CMakeLists.txt b/resources/CMakeLists.txt index 8bc4c40f..c7aa6732 100644 --- a/resources/CMakeLists.txt +++ b/resources/CMakeLists.txt @@ -19,16 +19,5 @@ endif() include_directories(${CMAKE_SOURCE_DIR} ${CMAKE_CURRENT_BINARY_DIR}) -if(APPLE OR WIN32) - set(_default_kwallet_option 0) -else() - set(_default_kwallet_option 1) -endif() -option(USE_KWALLET "Use KWallet for password storage (default 1 on Unix, 0 on Windows/Mac)" ${_default_kwallet_option}) -if(USE_KWALLET) - find_package(KF5Wallet ${KF5_VERSION} CONFIG REQUIRED) -endif() -configure_file(config-kwallet.h.cmake ${CMAKE_CURRENT_BINARY_DIR}/config-kwallet.h) - add_subdirectory(salesforce) add_subdirectory(sugarcrm) diff --git a/resources/config-kwallet.h.cmake b/resources/config-kwallet.h.cmake deleted file mode 100644 index 52899503..00000000 --- a/resources/config-kwallet.h.cmake +++ /dev/null @@ -1 +0,0 @@ -#cmakedefine01 USE_KWALLET diff --git a/resources/sugarcrm/CMakeLists.txt b/resources/sugarcrm/CMakeLists.txt index a1bbf937..a22a40aa 100644 --- a/resources/sugarcrm/CMakeLists.txt +++ b/resources/sugarcrm/CMakeLists.txt @@ -72,6 +72,7 @@ target_link_libraries(akonadi_sugarcrm_resource_private KF5::CalendarCore KF5::AkonadiContact KF5::WindowSystem + qt5keychain PUBLIC KF5::AkonadiAgentBase KF5::Contacts @@ -81,11 +82,7 @@ if(KCALENDARCORE_REQUIRES_KDE4SUPPORT) KF5::KDELibs4Support ) endif() -if(USE_KWALLET) - target_link_libraries(akonadi_sugarcrm_resource_private PRIVATE - KF5::Wallet - ) -endif() + target_link_libraries(akonadi_sugarcrm_resource_private PRIVATE KDReports::kdreports) target_link_libraries(akonadi_sugarcrm_resource PRIVATE diff --git a/resources/sugarcrm/passwordhandler.cpp b/resources/sugarcrm/passwordhandler.cpp index 4b15d0ac..b9c9a017 100644 --- a/resources/sugarcrm/passwordhandler.cpp +++ b/resources/sugarcrm/passwordhandler.cpp @@ -22,12 +22,9 @@ #include "sugarcrmresource_debug.h" #include -#if USE_KWALLET -#include -using KWallet::Wallet; +#include -static const char s_walletFolderName[] = "SugarCRM"; -#endif +static const char s_keychainFolderName[] = "SugarCRM"; // Implementation inspired from kdepim-runtime/resources/imap/settings.cpp branch KDE/4.14 @@ -35,66 +32,55 @@ PasswordHandler::PasswordHandler(const QString &resourceId, QObject *parent) : QObject(parent), mResourceId(resourceId) { -#if USE_KWALLET - // We have no GUI to use on startup - // We could at least use the config dialog though. - m_winId = 0; - mWalletOpened = false; - - Wallet *wallet = Wallet::openWallet( Wallet::NetworkWallet(), m_winId, Wallet::Asynchronous ); - if (wallet) { - connect(wallet, SIGNAL(walletOpened(bool)), - this, SLOT(onWalletOpened(bool))); - } else { - qCWarning(FATCRM_SUGARCRMRESOURCE_LOG) << "openWallet(Asynchronous) failed!"; - } -#endif + auto *job = new QKeychain::ReadPasswordJob(s_keychainFolderName); + + job->setKey(resourceId); + + connect(job, &QKeychain::Job::finished, this, [this, job] { + if (job->error()) { + qWarning() << "Error reading password for" << mResourceId << job->errorString(); + + if (job->error() == QKeychain::AccessDeniedByUser) { + mDeniedByUser = true; + } + } else { + mPassword = job->textData(); + mKeychainOpened = true; + Q_EMIT passwordAvailable(); + } + }); + + job->start(); } bool PasswordHandler::isPasswordAvailable() { -#if USE_KWALLET - if (!mWalletOpened) - return false; - QScopedPointer wallet(Wallet::openWallet(Wallet::NetworkWallet(), m_winId)); - return wallet && wallet->isOpen(); -#else - return true; -#endif + return mKeychainOpened; } QString PasswordHandler::password(bool *userRejected) { if (userRejected != nullptr) { - *userRejected = false; - } + *userRejected = mDeniedByUser; - if (!mPassword.isEmpty()) - return mPassword; -#if USE_KWALLET - QScopedPointer wallet(Wallet::openWallet(Wallet::NetworkWallet(), m_winId)); - if (wallet && wallet->isOpen()) { - if (wallet->hasFolder(QString(s_walletFolderName))) { - wallet->setFolder(QString(s_walletFolderName)); - wallet->readPassword(mResourceId, mPassword); - } else { - wallet->createFolder(QString(s_walletFolderName)); + if (mDeniedByUser) { + return QString(); } - // Initial migration: password not in wallet yet - if (mPassword.isEmpty()) { - mPassword = Settings::password(); - if (!mPassword.isEmpty()) { - savePassword(); - } + } + + if (!mKeychainOpened) { + return QString(); + } + + // Initial migration: password not in keychain yet + if (mPassword.isEmpty()) { + mPassword = Settings::password(); + if (!mPassword.isEmpty()) { + savePassword(); } - } else if (userRejected != nullptr) { - *userRejected = true; } -#else - mPassword = Settings::password(); -#endif - return mPassword; + return mPassword; } void PasswordHandler::setPassword(const QString &password) @@ -103,47 +89,25 @@ void PasswordHandler::setPassword(const QString &password) return; mPassword = password; - -#if USE_KWALLET savePassword(); -#else - Settings::setPassword(mPassword); - Settings::self()->writeConfig(); -#endif -} - -void PasswordHandler::onWalletOpened(bool success) -{ -#if USE_KWALLET - Wallet *wallet = qobject_cast( sender() ); - mWalletOpened = success; - if (wallet && success) { - // read and store the password - password(); - emit passwordAvailable(); - } - if (wallet) { - wallet->deleteLater(); - } -#else - Q_UNUSED(success); -#endif } -#if USE_KWALLET bool PasswordHandler::savePassword() { - QScopedPointer wallet(Wallet::openWallet(Wallet::NetworkWallet(), m_winId)); - if (wallet && wallet->isOpen()) { - if (!wallet->hasFolder(QString(s_walletFolderName))) - wallet->createFolder(QString(s_walletFolderName)); - wallet->setFolder(QString(s_walletFolderName)); - wallet->writePassword(mResourceId, mPassword); - wallet->sync(); - Settings::setPassword(QString()); // ensure no plain-text password from before in the config file - Settings::self()->save(); - return true; - } - return false; + auto *job = new QKeychain::WritePasswordJob(s_keychainFolderName); + job->setKey(mResourceId); + job->setTextData(mPassword); + + connect(job, &QKeychain::Job::finished, this, [this, job] { + if (job->error()) { + qWarning() << "password save error" << job->errorString(); + } + }); + + job->start(); + + Settings::setPassword(QString()); // ensure no plain-text password from before in the config file + Settings::self()->save(); + + return true; } -#endif diff --git a/resources/sugarcrm/passwordhandler.h b/resources/sugarcrm/passwordhandler.h index 7ff758e6..8674682a 100644 --- a/resources/sugarcrm/passwordhandler.h +++ b/resources/sugarcrm/passwordhandler.h @@ -24,8 +24,6 @@ #include #include -#include - class PasswordHandler : public QObject { Q_OBJECT @@ -39,18 +37,12 @@ class PasswordHandler : public QObject Q_SIGNALS: void passwordAvailable(); -private Q_SLOTS: - void onWalletOpened(bool success); - private: -#if USE_KWALLET bool savePassword(); - - WId m_winId; - bool mWalletOpened; -#endif QString mPassword; const QString mResourceId; + bool mKeychainOpened = false; + bool mDeniedByUser = false; }; #endif // PASSWORDHANDLER_H