Enhance update checker

* Reduce initial update check notification to 500 ms to prevent inappropriately stealing focus from user
* Add build flag WITH_XC_UPDATECHECK which defaults to ON
* Update checks are resolved every 7 days instead of every time the application is started
* Better checks for beta builds; ignore snapshots
* Increase test cases

11 files changed, 135 insertions, 55 deletions

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 3e7928570..536b08d9d 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -49,6 +49,7 @@ option(WITH_XC_YUBIKEY "Include YubiKey support." OFF)
 option(WITH_XC_SSHAGENT "Include SSH agent support." OFF)
 option(WITH_XC_KEESHARE "Sharing integration with KeeShare" OFF)
 option(WITH_XC_KEESHARE_SECURE "Sharing integration with secured KeeShare containers" OFF)
+option(WITH_XC_UPDATECHECK "Include automatic update checks; disable for controlled distributions" ON)
 if(APPLE)
     option(WITH_XC_TOUCHID "Include TouchID support for macOS." OFF)
 endif()
@@ -76,6 +77,10 @@ else()
   set(WITH_XC_CRYPTO_SSH OFF)
 endif()
 
+if(WITH_XC_UPDATECHECK)
+    set(WITH_XC_NETWORKING ON)
+endif()
+
 set(KEEPASSXC_VERSION_MAJOR "2")
 set(KEEPASSXC_VERSION_MINOR "4")
 set(KEEPASSXC_VERSION_PATCH "0")
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index b2cd27232..d8eb681e3 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -195,6 +195,7 @@ add_feature_info(SSHAgent WITH_XC_SSHAGENT "SSH agent integration compatible wit
 add_feature_info(KeeShare WITH_XC_KEESHARE "Sharing integration with KeeShare")
 add_feature_info(KeeShare-Secure WITH_XC_KEESHARE_SECURE "Sharing integration with KeeShare with secure sources")
 add_feature_info(YubiKey WITH_XC_YUBIKEY "YubiKey HMAC-SHA1 challenge-response")
+add_feature_info(UpdateCheck WITH_XC_UPDATECHECK "Automatic update checking")
 if(APPLE)
     add_feature_info(TouchID WITH_XC_TOUCHID "TouchID integration")
 endif()
diff --git a/src/config-keepassx.h.cmake b/src/config-keepassx.h.cmake
index 7d7018861..2acff4466 100644
--- a/src/config-keepassx.h.cmake
+++ b/src/config-keepassx.h.cmake
@@ -20,6 +20,7 @@
 #cmakedefine WITH_XC_KEESHARE
 #cmakedefine WITH_XC_KEESHARE_INSECURE
 #cmakedefine WITH_XC_KEESHARE_SECURE
+#cmakedefine WITH_XC_UPDATECHECK
 #cmakedefine WITH_XC_TOUCHID
 
 #cmakedefine KEEPASSXC_BUILD_TYPE "@KEEPASSXC_BUILD_TYPE@"
diff --git a/src/core/Clock.cpp b/src/core/Clock.cpp
index 88ac4fb77..be9e91dcf 100644
--- a/src/core/Clock.cpp
+++ b/src/core/Clock.cpp
@@ -30,6 +30,7 @@ QDateTime Clock::currentDateTime()
 
 uint Clock::currentSecondsSinceEpoch()
 {
+    // TODO: change to toSecsSinceEpoch() when min Qt >= 5.8
     return instance().currentDateTimeImpl().toTime_t();
 }
 
diff --git a/src/gui/ApplicationSettingsWidget.cpp b/src/gui/ApplicationSettingsWidget.cpp
index 849df03ae..22a49dece 100644
--- a/src/gui/ApplicationSettingsWidget.cpp
+++ b/src/gui/ApplicationSettingsWidget.cpp
@@ -92,8 +92,15 @@ ApplicationSettingsWidget::ApplicationSettingsWidget(QWidget* parent)
             m_secUi->touchIDResetSpinBox, SLOT(setEnabled(bool)));
     // clang-format on
 
-#ifndef WITH_XC_NETWORKING
+#ifdef WITH_XC_UPDATECHECK
+    connect(m_generalUi->checkForUpdatesOnStartupCheckBox, SIGNAL(toggled(bool)), SLOT(checkUpdatesToggled(bool)));
+#else
     m_generalUi->checkForUpdatesOnStartupCheckBox->setVisible(false);
