More password stats and code cleanup

keepassxreboot · Oct 12, 2019 · b84c0e8 · b84c0e8
1 parent bb22b4c
commit b84c0e8
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 57 deletions.
diff --git a/src/gui/dbsettings/DatabaseSettingsWidgetStatistics.cpp b/src/gui/dbsettings/DatabaseSettingsWidgetStatistics.cpp
@@ -22,10 +22,10 @@
 #include "core/FilePath.h"
 #include "core/Group.h"
 #include "core/Metadata.h"
+#include "zxcvbn/zxcvbn.h"
 
 #include <QFileInfo>
 #include <QHash>
-#include <QMessageBox>
 #include <QStandardItemModel>
 
 namespace
@@ -38,15 +38,17 @@ namespace
         int nGroups = 0; // Number of groups in the database
         int nEntries = 0; // Number of entries (across all groups)
         int nExpired = 0; // Number of expired entries
+        int nPwdsWeak = 0; // Number of weak or poor passwords
+        int nPwdsShort = 0; // Number of passwords 8 characters or less in size
         int nPwdsUnique = 0; // Number of unique passwords
         int nPwdsReused = 0; // Number of non-unique passwords
         int pwdTotalLen = 0; // Total length of all passwords
 
         // Ctor does all the work
-        Stats(QSharedPointer<Database> db)
+        explicit Stats(QSharedPointer<Database> db)
             : modified(QFileInfo(db->filePath()).lastModified())
         {
-            countGroup(*db->rootGroup());
+            gatherStats(db->rootGroup()->groupsRecursive(true));
         }
 
         // Get average password length
@@ -91,43 +93,43 @@ namespace
     private:
         QHash<QString, int> m_passwords;
 
-        void countGroup(const Group& group)
+        void gatherStats(const QList<Group*>& groups)
         {
-            // Skip the recycle bin
-            if (&group == group.database()->metadata()->recycleBin())
-                return;
-
-            // Handle this group
-            ++nGroups;
-            nEntries += group.entries().size();
-
-            // Handle the entries of this group
-            for (const auto& entry : group.entries()) {
-
-                if (entry->isExpired()) {
-                    ++nExpired;
+            for (const auto* group : groups) {
+                // Don't count anything in the recycle bin
+                if (group == group->database()->metadata()->recycleBin()) {
+                    continue;
                 }
 
-                // Get password statistics
-                const auto pwd = entry->password();
-                if (!pwd.isEmpty()) {
-                    if (m_passwords.find(pwd) == m_passwords.end()) {
-                        ++nPwdsUnique;
-                    } else {
-                        ++nPwdsReused;
-                    }
+                ++nGroups;
 
-                    pwdTotalLen += pwd.size();
+                for (const auto* entry : group->entries()) {
+                    ++nEntries;
 
-                    m_passwords[pwd]++;
-                }
-            }
+                    if (entry->isExpired()) {
+                        ++nExpired;
+                    }
 
-            // Recursively handle sub-groups
-            const auto& children = group.children();
-            for (const auto child : children) {
-                if (child) {
-                    countGroup(*child);
+                    // Get password statistics
+                    const auto pwd = entry->password();
+                    if (!pwd.isEmpty()) {
+                        if (!m_passwords.contains(pwd)) {
+                            ++nPwdsUnique;
+                        } else {
+                            ++nPwdsReused;
+                        }
+
+                        if (pwd.size() < 8) {
+                            ++nPwdsShort;
+                        }
+
+                        if (ZxcvbnMatch(pwd.toLatin1(), nullptr, nullptr) < 65) {
+                            ++nPwdsWeak;
+                        }
+
+                        pwdTotalLen += pwd.size();
+                        m_passwords[pwd]++;
+                    }
                 }
             }
         }
@@ -175,26 +177,33 @@ void DatabaseSettingsWidgetStatistics::loadSettings(QSharedPointer<Database> db)
                 db->isModified() ? tr("yes") : tr("no"),
                 db->isModified(),
                 tr("The database was modified, but the changes have not yet been saved to disk."));
-    addStatsRow(tr("Number of groups"), QString("%L1").arg(stats.nGroups));
-    addStatsRow(tr("Number of entries"), QString("%L1").arg(stats.nEntries));
+    addStatsRow(tr("Number of groups"), QString::number(stats.nGroups));
+    addStatsRow(tr("Number of entries"), QString::number(stats.nEntries));
     addStatsRow(tr("Number of expired entries"),
-                QString("%L1").arg(stats.nExpired),
+                QString::number(stats.nExpired),
                 stats.isAnyExpired(),
                 tr("The database contains entries that have expired."));
-    addStatsRow(tr("Unique passwords"), QString("%L1").arg(stats.nPwdsUnique));
+    addStatsRow(tr("Unique passwords"), QString::number(stats.nPwdsUnique));
     addStatsRow(tr("Non-unique passwords"),
-                QString("%L1").arg(stats.nPwdsReused),
+                QString::number(stats.nPwdsReused),
                 stats.areTooManyPwdsReused(),
-                tr("More than 10 % of the passwords are used in more than one entry. If possible, use unique passwords "
-                   "whereever possible."));
+                tr("More than 10% of passwords are reused. Use unique passwords when possible."));
     addStatsRow(tr("Maximum password reuse"),
-                QString("%L1").arg(stats.maxPwdReuse()),
+                QString::number(stats.maxPwdReuse()),
                 stats.arePwdsReusedTooOften(),
-                tr("Some passwords are used more than three times. If possible, don't use a password more than once."));
+                tr("Some passwords are used more than three times. Use unique passwords when possible."));
+    addStatsRow(tr("Number of short passwords"),
+                QString::number(stats.nPwdsShort),
+                stats.nPwdsShort > 0,
+                tr("Recommended minimum password length is at least 8 characters."));
+    addStatsRow(tr("Number of weak passwords"),
+                QString::number(stats.nPwdsWeak),
+                stats.nPwdsWeak > 0,
+                tr("Recommend using long, randomized passwords with a rating of 'good' or 'excellent'."));
     addStatsRow(tr("Average password length"),
-                QString("%L1").arg(stats.averagePwdLength()),
+                tr("%1 characters").arg(stats.averagePwdLength()),
                 stats.isAvgPwdTooShort(),
-                tr("The average password length is less than ten characters. Longer passwords provide more security."));
+                tr("Average password length is less than ten characters. Longer passwords provide more security."));
 
     m_ui->sharedGroupsView->setModel(m_referencesModel.data());
     m_ui->sharedGroupsView->horizontalHeader()->setSectionResizeMode(QHeaderView::ResizeToContents);

