CLI password generation options cleanup (#3275)

Summary of changes:
* Extract function for creating password generator from options into
`Generate` command. This function is now reused in `Add` and `Edit`
commands.
* Updated manpage with missing password generation options.
* Updated manpage with missing longer forms of password generation options.
* Added unit tests for new password generation options in `Add` and
`Edit`.
* Handle case when `-g` and `-p` options are used at the same time.

This PR adds password generation functionalities while reducing
code duplication, but at the cost of 2 small breaking changes:
* The password generation option for `Add` and `Edit` for specifying
password length is now `-L` instead of `-l`, to not clash with the
`-l --lower` option.
* The `-u` shorthand for the `--upper` option has to be removed, to not
clash with the `-u --username` option.
* Add -U variant for uppercase.
This commit is contained in:
louib 2019-08-30 22:50:32 -04:00 committed by Jonathan White
parent 79bb991a61
commit eb1882453f
8 changed files with 221 additions and 88 deletions

View file

@ -20,6 +20,7 @@
#include "Add.h"
#include "cli/Generate.h"
#include "cli/TextStream.h"
#include "cli/Utils.h"
#include "core/Database.h"
@ -44,11 +45,6 @@ const QCommandLineOption Add::GenerateOption = QCommandLineOption(QStringList()
<< "generate",
QObject::tr("Generate a password for the entry."));
const QCommandLineOption Add::PasswordLengthOption =
QCommandLineOption(QStringList() << "l"
<< "password-length",
QObject::tr("Length for the generated password."),
QObject::tr("length"));
Add::Add()
{
@ -57,9 +53,19 @@ Add::Add()
options.append(Add::UsernameOption);
options.append(Add::UrlOption);
options.append(Add::PasswordPromptOption);
options.append(Add::GenerateOption);
options.append(Add::PasswordLengthOption);
positionalArguments.append({QString("entry"), QObject::tr("Path of the entry to add."), QString("")});
// Password generation options.
options.append(Add::GenerateOption);
options.append(Generate::PasswordLengthOption);
options.append(Generate::LowerCaseOption);
options.append(Generate::UpperCaseOption);
options.append(Generate::NumbersOption);
options.append(Generate::SpecialCharsOption);
options.append(Generate::ExtendedAsciiOption);
options.append(Generate::ExcludeCharsOption);
options.append(Generate::ExcludeSimilarCharsOption);
options.append(Generate::IncludeEveryGroupOption);
}
int Add::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer<QCommandLineParser> parser)
@ -72,14 +78,22 @@ int Add::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer<Q
const QString& databasePath = args.at(0);
const QString& entryPath = args.at(1);
// Validating the password length here, before we actually create
// the entry.
QString passwordLength = parser->value(Add::PasswordLengthOption);
if (!passwordLength.isEmpty() && !passwordLength.toInt()) {
errorTextStream << QObject::tr("Invalid value for password length %1.").arg(passwordLength) << endl;
// Cannot use those 2 options at the same time!
if (parser->isSet(Add::GenerateOption) && parser->isSet(Add::PasswordPromptOption)) {
errorTextStream << QObject::tr("Cannot generate a password and prompt at the same time!") << endl;
return EXIT_FAILURE;
}
// Validating the password generator here, before we actually create
// the entry.
QSharedPointer<PasswordGenerator> passwordGenerator;
if (parser->isSet(Add::GenerateOption)) {
passwordGenerator = Generate::createGenerator(parser);
if (passwordGenerator.isNull()) {
return EXIT_FAILURE;
}
}
Entry* entry = database->rootGroup()->addEntryWithPath(entryPath);
if (!entry) {
errorTextStream << QObject::tr("Could not create entry with path %1.").arg(entryPath) << endl;
@ -101,17 +115,7 @@ int Add::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer<Q
QString password = Utils::getPassword(parser->isSet(Command::QuietOption) ? Utils::DEVNULL : Utils::STDOUT);
entry->setPassword(password);
} else if (parser->isSet(Add::GenerateOption)) {
PasswordGenerator passwordGenerator;
if (passwordLength.isEmpty()) {
passwordGenerator.setLength(PasswordGenerator::DefaultLength);
} else {
passwordGenerator.setLength(passwordLength.toInt());
}
passwordGenerator.setCharClasses(PasswordGenerator::DefaultCharset);
passwordGenerator.setFlags(PasswordGenerator::DefaultFlags);
QString password = passwordGenerator.generatePassword();
QString password = passwordGenerator->generatePassword();
entry->setPassword(password);
}

View file

@ -21,6 +21,7 @@
#include "Edit.h"
#include "cli/Add.h"
#include "cli/Generate.h"
#include "cli/TextStream.h"
#include "cli/Utils.h"
#include "core/Database.h"
@ -41,10 +42,20 @@ Edit::Edit()
options.append(Add::UsernameOption);
options.append(Add::UrlOption);
options.append(Add::PasswordPromptOption);
options.append(Add::GenerateOption);
options.append(Add::PasswordLengthOption);
options.append(Edit::TitleOption);
positionalArguments.append({QString("entry"), QObject::tr("Path of the entry to edit."), QString("")});
// Password generation options.
options.append(Add::GenerateOption);
options.append(Generate::PasswordLengthOption);
options.append(Generate::LowerCaseOption);
options.append(Generate::UpperCaseOption);
options.append(Generate::NumbersOption);
options.append(Generate::SpecialCharsOption);
options.append(Generate::ExtendedAsciiOption);
options.append(Generate::ExcludeCharsOption);
options.append(Generate::ExcludeSimilarCharsOption);
options.append(Generate::IncludeEveryGroupOption);
}
int Edit::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer<QCommandLineParser> parser)
@ -57,12 +68,23 @@ int Edit::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer<
const QString& databasePath = args.at(0);
const QString& entryPath = args.at(1);
QString passwordLength = parser->value(Add::PasswordLengthOption);
if (!passwordLength.isEmpty() && !passwordLength.toInt()) {
errorTextStream << QObject::tr("Invalid value for password length: %1").arg(passwordLength) << endl;
// Cannot use those 2 options at the same time!
if (parser->isSet(Add::GenerateOption) && parser->isSet(Add::PasswordPromptOption)) {
errorTextStream << QObject::tr("Cannot generate a password and prompt at the same time!") << endl;
return EXIT_FAILURE;
}
// Validating the password generator here, before we actually start
// the update.
QSharedPointer<PasswordGenerator> passwordGenerator;
bool generate = parser->isSet(Add::GenerateOption);
if (generate) {
passwordGenerator = Generate::createGenerator(parser);
if (passwordGenerator.isNull()) {
return EXIT_FAILURE;
}
}
Entry* entry = database->rootGroup()->findEntryByPath(entryPath);
if (!entry) {
errorTextStream << QObject::tr("Could not find entry with path %1.").arg(entryPath) << endl;
@ -72,7 +94,6 @@ int Edit::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer<
QString username = parser->value(Add::UsernameOption);
QString url = parser->value(Add::UrlOption);
QString title = parser->value(Edit::TitleOption);
bool generate = parser->isSet(Add::GenerateOption);
bool prompt = parser->isSet(Add::PasswordPromptOption);
if (username.isEmpty() && url.isEmpty() && title.isEmpty() && !prompt && !generate) {
errorTextStream << QObject::tr("Not changing any field for entry %1.").arg(entryPath) << endl;
@ -98,17 +119,7 @@ int Edit::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer<
QString password = Utils::getPassword(parser->isSet(Command::QuietOption) ? Utils::DEVNULL : Utils::STDOUT);
entry->setPassword(password);
} else if (generate) {
PasswordGenerator passwordGenerator;
if (passwordLength.isEmpty()) {
passwordGenerator.setLength(PasswordGenerator::DefaultLength);
} else {
passwordGenerator.setLength(static_cast<size_t>(passwordLength.toInt()));
}
passwordGenerator.setCharClasses(PasswordGenerator::DefaultCharset);
passwordGenerator.setFlags(PasswordGenerator::DefaultFlags);
QString password = passwordGenerator.generatePassword();
QString password = passwordGenerator->generatePassword();
entry->setPassword(password);
}

View file

@ -22,7 +22,6 @@
#include "cli/TextStream.h"
#include "cli/Utils.h"
#include "core/PasswordGenerator.h"
const QCommandLineOption Generate::PasswordLengthOption =
QCommandLineOption(QStringList() << "L"
@ -34,7 +33,7 @@ const QCommandLineOption Generate::LowerCaseOption = QCommandLineOption(QStringL
<< "lower",
QObject::tr("Use lowercase characters"));
const QCommandLineOption Generate::UpperCaseOption = QCommandLineOption(QStringList() << "u"
const QCommandLineOption Generate::UpperCaseOption = QCommandLineOption(QStringList() << "U"
<< "upper",
QObject::tr("Use uppercase characters"));
@ -75,26 +74,22 @@ Generate::Generate()
options.append(Generate::IncludeEveryGroupOption);
}
int Generate::execute(const QStringList& arguments)
/**
* Creates a password generator instance using the command line options
* of the parser object.
*/
QSharedPointer<PasswordGenerator> Generate::createGenerator(QSharedPointer<QCommandLineParser> parser)
{
QSharedPointer<QCommandLineParser> parser = getCommandLineParser(arguments);
if (parser.isNull()) {
return EXIT_FAILURE;
}
TextStream outputTextStream(Utils::STDOUT, QIODevice::WriteOnly);
TextStream errorTextStream(Utils::STDERR, QIODevice::WriteOnly);
const QStringList args = parser->positionalArguments();
PasswordGenerator passwordGenerator;
QSharedPointer<PasswordGenerator> passwordGenerator = QSharedPointer<PasswordGenerator>(new PasswordGenerator());
QString passwordLength = parser->value(Generate::PasswordLengthOption);
if (passwordLength.isEmpty()) {
passwordGenerator.setLength(PasswordGenerator::DefaultLength);
passwordGenerator->setLength(PasswordGenerator::DefaultLength);
} else if (passwordLength.toInt() <= 0) {
errorTextStream << QObject::tr("Invalid password length %1").arg(passwordLength) << endl;
return EXIT_FAILURE;
return QSharedPointer<PasswordGenerator>(nullptr);
} else {
passwordGenerator.setLength(passwordLength.toInt());
passwordGenerator->setLength(passwordLength.toInt());
}
PasswordGenerator::CharClasses classes = 0x0;
@ -124,16 +119,34 @@ int Generate::execute(const QStringList& arguments)
flags |= PasswordGenerator::CharFromEveryGroup;
}
passwordGenerator.setCharClasses(classes);
passwordGenerator.setFlags(flags);
passwordGenerator.setExcludedChars(parser->value(Generate::ExcludeCharsOption));
// The default charset will be used if no explicit class
// option was set.
passwordGenerator->setCharClasses(classes);
passwordGenerator->setFlags(flags);
passwordGenerator->setExcludedChars(parser->value(Generate::ExcludeCharsOption));
if (!passwordGenerator.isValid()) {
if (!passwordGenerator->isValid()) {
errorTextStream << QObject::tr("invalid password generator after applying all options") << endl;
return QSharedPointer<PasswordGenerator>(nullptr);
}
return passwordGenerator;
}
int Generate::execute(const QStringList& arguments)
{
QSharedPointer<QCommandLineParser> parser = getCommandLineParser(arguments);
if (parser.isNull()) {
return EXIT_FAILURE;
}
QString password = passwordGenerator.generatePassword();
QSharedPointer<PasswordGenerator> passwordGenerator = Generate::createGenerator(parser);
if (passwordGenerator.isNull()) {
return EXIT_FAILURE;
}
TextStream outputTextStream(Utils::STDOUT, QIODevice::WriteOnly);
QString password = passwordGenerator->generatePassword();
outputTextStream << password << endl;
return EXIT_SUCCESS;

View file

@ -20,12 +20,16 @@
#include "Command.h"
#include "core/PasswordGenerator.h"
class Generate : public Command
{
public:
Generate();
int execute(const QStringList& arguments) override;
static QSharedPointer<PasswordGenerator> createGenerator(QSharedPointer<QCommandLineParser> parser);
static const QCommandLineOption PasswordLengthOption;
static const QCommandLineOption LowerCaseOption;
static const QCommandLineOption UpperCaseOption;

View file

@ -1,4 +1,4 @@
.TH KEEPASSXC-CLI 1 "Nov 04, 2018"
.TH KEEPASSXC-CLI 1 "June 15, 2019"
.SH NAME
keepassxc-cli \- command line interface for the \fBKeePassXC\fP password manager.
@ -15,6 +15,7 @@ keepassxc-cli \- command line interface for the \fBKeePassXC\fP password manager
.IP "add [options] <database> <entry>"
Adds a new entry to a database. A password can be generated (\fI-g\fP option), or a prompt can be displayed to input the password (\fI-p\fP option).
The same password generation options as documented for the generate command can be used when the \fI-g\fP option is set.
.IP "analyze [options] <database>"
Analyze passwords in a database for weaknesses.
@ -30,6 +31,7 @@ Generate a random diceware passphrase.
.IP "edit [options] <database> <entry>"
Edits a database entry. A password can be generated (\fI-g\fP option), or a prompt can be displayed to input the password (\fI-p\fP option).
The same password generation options as documented for the generate command can be used when the \fI-g\fP option is set.
.IP "estimate [options] [password]"
Estimates the entropy of a password. The password to estimate can be provided as a positional argument, or using the standard input.
@ -94,6 +96,8 @@ Use the same credentials for unlocking both database.
.SS "Add and edit options"
The same password generation options as documented for the generate command can be used
with those 2 commands when the -g option is set.
.IP "-u, --username <username>"
Specify the username of the entry.
@ -107,9 +111,6 @@ Use a password prompt for the entry's password.
.IP "-g, --generate"
Generate a new password for the entry.
.IP "-l, --password-length"
Specify the length of the password to generate.
.SS "Edit options"
@ -176,21 +177,29 @@ Flattens the output to single lines. When this option is enabled, subgroups and
.IP "-L, --length <length>"
Desired length for the generated password. [Default: 16]
.IP "-l"
.IP "-l --lower"
Use lowercase characters for the generated password. [Default: Enabled]
.IP "-u"
.IP "-U --upper"
Use uppercase characters for the generated password. [Default: Enabled]
.IP "-n"
.IP "-n --numeric"
Use numbers characters for the generated password. [Default: Enabled]
.IP "-s"
.IP "-s --special"
Use special characters for the generated password. [Default: Disabled]
.IP "-e"
.IP "-e --extended"
Use extended ASCII characters for the generated password. [Default: Disabled]
.IP "-x --exclude <chars>"
Comma-separated list of characters to exclude from the generated password. None is excluded by default.
.IP "--exclude-similar"
Exclude similar looking characters. [Default: Disabled]
.IP "--every-group"
Include characters from every selected group. [Default: Disabled]
.SH REPORTING BUGS