✨✨ CLI Command cleanup ✨✨

This PR cleans up the `Command` classes in the CLI, introducing a `DatabaseCommand` class for the commands operating on a database, and a `getCommandLineParser` command to centralize the arguments parsing and validation. The opening of the database based on the CLI arguments and options is now centralized in `DatabaseCommand.execute`, making it easy to add new database opening features (like YubiKey support for the CLI). Also a couple of bugs fixed: * `Create` was still using `stdout` for some error messages. * `Diceware` and `Generate` were not validating that the word count was an integer. * `Diceware` was also using `stdout` for some error messages.
keepassxreboot · Jun 14, 2019 · 04360ed · 04360ed
1 parent 3cf171c
commit 04360ed
Show file tree

Hide file tree

Showing 31 changed files with 591 additions and 637 deletions.
diff --git a/src/cli/Add.cpp b/src/cli/Add.cpp
@@ -1,5 +1,5 @@
 /*
- *  Copyright (C) 2017 KeePassXC Team <team@keepassxc.org>
+ *  Copyright (C) 2019 KeePassXC Team <team@keepassxc.org>
  *
  *  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
@@ -20,115 +20,91 @@
 
 #include "Add.h"
 
-#include <QCommandLineParser>
-
 #include "cli/TextStream.h"
 #include "cli/Utils.h"
 #include "core/Database.h"
 #include "core/Entry.h"
 #include "core/Group.h"
 #include "core/PasswordGenerator.h"
 
+const QCommandLineOption Add::UsernameOption = QCommandLineOption(QStringList() << "u"
+                                                                                << "username",
+                                                                  QObject::tr("Username for the entry."),
+                                                                  QObject::tr("username"));
+
+const QCommandLineOption Add::UrlOption =
+    QCommandLineOption(QStringList() << "url", QObject::tr("URL for the entry."), QObject::tr("URL"));
+
+const QCommandLineOption Add::PasswordPromptOption =
+    QCommandLineOption(QStringList() << "p"
+                                     << "password-prompt",
+                       QObject::tr("Prompt for the entry's password."));
+
+const QCommandLineOption Add::GenerateOption = QCommandLineOption(QStringList() << "g"
+                                                                                << "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()
 {
     name = QString("add");
     description = QObject::tr("Add a new entry to a database.");
+    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("")});
 }
 
 Add::~Add()
 {
 }
 
-int Add::execute(const QStringList& arguments)
+int Add::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer<QCommandLineParser> parser)
 {
     TextStream inputTextStream(Utils::STDIN, QIODevice::ReadOnly);
     TextStream outputTextStream(Utils::STDOUT, QIODevice::WriteOnly);
     TextStream errorTextStream(Utils::STDERR, QIODevice::WriteOnly);
 
-    QCommandLineParser parser;
-    parser.setApplicationDescription(description);
-    parser.addPositionalArgument("database", QObject::tr("Path of the database."));
-    parser.addOption(Command::QuietOption);
-    parser.addOption(Command::KeyFileOption);
-    parser.addOption(Command::NoPasswordOption);
-
-    QCommandLineOption username(QStringList() << "u"
-                                              << "username",
-                                QObject::tr("Username for the entry."),
-                                QObject::tr("username"));
-    parser.addOption(username);
-
-    QCommandLineOption url(QStringList() << "url", QObject::tr("URL for the entry."), QObject::tr("URL"));
-    parser.addOption(url);
-
-    QCommandLineOption prompt(QStringList() << "p"
-                                            << "password-prompt",
-                              QObject::tr("Prompt for the entry's password."));
-    parser.addOption(prompt);
-
-    QCommandLineOption generate(QStringList() << "g"
-                                              << "generate",
-                                QObject::tr("Generate a password for the entry."));
-    parser.addOption(generate);
-
-    QCommandLineOption length(QStringList() << "l"
-                                            << "password-length",
-                              QObject::tr("Length for the generated password."),
-                              QObject::tr("length"));
-    parser.addOption(length);
-
-    parser.addPositionalArgument("entry", QObject::tr("Path of the entry to add."));
-
-    parser.addHelpOption();
-    parser.process(arguments);
-
-    const QStringList args = parser.positionalArguments();
-    if (args.size() != 2) {
-        errorTextStream << parser.helpText().replace("[options]", "add [options]");
-        return EXIT_FAILURE;
-    }
-
+    const QStringList args = parser->positionalArguments();
     const QString& databasePath = args.at(0);
     const QString& entryPath = args.at(1);
 
-    auto db = Utils::unlockDatabase(databasePath,
-                                    !parser.isSet(Command::NoPasswordOption),
-                                    parser.value(Command::KeyFileOption),
-                                    parser.isSet(Command::QuietOption) ? Utils::DEVNULL : Utils::STDOUT,
-                                    Utils::STDERR);
-    if (!db) {
-        return EXIT_FAILURE;
-    }
-
     // Validating the password length here, before we actually create
     // the entry.
-    QString passwordLength = parser.value(length);
+    QString passwordLength = parser->value(Add::PasswordLengthOption);
     if (!passwordLength.isEmpty() && !passwordLength.toInt()) {
         errorTextStream << QObject::tr("Invalid value for password length %1.").arg(passwordLength) << endl;
         return EXIT_FAILURE;
     }
 
-    Entry* entry = db->rootGroup()->addEntryWithPath(entryPath);
+    Entry* entry = database->rootGroup()->addEntryWithPath(entryPath);
     if (!entry) {
         errorTextStream << QObject::tr("Could not create entry with path %1.").arg(entryPath) << endl;
         return EXIT_FAILURE;
     }
 
-    if (!parser.value("username").isEmpty()) {
-        entry->setUsername(parser.value("username"));
+    if (!parser->value(Add::UsernameOption).isEmpty()) {
+        entry->setUsername(parser->value(Add::UsernameOption));
     }
 
-    if (!parser.value("url").isEmpty()) {
-        entry->setUrl(parser.value("url"));
+    if (!parser->value(Add::UrlOption).isEmpty()) {
+        entry->setUrl(parser->value(Add::UrlOption));
     }
 
-    if (parser.isSet(prompt)) {
-        if (!parser.isSet(Command::QuietOption)) {
+    if (parser->isSet(Add::PasswordPromptOption)) {
+        if (!parser->isSet(Command::QuietOption)) {
             outputTextStream << QObject::tr("Enter password for new entry: ") << flush;
         }
-        QString password = Utils::getPassword(parser.isSet(Command::QuietOption) ? Utils::DEVNULL : Utils::STDOUT);
+        QString password = Utils::getPassword(parser->isSet(Command::QuietOption) ? Utils::DEVNULL : Utils::STDOUT);
         entry->setPassword(password);
-    } else if (parser.isSet(generate)) {
+    } else if (parser->isSet(Add::GenerateOption)) {
         PasswordGenerator passwordGenerator;
 
         if (passwordLength.isEmpty()) {
@@ -144,12 +120,12 @@ int Add::execute(const QStringList& arguments)
     }
 
     QString errorMessage;
-    if (!db->save(databasePath, &errorMessage, true, false)) {
+    if (!database->save(databasePath, &errorMessage, true, false)) {
         errorTextStream << QObject::tr("Writing the database failed %1.").arg(errorMessage) << endl;
         return EXIT_FAILURE;
     }
 
-    if (!parser.isSet(Command::QuietOption)) {
+    if (!parser->isSet(Command::QuietOption)) {
         outputTextStream << QObject::tr("Successfully added entry %1.").arg(entry->title()) << endl;
     }
     return EXIT_SUCCESS;

diff --git a/src/cli/Add.h b/src/cli/Add.h
@@ -1,5 +1,5 @@
 /*
- *  Copyright (C) 2017 KeePassXC Team <team@keepassxc.org>
+ *  Copyright (C) 2019 KeePassXC Team <team@keepassxc.org>
  *
  *  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
@@ -18,14 +18,21 @@
 #ifndef KEEPASSXC_ADD_H
 #define KEEPASSXC_ADD_H
 
-#include "Command.h"
+#include "DatabaseCommand.h"
 
-class Add : public Command
+class Add : public DatabaseCommand
 {
 public:
     Add();
     ~Add();
-    int execute(const QStringList& arguments) override;
+
+    int executeWithDatabase(QSharedPointer<Database> db, QSharedPointer<QCommandLineParser> parser);
+
+    static const QCommandLineOption UsernameOption;
+    static const QCommandLineOption UrlOption;
+    static const QCommandLineOption PasswordPromptOption;
+    static const QCommandLineOption GenerateOption;
+    static const QCommandLineOption PasswordLengthOption;
 };
 
 #endif // KEEPASSXC_ADD_H
diff --git a/src/cli/CMakeLists.txt b/src/cli/CMakeLists.txt
@@ -1,4 +1,4 @@
-#  Copyright (C) 2017 KeePassXC Team
+#  Copyright (C) 2019 KeePassXC Team
 #
 #  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
@@ -18,6 +18,7 @@ set(cli_SOURCES
         Clip.cpp
         Create.cpp
         Command.cpp
+        DatabaseCommand.cpp
         Diceware.cpp
         Edit.cpp
         Estimate.cpp

diff --git a/src/cli/Clip.cpp b/src/cli/Clip.cpp
@@ -1,5 +1,5 @@
 /*
- *  Copyright (C) 2017 KeePassXC Team <team@keepassxc.org>
+ *  Copyright (C) 2019 KeePassXC Team <team@keepassxc.org>
  *
  *  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
@@ -22,80 +22,52 @@
 
 #include "Clip.h"
 
-#include <QCommandLineParser>
-
 #include "cli/TextStream.h"
 #include "cli/Utils.h"
 #include "core/Database.h"
 #include "core/Entry.h"
 #include "core/Group.h"
 
+const QCommandLineOption Clip::TotpOption = QCommandLineOption(QStringList() << "t"
+                                                                             << "totp",
+                                                               QObject::tr("Copy the current TOTP to the clipboard."));
+
 Clip::Clip()
 {
     name = QString("clip");
     description = QObject::tr("Copy an entry's password to the clipboard.");
+    options.append(Clip::TotpOption);
+    positionalArguments.append(
+        {QString("entry"), QObject::tr("Path of the entry to clip.", "clip = copy to clipboard"), QString("")});
+    optionalArguments.append(
+        {QString("timeout"), QObject::tr("Timeout in seconds before clearing the clipboard."), QString("[timeout]")});
 }
 
 Clip::~Clip()
 {
 }
 
-int Clip::execute(const QStringList& arguments)
+int Clip::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer<QCommandLineParser> parser)
 {
-    TextStream errorTextStream(Utils::STDERR, QIODevice::WriteOnly);
-
-    QCommandLineParser parser;
-    parser.setApplicationDescription(description);
-    parser.addPositionalArgument("database", QObject::tr("Path of the database."));
-    parser.addOption(Command::QuietOption);
-    parser.addOption(Command::KeyFileOption);
-    parser.addOption(Command::NoPasswordOption);
-
-    QCommandLineOption totp(QStringList() << "t"
-                                          << "totp",
-                            QObject::tr("Copy the current TOTP to the clipboard."));
-    parser.addOption(totp);
-    parser.addPositionalArgument("entry", QObject::tr("Path of the entry to clip.", "clip = copy to clipboard"));
-    parser.addPositionalArgument(
-        "timeout", QObject::tr("Timeout in seconds before clearing the clipboard."), "[timeout]");
-    parser.addHelpOption();
-    parser.process(arguments);
-
-    const QStringList args = parser.positionalArguments();
-    if (args.size() != 2 && args.size() != 3) {
-        errorTextStream << parser.helpText().replace("[options]", "clip [options]");
-        return EXIT_FAILURE;
-    }
-
-    auto db = Utils::unlockDatabase(args.at(0),
-                                    !parser.isSet(Command::NoPasswordOption),
-                                    parser.value(Command::KeyFileOption),
-                                    parser.isSet(Command::QuietOption) ? Utils::DEVNULL : Utils::STDOUT,
-                                    Utils::STDERR);
-    if (!db) {
-        return EXIT_FAILURE;
+    const QStringList args = parser->positionalArguments();
+    QString entryPath = args.at(1);
+    QString timeout;
+    if (args.size() == 3) {
+        timeout = args.at(2);
     }
-
-    return clipEntry(db, args.at(1), args.value(2), parser.isSet(totp), parser.isSet(Command::QuietOption));
-}
-
-int Clip::clipEntry(const QSharedPointer<Database>& database,
-                    const QString& entryPath,
-                    const QString& timeout,
-                    bool clipTotp,
-                    bool silent)
-{
+    bool clipTotp = parser->isSet(Clip::TotpOption);
     TextStream errorTextStream(Utils::STDERR);
 
     int timeoutSeconds = 0;
-    if (!timeout.isEmpty() && !timeout.toInt()) {
+    if (!timeout.isEmpty() && timeout.toInt() <= 0) {
         errorTextStream << QObject::tr("Invalid timeout value %1.").arg(timeout) << endl;
         return EXIT_FAILURE;
     } else if (!timeout.isEmpty()) {
         timeoutSeconds = timeout.toInt();
     }
 
-    TextStream outputTextStream(silent ? Utils::DEVNULL : Utils::STDOUT, QIODevice::WriteOnly);
+    TextStream outputTextStream(parser->isSet(Command::QuietOption) ? Utils::DEVNULL : Utils::STDOUT,
+                                QIODevice::WriteOnly);
     Entry* entry = database->rootGroup()->findEntryByPath(entryPath);
     if (!entry) {
         errorTextStream << QObject::tr("Entry %1 not found.").arg(entryPath) << endl;

diff --git a/src/cli/Clip.h b/src/cli/Clip.h
@@ -1,5 +1,5 @@
 /*
- *  Copyright (C) 2017 KeePassXC Team <team@keepassxc.org>
+ *  Copyright (C) 2019 KeePassXC Team <team@keepassxc.org>
  *
  *  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
@@ -18,19 +18,17 @@
 #ifndef KEEPASSXC_CLIP_H
 #define KEEPASSXC_CLIP_H
 
-#include "Command.h"
+#include "DatabaseCommand.h"
 
-class Clip : public Command
+class Clip : public DatabaseCommand
 {
 public:
     Clip();
     ~Clip();
-    int execute(const QStringList& arguments) override;
-    int clipEntry(const QSharedPointer<Database>& database,
-                  const QString& entryPath,
-                  const QString& timeout,
-                  bool clipTotp,
-                  bool silent);
+
+    int executeWithDatabase(QSharedPointer<Database> db, QSharedPointer<QCommandLineParser> parser);
+
+    static const QCommandLineOption TotpOption;
 };
 
 #endif // KEEPASSXC_CLIP_H