Store the proxy password in the keychain

Fixes: #261

changelog/unreleased/261

@@ -0,0 +1,3 @@
+Enhancement: Store proxy password in keychain
+
+https://github.com/owncloud/client/issues/261

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:

src/gui/application.h

@@ -96,8 +96,6 @@ protected slots:
     QString _userEnforcedLanguage;
     QString _displayLanguage;
 
-    ClientProxy _proxy;
-
     QTimer _checkConnectionTimer;
 
     QScopedPointer<FolderMan> _folderManager;

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);

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

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";
     }

src/gui/networksettings.cpp

@@ -26,11 +26,20 @@
 #include <QString>
 #include <QtGui/QtEvents>
 
+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<QNetworkProxy::ProxyType>(_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

src/gui/networksettings.h

@@ -15,6 +15,8 @@
 #ifndef MIRALL_NETWORKSETTINGS_H
 #define MIRALL_NETWORKSETTINGS_H
 
+#include "libsync/creds/credentialmanager.h"
+
 #include <QWidget>
 
 
@@ -51,6 +53,8 @@ private slots:
 private:
     void loadProxySettings();
     void loadBWLimitSettings();
+    CredentialManager *_credentialManager;
+
 
     Ui::NetworkSettings *_ui;
 };

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();

src/libsync/configfile.h

@@ -18,8 +18,9 @@
 #include "common/result.h"
 #include "owncloudlib.h"
 
-#include <QSharedPointer>
+#include <QNetworkProxy>
 #include <QSettings>
+#include <QSharedPointer>
 #include <QString>
 #include <QVariant>
 
@@ -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;