diff --git a/.github/workflows/android.yml b/.github/workflows/android.yml index 7fc4e17c9..0a8f19030 100644 --- a/.github/workflows/android.yml +++ b/.github/workflows/android.yml @@ -240,6 +240,7 @@ jobs: -DQT_ANDROID_SIGN_APK=Yes \ -DQT_ANDROID_SIGN_AAB=Yes \ -DUSE_MM_SERVER_API_KEY=Yes \ + -DUSE_KEYCHAIN=No \ -DCMAKE_TOOLCHAIN_FILE=$QT_BASE/android_arm64_v8a/lib/cmake/Qt6/qt.toolchain.cmake \ -GNinja \ -DCMAKE_CXX_COMPILER_LAUNCHER=ccache \ diff --git a/.github/workflows/ios.yml b/.github/workflows/ios.yml index db341858d..581aa9e7f 100644 --- a/.github/workflows/ios.yml +++ b/.github/workflows/ios.yml @@ -196,6 +196,7 @@ jobs: -DQT_HOST_PATH=${{ github.workspace }}/Qt/${{ env.QT_VERSION }}/macos \ -DIOS=TRUE \ -DUSE_MM_SERVER_API_KEY=TRUE \ + -DUSE_KEYCHAIN=No \ -DCMAKE_INSTALL_PREFIX:PATH=../install-Input \ -DINPUT_SDK_PATH=${{ github.workspace }}/input-sdk/arm64-ios \ -G "Xcode" \ diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml index c8269a09b..ee631f8b4 100644 --- a/.github/workflows/linux.yml +++ b/.github/workflows/linux.yml @@ -142,13 +142,13 @@ jobs: -DCMAKE_BUILD_TYPE=Debug \ -DCMAKE_PREFIX_PATH=${{ github.workspace }}/Qt/${{ env.QT_VERSION }}/gcc_64 \ -DUSE_SERVER_API_KEY=TRUE \ + -DUSE_KEYCHAIN=No \ -DINPUT_SDK_PATH=${{ github.workspace }}/input-sdk/x64-linux \ -DQGIS_QUICK_DATA_PATH=${{ github.workspace }}/input/app/android/assets/qgis-data \ -DUSE_MM_SERVER_API_KEY=TRUE \ -DCOVERAGE=TRUE \ -DCMAKE_CXX_COMPILER_LAUNCHER=ccache \ -DENABLE_TESTS=TRUE \ - -DUSE_INSECURE_KEYCHAIN_FALLBACK=TRUE \ -GNinja \ -S ../input @@ -192,6 +192,7 @@ jobs: -DINPUT_SDK_PATH=${{ github.workspace }}/input-sdk/x64-linux \ -DQGIS_QUICK_DATA_PATH=${{ github.workspace }}/input/app/android/assets/qgis-data \ -DUSE_MM_SERVER_API_KEY=TRUE \ + -DUSE_KEYCHAIN=No \ -DCMAKE_CXX_COMPILER_LAUNCHER=ccache \ -GNinja \ -S ../input diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml index 749236393..07a8287e3 100644 --- a/.github/workflows/macos.yml +++ b/.github/workflows/macos.yml @@ -141,12 +141,12 @@ jobs: -DCMAKE_BUILD_TYPE=Debug \ -DCMAKE_PREFIX_PATH=${{ github.workspace }}/Qt/${{ env.QT_VERSION }}/macos \ -DUSE_SERVER_API_KEY=TRUE \ + -DUSE_KEYCHAIN=No \ -DINPUT_SDK_PATH=${{ github.workspace }}/input-sdk/x64-osx \ -DQGIS_QUICK_DATA_PATH=${{ github.workspace }}/input/app/android/assets/qgis-data \ -DCOVERAGE=TRUE \ -DCMAKE_CXX_COMPILER_LAUNCHER=ccache \ -DENABLE_TESTS=TRUE \ - -DUSE_INSECURE_KEYCHAIN_FALLBACK=TRUE \ -GNinja \ -S ../input @@ -174,6 +174,7 @@ jobs: -DCMAKE_INSTALL_PREFIX:PATH=../install-Input \ -DUSE_SERVER_API_KEY=TRUE \ -DUSE_MM_SERVER_API_KEY=TRUE \ + -DUSE_KEYCHAIN=No \ -DINPUT_SDK_PATH=${{ github.workspace }}/input-sdk/x64-osx \ -DQGIS_QUICK_DATA_PATH=${{ github.workspace }}/input/app/android/assets/qgis-data \ -DCMAKE_CXX_COMPILER_LAUNCHER=ccache \ diff --git a/.github/workflows/macos_arm64.yml b/.github/workflows/macos_arm64.yml index 322752a73..fdba772b7 100644 --- a/.github/workflows/macos_arm64.yml +++ b/.github/workflows/macos_arm64.yml @@ -144,6 +144,7 @@ jobs: -DCMAKE_INSTALL_PREFIX:PATH=../install-Input \ -DUSE_SERVER_API_KEY=TRUE \ -DUSE_MM_SERVER_API_KEY=TRUE \ + -DUSE_KEYCHAIN=No \ -DINPUT_SDK_PATH=${{ github.workspace }}/input-sdk/arm64-osx \ -DQGIS_QUICK_DATA_PATH=${{ github.workspace }}/input/app/android/assets/qgis-data \ -DCMAKE_CXX_COMPILER_LAUNCHER=ccache \ diff --git a/.github/workflows/win.yml b/.github/workflows/win.yml index 17599b35d..fcc87708a 100644 --- a/.github/workflows/win.yml +++ b/.github/workflows/win.yml @@ -153,6 +153,7 @@ jobs: -DCMAKE_INSTALL_PREFIX:PATH=${{ steps.vars.outputs.WORKSPACE_DIR }}/install-Input ^ -DINPUT_SDK_PATH:PATH=${{ steps.vars.outputs.WORKSPACE_DIR }}/input-sdk/x64-windows ^ -DUSE_MM_SERVER_API_KEY=TRUE ^ + -DUSE_KEYCHAIN=No ^ -DCMAKE_CXX_COMPILER_LAUNCHER=ccache ^ -G "NMake Makefiles" ^ -S ${{ steps.vars.outputs.WORKSPACE_DIR }}/input ^ diff --git a/CMakeLists.txt b/CMakeLists.txt index e0fe1f664..c04bd7b49 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -143,11 +143,11 @@ set(HAVE_BLUETOOTH CACHE BOOL "Building with bluetooth position provider" ) -set(USE_INSECURE_KEYCHAIN_FALLBACK +set(USE_KEYCHAIN FALSE CACHE BOOL - "Wheter to use insecure credential store if keychain is not available. Useful in CI" + "Wheter to use keychains/wallets to store credentials. If false, we use QSettings" ) set(QT6_VERSION diff --git a/core/CMakeLists.txt b/core/CMakeLists.txt index c6ff3aa61..2ea847944 100644 --- a/core/CMakeLists.txt +++ b/core/CMakeLists.txt @@ -14,7 +14,6 @@ set(MM_CORE_SRCS project.cpp geodiffutils.cpp projectchecksumcache.cpp - credentialstore.cpp ) set(MM_CORE_HDRS @@ -52,6 +51,14 @@ else () ) endif () +if (USE_KEYCHAIN) + set(MM_CORE_SRCS ${MM_CORE_SRCS} credentialstorekeychain.cpp) + message(STATUS "Using QtKeychain to store credentials.") +else() + set(MM_CORE_SRCS ${MM_CORE_SRCS} credentialstoreplaintext.cpp) + message(STATUS "Using QSettings to store credentials.") +endif() + add_library(mm_core OBJECT ${MM_CORE_SRCS} ${MM_CORE_HDRS}) target_include_directories(mm_core PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}) target_link_libraries(mm_core PRIVATE Qt6::Core Qt6::Network Geodiff::Geodiff) diff --git a/core/credentialstore.h b/core/credentialstore.h index 754fcdfe7..3f57c5b15 100644 --- a/core/credentialstore.h +++ b/core/credentialstore.h @@ -16,6 +16,12 @@ #include +/** + * \brief The CredentialStore class stores user credentials either to QtKeychain + * or QSettings, according to USE_KEYCHAIN cmake flag. + * + * \note Read and write operations are async when QtKeychain is used + */ class CredentialStore : public QObject { Q_OBJECT diff --git a/core/credentialstore.cpp b/core/credentialstorekeychain.cpp similarity index 99% rename from core/credentialstore.cpp rename to core/credentialstorekeychain.cpp index d8335501a..3f0de1f81 100644 --- a/core/credentialstore.cpp +++ b/core/credentialstorekeychain.cpp @@ -84,7 +84,7 @@ void CredentialStore::writeAuthData mWriteJob->start(); // - // 3. Clear any previous data from QSettings (migration from the previous insecure QSettings) + // 3. Clear any previous data from QSettings (migration from the previous QSettings) // // TODO: pass diff --git a/core/credentialstoreplaintext.cpp b/core/credentialstoreplaintext.cpp new file mode 100644 index 000000000..b2b4b37b9 --- /dev/null +++ b/core/credentialstoreplaintext.cpp @@ -0,0 +1,83 @@ +/*************************************************************************** + * * + * 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; either version 2 of the License, or * + * (at your option) any later version. * + * * + ***************************************************************************/ + +#include "credentialstore.h" +#include "coreutils.h" + +#include + +const QString CredentialStore::KEYCHAIN_GROUP = QStringLiteral( "Input/" ); +const QString CredentialStore::KEYCHAIN_ENTRY_CREDENTIALS = QStringLiteral( "" ); // unused +const QString CredentialStore::KEYCHAIN_ENTRY_TOKEN = QStringLiteral( "" ); // unused + +const QString CredentialStore::KEY_USERNAME = QStringLiteral( "username" ); +const QString CredentialStore::KEY_PASSWORD = QStringLiteral( "password" ); +const QString CredentialStore::KEY_USERID = QStringLiteral( "userId" ); +const QString CredentialStore::KEY_TOKEN = QStringLiteral( "token" ); +const QString CredentialStore::KEY_EXPIRE = QStringLiteral( "expire" ); + +// +// We store credentials in QSettings and read them in synchronous operation +// + +CredentialStore::CredentialStore( QObject *parent ) + : QObject( parent ) +{ + // mWriteJob and mReadJob are unused +} + +void CredentialStore::writeAuthData +( const QString &username, + const QString &password, + int userId, + const QString &token, + const QDateTime &tokenExpiration ) +{ + QSettings settings; + settings.beginGroup( KEYCHAIN_GROUP ); + + settings.setValue( KEY_USERNAME, username ); + settings.setValue( KEY_PASSWORD, password ); + settings.setValue( KEY_USERID, userId ); + settings.setValue( KEY_TOKEN, token ); + settings.setValue( KEY_EXPIRE, tokenExpiration ); + + settings.endGroup(); +} + +void CredentialStore::readAuthData() +{ + QString username, password; + int userid = -1; + QByteArray token; + QDateTime tokenExpiration; + + QSettings settings; + settings.beginGroup( KEYCHAIN_GROUP ); + + username = settings.value( KEY_USERNAME ).toString(); + password = settings.value( KEY_PASSWORD ).toString(); + userid = settings.value( KEY_USERID ).toInt(); + token = settings.value( KEY_TOKEN ).toByteArray(); + tokenExpiration = settings.value( KEY_EXPIRE ).toDateTime(); + + settings.endGroup(); + + emit authDataRead( username, password, userid, token, tokenExpiration ); +} + +void CredentialStore::readKeyRecursively( const QString &key ) +{ + // no op +} + +void CredentialStore::finishReadingOperation() +{ + // no op +} diff --git a/core/merginapi.cpp b/core/merginapi.cpp index 8f5c33e22..4b5a9a976 100644 --- a/core/merginapi.cpp +++ b/core/merginapi.cpp @@ -55,12 +55,6 @@ MerginApi::MerginApi( LocalProjectsManager &localProjects, QObject *parent ) QSettings cache; if ( cache.contains( QStringLiteral( "Input/apiRoot" ) ) ) { - QObject::connect( mUserAuth, &MerginUserAuth::credentialsLoaded, this, [this]() - { - QObject::connect( this, &MerginApi::pingMerginFinished, this, &MerginApi::getUserInfo, Qt::SingleShotConnection ); - QObject::connect( this, &MerginApi::userInfoReplyFinished, this, &MerginApi::getWorkspaceInfo, Qt::SingleShotConnection ); - } ); - loadCache(); } else @@ -111,6 +105,12 @@ MerginApi::MerginApi( LocalProjectsManager &localProjects, QObject *parent ) getServerConfig(); pingMergin(); + + if ( mUserAuth->hasAuthData() ) + { + QObject::connect( this, &MerginApi::pingMerginFinished, this, &MerginApi::getUserInfo, Qt::SingleShotConnection ); + QObject::connect( this, &MerginApi::userInfoReplyFinished, this, &MerginApi::getWorkspaceInfo, Qt::SingleShotConnection ); + } } void MerginApi::loadCache() diff --git a/core/merginuserauth.cpp b/core/merginuserauth.cpp index f9409d043..b409c5209 100644 --- a/core/merginuserauth.cpp +++ b/core/merginuserauth.cpp @@ -11,11 +11,6 @@ MerginUserAuth::MerginUserAuth( QObject *parent ) : QObject( parent ) - , mUsername( "" ) - , mPassword( "" ) - , mUserId( -1 ) - , mAuthToken() - , mTokenExpiration() , mCredentialStore( new CredentialStore( this ) ) { clear();