+    m_generalUi->checkForUpdatesIncludeBetasCheckBox->setVisible(false);
+    m_generalUi->checkUpdatesSpacer->changeSize(0,0, QSizePolicy::Fixed, QSizePolicy::Fixed);
+#endif
+
+#ifndef WITH_XC_NETWORKING
     m_secUi->privacy->setVisible(false);
 #endif
 
@@ -350,3 +357,8 @@ void ApplicationSettingsWidget::rememberDatabasesToggled(bool checked)
     m_generalUi->rememberLastKeyFilesCheckBox->setEnabled(checked);
     m_generalUi->openPreviousDatabasesOnStartupCheckBox->setEnabled(checked);
 }
+
+void ApplicationSettingsWidget::checkUpdatesToggled(bool checked)
+{
+    m_generalUi->checkForUpdatesIncludeBetasCheckBox->setEnabled(checked);
+}
diff --git a/src/gui/ApplicationSettingsWidget.h b/src/gui/ApplicationSettingsWidget.h
index 85b3b4700..dfffbddbd 100644
--- a/src/gui/ApplicationSettingsWidget.h
+++ b/src/gui/ApplicationSettingsWidget.h
@@ -57,6 +57,7 @@ private slots:
     void systrayToggled(bool checked);
     void toolbarSettingsToggled(bool checked);
     void rememberDatabasesToggled(bool checked);
+    void checkUpdatesToggled(bool checked);
 
 private:
     QWidget* const m_secWidget;
diff --git a/src/gui/ApplicationSettingsWidgetGeneral.ui b/src/gui/ApplicationSettingsWidgetGeneral.ui
index f84c6a550..9f03bbb50 100644
--- a/src/gui/ApplicationSettingsWidgetGeneral.ui
+++ b/src/gui/ApplicationSettingsWidgetGeneral.ui
@@ -141,10 +141,40 @@
           <item>
            <widget class="QCheckBox" name="checkForUpdatesOnStartupCheckBox">
             <property name="text">
-             <string>Check for updates at application startup</string>
+             <string>Check for updates at application startup once per week</string>
             </property>
            </widget>
           </item>
+          <item>
+           <layout class="QHBoxLayout" name="checkUpdatesSubLayout">
+            <property name="spacing">
+             <number>0</number>
+            </property>
+            <item>
+             <spacer name="checkUpdatesSpacer">
+              <property name="orientation">
+               <enum>Qt::Horizontal</enum>
+              </property>
+              <property name="sizeType">
+               <enum>QSizePolicy::Fixed</enum>
+              </property>
+              <property name="sizeHint" stdset="0">
+               <size>
+                <width>20</width>
+                <height>20</height>
+               </size>
+              </property>
+             </spacer>
+            </item>
+            <item>
+             <widget class="QCheckBox" name="checkForUpdatesIncludeBetasCheckBox">
+              <property name="text">
+               <string>Include beta releases when checking for updates</string>
+              </property>
+             </widget>
+            </item>
+           </layout>
+          </item>
          </layout>
         </widget>
        </item>
@@ -242,13 +272,6 @@
          </property>
          <layout class="QVBoxLayout" name="verticalLayout_7">
           <item>
-           <widget class="QCheckBox" name="checkForUpdatesIncludeBetasCheckBox">
-            <property name="text">
-             <string>Include pre-releases when checking for updates</string>
-            </property>
-           </widget>
-          </item>
-          <item>
            <widget class="QCheckBox" name="toolbarHideCheckBox">
             <property name="text">
              <string>Hide toolbar (icons)</string>
diff --git a/src/gui/MainWindow.cpp b/src/gui/MainWindow.cpp
index 9e60b53e2..bf8c6654e 100644
--- a/src/gui/MainWindow.cpp
+++ b/src/gui/MainWindow.cpp
@@ -41,7 +41,7 @@
 #include "keys/FileKey.h"
 #include "keys/PasswordKey.h"
 
-#ifdef WITH_XC_NETWORKING
+#ifdef WITH_XC_UPDATECHECK
 #include "gui/MessageBox.h"
 #include "gui/UpdateCheckDialog.h"
 #include "updatecheck/UpdateChecker.h"
