Refactor the password health class

We now have two classes, PasswordHealth to deal with a single
password and HealthChecker to deal with all passwords of a
database.

Fixes keepassxreboot#3993

src/core/PasswordHealth.cpp

@@ -24,6 +24,19 @@
 #include "PasswordHealth.h"
 #include "zxcvbn.h"
 
+namespace {
+    /*
+     * Helper function to add a string to another as a new line
+     */
+    void addTo(QString& to, QString newText)
+    {
+        if (!to.isEmpty()) {
+            to += '\n';
+        }
+        to += newText;
+    }
+}
+
 PasswordHealth::PasswordHealth(double entropy)
     : m_entropy(entropy)
     , m_score(entropy)
@@ -53,66 +66,106 @@ PasswordHealth::PasswordHealth(QString pwd)
 {
 }
 
-PasswordHealth::PasswordHealth(QSharedPointer<Database> db, QString pwd, Cache* cache)
-    : PasswordHealth(ZxcvbnMatch(pwd.toLatin1(), nullptr, nullptr))
+PasswordHealth::Quality PasswordHealth::quality() const
 {
-    // Whenever the password is re-used, reduce score by
-    // this many points:
-    constexpr auto penalty = 15;
+    const auto s = score();
+    return s <= 0 ? Quality::bad
+                  : s < 40 ? Quality::poor : s < 65 ? Quality::weak : s < 100 ? Quality::good : Quality::excellent;
+}
 
