code review #1

keepassxreboot · Aug 23, 2019 · d8a0b85 · d8a0b85
1 parent 682e384
commit d8a0b85
Show file tree

Hide file tree

Showing 6 changed files with 6 additions and 18 deletions.
diff --git a/src/cli/Utils.cpp b/src/cli/Utils.cpp
@@ -141,17 +141,13 @@ namespace Utils
         if (!yubiKeySlot.isEmpty()) {
             bool ok = false;
             int slot = yubiKeySlot.toInt(&ok, 10);
-            if (!ok) {
-                err << QString("Invalid YubiKey slot %1").arg(yubiKeySlot) << endl;
-                return {};
-            }
-            if (slot != 1 && slot != 2) {
+            if (!ok || (slot != 1 && slot != 2)) {
                 err << QString("Invalid YubiKey slot %1").arg(yubiKeySlot) << endl;
                 return {};
             }
 
             QString errorMessage;
-            bool blocking = YubiKey::instance()->isBlocking(slot, errorMessage);
+            bool blocking = YubiKey::instance()->checkSlotIsBlocking(slot, errorMessage);
             if (!errorMessage.isEmpty()) {
                 err << errorMessage << endl;
                 return {};
@@ -161,7 +157,6 @@ namespace Utils
                 slot,
                 blocking,
                 QString("Please touch the button on your YubiKey to unlock " + databaseFilename),
-                QString("Challenge-response with YubiKey " + QString::number(slot) + " succeeded!!!"),
                 QString("Failed to unlock " + databaseFilename + " with YubiKey " + QString::number(slot)),
                 outputDescriptor,
                 errorDescriptor));

diff --git a/src/keys/YkChallengeResponseKeyCLI.cpp b/src/keys/YkChallengeResponseKeyCLI.cpp
@@ -31,15 +31,13 @@ QUuid YkChallengeResponseKeyCLI::UUID("e2be77c0-c810-417a-8437-32f41d00bd1d");
 YkChallengeResponseKeyCLI::YkChallengeResponseKeyCLI(int slot,
                                                      bool blocking,
                                                      QString messageInteraction,
-                                                     QString messageSuccess,
                                                      QString messageFailure,
                                                      FILE* outputDescriptor,
                                                      FILE* errorDescriptor)
     : ChallengeResponseKey(UUID)
     , m_slot(slot)
     , m_blocking(blocking)
     , m_messageInteraction(messageInteraction)
-    , m_messageSuccess(messageSuccess)
     , m_messageFailure(messageFailure)
     , m_out(outputDescriptor)
     , m_err(errorDescriptor)
@@ -71,9 +69,6 @@ bool YkChallengeResponseKeyCLI::challenge(const QByteArray& challenge, unsigned
         }
         YubiKey::ChallengeResult result = YubiKey::instance()->challenge(m_slot, m_blocking, challenge, m_key);
         if (result == YubiKey::SUCCESS) {
-            if (m_blocking) {
-                out << m_messageSuccess << endl;
-            }
             return true;
         }
     } while (retries > 0);

diff --git a/src/keys/YkChallengeResponseKeyCLI.h b/src/keys/YkChallengeResponseKeyCLI.h
@@ -35,7 +35,6 @@ class YkChallengeResponseKeyCLI : public QObject, public ChallengeResponseKey
     explicit YkChallengeResponseKeyCLI(int slot,
                                        bool blocking,
                                        QString messageInteraction,
-                                       QString messageSuccess,
                                        QString messageFailure,
                                        FILE* outputDescriptor,
                                        FILE* errorDescriptor);
@@ -49,7 +48,6 @@ class YkChallengeResponseKeyCLI : public QObject, public ChallengeResponseKey
     int m_slot;
     bool m_blocking;
     QString m_messageInteraction;
-    QString m_messageSuccess;
     QString m_messageFailure;
     FILE* m_out;
     FILE* m_err;

diff --git a/src/keys/drivers/YubiKey.cpp b/src/keys/drivers/YubiKey.cpp
@@ -135,7 +135,7 @@ void YubiKey::detect()
     // Check slot 1 and 2 for Challenge-Response HMAC capability
     for (int i = 1; i <= 2; ++i) {
         QString errorMsg;
-        bool isBlocking = this->isBlocking(i, errorMsg);
+        bool isBlocking = checkSlotIsBlocking(i, errorMsg);
         if (errorMsg.isEmpty()) {
             found = true;
             emit detected(i, isBlocking);
@@ -152,7 +152,7 @@ void YubiKey::detect()
     }
 }
 
-bool YubiKey::isBlocking(int slot, QString& errorMessage)
+bool YubiKey::checkSlotIsBlocking(int slot, QString& errorMessage)
 {
     if (!init()) {
         errorMessage = QString("Could not initialize YubiKey.");

diff --git a/src/keys/drivers/YubiKey.h b/src/keys/drivers/YubiKey.h
@@ -96,7 +96,7 @@ class YubiKey : public QObject
      *
      * @return whether the key is blocking or not.
      */
-    bool isBlocking(int slot, QString& errorMessage);
+    bool checkSlotIsBlocking(int slot, QString& errorMessage);
 signals:
     /** Emitted in response to detect() when a device is found
      *

diff --git a/tests/TestCli.cpp b/tests/TestCli.cpp
@@ -1348,7 +1348,7 @@ void TestCli::testYubiKeyOption()
     }
 
     QString errorMessage;
-    bool isBlocking = YubiKey::instance()->isBlocking(2, errorMessage);
+    bool isBlocking = YubiKey::instance()->checkSlotIsBlocking(2, errorMessage);
     if (isBlocking && errorMessage.isEmpty()) {
         QSKIP("Skipping YubiKey in press mode.");
     }