From 3af230746874fba362d186d8cd57f860509c770e Mon Sep 17 00:00:00 2001 From: Felix Geyer Date: Sun, 12 Jan 2014 23:33:36 +0100 Subject: [PATCH] Reworked the PasswordGeneratorWidget. It's loosely based on OS X PasswordAssistant. Generation happens as soon as a change is made, and on open of the widget. A combobox has been added to allow one to choose from some randomally-generated alternatives, and the UI is generally been made a bit more compact. Written by Michael Curtis and revised by me. Closes #119 https://github.com/keepassx/keepassx/pull/38 --- src/CMakeLists.txt | 2 + src/core/PasswordGenerator.cpp | 89 +++++---- src/core/PasswordGenerator.h | 31 ++- src/gui/PasswordComboBox.cpp | 88 ++++++++ src/gui/PasswordComboBox.h | 46 +++++ src/gui/PasswordGeneratorWidget.cpp | 80 ++++++-- src/gui/PasswordGeneratorWidget.h | 12 +- src/gui/PasswordGeneratorWidget.ui | 298 +++++++++++++++++----------- 8 files changed, 449 insertions(+), 197 deletions(-) create mode 100644 src/gui/PasswordComboBox.cpp create mode 100644 src/gui/PasswordComboBox.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 78e8a046a..b53aa639f 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -85,6 +85,7 @@ set(keepassx_SOURCES gui/MessageBox.cpp gui/PasswordEdit.cpp gui/PasswordGeneratorWidget.cpp + gui/PasswordComboBox.cpp gui/SettingsWidget.cpp gui/SortFilterHideProxyModel.cpp gui/UnlockDatabaseWidget.cpp @@ -163,6 +164,7 @@ set(keepassx_MOC gui/MainWindow.h gui/PasswordEdit.h gui/PasswordGeneratorWidget.h + gui/PasswordComboBox.h gui/SettingsWidget.h gui/SortFilterHideProxyModel.h gui/UnlockDatabaseWidget.h diff --git a/src/core/PasswordGenerator.cpp b/src/core/PasswordGenerator.cpp index f8daec695..844f3bf82 100644 --- a/src/core/PasswordGenerator.cpp +++ b/src/core/PasswordGenerator.cpp @@ -19,15 +19,33 @@ #include "crypto/Random.h" -PasswordGenerator* PasswordGenerator::m_instance = Q_NULLPTR; - -QString PasswordGenerator::generatePassword(int length, - const PasswordGenerator::CharClasses& classes, - const PasswordGenerator::GeneratorFlags& flags) +PasswordGenerator::PasswordGenerator() + : m_length(0) + , m_classes(0) + , m_flags(0) { - Q_ASSERT(isValidCombination(length, classes, flags)); +} - QVector groups = passwordGroups(classes, flags); +void PasswordGenerator::setLength(int length) +{ + m_length = length; +} + +void PasswordGenerator::setCharClasses(const CharClasses& classes) +{ + m_classes = classes; +} + +void PasswordGenerator::setFlags(const GeneratorFlags& flags) +{ + m_flags = flags; +} + +QString PasswordGenerator::generatePassword() const +{ + Q_ASSERT(isValid()); + + QVector groups = passwordGroups(); QVector passwordChars; Q_FOREACH (const PasswordGroup& group, groups) { @@ -38,14 +56,14 @@ QString PasswordGenerator::generatePassword(int length, QString password; - if (flags & CharFromEveryGroup) { + if (m_flags & CharFromEveryGroup) { for (int i = 0; i < groups.size(); i++) { int pos = randomGen()->randomUInt(groups[i].size()); password.append(groups[i][pos]); } - for (int i = groups.size(); i < length; i++) { + for (int i = groups.size(); i < m_length; i++) { int pos = randomGen()->randomUInt(passwordChars.size()); password.append(passwordChars[pos]); @@ -61,7 +79,7 @@ QString PasswordGenerator::generatePassword(int length, } } else { - for (int i = 0; i < length; i++) { + for (int i = 0; i < m_length; i++) { int pos = randomGen()->randomUInt(passwordChars.size()); password.append(passwordChars[pos]); @@ -71,34 +89,31 @@ QString PasswordGenerator::generatePassword(int length, return password; } -bool PasswordGenerator::isValidCombination(int length, - const PasswordGenerator::CharClasses& classes, - const PasswordGenerator::GeneratorFlags& flags) +bool PasswordGenerator::isValid() const { - if (classes == 0) { + if (m_classes == 0) { return false; } - else if (length == 0) { + else if (m_length == 0) { return false; } - if ((flags & CharFromEveryGroup) && (length < numCharClasses(classes))) { + if ((m_flags & CharFromEveryGroup) && (m_length < numCharClasses())) { return false; } return true; } -QVector PasswordGenerator::passwordGroups(const PasswordGenerator::CharClasses& classes, - const PasswordGenerator::GeneratorFlags& flags) +QVector PasswordGenerator::passwordGroups() const { QVector passwordGroups; - if (classes & LowerLetters) { + if (m_classes & LowerLetters) { PasswordGroup group; for (int i = 97; i < (97 + 26); i++) { - if ((flags & ExcludeLookAlike) && (i == 108)) { // "l" + if ((m_flags & ExcludeLookAlike) && (i == 108)) { // "l" continue; } @@ -107,11 +122,11 @@ QVector PasswordGenerator::passwordGroups(const PasswordGenerator passwordGroups.append(group); } - if (classes & UpperLetters) { + if (m_classes & UpperLetters) { PasswordGroup group; for (int i = 65; i < (65 + 26); i++) { - if ((flags & ExcludeLookAlike) && (i == 73 || i == 79)) { // "I" and "O" + if ((m_flags & ExcludeLookAlike) && (i == 73 || i == 79)) { // "I" and "O" continue; } @@ -120,11 +135,11 @@ QVector PasswordGenerator::passwordGroups(const PasswordGenerator passwordGroups.append(group); } - if (classes & Numbers) { + if (m_classes & Numbers) { PasswordGroup group; for (int i = 48; i < (48 + 10); i++) { - if ((flags & ExcludeLookAlike) && (i == 48 || i == 49)) { // "0" and "1" + if ((m_flags & ExcludeLookAlike) && (i == 48 || i == 49)) { // "0" and "1" continue; } @@ -133,7 +148,7 @@ QVector PasswordGenerator::passwordGroups(const PasswordGenerator passwordGroups.append(group); } - if (classes & SpecialCharacters) { + if (m_classes & SpecialCharacters) { PasswordGroup group; for (int i = 33; i <= 47; i++) { @@ -149,7 +164,7 @@ QVector PasswordGenerator::passwordGroups(const PasswordGenerator } for (int i = 123; i <= 126; i++) { - if ((flags & ExcludeLookAlike) && (i == 124)) { // "|" + if ((m_flags & ExcludeLookAlike) && (i == 124)) { // "|" continue; } @@ -162,35 +177,23 @@ QVector PasswordGenerator::passwordGroups(const PasswordGenerator return passwordGroups; } -int PasswordGenerator::numCharClasses(const PasswordGenerator::CharClasses& classes) +int PasswordGenerator::numCharClasses() const { int numClasses = 0; - if (classes & LowerLetters) { + if (m_classes & LowerLetters) { numClasses++; } - if (classes & UpperLetters) { + if (m_classes & UpperLetters) { numClasses++; } - if (classes & Numbers) { + if (m_classes & Numbers) { numClasses++; } - if (classes & SpecialCharacters) { + if (m_classes & SpecialCharacters) { numClasses++; } return numClasses; } -PasswordGenerator* PasswordGenerator::instance() -{ - if (!m_instance) { - m_instance = new PasswordGenerator(); - } - - return m_instance; -} - -PasswordGenerator::PasswordGenerator() -{ -} diff --git a/src/core/PasswordGenerator.h b/src/core/PasswordGenerator.h index 2a9a47b82..e9e05660e 100644 --- a/src/core/PasswordGenerator.h +++ b/src/core/PasswordGenerator.h @@ -45,29 +45,28 @@ public: }; Q_DECLARE_FLAGS(GeneratorFlags, GeneratorFlag) - QString generatePassword(int length, const PasswordGenerator::CharClasses& classes, - const PasswordGenerator::GeneratorFlags& flags); - bool isValidCombination(int length, const PasswordGenerator::CharClasses& classes, - const PasswordGenerator::GeneratorFlags& flags); - - static PasswordGenerator* instance(); - -private: +public: PasswordGenerator(); - QVector passwordGroups(const PasswordGenerator::CharClasses& classes, - const PasswordGenerator::GeneratorFlags& flags); - int numCharClasses(const PasswordGenerator::CharClasses& classes); + void setLength(int length); + void setCharClasses(const CharClasses& classes); + void setFlags(const GeneratorFlags& flags); - static PasswordGenerator* m_instance; + bool isValid() const; + + QString generatePassword() const; +private: + + QVector passwordGroups() const; + int numCharClasses() const; + + int m_length; + CharClasses m_classes; + GeneratorFlags m_flags; Q_DISABLE_COPY(PasswordGenerator) }; -inline PasswordGenerator* passwordGenerator() { - return PasswordGenerator::instance(); -} - Q_DECLARE_OPERATORS_FOR_FLAGS(PasswordGenerator::CharClasses) Q_DECLARE_OPERATORS_FOR_FLAGS(PasswordGenerator::GeneratorFlags) diff --git a/src/gui/PasswordComboBox.cpp b/src/gui/PasswordComboBox.cpp new file mode 100644 index 000000000..ca02635c5 --- /dev/null +++ b/src/gui/PasswordComboBox.cpp @@ -0,0 +1,88 @@ +/* + * Copyright (C) 2013 Michael Curtis + * Copyright (C) 2014 Felix Geyer + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 or (at your option) + * version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "PasswordComboBox.h" + +#include + +#include "core/PasswordGenerator.h" + +PasswordComboBox::PasswordComboBox(QWidget* parent) + : QComboBox(parent) + , m_generator(Q_NULLPTR) + , m_alternatives(10) +{ + setEditable(true); + setEcho(false); +} + +PasswordComboBox::~PasswordComboBox() +{ +} + +void PasswordComboBox::setEcho(bool echo) +{ + lineEdit()->setEchoMode(echo ? QLineEdit::Normal : QLineEdit::Password); + + QString current = currentText(); + + if (echo) { + // add fake item to show visual indication that a popup is available + addItem(""); + } + else { + // clear items so the combobox indicates that no popup menu is available + clear(); + } + + setEditText(current); +} + +void PasswordComboBox::setGenerator(PasswordGenerator* generator) +{ + m_generator = generator; +} + +void PasswordComboBox::setNumberAlternatives(int alternatives) +{ + m_alternatives = alternatives; +} + +void PasswordComboBox::showPopup() +{ + // no point in showing a bunch of hidden passwords + if (lineEdit()->echoMode() == QLineEdit::Password) { + hidePopup(); + return; + } + + // keep existing password as the first item in the popup + QString current = currentText(); + clear(); + addItem(current); + + if (m_generator && m_generator->isValid()) { + for (int alternative = 0; alternative < m_alternatives; alternative++) { + QString password = m_generator->generatePassword(); + + addItem(password); + } + } + + QComboBox::showPopup(); +} diff --git a/src/gui/PasswordComboBox.h b/src/gui/PasswordComboBox.h new file mode 100644 index 000000000..3784a6531 --- /dev/null +++ b/src/gui/PasswordComboBox.h @@ -0,0 +1,46 @@ +/* + * Copyright (C) 2013 Michael Curtis + * Copyright (C) 2014 Felix Geyer + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 or (at your option) + * version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#ifndef KEEPASSX_PASSWORDCOMBOBOX_H +#define KEEPASSX_PASSWORDCOMBOBOX_H + +#include + +#include "core/Global.h" + +class PasswordGenerator; + +class PasswordComboBox : public QComboBox +{ + Q_OBJECT + +public: + explicit PasswordComboBox(QWidget* parent = Q_NULLPTR); + ~PasswordComboBox(); + + void setEcho(bool echo); + void setGenerator(PasswordGenerator* generator); + void setNumberAlternatives(int alternatives); + void showPopup(); + +private: + PasswordGenerator* m_generator; + int m_alternatives; +}; + +#endif // KEEPASSX_PASSWORDCOMBOBOX_H diff --git a/src/gui/PasswordGeneratorWidget.cpp b/src/gui/PasswordGeneratorWidget.cpp index 756635b31..044484d25 100644 --- a/src/gui/PasswordGeneratorWidget.cpp +++ b/src/gui/PasswordGeneratorWidget.cpp @@ -18,22 +18,36 @@ #include "PasswordGeneratorWidget.h" #include "ui_PasswordGeneratorWidget.h" +#include + #include "core/Config.h" +#include "core/PasswordGenerator.h" +#include "core/FilePath.h" PasswordGeneratorWidget::PasswordGeneratorWidget(QWidget* parent) : QWidget(parent) + , m_updatingSpinBox(false) + , m_generator(new PasswordGenerator()) , m_ui(new Ui::PasswordGeneratorWidget()) { m_ui->setupUi(this); - connect(m_ui->editNewPassword, SIGNAL(textChanged(QString)), SLOT(updateApplyEnabled(QString))); + m_ui->togglePasswordButton->setIcon(filePath()->onOffIcon("actions", "password-show")); + + connect(m_ui->editNewPassword->lineEdit(), SIGNAL(textChanged(QString)), SLOT(updateApplyEnabled(QString))); connect(m_ui->togglePasswordButton, SIGNAL(toggled(bool)), SLOT(togglePassword(bool))); - connect(m_ui->buttonGenerate, SIGNAL(clicked()), SLOT(generatePassword())); connect(m_ui->buttonApply, SIGNAL(clicked()), SLOT(emitNewPassword())); connect(m_ui->buttonApply, SIGNAL(clicked()), SLOT(saveSettings())); - reset(); + connect(m_ui->sliderLength, SIGNAL(valueChanged(int)), SLOT(sliderMoved())); + connect(m_ui->spinBoxLength, SIGNAL(valueChanged(int)), SLOT(spinBoxChanged())); + + connect(m_ui->optionButtons, SIGNAL(buttonClicked(int)), SLOT(updateGenerator())); + + m_ui->editNewPassword->setGenerator(m_generator.data()); + loadSettings(); + reset(); } PasswordGeneratorWidget::~PasswordGeneratorWidget() @@ -68,8 +82,10 @@ void PasswordGeneratorWidget::saveSettings() void PasswordGeneratorWidget::reset() { - m_ui->editNewPassword->setText(""); - m_ui->togglePasswordButton->setChecked(true); + m_ui->editNewPassword->lineEdit()->setText(""); + m_ui->togglePasswordButton->setChecked(config()->get("security/passwordscleartext").toBool()); + + updateGenerator(); } void PasswordGeneratorWidget::updateApplyEnabled(const QString& password) @@ -79,27 +95,35 @@ void PasswordGeneratorWidget::updateApplyEnabled(const QString& password) void PasswordGeneratorWidget::togglePassword(bool checked) { - m_ui->editNewPassword->setEchoMode(checked ? QLineEdit::Password : QLineEdit::Normal); -} - -void PasswordGeneratorWidget::generatePassword() -{ - int length = m_ui->spinBoxLength->value(); - PasswordGenerator::CharClasses classes = charClasses(); - PasswordGenerator::GeneratorFlags flags = generatorFlags(); - - if (!passwordGenerator()->isValidCombination(length, classes, flags)) { - // TODO: display error - return; - } - - QString password = passwordGenerator()->generatePassword(length, classes, flags); - m_ui->editNewPassword->setText(password); + m_ui->editNewPassword->setEcho(checked); } void PasswordGeneratorWidget::emitNewPassword() { - Q_EMIT newPassword(m_ui->editNewPassword->text()); + Q_EMIT newPassword(m_ui->editNewPassword->lineEdit()->text()); +} + +void PasswordGeneratorWidget::sliderMoved() +{ + if (m_updatingSpinBox) { + return; + } + + m_ui->spinBoxLength->setValue(m_ui->sliderLength->value()); + + updateGenerator(); +} + +void PasswordGeneratorWidget::spinBoxChanged() +{ + // Interlock so that we don't update twice - this causes issues as the spinbox can go higher than slider + m_updatingSpinBox = true; + + m_ui->sliderLength->setValue(m_ui->spinBoxLength->value()); + + m_updatingSpinBox = false; + + updateGenerator(); } PasswordGenerator::CharClasses PasswordGeneratorWidget::charClasses() @@ -139,3 +163,15 @@ PasswordGenerator::GeneratorFlags PasswordGeneratorWidget::generatorFlags() return flags; } + +void PasswordGeneratorWidget::updateGenerator() +{ + m_generator->setLength(m_ui->spinBoxLength->value()); + m_generator->setCharClasses(charClasses()); + m_generator->setFlags(generatorFlags()); + + if (m_generator->isValid()) { + QString password = m_generator->generatePassword(); + m_ui->editNewPassword->setEditText(password); + } +} diff --git a/src/gui/PasswordGeneratorWidget.h b/src/gui/PasswordGeneratorWidget.h index 2e5d98f70..465f03d78 100644 --- a/src/gui/PasswordGeneratorWidget.h +++ b/src/gui/PasswordGeneratorWidget.h @@ -19,6 +19,7 @@ #define KEEPASSX_PASSWORDGENERATORWIDGET_H #include +#include #include "core/Global.h" #include "core/PasswordGenerator.h" @@ -27,6 +28,8 @@ namespace Ui { class PasswordGeneratorWidget; } +class PasswordGenerator; + class PasswordGeneratorWidget : public QWidget { Q_OBJECT @@ -43,14 +46,21 @@ Q_SIGNALS: private Q_SLOTS: void updateApplyEnabled(const QString& password); void togglePassword(bool checked); - void generatePassword(); + void emitNewPassword(); void saveSettings(); + void sliderMoved(); + void spinBoxChanged(); + + void updateGenerator(); private: + bool m_updatingSpinBox; + PasswordGenerator::CharClasses charClasses(); PasswordGenerator::GeneratorFlags generatorFlags(); + const QScopedPointer m_generator; const QScopedPointer m_ui; }; diff --git a/src/gui/PasswordGeneratorWidget.ui b/src/gui/PasswordGeneratorWidget.ui index 2b6d7a69f..342f191a8 100644 --- a/src/gui/PasswordGeneratorWidget.ui +++ b/src/gui/PasswordGeneratorWidget.ui @@ -6,66 +6,79 @@ 0 0 - 468 - 298 + 434 + 250 - + - - - Use the following password groups: - - - - - - - - - Qt::Horizontal + + + + + Password: - - QSizePolicy::Maximum - - - - 20 - 1 - - - + - - - - - - Lower letters + + + + QLayout::SetNoConstraint + + + + + true - - - - Numbers + + + + true - - - - Upper letters + + + + + + Length: + + + + + + + + + 1 + + + 64 + + + Qt::Horizontal + + + QSlider::TicksBelow + + + 8 - - - - Special characters + + + + 1 + + + 999 @@ -74,38 +87,117 @@ - - - Exclude look-alike characters + + + Character Types + + + + + + + Upper Case Letters + + + A-Z + + + true + + + optionButtons + + + + + + + Lower Case Letters + + + a-z + + + true + + + optionButtons + + + + + + + Numbers + + + 0-9 + + + true + + + optionButtons + + + + + + + Special Characters + + + /*_& ... + + + true + + + optionButtons + + + + + + + Qt::Horizontal + + + + 40 + 20 + + + + + + + + + + Exclude look-alike characters + + + optionButtons + + + + + + + Ensure that the password contains characters from every group + + + optionButtons + + + + - - - Ensure that the password contains characters from every group - - - - - - - - - Length: - - - - - - - 1 - - - 999 - - - + @@ -119,51 +211,13 @@ - - - - - - - - New password: - - - - - - - QLineEdit::Password - - - - - - - ... - - - true - - - true - - - - - - - Generate - - - false - Apply + Accept @@ -171,19 +225,33 @@ + + + PasswordComboBox + QComboBox +
gui/PasswordComboBox.h
+
+
+ editNewPassword + togglePasswordButton + sliderLength + spinBoxLength + checkBoxUpper checkBoxLower checkBoxNumbers - checkBoxUpper checkBoxSpecialChars checkBoxExcludeAlike checkBoxEnsureEvery - spinBoxLength - editNewPassword - togglePasswordButton - buttonGenerate buttonApply + + + + false + + +