-    if (!db || !db->rootGroup()) {
-        return;
-    }
+QColor PasswordHealth::color() const
+{
+    switch (quality()) {
+    case Quality::bad:
+        return QColor("red");
+
+    case Quality::poor:
+        return QColor("orange");
+
+    case Quality::weak:
+        return QColor("yellow");
+
+    case Quality::good:
+        return QColor("green");
 
-    // Set up the cache if not yet done (and use our own cache if
-    // the caller didn't give us one)
-    Cache mycache;
-    if (!cache) {
-        cache = &mycache;
+    case Quality::excellent:
+        return QColor("green");
     }
-    if (cache->isEmpty()) {
-        const auto groups = db->rootGroup()->groupsRecursive(true);
-        for (const auto* group : groups) {
-            if (!group->isRecycled()) {
-                for (const auto* entry : group->entries()) {
-                    if (!entry->isRecycled()) {
-                        (*cache)[entry->password()]
-                            << QApplication::tr("Used in %1/%2").arg(group->hierarchy().join('/'), entry->title());
-                    }
+
+    // Unreachable. Using "return" here instead of "default"
+    // above because we want a compiler warning when someone
+    // adds new enum values.
+    return QColor();
+}
+
+/**
+ * Ctor of the health checker class.
+ *
+ * This class provides additional information about password health
+ * than can be derived from the password itself (re-use, expiry).
+ */
+HealthChecker::HealthChecker(QSharedPointer<Database> db)
+{
+    // Build the cache of re-used passwords
+    for (const auto* group : db->rootGroup()->groupsRecursive(true)) {
+        if (!group->isRecycled()) {
+            for (const auto* entry : group->entries()) {
+                if (!entry->isRecycled()) {
+                    m_reuse[entry->password()]
+                        << QApplication::tr("Used in %1/%2").arg(group->hierarchy().join('/'), entry->title());
                 }
             }
         }
     }
+}
+
+/**
+ * Call operator of the Health Checker class.
+ *
+ * Returns the health of the password in `entry`, considering
+ * password entropy, re-use, expiration, etc.
+ */
+const PasswordHealth& HealthChecker::operator()(const Entry& entry)
+{
+    // Return from cache if we saw it before
+    const auto p = m_cache.find(entry.uuid());
+    if (p!=m_cache.end()) {
+        return *p;
+    }
+
+    // Got this entry for the first time. Store its health here:
+    auto& health = m_cache[entry.uuid()];
+
+    // First analyse the password itself
+    const auto pwd = entry.password();
+    health = PasswordHealth(pwd);
 
-    // If the password is in the database more than once,
+    // Second, if the password is in the database more than once,
     // reduce the score accordingly
-    const auto& used = (*cache)[pwd];
+    const auto& used = m_reuse[pwd];
     const auto count = used.size();
     if (count > 1) {
-        m_score -= penalty * (count - 1);
-        addTo(m_reason, QApplication::tr("Password is used %1 times").arg(QString::number(count)));
-        addTo(m_details, used.join('\n'));
+        constexpr auto penalty = 15;
+        health.m_score -= penalty * (count - 1);
+        addTo(health.m_reason, QApplication::tr("Password is used %1 times").arg(QString::number(count)));
+        addTo(health.m_details, used.join('\n'));
 
         // Don't allow re-used passwords to be considered "good"
         // no matter how great their entropy is.
-        if (m_score > 64) {
-            m_score = 64;
+        if (health.m_score > 64) {
+            health.m_score = 64;
         }
     }
-}
 
-PasswordHealth::PasswordHealth(QSharedPointer<Database> db, const Entry& entry, Cache* cache)
-    : PasswordHealth(db, entry.password(), cache)
-{
-    // If the password has already expired, reduce score to 0.
-    // Else, if the password is going to expire in the next
-    // 30 days, reduce score by 2 points per day.
+    // Third, if the password has already expired, reduce score to 0;
+    // or, if the password is going to expire in the next 30 days,
+    // reduce score by 2 points per day.
     if (entry.isExpired()) {
-        m_score = 0;
-        addTo(m_reason, QApplication::tr("Password has expired"));
-        addTo(m_details,
+        health.m_score = 0;
+        addTo(health.m_reason, QApplication::tr("Password has expired"));
+        addTo(health.m_details,
               QApplication::tr("Password expiry was %1")
-                  .arg(entry.timeInfo().expiryTime().toString(Qt::DefaultLocaleShortDate)));
+              .arg(entry.timeInfo().expiryTime().toString(Qt::DefaultLocaleShortDate)));
     } else if (entry.timeInfo().expires()) {
         const auto days = QDateTime::currentDateTime().daysTo(entry.timeInfo().expiryTime());
         if (days <= 30) {
@@ -121,57 +174,20 @@ PasswordHealth::PasswordHealth(QSharedPointer<Database> db, const Entry& entry,
             // reduce the score by 2 points for every day that
             // we get closer to expiry. days<=0 has already
             // been handled above ("isExpired()").
-            if (m_score > 60) {
-                m_score = 60;
+            if (health.m_score > 60) {
+                health.m_score = 60;
             }
-            m_score -= (30 - days) * 2;
-            addTo(m_reason,
+            health.m_score -= (30 - days) * 2;
+            addTo(health.m_reason,
                   days <= 2 ? QApplication::tr("Password is about to expire")
-                            : days <= 10 ? QApplication::tr("Password expires in %1 days").arg(days)
-                                         : QApplication::tr("Password will expire soon"));
-            addTo(m_details,
+                  : days <= 10 ? QApplication::tr("Password expires in %1 days").arg(days)
+                  : QApplication::tr("Password will expire soon"));
+            addTo(health.m_details,
                   QApplication::tr("Password expires on %1")
-                      .arg(entry.timeInfo().expiryTime().toString(Qt::DefaultLocaleShortDate)));
+                  .arg(entry.timeInfo().expiryTime().toString(Qt::DefaultLocaleShortDate)));
         }
     }
-}
-
-void PasswordHealth::addTo(QString& to, QString newText)
-{
-    if (!to.isEmpty()) {
-        to += '\n';
-    }
-    to += newText;
-}
-
-PasswordHealth::Quality PasswordHealth::quality() const
-{
-    const auto s = score();
-    return s <= 0 ? Quality::bad
-                  : s < 40 ? Quality::poor : s < 65 ? Quality::weak : s < 100 ? Quality::good : Quality::excellent;
-}
-
-QColor PasswordHealth::color() const
-{
-    switch (quality()) {
-    case Quality::bad:
-        return QColor("red");
-
-    case Quality::poor:
-        return QColor("orange");
-
-    case Quality::weak:
-        return QColor("yellow");
-
-    case Quality::good:
-        return QColor("green");
-
-    case Quality::excellent:
-        return QColor("green");
-    }
 
-    // Unreachable. Using "return" here instead of "default"
-    // above because we want a compiler warning when someone
-    // adds new enum values.
-    return QColor();
+    // Return the result
+    return health;
 }

src/core/PasswordHealth.h

@@ -26,24 +26,20 @@ class Database;
 class Entry;
 class QString;
 
+/**
+ * Health status of a single password.
+ *
+ * @see HealthChecker
+ */
 class PasswordHealth
 {
-public:
-    // first = the password
-    // second = paths of where this password is used
-    using Cache = QHash<QString, QStringList>;
+    friend class HealthChecker;
 
-    /*
-     * Constructors.
-     * Callers may pass a pointer to a Cache object in order to
-     * speed up repeated calls on the same database. (Don't re-use
-     * a cache across databases.)
-     */
+public:
+    // Ctors
     PasswordHealth() = default;
     explicit PasswordHealth(double entropy);
     explicit PasswordHealth(QString pwd);
-    PasswordHealth(QSharedPointer<Database> db, QString pwd, Cache* cache = nullptr);
-    PasswordHealth(QSharedPointer<Database> db, const Entry& entry, Cache* cache = nullptr);
 
     /*
      * The password score is defined to be the greater the better
@@ -101,7 +97,24 @@ class PasswordHealth
     double m_entropy = 0.0;
     int m_score = 0;
     QString m_reason, m_details;
-    void addTo(QString& to, QString newText);
+};
+
+/**
+ * Password health check for all entries of a database.
+ *
+ * @see PasswordHealth
+ */
+class HealthChecker {
+public:
+    // Ctor
+    explicit HealthChecker(QSharedPointer<Database>);
+
+    // Get the health status of an entry
+    const PasswordHealth& operator()(const Entry&);
+
+private:
+    QHash<QUuid, PasswordHealth> m_cache;   // Result cache (first=entry UUID)
+    QHash<QString, QStringList> m_reuse;    // first=password, second=where it is used
 };
 
 #endif // KEEPASSX_PASSWORDHEALTH_H

src/gui/reports/ReportsWidgetHealthcheck.cpp

@@ -34,17 +34,17 @@ namespace
     public:
         struct Item
         {
-            PasswordHealth health;
             const Group* group = nullptr;
             const Entry* entry = nullptr;
+            PasswordHealth health;
 
             Item()
             {
             }
-            Item(QSharedPointer<Database> db, const Group& g, const Entry& e, PasswordHealth::Cache& cache)
-                : health(db, e, &cache)
-                , group(&g)
+            Item(const Group& g, const Entry& e, const PasswordHealth& h)
+                : group(&g)
                 , entry(&e)
+                , health(h)
             {
             }
 
@@ -54,11 +54,7 @@ namespace
             }
         };
 
-        explicit Health(QSharedPointer<Database> db)
-            : m_db(db)
-        {
-            gatherHealth(db->rootGroup()->groupsRecursive(true));
-        }
+        explicit Health(QSharedPointer<Database>);
 
         const QVector<Item>& items() const
         {
@@ -67,16 +63,17 @@ namespace
 
     private:
         QSharedPointer<Database> m_db;
+        HealthChecker m_checker;
         QVector<Item> m_items;
-        PasswordHealth::Cache m_cache;
 
         void gatherHealth(const QList<Group*>&);
     };
 } // namespace
 
-void Health::gatherHealth(const QList<Group*>& groups)
+Health::Health(QSharedPointer<Database> db)
+    : m_db(db), m_checker(db)
 {
-    for (const auto* group : groups) {
+    for (const auto* group : db->rootGroup()->groupsRecursive(true)) {
         // Skip recycle bin
         if (group->isRecycled()) {
             continue;
@@ -93,7 +90,7 @@ void Health::gatherHealth(const QList<Group*>& groups)
             }
 
             // Add entry if its password isn't at least "good"
-            const auto item = Item(m_db, *group, *entry, m_cache);
+            const auto item = Item(*group, *entry, m_checker(*entry));
             if (item.health.quality() < PasswordHealth::Quality::good) {
                 m_items.append(item);
             }

src/gui/reports/ReportsWidgetStatistics.cpp

@@ -98,6 +98,8 @@ namespace
 
         void gatherStats(const QList<Group*>& groups)
         {
+            auto checker = HealthChecker(m_db);
+
             for (const auto* group : groups) {
                 // Don't count anything in the recycle bin
                 if (group->isRecycled()) {
@@ -133,7 +135,7 @@ namespace
 
                         // Speed up Zxcvbn process by excluding very long passwords and most passphrases
                         if (pwd.size() < 25
-                            && PasswordHealth(m_db, *entry).quality() <= PasswordHealth::Quality::weak) {
+                            && checker(*entry).quality() <= PasswordHealth::Quality::weak) {
                             ++nPwdsWeak;
                         }