Skip to content

Commit

Permalink
Add entry view column for password strength
Browse files Browse the repository at this point in the history
* Closes keepassxreboot#4216

Reduced to three-tiered rating system and fixed column implementation. Hide password strength indicator in entry view if excluded from reports.

Introduce password health caching to prevent unnecessary calculations.
  • Loading branch information
JJuiice authored and droidmonkey committed Feb 27, 2021
1 parent c9c19d0 commit 0221544
Show file tree
Hide file tree
Showing 23 changed files with 213 additions and 187 deletions.
1 change: 1 addition & 0 deletions COPYING
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions share/icons/application/scalable/actions/lock-question.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions share/icons/icons.qrc
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
<file>application/scalable/actions/help-about.svg</file>
<file>application/scalable/actions/hibp.svg</file>
<file>application/scalable/actions/key-enter.svg</file>
<file>application/scalable/actions/lock-question.svg</file>
<file>application/scalable/actions/keyboard-shortcuts.svg</file>
<file>application/scalable/actions/message-close.svg</file>
<file>application/scalable/actions/move-down.svg</file>
Expand Down
2 changes: 1 addition & 1 deletion src/cli/Estimate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
1 change: 1 addition & 0 deletions src/core/CustomData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions src/core/CustomData.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ class CustomData : public QObject
static const QString Created;
static const QString BrowserKeyPrefix;
static const QString BrowserLegacyKeyPrefix;
static const QString ExcludeFromReports;

signals:
void customDataModified();
Expand Down
25 changes: 22 additions & 3 deletions src/core/Entry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -245,6 +243,25 @@ QString Entry::defaultAutoTypeSequence() const
return m_data.defaultAutoTypeSequence;
}

const QSharedPointer<PasswordHealth>& 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
Expand Down Expand Up @@ -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));
}

Expand Down
7 changes: 6 additions & 1 deletion src/core/Entry.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@

class Database;
class Group;
class PasswordHealth;

namespace Totp
{
struct Settings;
Expand Down Expand Up @@ -66,6 +68,7 @@ struct EntryData
QString defaultAutoTypeSequence;
TimeInfo timeInfo;
QSharedPointer<Totp::Settings> totpSettings;
QSharedPointer<PasswordHealth> passwordHealth;

bool operator==(const EntryData& other) const;
bool operator!=(const EntryData& other) const;
Expand Down Expand Up @@ -94,7 +97,6 @@ class Entry : public QObject
int autoTypeObfuscation() const;
QString defaultAutoTypeSequence() const;
QString effectiveAutoTypeSequence() const;
QString effectiveNewAutoTypeSequence() const;
QList<QString> autoTypeSequences(const QString& pattern = {}) const;
AutoTypeAssociations* autoTypeAssociations();
const AutoTypeAssociations* autoTypeAssociations() const;
Expand All @@ -111,6 +113,9 @@ class Entry : public QObject
QSharedPointer<Totp::Settings> totpSettings() const;
int size() const;
QString path() const;
const QSharedPointer<PasswordHealth>& passwordHealth();
bool excludeFromReports() const;
void setExcludeFromReports(bool state);

bool hasTotp() const;
bool isExpired() const;
Expand Down
7 changes: 2 additions & 5 deletions src/core/PasswordHealth.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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))
{
}

Expand Down
10 changes: 1 addition & 9 deletions src/core/PasswordHealth.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -83,14 +83,6 @@ class PasswordHealth
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;
Expand Down
13 changes: 4 additions & 9 deletions src/gui/entry/EditEntryWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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()) {
Expand Down
6 changes: 3 additions & 3 deletions src/gui/entry/EditEntryWidgetAdvanced.ui
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,9 @@
</widget>
</item>
<item>
<widget class="QCheckBox" name="knownBadCheckBox">
<widget class="QCheckBox" name="excludeReportsCheckBox">
<property name="toolTip">
<string>&lt;html&gt;&lt;head/&gt;&lt;body&gt;&lt;p&gt;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.&lt;/p&gt;&lt;/body&gt;&lt;/html&gt;</string>
<string>If checked, the entry will not appear in reports like Health Check and HIBP even if it doesn't match the quality requirements.</string>
</property>
<property name="text">
<string>Exclude from database reports</string>
Expand Down Expand Up @@ -327,7 +327,7 @@
<tabstop>editAttributeButton</tabstop>
<tabstop>protectAttributeButton</tabstop>
<tabstop>revealAttributeButton</tabstop>
<tabstop>knownBadCheckBox</tabstop>
<tabstop>excludeReportsCheckBox</tabstop>
<tabstop>fgColorCheckBox</tabstop>
<tabstop>fgColorButton</tabstop>
<tabstop>bgColorCheckBox</tabstop>
Expand Down
47 changes: 39 additions & 8 deletions src/gui/entry/EntryModel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,17 @@
#include "EntryModel.h"

#include <QDateTime>
#include <QFont>
#include <QMimeData>
#include <QPainter>
#include <QPalette>

#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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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;
Expand All @@ -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();
}
}

Expand Down Expand Up @@ -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) {
Expand All @@ -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:
Expand All @@ -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");
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/gui/entry/EntryModel.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ class EntryModel : public QAbstractTableModel
Paperclip = 10,
Attachments = 11,
Totp = 12,
Size = 13
Size = 13,
PasswordStrength = 14
};

explicit EntryModel(QObject* parent = nullptr);
Expand Down
Loading

0 comments on commit 0221544

Please sign in to comment.