diff --git a/COPYING b/COPYING index aa11b14fa..a20889ac9 100644 --- a/COPYING +++ b/COPYING @@ -166,6 +166,7 @@ Files: share/icons/application/scalable/actions/chevron-double-down.svg share/icons/application/scalable/actions/group-new.svg share/icons/application/scalable/actions/help-about.svg share/icons/application/scalable/actions/key-enter.svg + share/icons/application/scalable/actions/lock-question.svg share/icons/application/scalable/actions/message-close.svg share/icons/application/scalable/actions/move-down.svg share/icons/application/scalable/actions/move-up.svg diff --git a/share/icons/application/scalable/actions/lock-question.svg b/share/icons/application/scalable/actions/lock-question.svg new file mode 100644 index 000000000..99495d9fd --- /dev/null +++ b/share/icons/application/scalable/actions/lock-question.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/share/icons/icons.qrc b/share/icons/icons.qrc index cf73b4958..d42bfc52b 100644 --- a/share/icons/icons.qrc +++ b/share/icons/icons.qrc @@ -46,6 +46,7 @@ application/scalable/actions/help-about.svg application/scalable/actions/hibp.svg application/scalable/actions/key-enter.svg + application/scalable/actions/lock-question.svg application/scalable/actions/keyboard-shortcuts.svg application/scalable/actions/message-close.svg application/scalable/actions/move-down.svg diff --git a/src/cli/Estimate.cpp b/src/cli/Estimate.cpp index 63044e50a..878cd0e16 100644 --- a/src/cli/Estimate.cpp +++ b/src/cli/Estimate.cpp @@ -172,6 +172,6 @@ int Estimate::execute(const QStringList& arguments) password = in.readLine(); } - estimate(password.toLatin1(), parser->isSet(Estimate::AdvancedOption)); + estimate(password.toUtf8(), parser->isSet(Estimate::AdvancedOption)); return EXIT_SUCCESS; } diff --git a/src/core/CustomData.cpp b/src/core/CustomData.cpp index b421ec3a0..bb287ec45 100644 --- a/src/core/CustomData.cpp +++ b/src/core/CustomData.cpp @@ -24,6 +24,7 @@ const QString CustomData::LastModified = QStringLiteral("_LAST_MODIFIED"); const QString CustomData::Created = QStringLiteral("_CREATED"); const QString CustomData::BrowserKeyPrefix = QStringLiteral("KPXC_BROWSER_"); const QString CustomData::BrowserLegacyKeyPrefix = QStringLiteral("Public Key: "); +const QString CustomData::ExcludeFromReports = QStringLiteral("KnownBad"); CustomData::CustomData(QObject* parent) : QObject(parent) diff --git a/src/core/CustomData.h b/src/core/CustomData.h index 93b78c46a..aeaa136ef 100644 --- a/src/core/CustomData.h +++ b/src/core/CustomData.h @@ -51,6 +51,7 @@ public: static const QString Created; static const QString BrowserKeyPrefix; static const QString BrowserLegacyKeyPrefix; + static const QString ExcludeFromReports; signals: void customDataModified(); diff --git a/src/core/Entry.cpp b/src/core/Entry.cpp index 61f48b780..49beb1e9e 100644 --- a/src/core/Entry.cpp +++ b/src/core/Entry.cpp @@ -17,14 +17,12 @@ */ #include "Entry.h" -#include "config-keepassx.h" - -#include "core/Clock.h" #include "core/Config.h" #include "core/Database.h" #include "core/DatabaseIcons.h" #include "core/Group.h" #include "core/Metadata.h" +#include "core/PasswordHealth.h" #include "core/Tools.h" #include "totp/totp.h" @@ -245,6 +243,25 @@ QString Entry::defaultAutoTypeSequence() const return m_data.defaultAutoTypeSequence; } +const QSharedPointer& Entry::passwordHealth() +{ + if (!m_data.passwordHealth) { + m_data.passwordHealth.reset(new PasswordHealth(resolvePlaceholder(password()))); + } + return m_data.passwordHealth; +} + +bool Entry::excludeFromReports() const +{ + return customData()->contains(CustomData::ExcludeFromReports) + && customData()->value(CustomData::ExcludeFromReports) == TRUE_STR; +} + +void Entry::setExcludeFromReports(bool state) +{ + customData()->set(CustomData::ExcludeFromReports, state ? TRUE_STR : FALSE_STR); +} + /** * Determine the effective sequence that will be injected * This function return an empty string if a parent group has autotype disabled or if the entry has no parent @@ -673,6 +690,8 @@ void Entry::setUsername(const QString& username) void Entry::setPassword(const QString& password) { + // Reset Password Health + m_data.passwordHealth.reset(); m_attributes->set(EntryAttributes::PasswordKey, password, m_attributes->isProtected(EntryAttributes::PasswordKey)); } diff --git a/src/core/Entry.h b/src/core/Entry.h index be666b9a0..88a89761a 100644 --- a/src/core/Entry.h +++ b/src/core/Entry.h @@ -36,6 +36,8 @@ class Database; class Group; +class PasswordHealth; + namespace Totp { struct Settings; @@ -66,6 +68,7 @@ struct EntryData QString defaultAutoTypeSequence; TimeInfo timeInfo; QSharedPointer totpSettings; + QSharedPointer passwordHealth; bool operator==(const EntryData& other) const; bool operator!=(const EntryData& other) const; @@ -94,7 +97,6 @@ public: int autoTypeObfuscation() const; QString defaultAutoTypeSequence() const; QString effectiveAutoTypeSequence() const; - QString effectiveNewAutoTypeSequence() const; QList autoTypeSequences(const QString& pattern = {}) const; AutoTypeAssociations* autoTypeAssociations(); const AutoTypeAssociations* autoTypeAssociations() const; @@ -111,6 +113,9 @@ public: QSharedPointer totpSettings() const; int size() const; QString path() const; + const QSharedPointer& passwordHealth(); + bool excludeFromReports() const; + void setExcludeFromReports(bool state); bool hasTotp() const; bool isExpired() const; diff --git a/src/core/PasswordHealth.cpp b/src/core/PasswordHealth.cpp index c2551a519..583509eb0 100644 --- a/src/core/PasswordHealth.cpp +++ b/src/core/PasswordHealth.cpp @@ -24,9 +24,6 @@ #include "PasswordHealth.h" #include "zxcvbn.h" -// Define the static member variable with the custom field name -const QString PasswordHealth::OPTION_KNOWN_BAD = QStringLiteral("KnownBad"); - PasswordHealth::PasswordHealth(double entropy) : m_score(entropy) , m_entropy(entropy) @@ -49,8 +46,8 @@ PasswordHealth::PasswordHealth(double entropy) } } -PasswordHealth::PasswordHealth(QString pwd) - : PasswordHealth(ZxcvbnMatch(pwd.toLatin1(), nullptr, nullptr)) +PasswordHealth::PasswordHealth(const QString& pwd) + : PasswordHealth(ZxcvbnMatch(pwd.toUtf8(), nullptr, nullptr)) { } diff --git a/src/core/PasswordHealth.h b/src/core/PasswordHealth.h index ef3249380..6f1139997 100644 --- a/src/core/PasswordHealth.h +++ b/src/core/PasswordHealth.h @@ -34,7 +34,7 @@ class PasswordHealth { public: explicit PasswordHealth(double entropy); - explicit PasswordHealth(QString pwd); + explicit PasswordHealth(const QString& pwd); /* * The password score is defined to be the greater the better @@ -83,14 +83,6 @@ public: return m_entropy; } - /** - * Name of custom data field that holds the "this is a known - * bad password" flag. Legal values of the field are TRUE_STR - * and FALSE_STR, the default (used if the field doesn't exist) - * is false. - */ - static const QString OPTION_KNOWN_BAD; - private: int m_score = 0; double m_entropy = 0.0; diff --git a/src/gui/entry/EditEntryWidget.cpp b/src/gui/entry/EditEntryWidget.cpp index 3a8a606ec..b4339034a 100644 --- a/src/gui/entry/EditEntryWidget.cpp +++ b/src/gui/entry/EditEntryWidget.cpp @@ -440,7 +440,7 @@ void EditEntryWidget::setupEntryUpdate() // Advanced tab connect(m_advancedUi->attributesEdit, SIGNAL(textChanged()), this, SLOT(setModified())); connect(m_advancedUi->protectAttributeButton, SIGNAL(stateChanged(int)), this, SLOT(setModified())); - connect(m_advancedUi->knownBadCheckBox, SIGNAL(stateChanged(int)), this, SLOT(setModified())); + connect(m_advancedUi->excludeReportsCheckBox, SIGNAL(stateChanged(int)), this, SLOT(setModified())); connect(m_advancedUi->fgColorCheckBox, SIGNAL(stateChanged(int)), this, SLOT(setModified())); connect(m_advancedUi->bgColorCheckBox, SIGNAL(stateChanged(int)), this, SLOT(setModified())); connect(m_advancedUi->attachmentsWidget, SIGNAL(widgetUpdated()), this, SLOT(setModified())); @@ -861,9 +861,7 @@ void EditEntryWidget::setForms(Entry* entry, bool restore) editTriggers = QAbstractItemView::DoubleClicked; } m_advancedUi->attributesView->setEditTriggers(editTriggers); - m_advancedUi->knownBadCheckBox->setChecked(entry->customData()->contains(PasswordHealth::OPTION_KNOWN_BAD) - && entry->customData()->value(PasswordHealth::OPTION_KNOWN_BAD) - == TRUE_STR); + m_advancedUi->excludeReportsCheckBox->setChecked(entry->excludeFromReports()); setupColorButton(true, entry->foregroundColor()); setupColorButton(false, entry->backgroundColor()); m_iconsWidget->setEnabled(!m_history); @@ -1126,11 +1124,8 @@ void EditEntryWidget::updateEntryData(Entry* entry) const entry->setNotes(m_mainUi->notesEdit->toPlainText()); - const auto wasKnownBad = entry->customData()->contains(PasswordHealth::OPTION_KNOWN_BAD) - && entry->customData()->value(PasswordHealth::OPTION_KNOWN_BAD) == TRUE_STR; - const auto isKnownBad = m_advancedUi->knownBadCheckBox->isChecked(); - if (isKnownBad != wasKnownBad) { - entry->customData()->set(PasswordHealth::OPTION_KNOWN_BAD, isKnownBad ? TRUE_STR : FALSE_STR); + if (entry->excludeFromReports() != m_advancedUi->excludeReportsCheckBox->isChecked()) { + entry->setExcludeFromReports(m_advancedUi->excludeReportsCheckBox->isChecked()); } if (m_advancedUi->fgColorCheckBox->isChecked() && m_advancedUi->fgColorButton->property("color").isValid()) { diff --git a/src/gui/entry/EditEntryWidgetAdvanced.ui b/src/gui/entry/EditEntryWidgetAdvanced.ui index 80841eb64..c9ff9e87c 100644 --- a/src/gui/entry/EditEntryWidgetAdvanced.ui +++ b/src/gui/entry/EditEntryWidgetAdvanced.ui @@ -187,9 +187,9 @@ - + - <html><head/><body><p>If checked, the entry will not appear in reports like Health Check and HIBP even if it doesn't match the quality requirements (e. g. password entropy or re-use). You can set the check mark if the password is beyond your control (e. g. if it needs to be a four-digit PIN) to prevent it from cluttering the reports.</p></body></html> + If checked, the entry will not appear in reports like Health Check and HIBP even if it doesn't match the quality requirements. Exclude from database reports @@ -327,7 +327,7 @@ editAttributeButton protectAttributeButton revealAttributeButton - knownBadCheckBox + excludeReportsCheckBox fgColorCheckBox fgColorButton bgColorCheckBox diff --git a/src/gui/entry/EntryModel.cpp b/src/gui/entry/EntryModel.cpp index ccd9b95fb..46c559a5a 100644 --- a/src/gui/entry/EntryModel.cpp +++ b/src/gui/entry/EntryModel.cpp @@ -18,18 +18,17 @@ #include "EntryModel.h" #include -#include #include #include #include -#include "core/Config.h" #include "core/DatabaseIcons.h" #include "core/Entry.h" -#include "core/Global.h" #include "core/Group.h" #include "core/Metadata.h" +#include "core/PasswordHealth.h" #include "gui/Icons.h" +#include "gui/styles/StateColorPalette.h" #ifdef Q_OS_MACOS #include "gui/osutils/macutils/MacUtils.h" #endif @@ -128,7 +127,7 @@ int EntryModel::columnCount(const QModelIndex& parent) const return 0; } - return 14; + return 15; } QVariant EntryModel::data(const QModelIndex& index, int role) const @@ -249,6 +248,12 @@ QVariant EntryModel::data(const QModelIndex& index, int role) const return entry->resolveMultiplePlaceholders(entry->username()); case Password: return entry->resolveMultiplePlaceholders(entry->password()); + case PasswordStrength: { + if (!entry->password().isEmpty() && !entry->excludeFromReports()) { + return entry->passwordHealth()->score(); + } + return 0; + } case Expires: // There seems to be no better way of expressing 'infinity' return entry->timeInfo().expires() ? entry->timeInfo().expiryTime() : QDateTime(QDate(9999, 1, 1)); @@ -290,6 +295,28 @@ QVariant EntryModel::data(const QModelIndex& index, int role) const return icons()->icon("chronometer"); } break; + case PasswordStrength: + if (!entry->password().isEmpty() && !entry->excludeFromReports()) { + StateColorPalette statePalette; + QColor color = statePalette.color(StateColorPalette::Error); + + switch (entry->passwordHealth()->quality()) { + case PasswordHealth::Quality::Bad: + case PasswordHealth::Quality::Poor: + color = statePalette.color(StateColorPalette::HealthCritical); + break; + case PasswordHealth::Quality::Weak: + color = statePalette.color(StateColorPalette::HealthBad); + break; + case PasswordHealth::Quality::Good: + case PasswordHealth::Quality::Excellent: + color = statePalette.color(StateColorPalette::HealthExcellent); + break; + } + + return color; + } + break; } } else if (role == Qt::FontRole) { QFont font; @@ -316,9 +343,9 @@ QVariant EntryModel::data(const QModelIndex& index, int role) const if (backgroundColor.isValid()) { return QVariant(backgroundColor); } - } else if (role == Qt::TextAlignmentRole) { - if (index.column() == Paperclip) { - return Qt::AlignCenter; + } else if (role == Qt::ToolTipRole) { + if (index.column() == PasswordStrength && !entry->password().isEmpty() && !entry->excludeFromReports()) { + return entry->passwordHealth()->scoreReason(); } } @@ -363,6 +390,8 @@ QVariant EntryModel::headerData(int section, Qt::Orientation orientation, int ro return icons()->icon("paperclip"); case Totp: return icons()->icon("chronometer"); + case PasswordStrength: + return icons()->icon("lock-question"); } } else if (role == Qt::ToolTipRole) { switch (section) { @@ -374,6 +403,8 @@ QVariant EntryModel::headerData(int section, Qt::Orientation orientation, int ro return tr("Username"); case Password: return tr("Password"); + case PasswordStrength: + return tr("Password Strength"); case Url: return tr("URL"); case Notes: @@ -393,7 +424,7 @@ QVariant EntryModel::headerData(int section, Qt::Orientation orientation, int ro case Paperclip: return tr("Has attachments"); case Totp: - return tr("Has TOTP one-time password"); + return tr("Has TOTP"); } } diff --git a/src/gui/entry/EntryModel.h b/src/gui/entry/EntryModel.h index 01c2483a9..cd34da7d4 100644 --- a/src/gui/entry/EntryModel.h +++ b/src/gui/entry/EntryModel.h @@ -46,7 +46,8 @@ public: Paperclip = 10, Attachments = 11, Totp = 12, - Size = 13 + Size = 13, + PasswordStrength = 14 }; explicit EntryModel(QObject* parent = nullptr); diff --git a/src/gui/entry/EntryView.cpp b/src/gui/entry/EntryView.cpp index 54c1547c7..9dd021453 100644 --- a/src/gui/entry/EntryView.cpp +++ b/src/gui/entry/EntryView.cpp @@ -20,12 +20,42 @@ #include #include -#include #include +#include #include +#include #include "gui/SortFilterHideProxyModel.h" +#define ICON_ONLY_SECTION_SIZE 26 + +class PasswordStrengthItemDelegate : public QStyledItemDelegate +{ +public: + explicit PasswordStrengthItemDelegate(QObject* parent) + : QStyledItemDelegate(parent){}; + + void initStyleOption(QStyleOptionViewItem* option, const QModelIndex& index) const override + { + QStyledItemDelegate::initStyleOption(option, index); + auto value = index.data(Qt::DecorationRole); + if (value.isValid() && value.type() == QVariant::Color && option->rect.width() > 0) { + // Rebuild the password strength icon to add a dark border + QColor pen(Qt::black); + if (option->widget) { + pen = option->widget->palette().color(QPalette::Shadow); + } + auto size = option->decorationSize; + QImage image(size.width(), size.height(), QImage::Format_ARGB32_Premultiplied); + QPainter p(&image); + p.setBrush(value.value()); + p.setPen(pen); + p.drawRect(0, 0, size.width() - 1, size.height() - 1); + option->icon = QIcon(QPixmap::fromImage(image)); + } + } +}; + EntryView::EntryView(QWidget* parent) : QTreeView(parent) , m_model(new EntryModel(this)) @@ -41,6 +71,7 @@ EntryView::EntryView(QWidget* parent) // Use Qt::UserRole as sort role, see EntryModel::data() m_sortModel->setSortRole(Qt::UserRole); QTreeView::setModel(m_sortModel); + QTreeView::setItemDelegateForColumn(EntryModel::PasswordStrength, new PasswordStrengthItemDelegate(this)); setUniformRowHeights(true); setRootIsDecorated(false); @@ -52,10 +83,10 @@ EntryView::EntryView(QWidget* parent) // QAbstractItemView::startDrag() uses this property as the default drag action setDefaultDropAction(Qt::MoveAction); - // clang-format off connect(this, SIGNAL(doubleClicked(QModelIndex)), SLOT(emitEntryActivated(QModelIndex))); - connect(selectionModel(), SIGNAL(selectionChanged(QItemSelection,QItemSelection)), SLOT(emitEntrySelectionChanged())); - // clang-format on + connect(selectionModel(), &QItemSelectionModel::selectionChanged, this, [this] { + emit entrySelectionChanged(currentEntry()); + }); new QShortcut(Qt::CTRL + Qt::Key_F10, this, SLOT(contextMenuShortcutPressed()), nullptr, Qt::WidgetShortcut); @@ -68,13 +99,11 @@ EntryView::EntryView(QWidget* parent) for (int visualIndex = 1; visualIndex < header()->count(); ++visualIndex) { int logicalIndex = header()->logicalIndex(visualIndex); QString caption = m_model->headerData(logicalIndex, Qt::Horizontal, Qt::DisplayRole).toString(); - if (logicalIndex == EntryModel::Paperclip) { - caption = tr("Has attachments", "Entry attachment icon toggle"); - } else if (logicalIndex == EntryModel::Totp) { - caption = tr("Has TOTP", "Entry TOTP icon toggle"); + if (caption.isEmpty()) { + caption = m_model->headerData(logicalIndex, Qt::Horizontal, Qt::ToolTipRole).toString(); } - QAction* action = m_headerMenu->addAction(caption); + auto action = m_headerMenu->addAction(caption); action->setCheckable(true); action->setData(logicalIndex); m_columnActions->addAction(action); @@ -82,7 +111,8 @@ EntryView::EntryView(QWidget* parent) connect(m_columnActions, SIGNAL(triggered(QAction*)), this, SLOT(toggleColumnVisibility(QAction*))); connect(header(), &QHeaderView::sortIndicatorChanged, [this](int index, Qt::SortOrder order) { Q_UNUSED(order) - header()->setSortIndicatorShown(index != EntryModel::Paperclip && index != EntryModel::Totp); + header()->setSortIndicatorShown(index != EntryModel::Paperclip && index != EntryModel::Totp + && index != EntryModel::PasswordStrength); }); m_headerMenu->addSeparator(); @@ -101,8 +131,6 @@ EntryView::EntryView(QWidget* parent) connect(header(), SIGNAL(sectionMoved(int, int, int)), SIGNAL(viewStateChanged())); connect(header(), SIGNAL(sectionResized(int, int, int)), SIGNAL(viewStateChanged())); connect(header(), SIGNAL(sortIndicatorChanged(int, Qt::SortOrder)), SLOT(sortIndicatorChanged(int, Qt::SortOrder))); - - // clang-format off } void EntryView::contextMenuShortcutPressed() @@ -130,10 +158,10 @@ void EntryView::sortIndicatorChanged(int logicalIndex, Qt::SortOrder order) // do not emit any signals, header()->setSortIndicator recursively calls this // function and the signals are emitted in the else part } else { - // call emitEntrySelectionChanged even though the selection did not really change + // emit entrySelectionChanged even though the selection did not really change // this triggers the evaluation of the menu activation and anyway, the position // of the selected entry within the widget did change - emitEntrySelectionChanged(); + emit entrySelectionChanged(currentEntry()); emit viewStateChanged(); } } @@ -228,11 +256,6 @@ void EntryView::emitEntryActivated(const QModelIndex& index) emit entryActivated(entry, static_cast(m_sortModel->mapToSource(index).column())); } -void EntryView::emitEntrySelectionChanged() -{ - emit entrySelectionChanged(currentEntry()); -} - void EntryView::setModel(QAbstractItemModel* model) { Q_UNUSED(model); @@ -391,18 +414,15 @@ void EntryView::fitColumnsToContents() } /** - * Mark icon-only columns as fixed and resize them to their minimum section size. + * Mark icon-only columns as fixed and resize them to icon-only section size */ void EntryView::resetFixedColumns() { - if (!isColumnHidden(EntryModel::Paperclip)) { - header()->setSectionResizeMode(EntryModel::Paperclip, QHeaderView::Fixed); - header()->resizeSection(EntryModel::Paperclip, header()->minimumSectionSize()); - } - - if (!isColumnHidden(EntryModel::Totp)) { - header()->setSectionResizeMode(EntryModel::Totp, QHeaderView::Fixed); - header()->resizeSection(EntryModel::Totp, header()->minimumSectionSize()); + for (const auto& col : {EntryModel::Paperclip, EntryModel::Totp, EntryModel::PasswordStrength}) { + if (!isColumnHidden(col)) { + header()->setSectionResizeMode(col, QHeaderView::Fixed); + header()->resizeSection(col, ICON_ONLY_SECTION_SIZE); + } } } @@ -431,6 +451,7 @@ void EntryView::resetViewToDefaults() header()->hideSection(EntryModel::Accessed); header()->hideSection(EntryModel::Attachments); header()->hideSection(EntryModel::Size); + header()->hideSection(EntryModel::PasswordStrength); // Reset column order to logical indices for (int i = 0; i < header()->count(); ++i) { diff --git a/src/gui/entry/EntryView.h b/src/gui/entry/EntryView.h index ad2fd037c..4608087fd 100644 --- a/src/gui/entry/EntryView.h +++ b/src/gui/entry/EntryView.h @@ -63,7 +63,6 @@ protected: private slots: void emitEntryActivated(const QModelIndex& index); - void emitEntrySelectionChanged(); void showHeaderMenu(const QPoint& position); void toggleColumnVisibility(QAction* action); void fitColumnsToWindow(); diff --git a/src/gui/reports/ReportsWidgetHealthcheck.cpp b/src/gui/reports/ReportsWidgetHealthcheck.cpp index ea52e1d4a..754f883cd 100644 --- a/src/gui/reports/ReportsWidgetHealthcheck.cpp +++ b/src/gui/reports/ReportsWidgetHealthcheck.cpp @@ -41,14 +41,13 @@ namespace QPointer group; QPointer entry; QSharedPointer health; - bool knownBad = false; + bool exclude = false; Item(const Group* g, const Entry* e, QSharedPointer h) : group(g) , entry(e) , health(h) - , knownBad(e->customData()->contains(PasswordHealth::OPTION_KNOWN_BAD) - && e->customData()->value(PasswordHealth::OPTION_KNOWN_BAD) == TRUE_STR) + , exclude(e->excludeFromReports()) { } @@ -121,7 +120,7 @@ Health::Health(QSharedPointer db) // Evaluate this entry const auto item = QSharedPointer(new Item(group, entry, m_checker.evaluate(entry))); - if (item->knownBad) { + if (item->exclude) { m_anyKnownBad = true; } @@ -140,7 +139,6 @@ Health::Health(QSharedPointer db) ReportsWidgetHealthcheck::ReportsWidgetHealthcheck(QWidget* parent) : QWidget(parent) , m_ui(new Ui::ReportsWidgetHealthcheck()) - , m_errorIcon(icons()->icon("dialog-error")) , m_referencesModel(new QStandardItemModel(this)) , m_modelProxy(new ReportSortProxyModel(this)) { @@ -258,18 +256,18 @@ void ReportsWidgetHealthcheck::calculateHealth() const QScopedPointer health(AsyncTask::runAndWaitForFuture([this] { return new Health(m_db); })); // Display entries that are marked as "known bad"? - const auto showKnownBad = m_ui->showKnownBadCheckBox->isChecked(); + const auto showExcluded = m_ui->showKnownBadCheckBox->isChecked(); // Display the entries m_rowToEntry.clear(); for (const auto& item : health->items()) { - if (item->knownBad && !showKnownBad) { + if (item->exclude && !showExcluded) { // Exclude this entry from the report continue; } // Show the entry in the report - addHealthRow(item->health, item->group, item->entry, item->knownBad); + addHealthRow(item->health, item->group, item->entry, item->exclude); } // Set the table header @@ -330,12 +328,16 @@ void ReportsWidgetHealthcheck::customMenuRequested(QPoint pos) connect(edit, SIGNAL(triggered()), SLOT(editFromContextmenu())); // Create the "exclude from reports" menu item - const auto knownbad = new QAction(icons()->icon("reports-exclude"), tr("Exclude from reports"), this); - knownbad->setCheckable(true); - knownbad->setChecked(m_contextmenuEntry->customData()->contains(PasswordHealth::OPTION_KNOWN_BAD) - && m_contextmenuEntry->customData()->value(PasswordHealth::OPTION_KNOWN_BAD) == TRUE_STR); - menu->addAction(knownbad); - connect(knownbad, SIGNAL(toggled(bool)), SLOT(toggleKnownBad(bool))); + const auto exclude = new QAction(icons()->icon("reports-exclude"), tr("Exclude from reports"), this); + exclude->setCheckable(true); + exclude->setChecked(m_contextmenuEntry->excludeFromReports()); + menu->addAction(exclude); + connect(exclude, &QAction::toggled, exclude, [this](bool state) { + if (m_contextmenuEntry) { + m_contextmenuEntry->setExcludeFromReports(state); + calculateHealth(); + } + }); // Show the context menu menu->popup(m_ui->healthcheckTableView->viewport()->mapToGlobal(pos)); @@ -348,17 +350,6 @@ void ReportsWidgetHealthcheck::editFromContextmenu() } } -void ReportsWidgetHealthcheck::toggleKnownBad(bool isKnownBad) -{ - if (!m_contextmenuEntry) { - return; - } - - m_contextmenuEntry->customData()->set(PasswordHealth::OPTION_KNOWN_BAD, isKnownBad ? TRUE_STR : FALSE_STR); - - calculateHealth(); -} - void ReportsWidgetHealthcheck::saveSettings() { // nothing to do - the tab is passive diff --git a/src/gui/reports/ReportsWidgetHealthcheck.h b/src/gui/reports/ReportsWidgetHealthcheck.h index 58ac0a03b..510301746 100644 --- a/src/gui/reports/ReportsWidgetHealthcheck.h +++ b/src/gui/reports/ReportsWidgetHealthcheck.h @@ -41,7 +41,7 @@ class ReportsWidgetHealthcheck : public QWidget Q_OBJECT public: explicit ReportsWidgetHealthcheck(QWidget* parent = nullptr); - ~ReportsWidgetHealthcheck(); + ~ReportsWidgetHealthcheck() override; void loadSettings(QSharedPointer db); void saveSettings(); @@ -57,7 +57,6 @@ public slots: void emitEntryActivated(const QModelIndex& index); void customMenuRequested(QPoint); void editFromContextmenu(); - void toggleKnownBad(bool); private: void addHealthRow(QSharedPointer, const Group*, const Entry*, bool knownBad); @@ -65,7 +64,6 @@ private: QScopedPointer m_ui; bool m_healthCalculated = false; - QIcon m_errorIcon; QScopedPointer m_referencesModel; QScopedPointer m_modelProxy; QSharedPointer m_db; diff --git a/src/gui/reports/ReportsWidgetHibp.cpp b/src/gui/reports/ReportsWidgetHibp.cpp index 6315f205c..cc8eb91e8 100644 --- a/src/gui/reports/ReportsWidgetHibp.cpp +++ b/src/gui/reports/ReportsWidgetHibp.cpp @@ -32,20 +32,6 @@ namespace { - /* - * Check if an entry has been marked as "known bad password". - * These entries are to be excluded from the HIBP report. - * - * Question to reviewer: Should this be a member function of Entry? - * It's duplicated in EditEntryWidget::setForms, EditEntryWidget::updateEntryData, - * ReportsWidgetHealthcheck::customMenuRequested, and Health::Item::Item. - */ - bool isKnownBad(const Entry* entry) - { - return entry->customData()->contains(PasswordHealth::OPTION_KNOWN_BAD) - && entry->customData()->value(PasswordHealth::OPTION_KNOWN_BAD) == TRUE_STR; - } - class ReportSortProxyModel : public QSortFilterProxyModel { public: @@ -153,25 +139,23 @@ void ReportsWidgetHibp::makeHibpTable() }); // Display entries that are marked as "known bad"? - const auto showKnownBad = m_ui->showKnownBadCheckBox->isChecked(); + const auto showExcluded = m_ui->showKnownBadCheckBox->isChecked(); // The colors for table cells const auto red = QBrush("red"); // Build the table - bool anyKnownBad = false; + bool anyExcluded = false; for (const auto& item : items) { const auto entry = item.first; const auto group = entry->group(); const auto count = item.second; auto title = entry->title(); - // If the entry is marked as known bad, hide it unless the - // checkbox is set. - bool knownBad = isKnownBad(entry); - if (knownBad) { - anyKnownBad = true; - if (!showKnownBad) { + // Hide entry if excluded unless explicitly requested + if (entry->excludeFromReports()) { + anyExcluded = true; + if (!showExcluded) { continue; } @@ -183,7 +167,7 @@ void ReportsWidgetHibp::makeHibpTable() << new QStandardItem(group->iconPixmap(), group->hierarchy().join("/")) << new QStandardItem(countToText(count)); - if (knownBad) { + if (entry->excludeFromReports()) { row[1]->setToolTip(tr("This entry is being excluded from reports")); } @@ -213,7 +197,7 @@ void ReportsWidgetHibp::makeHibpTable() // Show the "show known bad entries" checkbox if there's any known // bad entry in the database. - if (anyKnownBad) { + if (anyExcluded) { m_ui->showKnownBadCheckBox->show(); } else { m_ui->showKnownBadCheckBox->hide(); @@ -331,7 +315,7 @@ void ReportsWidgetHibp::emitEntryActivated(const QModelIndex& index) // Found it, invoke entry editor m_editedEntry = entry; m_editedPassword = entry->password(); - m_editedKnownBad = isKnownBad(entry); + m_editedExcluded = entry->excludeFromReports(); emit entryActivated(const_cast(entry)); } } @@ -350,7 +334,7 @@ void ReportsWidgetHibp::refreshAfterEdit() // No need to re-validate if there was no change that affects // the HIBP result (i. e., change to the password or to the // "known bad" flag) - if (m_editedEntry->password() == m_editedPassword && isKnownBad(m_editedEntry) == m_editedKnownBad) { + if (m_editedEntry->password() == m_editedPassword && m_editedEntry->excludeFromReports() == m_editedExcluded) { // Don't go through HIBP but still rebuild the table, the user might // have edited the entry title. makeHibpTable(); @@ -392,12 +376,16 @@ void ReportsWidgetHibp::customMenuRequested(QPoint pos) connect(edit, SIGNAL(triggered()), SLOT(editFromContextmenu())); // Create the "exclude from reports" menu item - const auto knownbad = new QAction(icons()->icon("reports-exclude"), tr("Exclude from reports"), this); - knownbad->setCheckable(true); - knownbad->setChecked(m_contextmenuEntry->customData()->contains(PasswordHealth::OPTION_KNOWN_BAD) - && m_contextmenuEntry->customData()->value(PasswordHealth::OPTION_KNOWN_BAD) == TRUE_STR); - menu->addAction(knownbad); - connect(knownbad, SIGNAL(toggled(bool)), SLOT(toggleKnownBad(bool))); + const auto exclude = new QAction(icons()->icon("reports-exclude"), tr("Exclude from reports"), this); + exclude->setCheckable(true); + exclude->setChecked(m_contextmenuEntry->excludeFromReports()); + menu->addAction(exclude); + connect(exclude, &QAction::toggled, exclude, [this](bool state) { + if (m_contextmenuEntry) { + m_contextmenuEntry->setExcludeFromReports(state); + makeHibpTable(); + } + }); // Show the context menu menu->popup(m_ui->hibpTableView->viewport()->mapToGlobal(pos)); @@ -410,17 +398,6 @@ void ReportsWidgetHibp::editFromContextmenu() } } -void ReportsWidgetHibp::toggleKnownBad(bool isKnownBad) -{ - if (!m_contextmenuEntry) { - return; - } - - m_contextmenuEntry->customData()->set(PasswordHealth::OPTION_KNOWN_BAD, isKnownBad ? TRUE_STR : FALSE_STR); - - makeHibpTable(); -} - void ReportsWidgetHibp::saveSettings() { // nothing to do - the tab is passive diff --git a/src/gui/reports/ReportsWidgetHibp.h b/src/gui/reports/ReportsWidgetHibp.h index 0d76c07fe..5907cb6d8 100644 --- a/src/gui/reports/ReportsWidgetHibp.h +++ b/src/gui/reports/ReportsWidgetHibp.h @@ -62,7 +62,6 @@ public slots: void makeHibpTable(); void customMenuRequested(QPoint); void editFromContextmenu(); - void toggleKnownBad(bool); private: void startValidation(); @@ -78,7 +77,7 @@ private: QList m_rowToEntry; // List index is table row QPointer m_editedEntry; // The entry we're currently editing QString m_editedPassword; // The old password of the entry we're editing - bool m_editedKnownBad; // The old "known bad" flag of the entry we're editing + bool m_editedExcluded; // The old "known bad" flag of the entry we're editing Entry* m_contextmenuEntry = nullptr; // The entry that was right-clicked #ifdef WITH_XC_NETWORKING diff --git a/src/gui/reports/ReportsWidgetStatistics.cpp b/src/gui/reports/ReportsWidgetStatistics.cpp index 338fd4c9a..200eae940 100644 --- a/src/gui/reports/ReportsWidgetStatistics.cpp +++ b/src/gui/reports/ReportsWidgetStatistics.cpp @@ -26,8 +26,6 @@ #include "core/PasswordHealth.h" #include "gui/Icons.h" -#include -#include #include namespace @@ -37,15 +35,15 @@ namespace public: // The statistics we collect: QDateTime modified; // File modification time - 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 nKnownBad = 0; // Number of known bad entries - int pwdTotalLen = 0; // Total length of all passwords + int groupCount = 0; // Number of groups in the database + int entryCount = 0; // Number of entries (across all groups) + int expiredEntries = 0; // Number of expired entries + int excludedEntries = 0; // Number of known bad entries + int weakPasswords = 0; // Number of weak or poor passwords + int shortPasswords = 0; // Number of passwords 8 characters or less in size + int uniquePasswords = 0; // Number of unique passwords + int reusedPasswords = 0; // Number of non-unique passwords + int totalPasswordLength = 0; // Total length of all passwords // Ctor does all the work explicit Stats(QSharedPointer db) @@ -58,8 +56,8 @@ namespace // Get average password length int averagePwdLength() const { - const auto nPwds = nPwdsUnique + nPwdsReused; - return nPwds == 0 ? 0 : std::round(pwdTotalLen / double(nPwds)); + const auto passwords = uniquePasswords + reusedPasswords; + return passwords == 0 ? 0 : std::round(totalPasswordLength / double(passwords)); } // Get max number of password reuse (=how many entries @@ -77,12 +75,12 @@ namespace // following returns true. bool isAnyExpired() const { - return nExpired > 0; + return expiredEntries > 0; } bool areTooManyPwdsReused() const { - return nPwdsReused > nPwdsUnique / 10; + return reusedPasswords > uniquePasswords / 10; } bool arePwdsReusedTooOften() const @@ -109,7 +107,7 @@ namespace continue; } - ++nGroups; + ++groupCount; for (const auto* entry : group->entries()) { // Don't count anything in the recycle bin @@ -117,36 +115,35 @@ namespace continue; } - ++nEntries; + ++entryCount; if (entry->isExpired()) { - ++nExpired; + ++expiredEntries; } // Get password statistics const auto pwd = entry->password(); if (!pwd.isEmpty()) { if (!m_passwords.contains(pwd)) { - ++nPwdsUnique; + ++uniquePasswords; } else { - ++nPwdsReused; + ++reusedPasswords; } if (pwd.size() < 8) { - ++nPwdsShort; + ++shortPasswords; } // Speed up Zxcvbn process by excluding very long passwords and most passphrases if (pwd.size() < 25 && checker.evaluate(entry)->quality() <= PasswordHealth::Quality::Weak) { - ++nPwdsWeak; + ++weakPasswords; } - if (entry->customData()->contains(PasswordHealth::OPTION_KNOWN_BAD) - && entry->customData()->value(PasswordHealth::OPTION_KNOWN_BAD) == TRUE_STR) { - ++nKnownBad; + if (entry->excludeFromReports()) { + ++excludedEntries; } - pwdTotalLen += pwd.size(); + totalPasswordLength += pwd.size(); m_passwords[pwd]++; } } @@ -220,15 +217,15 @@ void ReportsWidgetStatistics::calculateStats() m_db->isModified() ? tr("yes") : tr("no"), m_db->isModified(), tr("The database was modified, but the changes have not yet been saved to disk.")); - addStatsRow(tr("Number of groups"), QString::number(stats->nGroups)); - addStatsRow(tr("Number of entries"), QString::number(stats->nEntries)); + addStatsRow(tr("Number of groups"), QString::number(stats->groupCount)); + addStatsRow(tr("Number of entries"), QString::number(stats->entryCount)); addStatsRow(tr("Number of expired entries"), - QString::number(stats->nExpired), + QString::number(stats->expiredEntries), stats->isAnyExpired(), tr("The database contains entries that have expired.")); - addStatsRow(tr("Unique passwords"), QString::number(stats->nPwdsUnique)); + addStatsRow(tr("Unique passwords"), QString::number(stats->uniquePasswords)); addStatsRow(tr("Non-unique passwords"), - QString::number(stats->nPwdsReused), + QString::number(stats->reusedPasswords), stats->areTooManyPwdsReused(), tr("More than 10% of passwords are reused. Use unique passwords when possible.")); addStatsRow(tr("Maximum password reuse"), @@ -236,16 +233,16 @@ void ReportsWidgetStatistics::calculateStats() stats->arePwdsReusedTooOften(), 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, + QString::number(stats->shortPasswords), + stats->shortPasswords > 0, tr("Recommended minimum password length is at least 8 characters.")); addStatsRow(tr("Number of weak passwords"), - QString::number(stats->nPwdsWeak), - stats->nPwdsWeak > 0, + QString::number(stats->weakPasswords), + stats->weakPasswords > 0, tr("Recommend using long, randomized passwords with a rating of 'good' or 'excellent'.")); addStatsRow(tr("Entries excluded from reports"), - QString::number(stats->nKnownBad), - stats->nKnownBad > 0, + QString::number(stats->excludedEntries), + stats->excludedEntries > 0, tr("Excluding entries from reports, e. g. because they are known to have a poor password, isn't " "necessarily a problem but you should keep an eye on them.")); addStatsRow(tr("Average password length"), diff --git a/tests/TestEntryModel.cpp b/tests/TestEntryModel.cpp index 5df4b9ed9..52b9dbec5 100644 --- a/tests/TestEntryModel.cpp +++ b/tests/TestEntryModel.cpp @@ -321,7 +321,7 @@ void TestEntryModel::testProxyModel() */ QSignalSpy spyColumnRemove(modelProxy, SIGNAL(columnsAboutToBeRemoved(QModelIndex, int, int))); modelProxy->hideColumn(0, true); - QCOMPARE(modelProxy->columnCount(), 13); + QCOMPARE(modelProxy->columnCount(), 14); QVERIFY(!spyColumnRemove.isEmpty()); int oldSpyColumnRemoveSize = spyColumnRemove.size(); @@ -343,7 +343,7 @@ void TestEntryModel::testProxyModel() */ QSignalSpy spyColumnInsert(modelProxy, SIGNAL(columnsAboutToBeInserted(QModelIndex, int, int))); modelProxy->hideColumn(0, false); - QCOMPARE(modelProxy->columnCount(), 14); + QCOMPARE(modelProxy->columnCount(), 15); QVERIFY(!spyColumnInsert.isEmpty()); int oldSpyColumnInsertSize = spyColumnInsert.size(); diff --git a/tests/gui/TestGui.cpp b/tests/gui/TestGui.cpp index 5ce371668..f65090044 100644 --- a/tests/gui/TestGui.cpp +++ b/tests/gui/TestGui.cpp @@ -461,14 +461,13 @@ void TestGui::testEditEntry() // Test the "known bad" checkbox editEntryWidget->setCurrentPage(1); - auto knownBadCheckBox = editEntryWidget->findChild("knownBadCheckBox"); - QVERIFY(knownBadCheckBox); - QCOMPARE(knownBadCheckBox->isChecked(), false); - knownBadCheckBox->setChecked(true); + auto excludeReportsCheckBox = editEntryWidget->findChild("excludeReportsCheckBox"); + QVERIFY(excludeReportsCheckBox); + QCOMPARE(excludeReportsCheckBox->isChecked(), false); + excludeReportsCheckBox->setChecked(true); QTest::mouseClick(applyButton, Qt::LeftButton); QCOMPARE(entry->historyItems().size(), ++editCount); - QCOMPARE(entry->customData()->contains(PasswordHealth::OPTION_KNOWN_BAD), true); - QCOMPARE(entry->customData()->value(PasswordHealth::OPTION_KNOWN_BAD), TRUE_STR); + QVERIFY(entry->excludeFromReports()); // Test entry colors (simulate choosing a color) editEntryWidget->setCurrentPage(1);