Merge pull request #11726 from FranciscoPombal/fix_#11724

WebUI: Implement "Secure" flag for session cookie. Closes #11724
qbittorrent · Dec 25, 2019 · fea39fb · fea39fb
2 parents c94417b + 691d5e5
commit fea39fb
Show file tree

Hide file tree

Showing 8 changed files with 37 additions and 0 deletions.
diff --git a/src/base/preferences.cpp b/src/base/preferences.cpp
@@ -653,6 +653,16 @@ void Preferences::setWebUiCSRFProtectionEnabled(const bool enabled)
     setValue("Preferences/WebUI/CSRFProtection", enabled);
 }
 
+bool Preferences::isWebUiSecureCookieEnabled() const
+{
+    return value("Preferences/WebUI/SecureCookie", true).toBool();
+}
+
+void Preferences::setWebUiSecureCookieEnabled(const bool enabled)
+{
+    setValue("Preferences/WebUI/SecureCookie", enabled);
+}
+
 bool Preferences::isWebUIHostHeaderValidationEnabled() const
 {
     return value("Preferences/WebUI/HostHeaderValidation", true).toBool();

diff --git a/src/base/preferences.h b/src/base/preferences.h
@@ -202,6 +202,8 @@ class Preferences : public QObject
     void setWebUiClickjackingProtectionEnabled(bool enabled);
     bool isWebUiCSRFProtectionEnabled() const;
     void setWebUiCSRFProtectionEnabled(bool enabled);
+    bool isWebUiSecureCookieEnabled () const;
+    void setWebUiSecureCookieEnabled(bool enabled);
     bool isWebUIHostHeaderValidationEnabled() const;
     void setWebUIHostHeaderValidationEnabled(bool enabled);
 

diff --git a/src/gui/optionsdialog.cpp b/src/gui/optionsdialog.cpp
@@ -410,6 +410,8 @@ OptionsDialog::OptionsDialog(QWidget *parent)
     connect(m_ui->spinSessionTimeout, qSpinBoxValueChanged, this, &ThisType::enableApplyButton);
     connect(m_ui->checkClickjacking, &QCheckBox::toggled, this, &ThisType::enableApplyButton);
     connect(m_ui->checkCSRFProtection, &QCheckBox::toggled, this, &ThisType::enableApplyButton);
+    connect(m_ui->checkWebUiHttps, &QGroupBox::toggled, m_ui->checkSecureCookie, &QWidget::setEnabled);
+    connect(m_ui->checkSecureCookie, &QCheckBox::toggled, this, &ThisType::enableApplyButton);
     connect(m_ui->groupHostHeaderValidation, &QGroupBox::toggled, this, &ThisType::enableApplyButton);
     connect(m_ui->checkDynDNS, &QGroupBox::toggled, this, &ThisType::enableApplyButton);
     connect(m_ui->comboDNSService, qComboBoxCurrentIndexChanged, this, &ThisType::enableApplyButton);
@@ -797,6 +799,7 @@ void OptionsDialog::saveOptions()
         // Security
         pref->setWebUiClickjackingProtectionEnabled(m_ui->checkClickjacking->isChecked());
         pref->setWebUiCSRFProtectionEnabled(m_ui->checkCSRFProtection->isChecked());
+        pref->setWebUiSecureCookieEnabled(m_ui->checkSecureCookie->isChecked());
         pref->setWebUIHostHeaderValidationEnabled(m_ui->groupHostHeaderValidation->isChecked());
         // DynDNS
         pref->setDynDNSEnabled(m_ui->checkDynDNS->isChecked());
@@ -1174,6 +1177,8 @@ void OptionsDialog::loadOptions()
     // Security
     m_ui->checkClickjacking->setChecked(pref->isWebUiClickjackingProtectionEnabled());
     m_ui->checkCSRFProtection->setChecked(pref->isWebUiCSRFProtectionEnabled());
+    m_ui->checkSecureCookie->setEnabled(pref->isWebUiHttpsEnabled());
+    m_ui->checkSecureCookie->setChecked(pref->isWebUiSecureCookieEnabled());
     m_ui->groupHostHeaderValidation->setChecked(pref->isWebUIHostHeaderValidationEnabled());
 
     m_ui->checkDynDNS->setChecked(pref->isDynDNSEnabled());

diff --git a/src/gui/optionsdialog.ui b/src/gui/optionsdialog.ui
@@ -3061,6 +3061,13 @@ Specify an IPv4 or IPv6 address. You can specify &quot;0.0.0.0&quot; for any IPv
                     </property>
                    </widget>
                   </item>
+                  <item>
+                   <widget class="QCheckBox" name="checkSecureCookie">
+                    <property name="text">
+                     <string>Enable cookie Secure flag (requires HTTPS)</string>
+                    </property>
+                   </widget>
+                  </item>
                   <item>
                    <widget class="QGroupBox" name="groupHostHeaderValidation">
                     <property name="title">

diff --git a/src/webui/api/appcontroller.cpp b/src/webui/api/appcontroller.cpp
@@ -239,6 +239,7 @@ void AppController::preferencesAction()
     // Security
     data["web_ui_clickjacking_protection_enabled"] = pref->isWebUiClickjackingProtectionEnabled();
     data["web_ui_csrf_protection_enabled"] = pref->isWebUiCSRFProtectionEnabled();
+    data["web_ui_secure_cookie_enabled"] = pref->isWebUiSecureCookieEnabled();
     data["web_ui_host_header_validation_enabled"] = pref->isWebUIHostHeaderValidationEnabled();
     // Update my dynamic domain name
     data["dyndns_enabled"] = pref->isDynDNSEnabled();
@@ -608,6 +609,8 @@ void AppController::setPreferencesAction()
         pref->setWebUiClickjackingProtectionEnabled(it.value().toBool());
     if (hasKey("web_ui_csrf_protection_enabled"))
         pref->setWebUiCSRFProtectionEnabled(it.value().toBool());
+    if (hasKey("web_ui_secure_cookie_enabled"))
+        pref->setWebUiSecureCookieEnabled(it.value().toBool());
     if (hasKey("web_ui_host_header_validation_enabled"))
         pref->setWebUIHostHeaderValidationEnabled(it.value().toBool());
     // Update my dynamic domain name

diff --git a/src/webui/webapplication.cpp b/src/webui/webapplication.cpp
@@ -337,6 +337,7 @@ void WebApplication::configure()
 
     m_isClickjackingProtectionEnabled = pref->isWebUiClickjackingProtectionEnabled();
     m_isCSRFProtectionEnabled = pref->isWebUiCSRFProtectionEnabled();
+    m_isSecureCookieEnabled = pref->isWebUiSecureCookieEnabled();
     m_isHostHeaderValidationEnabled = pref->isWebUIHostHeaderValidationEnabled();
     m_isHttpsEnabled = pref->isWebUiHttpsEnabled();
 
@@ -535,6 +536,7 @@ void WebApplication::sessionStart()
 
     QNetworkCookie cookie(C_SID, m_currentSession->id().toUtf8());
     cookie.setHttpOnly(true);
+    cookie.setSecure(m_isSecureCookieEnabled && m_isHttpsEnabled);
     cookie.setPath(QLatin1String("/"));
     QByteArray cookieRawForm = cookie.toRawForm();
     if (m_isCSRFProtectionEnabled)

diff --git a/src/webui/webapplication.h b/src/webui/webapplication.h
@@ -153,6 +153,7 @@ class WebApplication
     QStringList m_domainList;
     bool m_isClickjackingProtectionEnabled;
     bool m_isCSRFProtectionEnabled;
+    bool m_isSecureCookieEnabled;
     bool m_isHostHeaderValidationEnabled;
     bool m_isHttpsEnabled;
     QString m_contentSecurityPolicy;

diff --git a/src/webui/www/private/views/preferences.html b/src/webui/www/private/views/preferences.html
@@ -756,6 +756,10 @@
                 <input type="checkbox" id="csrf_protection_checkbox" />
                 <label for="csrf_protection_checkbox">QBT_TR(Enable Cross-Site Request Forgery (CSRF) protection)QBT_TR[CONTEXT=OptionsDialog]</label>
             </div>
+            <div class="formRow">
+                <input type="checkbox" id="secureCookieCheckbox" />
+                <label for="secureCookieCheckbox">QBT_TR(Enable cookie Secure flag (requires HTTPS))QBT_TR[CONTEXT=OptionsDialog]</label>
+            </div>
 
             <fieldset class="settings">
                 <legend>
@@ -1350,6 +1354,7 @@
             const isUseHttpsEnabled = $('use_https_checkbox').getProperty('checked');
             $('ssl_cert_text').setProperty('disabled', !isUseHttpsEnabled);
             $('ssl_key_text').setProperty('disabled', !isUseHttpsEnabled);
+            $('secureCookieCheckbox').setProperty('disabled', !isUseHttpsEnabled);
         };
 
         const updateBypasssAuthSettings = function() {
@@ -1717,6 +1722,7 @@
                         // Security
                         $('clickjacking_protection_checkbox').setProperty('checked', pref.web_ui_clickjacking_protection_enabled);
                         $('csrf_protection_checkbox').setProperty('checked', pref.web_ui_csrf_protection_enabled);
+                        $('secureCookieCheckbox').setProperty('checked', pref.web_ui_secure_cookie_enabled);
                         $('host_header_validation_checkbox').setProperty('checked', pref.web_ui_host_header_validation_enabled);
                         updateHostHeaderValidationSettings();
 
@@ -2082,6 +2088,7 @@
 
             settings.set('web_ui_clickjacking_protection_enabled', $('clickjacking_protection_checkbox').getProperty('checked'));
             settings.set('web_ui_csrf_protection_enabled', $('csrf_protection_checkbox').getProperty('checked'));
+            settings.set('web_ui_secure_cookie_enabled', $('secureCookieCheckbox').getProperty('checked'));
             settings.set('web_ui_host_header_validation_enabled', $('host_header_validation_checkbox').getProperty('checked'));
 
             // Update my dynamic domain name