Netman testing (#236)

* Let NetMan handle testing results

* combine stopReply and tidyReply into cleanupReply

---------

Co-authored-by: Chris Rizzitello <crizzitello@ics.com>

src/components/tagging/tag_cache/tagcache.cpp

@@ -3,7 +3,7 @@
 #include <QNetworkReply>
 
 #include "helpers/netman.h"
-#include "helpers/stopreply.h"
+#include "helpers/cleanupreply.h"
 
 
 TagCache::TagCache(QObject *parent): QObject(parent) {
@@ -12,7 +12,7 @@ TagCache::TagCache(QObject *parent): QObject(parent) {
 
 TagCache::~TagCache() {
   for (auto entry : qAsConst(tagRequests)) {
-    stopReply(&(entry));
+    cleanUpReply(&(entry));
   }
 }
 
@@ -73,5 +73,5 @@ void TagCache::onGetTagsComplete(QNetworkReply* reply, QString operationSlug) {
     }
   }
 
-  tidyReply(&reply);
+  cleanUpReply(&reply);
 }

src/components/tagging/tageditor.cpp

@@ -12,7 +12,7 @@
 
 #include "components/loading/qprogressindicator.h"
 #include "helpers/netman.h"
-#include "helpers/stopreply.h"
+#include "helpers/cleanupreply.h"
 #include "tag_cache/tagcache.h"
 
 TagEditor::TagEditor(QWidget *parent)
@@ -30,9 +30,9 @@ TagEditor::TagEditor(QWidget *parent)
 
 TagEditor::~TagEditor() {
   for (auto& entry : activeRequests) {
-    stopReply(&(entry));
+    cleanUpReply(&(entry));
   }
-  stopReply(&createTagReply);
+  cleanUpReply(&createTagReply);
 }
 
 void TagEditor::buildUi() {
@@ -123,7 +123,7 @@ void TagEditor::updateCompleterModel() {
 }
 
 void TagEditor::clear() {
-  stopReply(&createTagReply);
+  cleanUpReply(&createTagReply);
   tagCompleteTextBox->clear();
   errorLabel->clear();
   tagView->clear();
@@ -197,7 +197,7 @@ void TagEditor::onCreateTagComplete() {
     QMessageBox::warning(this, tr("Tag Error"),tr("Could not create tag\n Please check your connection and try again."));
   }
   disconnect(createTagReply, &QNetworkReply::finished, this, &TagEditor::onCreateTagComplete);
-  tidyReply(&createTagReply);
+  cleanUpReply(&createTagReply);
   loading->stopAnimation();
   tagCompleteTextBox->setEnabled(true);
   tagCompleteTextBox->setFocus();

src/forms/add_operation/createoperation.cpp

@@ -10,7 +10,7 @@
 #include "dtos/ashirt_error.h"
 #include "components/loading_button/loadingbutton.h"
 #include "helpers/netman.h"
-#include "helpers/stopreply.h"
+#include "helpers/cleanupreply.h"
 
 CreateOperation::CreateOperation(QWidget* parent)
   : AShirtDialog(parent, AShirtDialog::commonWindowFlags)
@@ -44,7 +44,7 @@ CreateOperation::CreateOperation(QWidget* parent)
 }
 
 CreateOperation::~CreateOperation() {
-  stopReply(&createOpReply);
+  cleanUpReply(&createOpReply);
 }
 
 void CreateOperation::submitButtonClicked() {
@@ -97,5 +97,5 @@ void CreateOperation::onRequestComplete() {
   submitButton->stopAnimation();
   submitButton->setEnabled(true);
 
-  tidyReply(&createOpReply);
+  cleanUpReply(&createOpReply);
 }

src/forms/evidence/evidencemanager.cpp

@@ -18,7 +18,7 @@
 #include "forms/evidence_filter/evidencefilter.h"
 #include "forms/evidence_filter/evidencefilterform.h"
 #include "helpers/netman.h"
-#include "helpers/stopreply.h"
+#include "helpers/cleanupreply.h"
 
 enum ColumnIndexes {
   COL_DATE_CAPTURED = 0,
@@ -54,7 +54,7 @@ EvidenceManager::EvidenceManager(DatabaseConnection* db, QWidget* parent)
 }
 
 EvidenceManager::~EvidenceManager() {
-  stopReply(&uploadAssetReply);
+  cleanUpReply(&uploadAssetReply);
 }
 
 void EvidenceManager::buildEvidenceTableUi() {
@@ -510,7 +510,7 @@ void EvidenceManager::onUploadComplete() {
   loadingAnimation->stopAnimation();
   evidenceTable->setEnabled(true);
 
-  tidyReply(&uploadAssetReply);
+  cleanUpReply(&uploadAssetReply);
 }
 
 qint64 EvidenceManager::selectedRowEvidenceID() {

src/forms/getinfo/getinfo.cpp

@@ -10,7 +10,7 @@
 #include "components/loading_button/loadingbutton.h"
 #include "db/databaseconnection.h"
 #include "helpers/netman.h"
-#include "helpers/stopreply.h"
+#include "helpers/cleanupreply.h"
 
 GetInfo::GetInfo(DatabaseConnection* db, qint64 evidenceID, QWidget* parent)
     : AShirtDialog(parent, AShirtDialog::commonWindowFlags)
@@ -25,7 +25,7 @@ GetInfo::GetInfo(DatabaseConnection* db, qint64 evidenceID, QWidget* parent)
 
 GetInfo::~GetInfo() {
   delete evidenceEditor;
-  stopReply(&uploadAssetReply);
+  cleanUpReply(&uploadAssetReply);
 }
 
 void GetInfo::buildUi() {
@@ -150,5 +150,5 @@ void GetInfo::onUploadComplete()
     // one thing we might want to record: evidence uuid... not sure why we'd need it though.
     submitButton->stopAnimation();
     Q_EMIT setActionButtonsEnabled(true);
-    tidyReply(&uploadAssetReply);
+    cleanUpReply(&uploadAssetReply);
 }

src/forms/settings/settings.cpp

@@ -20,7 +20,7 @@
 #include "dtos/checkConnection.h"
 #include "helpers/http_status.h"
 #include "helpers/netman.h"
-#include "helpers/stopreply.h"
+#include "helpers/cleanupreply.h"
 #include "hotkeymanager.h"
 #include "components/custom_keyseq_edit/singlestrokekeysequenceedit.h"
 #include "components/loading_button/loadingbutton.h"
@@ -45,10 +45,6 @@ Settings::Settings(HotkeyManager *hotkeyManager, QWidget *parent)
   wireUi();
 }
 
-Settings::~Settings() {
-  stopReply(&currentTestReply);
-}
-
 void Settings::buildUi() {
   auto eviRepoBrowseButton = new QPushButton(tr("Browse"), this);
   connect(eviRepoBrowseButton, &QPushButton::clicked, this, &Settings::onBrowseClicked);
@@ -135,6 +131,7 @@ void Settings::buildUi() {
 }
 
 void Settings::wireUi() {
+  connect(NetMan::get(), &NetMan::testStatusChanged, this, &Settings::testStatusChanged);
   connect(testConnectionButton, &QPushButton::clicked, this, &Settings::onTestConnectionClicked);
   connect(captureAreaShortcutTextBox, &QKeySequenceEdit::keySequenceChanged, this, [this](const QKeySequence &keySequence){
     checkForDuplicateShortcuts(keySequence, captureAreaShortcutTextBox);
@@ -199,13 +196,11 @@ void Settings::closeEvent(QCloseEvent *event) {
 }
 
 void Settings::onCancelClicked() {
-  stopReply(&currentTestReply);
   hotkeyManager->enableHotkeys();
   reject();
 }
 
 void Settings::onSaveClicked() {
-  stopReply(&currentTestReply);
   connStatusLabel->clear();
 
   AppConfig::setValue(CONFIG::EVIDENCEREPO, QDir::fromNativeSeparators(eviRepoTextBox->text()));
@@ -250,46 +245,18 @@ void Settings::onTestConnectionClicked() {
     connStatusLabel->setText(tr("Please set Access Key, Secret key and Host Path first."));
     return;
   }
-  testConnectionButton->startAnimation();
-  testConnectionButton->setEnabled(false);
-  currentTestReply = NetMan::testConnection(
-      hostPathTextBox->text(), accessKeyTextBox->text(), secretKeyTextBox->text());
-  connect(currentTestReply, &QNetworkReply::finished, this, &Settings::onTestRequestComplete);
+  NetMan::testConnection(hostPathTextBox->text(), accessKeyTextBox->text(), secretKeyTextBox->text());
 }
 
-void Settings::onTestRequestComplete() {
-  bool ok = true;
-  auto statusCode =
-      currentTestReply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(&ok);
-
-  if (ok) {
-    dto::CheckConnection connectionCheckResp;
-
-    switch (statusCode) {
-      case HttpStatus::StatusOK:
-        connectionCheckResp = dto::CheckConnection::parseJson(currentTestReply->readAll());
-        if (connectionCheckResp.parsedCorrectly && connectionCheckResp.ok) {
-          connStatusLabel->setText(tr("Connected"));
-        }
-        else {
-          connStatusLabel->setText(tr("Unable to connect: Wrong or outdated server"));
-        }
-        break;
-      case HttpStatus::StatusUnauthorized:
-        connStatusLabel->setText(tr("Could not connect: Unauthorized (check access key and secret)"));
-        break;
-      case HttpStatus::StatusNotFound:
-        connStatusLabel->setText(tr("Could not connect: Not Found (check URL)"));
-        break;
-      default:
-        connStatusLabel->setText(tr("Could not connect: Unexpected Error (code: %1)").arg(statusCode));
-    }
+void Settings::testStatusChanged(int result)
+{
+  if(result == NetMan::INPROGRESS) {
+    testConnectionButton->startAnimation();
+    testConnectionButton->setEnabled(false);
+    connStatusLabel->setText("Testing");
+    return;
   }
-  else {
-    connStatusLabel->setText(tr("Could not connect: Unexpected Error (check network connection and URL)"));
-  }
-
   testConnectionButton->stopAnimation();
   testConnectionButton->setEnabled(true);
-  tidyReply(&currentTestReply);
+  connStatusLabel->setText(NetMan::lastTestError());
 }

src/forms/settings/settings.h

@@ -30,7 +30,7 @@ class Settings : public AShirtDialog {
    * @param parent
    */
   explicit Settings(HotkeyManager* hotkeyManager, QWidget* parent = nullptr);
-  ~Settings();
+  ~Settings() = default;
 
  private:
   /// buildUi creates the window structure.
@@ -55,8 +55,8 @@ class Settings : public AShirtDialog {
 
   /// onTestConnectionClicked acts upon the "test connection" button press. Checks the network.
   void onTestConnectionClicked();
-  /// onTestRequestComplete handles the network result action.
-  void onTestRequestComplete();
+  /// testStatusChanged handles the netman test results
+  void testStatusChanged(int result);
   /// onBrowseClicked triggers when the "browse" button is pressed. Shows a file dialog to the user.
   void onBrowseClicked();
 
@@ -64,8 +64,6 @@ class Settings : public AShirtDialog {
   /// hotkeyManager is a (shared) reference to the HotkeyManager. Not to be deleted.
   HotkeyManager* hotkeyManager;
 
-  QNetworkReply* currentTestReply = nullptr;
-
   // UI components
   QLabel* connStatusLabel = nullptr;
 

src/helpers/CMakeLists.txt

@@ -16,7 +16,7 @@ add_library (HELPERS STATIC
     netman.h
     request_builder.h
     screenshot.cpp screenshot.h
-    stopreply.cpp stopreply.h
+    cleanupreply.h
     string_helpers.h
     system_helpers.h
     ui_helpers.h

src/helpers/cleanupreply.h

@@ -0,0 +1,15 @@
+// Licensed under the terms of MIT. See LICENSE file in project root for terms.
+
+#pragma once
+
+#include <QNetworkReply>
+
+static void cleanUpReply(QNetworkReply **reply) {
+  if (*reply == nullptr) {
+    return;
+  }
+
+  (*reply)->isFinished() ? (*reply)->close() : (*reply)->abort();
+  (*reply)->deleteLater();
+  *reply = nullptr;
+}

src/helpers/netman.h

@@ -13,15 +13,23 @@
 #include "dtos/operation.h"
 #include "dtos/tag.h"
 #include "dtos/github_release.h"
+#include "dtos/checkConnection.h"
 #include "helpers/multipartparser.h"
-#include "helpers/stopreply.h"
+#include "helpers/cleanupreply.h"
+#include "helpers/http_status.h"
 #include "models/evidence.h"
 
-
 class NetMan : public QObject {
   Q_OBJECT
 public:
-  // type alias QList<dto::Operation> to provide shorter lines
+  //Result for Connection Test
+  enum TestResult {
+   INPROGRESS,
+   SUCCESS,
+   FAILURE
+  };
+
+ // type alias QList<dto::Operation> to provide shorter lines
   using OperationVector = QList<dto::Operation>;
   static NetMan* get() {
     static NetMan i;
@@ -50,12 +58,54 @@ public:
     return builder->execute(get()->nam);
   }
 
+  ///Return the last Test Error
+  static QString lastTestError() {return get()->_lastTestError;}
+
+  ///Processes the test result and emits sets the lastError and a emits a result.
+  static void processTestResults() {
+    bool ok = true;
+    auto statusCode = get()->testConnectionReply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(&ok);
+
+    if(!ok) {
+        get()->_lastTestError = tr("Server not Found, Check the Url");
+        Q_EMIT get()->testStatusChanged(FAILURE);
+        cleanUpReply(&get()->testConnectionReply);
+        return;
+    }
+
+    dto::CheckConnection connectionCheckResp;
+    switch (statusCode) {
+        case HttpStatus::StatusOK:
+          connectionCheckResp = dto::CheckConnection::parseJson(get()->testConnectionReply->readAll());
+          if (connectionCheckResp.parsedCorrectly && connectionCheckResp.ok) {
+            get()->_lastTestError = tr("Successfully Connected");
+            Q_EMIT get()->testStatusChanged(SUCCESS);
+          } else {
+            get()->_lastTestError = tr("Server Error Report to Admin");
+            Q_EMIT get()->testStatusChanged(TestResult::FAILURE);
+          }
+          break;
+        case HttpStatus::StatusUnauthorized:
+          get()->_lastTestError = tr("Authorization Failure, Check Api Keys");
+          Q_EMIT get()->testStatusChanged(TestResult::FAILURE);
+          break;
+        default:
+          get()->_lastTestError = tr("Code %1").arg(statusCode);
+          Q_EMIT get()->testStatusChanged(TestResult::FAILURE);
+    }
+    cleanUpReply(&get()->testConnectionReply);
+  }
+
   /// testConnection provides a mechanism to validate a given host, apikey and secret key, to test
   /// a connection to the ASHIRT API server
-  static QNetworkReply *testConnection(QString host, QString apiKey, QString secretKey) {
+  /// Connect to the testStatusChanged signal to see results;
+  static void testConnection(QString host, QString apiKey, QString secretKey) {
     auto builder = ashirtGet(QStringLiteral("/api/checkconnection"), host);
     addASHIRTAuth(builder, apiKey, secretKey);
-    return builder->execute(get()->nam);
+    get()->testConnectionReply = builder->execute(get()->nam);
+    get()->_lastTestError = tr("Testing in progress");
+    Q_EMIT get()->testStatusChanged(TestResult::INPROGRESS);
+    connect(get()->testConnectionReply, &QNetworkReply::finished, get(), processTestResults);
   }
 
   /// getAllOperations retrieves all (user-visble) operations from the configured ASHIRT API server.
@@ -132,12 +182,20 @@ public:
 signals:
  void operationListUpdated(bool success, NetMan::OperationVector  operations = NetMan::OperationVector());
  void releasesChecked(bool success, QList<dto::GithubRelease> releases = QList<dto::GithubRelease>());
+ void testStatusChanged(int newStatus);
 
 private:
  NetMan(QObject * parent = nullptr) : QObject(parent), nam(new QNetworkAccessManager(this)) { }
  NetMan(NetMan const &) = delete;
  void operator=(NetMan const &) = delete;
- ~NetMan() = default;
+
+ ~NetMan() {
+    cleanUpReply(&get()->allOpsReply);
+    cleanUpReply(&get()->testConnectionReply);
+    cleanUpReply(&get()->githubReleaseReply);
+ };
+
+ QString _lastTestError;
 
  /// ashirtGet generates a basic GET request to the ashirt API server. No authentication is
  /// provided (use addASHIRTAuth to do this)
@@ -215,7 +273,7 @@ private:
    } else {
      Q_EMIT get()->operationListUpdated(false);
    }
-   tidyReply(&get()->allOpsReply);
+   cleanUpReply(&get()->allOpsReply);
  }
 
  /// onGithubReleasesComplete is called when the network request associated with the method checkForNewRelease
@@ -229,9 +287,10 @@ private:
    } else {
      Q_EMIT get()->releasesChecked(false);
    }
-   tidyReply(&get()->githubReleaseReply);
+   cleanUpReply(&get()->githubReleaseReply);
  }
  QNetworkReply *allOpsReply = nullptr;
+ QNetworkReply *testConnectionReply = nullptr;
  QNetworkReply *githubReleaseReply = nullptr;
  QNetworkAccessManager *nam = nullptr;
 };

src/helpers/stopreply.cpp

@@ -1,30 +0,0 @@
-// Copyright 2020, Verizon Media
-// Licensed under the terms of MIT. See LICENSE file in project root for terms.
-
-#include "stopreply.h"
-
-// stopReply aborts a request, then cleans up the reply (via deleteLater)
-// also sets the reply pointer to nullptr. This is for use cases where the
-// reply is to be ignored.
-void stopReply(QNetworkReply **reply) {
-  if (*reply == nullptr) {
-    return;
-  }
-
-  (*reply)->abort();
-  (*reply)->deleteLater();
-  *reply = nullptr;
-}
-
-// tidyReply cleans up a "completed" reply, closing the connection and marking
-// for deletion. Additionally, this sets the reply pointer to nullptr. This is
-// for use cases where a reply has extracted all necessary information and clean-up is necessary.
-void tidyReply(QNetworkReply **reply) {
-  if (*reply == nullptr) {
-    return;
-  }
-
-  (*reply)->close();
-  (*reply)->deleteLater();
-  *reply = nullptr;
-}

src/helpers/stopreply.h

@@ -1,9 +0,0 @@
-// Copyright 2020, Verizon Media
-// Licensed under the terms of MIT. See LICENSE file in project root for terms.
-
-#pragma once
-
-#include <QNetworkReply>
-
-void stopReply(QNetworkReply **reply);
-void tidyReply(QNetworkReply **reply);