diff --git a/src/gui/dbsettings/DatabaseSettingsWidgetStatistics.h b/src/gui/dbsettings/DatabaseSettingsWidgetStatistics.h
@@ -19,12 +19,9 @@
 #define KEEPASSXC_DATABASESETTINGSWIDGETSTATISTICS_H
 
 #include <QIcon>
-#include <QPointer>
-#include <QScopedPointer>
 #include <QWidget>
 
 class Database;
-
 class QStandardItemModel;
 
 namespace Ui
@@ -44,6 +41,7 @@ class DatabaseSettingsWidgetStatistics : public QWidget
 
 private:
     QScopedPointer<Ui::DatabaseSettingsWidgetStatistics> m_ui;
+
     QIcon m_errIcon;
     QScopedPointer<QStandardItemModel> m_referencesModel;
 

diff --git a/src/gui/dbsettings/DatabaseSettingsWidgetStatistics.ui b/src/gui/dbsettings/DatabaseSettingsWidgetStatistics.ui
@@ -10,7 +10,7 @@
     <height>379</height>
    </rect>
   </property>
-  <layout class="QVBoxLayout" name="verticalLayout" stretch="0,0">
+  <layout class="QVBoxLayout" name="verticalLayout" stretch="0">
    <property name="leftMargin">
     <number>0</number>
    </property>
@@ -28,35 +28,47 @@
      <property name="title">
       <string>Statistics</string>
      </property>
-     <layout class="QGridLayout" name="gridLayout">
-      <item row="0" column="0" rowspan="2">
+     <layout class="QVBoxLayout" name="verticalLayout_2">
+      <item>
        <widget class="QTableView" name="sharedGroupsView">
-        <property name="alternatingRowColors">
-         <bool>true</bool>
-        </property>
         <property name="editTriggers">
          <set>QAbstractItemView::NoEditTriggers</set>
         </property>
         <property name="showDropIndicator" stdset="0">
          <bool>false</bool>
         </property>
+        <property name="alternatingRowColors">
+         <bool>true</bool>
+        </property>
         <property name="textElideMode">
          <enum>Qt::ElideMiddle</enum>
         </property>
         <property name="sortingEnabled">
          <bool>false</bool>
         </property>
-        <attribute name="horizontalHeaderStretchLastSection">
-         <bool>true</bool>
-        </attribute>
         <attribute name="horizontalHeaderVisible">
          <bool>false</bool>
         </attribute>
+        <attribute name="horizontalHeaderStretchLastSection">
+         <bool>true</bool>
+        </attribute>
         <attribute name="verticalHeaderVisible">
          <bool>false</bool>
         </attribute>
        </widget>
       </item>
+      <item>
+       <widget class="QLabel" name="label">
+        <property name="font">
+         <font>
+          <italic>true</italic>
+         </font>
+        </property>
+        <property name="text">
+         <string>Hover over lines with error icons for further information.</string>
+        </property>
+       </widget>
+      </item>
      </layout>
     </widget>
    </item>