diff --git a/changelog/unreleased/261 b/changelog/unreleased/261 new file mode 100644 index 00000000000..7e4b2df4223 --- /dev/null +++ b/changelog/unreleased/261 @@ -0,0 +1,3 @@ +Enhancement: Store proxy password in keychain + +https://github.com/owncloud/client/issues/261 diff --git a/src/gui/application.cpp b/src/gui/application.cpp index 16267836cfd..58bb46a6653 100644 --- a/src/gui/application.cpp +++ b/src/gui/application.cpp @@ -225,7 +225,6 @@ Application::Application(Platform *platform, bool debugMode, QObject *parent) _gui = new ownCloudGui(this); FolderMan::instance()->setupFolders(); - _proxy.setupQtProxyFromConfig(); // folders have to be defined first, than we set up the Qt proxy. // Enable word wrapping of QInputDialog (#4197) // TODO: diff --git a/src/gui/application.h b/src/gui/application.h index f33f1b0d512..01f50e3baff 100644 --- a/src/gui/application.h +++ b/src/gui/application.h @@ -96,8 +96,6 @@ protected slots: QString _userEnforcedLanguage; QString _displayLanguage; - ClientProxy _proxy; - QTimer _checkConnectionTimer; QScopedPointer _folderManager; diff --git a/src/gui/clientproxy.cpp b/src/gui/clientproxy.cpp index 62996a1c039..53853592a08 100644 --- a/src/gui/clientproxy.cpp +++ b/src/gui/clientproxy.cpp @@ -23,25 +23,29 @@ namespace OCC { Q_LOGGING_CATEGORY(lcClientProxy, "sync.clientproxy", QtInfoMsg) -ClientProxy::ClientProxy(QObject *parent) - : QObject(parent) -{ +namespace { + + QNetworkProxy proxyFromConfig(const QString &password, const ConfigFile &cfg) + { + QNetworkProxy proxy; + + if (cfg.proxyHostName().isEmpty()) + return QNetworkProxy(); + + proxy.setHostName(cfg.proxyHostName()); + proxy.setPort(cfg.proxyPort()); + if (cfg.proxyNeedsAuth()) { + proxy.setUser(cfg.proxyUser()); + proxy.setPassword(password); + } + return proxy; + } + } -static QNetworkProxy proxyFromConfig(const ConfigFile &cfg) +QString ClientProxy::printQNetworkProxy(const QNetworkProxy &proxy) { - QNetworkProxy proxy; - - if (cfg.proxyHostName().isEmpty()) - return QNetworkProxy(); - - proxy.setHostName(cfg.proxyHostName()); - proxy.setPort(cfg.proxyPort()); - if (cfg.proxyNeedsAuth()) { - proxy.setUser(cfg.proxyUser()); - proxy.setPassword(cfg.proxyPassword()); - } - return proxy; + return QStringLiteral("%1://%2:%3").arg(proxy.type()).arg(proxy.hostName()).arg(proxy.port()); } bool ClientProxy::isUsingSystemDefault() @@ -56,12 +60,7 @@ bool ClientProxy::isUsingSystemDefault() return false; } -QString printQNetworkProxy(const QNetworkProxy &proxy) -{ - return QStringLiteral("%1://%2:%3").arg(proxy.type()).arg(proxy.hostName()).arg(proxy.port()); -} - -void ClientProxy::setupQtProxyFromConfig() +void ClientProxy::setupQtProxyFromConfig(const QString &password) { OCC::ConfigFile cfg; int proxyType = QNetworkProxy::DefaultProxy; @@ -70,7 +69,7 @@ void ClientProxy::setupQtProxyFromConfig() // if there is no config file, default to system proxy. if (cfg.exists()) { proxyType = cfg.proxyType(); - proxy = proxyFromConfig(cfg); + proxy = proxyFromConfig(password, cfg); } switch (proxyType) { @@ -100,26 +99,6 @@ void ClientProxy::setupQtProxyFromConfig() } } -const char *ClientProxy::proxyTypeToCStr(QNetworkProxy::ProxyType type) -{ - switch (type) { - case QNetworkProxy::NoProxy: - return "NoProxy"; - case QNetworkProxy::DefaultProxy: - return "DefaultProxy"; - case QNetworkProxy::Socks5Proxy: - return "Socks5Proxy"; - case QNetworkProxy::HttpProxy: - return "HttpProxy"; - case QNetworkProxy::HttpCachingProxy: - return "HttpCachingProxy"; - case QNetworkProxy::FtpCachingProxy: - return "FtpCachingProxy"; - default: - return "NoProxy"; - } -} - void ClientProxy::lookupSystemProxyAsync(const QUrl &url, QObject *dst, const char *slot) { SystemProxyRunnable *runnable = new SystemProxyRunnable(url); diff --git a/src/gui/clientproxy.h b/src/gui/clientproxy.h index 7ef80f11449..81b368b475c 100644 --- a/src/gui/clientproxy.h +++ b/src/gui/clientproxy.h @@ -31,20 +31,12 @@ class ConfigFile; * @brief The ClientProxy class * @ingroup libsync */ -class ClientProxy : public QObject -{ - Q_OBJECT -public: - explicit ClientProxy(QObject *parent = nullptr); - - static bool isUsingSystemDefault(); - static void lookupSystemProxyAsync(const QUrl &url, QObject *dst, const char *slot); +namespace ClientProxy { + bool isUsingSystemDefault(); + void lookupSystemProxyAsync(const QUrl &url, QObject *dst, const char *slot); + void setupQtProxyFromConfig(const QString &password); -public slots: - void setupQtProxyFromConfig(); - -private: - const char *proxyTypeToCStr(QNetworkProxy::ProxyType type); + QString printQNetworkProxy(const QNetworkProxy &proxy); }; class SystemProxyRunnable : public QObject, public QRunnable @@ -59,8 +51,6 @@ class SystemProxyRunnable : public QObject, public QRunnable private: QUrl _url; }; - -QString printQNetworkProxy(const QNetworkProxy &proxy); } #endif // CLIENTPROXY_H diff --git a/src/gui/connectionvalidator.cpp b/src/gui/connectionvalidator.cpp index b77fe6c9a97..6c0c763b5ad 100644 --- a/src/gui/connectionvalidator.cpp +++ b/src/gui/connectionvalidator.cpp @@ -85,7 +85,7 @@ void ConnectionValidator::systemProxyLookupDone(const QNetworkProxy &proxy) } if (proxy.type() != QNetworkProxy::NoProxy) { - qCInfo(lcConnectionValidator) << "Setting QNAM proxy to be system proxy" << printQNetworkProxy(proxy); + qCInfo(lcConnectionValidator) << "Setting QNAM proxy to be system proxy" << ClientProxy::printQNetworkProxy(proxy); } else { qCInfo(lcConnectionValidator) << "No system proxy set by OS"; } diff --git a/src/gui/networksettings.cpp b/src/gui/networksettings.cpp index a5c3e2031c9..e34b980ecad 100644 --- a/src/gui/networksettings.cpp +++ b/src/gui/networksettings.cpp @@ -26,11 +26,20 @@ #include #include +namespace { +auto proxyPasswordC() +{ + return QStringLiteral("Proxy/Password"); +} +} namespace OCC { +Q_LOGGING_CATEGORY(lcNetworkSettings, "gui.networksettings.gui", QtInfoMsg) + NetworkSettings::NetworkSettings(QWidget *parent) : QWidget(parent) , _ui(new Ui::NetworkSettings) + , _credentialManager(new CredentialManager(this)) { _ui->setupUi(this); @@ -124,7 +133,23 @@ void NetworkSettings::loadProxySettings() _ui->portSpinBox->setValue(port); _ui->authRequiredcheckBox->setChecked(cfgFile.proxyNeedsAuth()); _ui->userLineEdit->setText(cfgFile.proxyUser()); - _ui->passwordLineEdit->setText(cfgFile.proxyPassword()); + + const QString legacyPasswordKey = QStringLiteral("Proxy/pass"); + const QString legacyPassword = QString::fromUtf8(QByteArray::fromBase64(ConfigFile().makeQSettings().value(legacyPasswordKey).toByteArray())); + if (!legacyPassword.isEmpty()) { + qCWarning(lcNetworkSettings) << "Migrating legacy proxy password to keychain"; + ConfigFile().makeQSettings().remove(legacyPasswordKey); + _credentialManager->set(proxyPasswordC(), legacyPassword); + _ui->passwordLineEdit->setText(legacyPassword); + ClientProxy::setupQtProxyFromConfig(legacyPassword); + } else { + auto job = _credentialManager->get(proxyPasswordC()); + connect(job, &CredentialJob::finished, this, [job, this] { + const QString password = job->data().toString(); + _ui->passwordLineEdit->setText(password); + ClientProxy::setupQtProxyFromConfig(password); + }); + } } void NetworkSettings::loadBWLimitSettings() @@ -162,19 +187,15 @@ void NetworkSettings::saveProxySettings() } else if (_ui->systemProxyRadioButton->isChecked()) { cfgFile.setProxyType(QNetworkProxy::DefaultProxy); } else if (_ui->manualProxyRadioButton->isChecked()) { - int type = _ui->typeComboBox->itemData(_ui->typeComboBox->currentIndex()).toInt(); - QString host = _ui->hostLineEdit->text(); - if (host.isEmpty()) + auto type = static_cast(_ui->typeComboBox->itemData(_ui->typeComboBox->currentIndex()).toInt()); + if (_ui->hostLineEdit->text().isEmpty()) { type = QNetworkProxy::NoProxy; - bool needsAuth = _ui->authRequiredcheckBox->isChecked(); - QString user = _ui->userLineEdit->text(); - QString pass = _ui->passwordLineEdit->text(); - cfgFile.setProxyType(type, _ui->hostLineEdit->text(), - _ui->portSpinBox->value(), needsAuth, user, pass); + } + _credentialManager->set(proxyPasswordC(), _ui->passwordLineEdit->text()); + cfgFile.setProxyType(type, _ui->hostLineEdit->text(), _ui->portSpinBox->value(), _ui->authRequiredcheckBox->isChecked(), _ui->userLineEdit->text()); } - ClientProxy proxy; - proxy.setupQtProxyFromConfig(); // Refresh the Qt proxy settings as the + ClientProxy::setupQtProxyFromConfig(_ui->passwordLineEdit->text()); // Refresh the Qt proxy settings as the // quota check can happen all the time. // ...and set the folders dirty, they refresh their proxy next time they diff --git a/src/gui/networksettings.h b/src/gui/networksettings.h index 59086cd95c8..6c6ef5f9e4f 100644 --- a/src/gui/networksettings.h +++ b/src/gui/networksettings.h @@ -15,6 +15,8 @@ #ifndef MIRALL_NETWORKSETTINGS_H #define MIRALL_NETWORKSETTINGS_H +#include "libsync/creds/credentialmanager.h" + #include @@ -51,6 +53,8 @@ private slots: private: void loadProxySettings(); void loadBWLimitSettings(); + CredentialManager *_credentialManager; + Ui::NetworkSettings *_ui; }; diff --git a/src/libsync/configfile.cpp b/src/libsync/configfile.cpp index 55b8adb7a02..9b313ef4151 100644 --- a/src/libsync/configfile.cpp +++ b/src/libsync/configfile.cpp @@ -75,8 +75,10 @@ const QString clientVersionC() { return QStringLiteral("clientVersion"); } const QString proxyHostC() { return QStringLiteral("Proxy/host"); } const QString proxyTypeC() { return QStringLiteral("Proxy/type"); } const QString proxyPortC() { return QStringLiteral("Proxy/port"); } -const QString proxyUserC() { return QStringLiteral("Proxy/user"); } -const QString proxyPassC() { return QStringLiteral("Proxy/pass"); } +const QString proxyUserC() +{ + return QStringLiteral("Proxy/user"); +} const QString proxyNeedsAuthC() { return QStringLiteral("Proxy/needsAuth"); } const QString useUploadLimitC() { return QStringLiteral("BWLimit/useUploadLimit"); } @@ -558,11 +560,7 @@ void ConfigFile::setUiLanguage(const QString &uiLanguage) settings.setValue(uiLanguageC(), uiLanguage); } -void ConfigFile::setProxyType(int proxyType, - const QString &host, - int port, bool needsAuth, - const QString &user, - const QString &pass) +void ConfigFile::setProxyType(QNetworkProxy::ProxyType proxyType, const QString &host, int port, bool needsAuth, const QString &user) { auto settings = makeQSettings(); @@ -573,7 +571,6 @@ void ConfigFile::setProxyType(int proxyType, settings.setValue(proxyPortC(), port); settings.setValue(proxyNeedsAuthC(), needsAuth); settings.setValue(proxyUserC(), user); - settings.setValue(proxyPassC(), pass.toUtf8().toBase64()); } settings.sync(); } @@ -646,12 +643,6 @@ QString ConfigFile::proxyUser() const return getValue(proxyUserC()).toString(); } -QString ConfigFile::proxyPassword() const -{ - QByteArray pass = getValue(proxyPassC()).toByteArray(); - return QString::fromUtf8(QByteArray::fromBase64(pass)); -} - int ConfigFile::useUploadLimit() const { return getValue(useUploadLimitC(), QString(), 0).toInt(); diff --git a/src/libsync/configfile.h b/src/libsync/configfile.h index 57daa69c6fa..1d6fdd6fb27 100644 --- a/src/libsync/configfile.h +++ b/src/libsync/configfile.h @@ -18,8 +18,9 @@ #include "common/result.h" #include "owncloudlib.h" -#include +#include #include +#include #include #include @@ -120,18 +121,14 @@ class OWNCLOUDSYNC_EXPORT ConfigFile bool showExperimentalOptions() const; // proxy settings - void setProxyType(int proxyType, - const QString &host = QString(), - int port = 0, bool needsAuth = false, - const QString &user = QString(), - const QString &pass = QString()); + void setProxyType( + QNetworkProxy::ProxyType proxyType, const QString &host = QString(), int port = 0, bool needsAuth = false, const QString &user = QString()); int proxyType() const; QString proxyHostName() const; int proxyPort() const; bool proxyNeedsAuth() const; QString proxyUser() const; - QString proxyPassword() const; /** 0: no limit, 1: manual, >0: automatic */ int useUploadLimit() const;