@@ -372,12 +372,12 @@ MainWindow::MainWindow()
     setUnifiedTitleAndToolBarOnMac(true);
 #endif
 
-#ifdef WITH_XC_NETWORKING
+#ifdef WITH_XC_UPDATECHECK
     connect(m_ui->actionCheckForUpdates, SIGNAL(triggered()), SLOT(showUpdateCheckDialog()));
     connect(UpdateChecker::instance(),
             SIGNAL(updateCheckFinished(bool, QString, bool)),
             SLOT(hasUpdateAvailable(bool, QString, bool)));
-    QTimer::singleShot(3000, this, SLOT(showUpdateCheckStartup()));
+    QTimer::singleShot(500, this, SLOT(showUpdateCheckStartup()));
 #else
     m_ui->actionCheckForUpdates->setVisible(false);
 #endif
@@ -670,7 +670,7 @@ void MainWindow::showAboutDialog()
 
 void MainWindow::showUpdateCheckStartup()
 {
-#ifdef WITH_XC_NETWORKING
+#ifdef WITH_XC_UPDATECHECK
     if (!config()->get("UpdateCheckMessageShown", false).toBool()) {
         auto result =
             MessageBox::question(this,
@@ -693,7 +693,7 @@ void MainWindow::showUpdateCheckStartup()
 
 void MainWindow::hasUpdateAvailable(bool hasUpdate, const QString& version, bool isManuallyRequested)
 {
-#ifdef WITH_XC_NETWORKING
+#ifdef WITH_XC_UPDATECHECK
     if (hasUpdate && !isManuallyRequested) {
         auto* updateCheckDialog = new UpdateCheckDialog(this);
         updateCheckDialog->showUpdateCheckResponse(hasUpdate, version);
@@ -708,7 +708,7 @@ void MainWindow::hasUpdateAvailable(bool hasUpdate, const QString& version, bool
 
 void MainWindow::showUpdateCheckDialog()
 {
-#ifdef WITH_XC_NETWORKING
+#ifdef WITH_XC_UPDATECHECK
     updateCheck()->checkForUpdates(true);
     auto* updateCheckDialog = new UpdateCheckDialog(this);
     updateCheckDialog->show();
diff --git a/src/updatecheck/UpdateChecker.cpp b/src/updatecheck/UpdateChecker.cpp
index 4272410b6..145312907 100644
--- a/src/updatecheck/UpdateChecker.cpp
+++ b/src/updatecheck/UpdateChecker.cpp
@@ -17,6 +17,7 @@
 
 #include "UpdateChecker.h"
 #include "config-keepassx.h"
+#include "core/Clock.h"
 #include "core/Config.h"
 #include <QJsonObject>
 #include <QNetworkAccessManager>
@@ -38,24 +39,28 @@ UpdateChecker::~UpdateChecker()
 
 void UpdateChecker::checkForUpdates(bool manuallyRequested)
 {
+    auto nextCheck = config()->get("GUI/CheckForUpdatesNextCheck", 0).toULongLong();
     m_isManuallyRequested = manuallyRequested;
-    m_bytesReceived.clear();
 
-    QString apiUrlStr = QString("https://api.github.com/repos/keepassxreboot/keepassxc/releases");
+    if (m_isManuallyRequested || Clock::currentSecondsSinceEpoch() >= nextCheck) {
+        m_bytesReceived.clear();
 
-    if (!config()->get("GUI/CheckForUpdatesIncludeBetas", false).toBool()) {
-        apiUrlStr += "/latest";
-    }
+        QString apiUrlStr = QString("https://api.github.com/repos/keepassxreboot/keepassxc/releases");
+
+        if (!config()->get("GUI/CheckForUpdatesIncludeBetas", false).toBool()) {
+            apiUrlStr += "/latest";
+        }
 
-    QUrl apiUrl = QUrl(apiUrlStr);
+        QUrl apiUrl = QUrl(apiUrlStr);
 
-    QNetworkRequest request(apiUrl);
-    request.setRawHeader("Accept", "application/json");
+        QNetworkRequest request(apiUrl);
+        request.setRawHeader("Accept", "application/json");
 
-    m_reply = m_netMgr->get(request);
+        m_reply = m_netMgr->get(request);
 
-    connect(m_reply, &QNetworkReply::finished, this, &UpdateChecker::fetchFinished);
-    connect(m_reply, &QIODevice::readyRead, this, &UpdateChecker::fetchReadyRead);
+        connect(m_reply, &QNetworkReply::finished, this, &UpdateChecker::fetchFinished);
+        connect(m_reply, &QIODevice::readyRead, this, &UpdateChecker::fetchReadyRead);
+    }
 }
 
 void UpdateChecker::fetchReadyRead()
@@ -84,8 +89,12 @@ void UpdateChecker::fetchFinished()
 
         if (!jsonObject.value("tag_name").isUndefined()) {
             version = jsonObject.value("tag_name").toString();
-            hasNewVersion = compareVersions(version, QString(KEEPASSXC_VERSION));
+            hasNewVersion = compareVersions(QString(KEEPASSXC_VERSION), version);
         }
+
+        // Check again in 7 days
+        // TODO: change to toSecsSinceEpoch() when min Qt >= 5.8
+        config()->set("GUI/CheckForUpdatesNextCheck", Clock::currentDateTime().addDays(7).toTime_t());
     } else {
         version = "error";
     }
@@ -93,38 +102,46 @@ void UpdateChecker::fetchFinished()
     emit updateCheckFinished(hasNewVersion, version, m_isManuallyRequested);
 }
 
-bool UpdateChecker::compareVersions(const QString& remoteVersion, const QString& localVersion)
+bool UpdateChecker::compareVersions(const QString& localVersion, const QString& remoteVersion)
 {
+    // Quick full-string equivalence check
     if (localVersion == remoteVersion) {
-        return false; // Currently using updated version
+        return false;
     }
 
-    QRegularExpression verRegex("^(\\d+(\\.\\d+){0,2})(-\\w+)?$", QRegularExpression::CaseInsensitiveOption);
+    QRegularExpression verRegex(R"(^((?:\d+\.){2}\d+)(?:-(\w+?)(\d+)?)?$)");
 
-    QRegularExpressionMatch lmatch = verRegex.match(localVersion);
-    QRegularExpressionMatch rmatch = verRegex.match(remoteVersion);
+    auto lmatch = verRegex.match(localVersion);
+    auto rmatch = verRegex.match(remoteVersion);
 
-    if (!lmatch.captured(1).isNull() && !rmatch.captured(1).isNull()) {
-        if (lmatch.captured(1) == rmatch.captured(1) && !lmatch.captured(3).isNull()) {
-            // Same version, but installed version has snapshot/beta suffix and should be updated to stable
-            return true;
-        }
+    auto lVersion = lmatch.captured(1).split(".");
+    auto lSuffix = lmatch.captured(2);
+    auto lBetaNum = lmatch.captured(3);
 
-        QStringList lparts = lmatch.captured(1).split(".");
-        QStringList rparts = rmatch.captured(1).split(".");
+    auto rVersion = rmatch.captured(1).split(".");
+    auto rSuffix = rmatch.captured(2);
+    auto rBetaNum = rmatch.captured(3);
 
-        if (lparts.length() < 3)
-            lparts << "0";
+    if (!lVersion.isEmpty() && !rVersion.isEmpty()) {
+        if (lSuffix.compare("snapshot", Qt::CaseInsensitive) == 0) {
+            // Snapshots are not checked for version updates
+            return false;
+        }
 
-        if (rparts.length() < 3)
-            rparts << "0";
+        // Check "-beta[X]" versions
+        if (lVersion == rVersion && !lSuffix.isEmpty()) {
+            // Check if stable version has been released or new beta is available
+            // otherwise the version numbers are equal
+            return rSuffix.isEmpty() || lBetaNum.toInt() < rBetaNum.toInt();
+        }
 
         for (int i = 0; i < 3; i++) {
-            int l = lparts[i].toInt();
-            int r = rparts[i].toInt();
+            int l = lVersion[i].toInt();
+            int r = rVersion[i].toInt();
 
-            if (l == r)
+            if (l == r) {
                 continue;
+            }
 
             if (l > r) {
                 return false; // Installed version is newer than release
diff --git a/src/updatecheck/UpdateChecker.h b/src/updatecheck/UpdateChecker.h
index ac6471d64..64430bda3 100644
--- a/src/updatecheck/UpdateChecker.h
+++ b/src/updatecheck/UpdateChecker.h
@@ -31,7 +31,7 @@ public:
     ~UpdateChecker() override;
 
     void checkForUpdates(bool manuallyRequested);
-    static bool compareVersions(const QString& remoteVersion, const QString& localVersion);
+    static bool compareVersions(const QString& localVersion, const QString& remoteVersion);
     static UpdateChecker* instance();
 
 signals:
diff --git a/tests/TestUpdateCheck.cpp b/tests/TestUpdateCheck.cpp
index 8cba43b1d..ff709cd56 100644
--- a/tests/TestUpdateCheck.cpp
+++ b/tests/TestUpdateCheck.cpp
@@ -29,13 +29,32 @@ void TestUpdateCheck::initTestCase()
 
 void TestUpdateCheck::testCompareVersion()
 {
-    // Remote Version , Installed Version
-    QCOMPARE(UpdateChecker::compareVersions(QString("2.4.0"), QString("2.3.4")), true);
-    QCOMPARE(UpdateChecker::compareVersions(QString("2.3.0"), QString("2.4.0")), false);
+    // No upgrade
     QCOMPARE(UpdateChecker::compareVersions(QString("2.3.0"), QString("2.3.0")), false);
-    QCOMPARE(UpdateChecker::compareVersions(QString("2.3.0"), QString("2.3.0-beta1")), true);
-    QCOMPARE(UpdateChecker::compareVersions(QString("2.3.0-beta2"), QString("2.3.0-beta1")), true);
-    QCOMPARE(UpdateChecker::compareVersions(QString("2.3.4"), QString("2.4.0-snapshot")), false);
-    QCOMPARE(UpdateChecker::compareVersions(QString("invalid"), QString("2.4.0")), false);
-    QCOMPARE(UpdateChecker::compareVersions(QString(""), QString("2.4.0")), false);
+
+    // First digit upgrade
+    QCOMPARE(UpdateChecker::compareVersions(QString("2.4.0"), QString("3.0.0")), true);
+    QCOMPARE(UpdateChecker::compareVersions(QString("3.0.0"), QString("2.4.0")), false);
+
+    // Second digit upgrade
+    QCOMPARE(UpdateChecker::compareVersions(QString("2.3.4"), QString("2.4.0")), true);
+    QCOMPARE(UpdateChecker::compareVersions(QString("2.4.0"), QString("2.3.4")), false);
+
+    // Third digit upgrade
+    QCOMPARE(UpdateChecker::compareVersions(QString("2.3.0"), QString("2.3.1")), true);
+    QCOMPARE(UpdateChecker::compareVersions(QString("2.3.1"), QString("2.3.0")), false);
+
+    // Beta builds
+    QCOMPARE(UpdateChecker::compareVersions(QString("2.3.0"), QString("2.3.0-beta1")), false);
+    QCOMPARE(UpdateChecker::compareVersions(QString("2.3.0"), QString("2.3.1-beta1")), true);
+    QCOMPARE(UpdateChecker::compareVersions(QString("2.3.0-beta1"), QString("2.3.0")), true);
+    QCOMPARE(UpdateChecker::compareVersions(QString("2.3.0-beta"), QString("2.3.0-beta1")), true);
+    QCOMPARE(UpdateChecker::compareVersions(QString("2.3.0-beta1"), QString("2.3.0-beta")), false);
+    QCOMPARE(UpdateChecker::compareVersions(QString("2.3.0-beta1"), QString("2.3.0-beta2")), true);
+    QCOMPARE(UpdateChecker::compareVersions(QString("2.3.0-beta2"), QString("2.3.0-beta1")), false);
+
+    // Snapshot and invalid data
+    QCOMPARE(UpdateChecker::compareVersions(QString("2.3.4-snapshot"), QString("2.4.0")), false);
+    QCOMPARE(UpdateChecker::compareVersions(QString("2.4.0"), QString("invalid")), false);
+    QCOMPARE(UpdateChecker::compareVersions(QString("2.4.0"), QString("")), false);
 }