From a1ef9bdbe7d38cb2144cd844d5d3a96722031b47 Mon Sep 17 00:00:00 2001 From: crizzitello <98421672+crizzitello@users.noreply.github.com> Date: Mon, 2 Oct 2023 14:04:23 -0400 Subject: [PATCH] Netman testing (#236) * Let NetMan handle testing results * combine stopReply and tidyReply into cleanupReply --------- Co-authored-by: Chris Rizzitello --- src/components/tagging/tag_cache/tagcache.cpp | 6 +- src/components/tagging/tageditor.cpp | 10 +-- src/forms/add_operation/createoperation.cpp | 6 +- src/forms/evidence/evidencemanager.cpp | 6 +- src/forms/getinfo/getinfo.cpp | 6 +- src/forms/settings/settings.cpp | 55 +++----------- src/forms/settings/settings.h | 8 +- src/helpers/CMakeLists.txt | 2 +- src/helpers/cleanupreply.h | 15 ++++ src/helpers/netman.h | 75 +++++++++++++++++-- src/helpers/stopreply.cpp | 30 -------- src/helpers/stopreply.h | 9 --- 12 files changed, 114 insertions(+), 114 deletions(-) create mode 100644 src/helpers/cleanupreply.h delete mode 100644 src/helpers/stopreply.cpp delete mode 100644 src/helpers/stopreply.h diff --git a/src/components/tagging/tag_cache/tagcache.cpp b/src/components/tagging/tag_cache/tagcache.cpp index 20e6443..93ea175 100644 --- a/src/components/tagging/tag_cache/tagcache.cpp +++ b/src/components/tagging/tag_cache/tagcache.cpp @@ -3,7 +3,7 @@ #include #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); } diff --git a/src/components/tagging/tageditor.cpp b/src/components/tagging/tageditor.cpp index 174d778..eef5013 100644 --- a/src/components/tagging/tageditor.cpp +++ b/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(); diff --git a/src/forms/add_operation/createoperation.cpp b/src/forms/add_operation/createoperation.cpp index d29b08a..7f1dacf 100644 --- a/src/forms/add_operation/createoperation.cpp +++ b/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); } diff --git a/src/forms/evidence/evidencemanager.cpp b/src/forms/evidence/evidencemanager.cpp index ecbe7e7..001b18e 100644 --- a/src/forms/evidence/evidencemanager.cpp +++ b/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() { diff --git a/src/forms/getinfo/getinfo.cpp b/src/forms/getinfo/getinfo.cpp index 3dc2a5a..c5470a7 100644 --- a/src/forms/getinfo/getinfo.cpp +++ b/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); } diff --git a/src/forms/settings/settings.cpp b/src/forms/settings/settings.cpp index d651070..17386d1 100644 --- a/src/forms/settings/settings.cpp +++ b/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(¤tTestReply); -} - 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(¤tTestReply); hotkeyManager->enableHotkeys(); reject(); } void Settings::onSaveClicked() { - stopReply(¤tTestReply); 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(¤tTestReply); + connStatusLabel->setText(NetMan::lastTestError()); } diff --git a/src/forms/settings/settings.h b/src/forms/settings/settings.h index 71541ae..ac9cbf9 100644 --- a/src/forms/settings/settings.h +++ b/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; diff --git a/src/helpers/CMakeLists.txt b/src/helpers/CMakeLists.txt index c1d23c2..bedd948 100644 --- a/src/helpers/CMakeLists.txt +++ b/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 diff --git a/src/helpers/cleanupreply.h b/src/helpers/cleanupreply.h new file mode 100644 index 0000000..14d0623 --- /dev/null +++ b/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 + +static void cleanUpReply(QNetworkReply **reply) { + if (*reply == nullptr) { + return; + } + + (*reply)->isFinished() ? (*reply)->close() : (*reply)->abort(); + (*reply)->deleteLater(); + *reply = nullptr; +} diff --git a/src/helpers/netman.h b/src/helpers/netman.h index 4726153..56d70c6 100644 --- a/src/helpers/netman.h +++ b/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 to provide shorter lines + //Result for Connection Test + enum TestResult { + INPROGRESS, + SUCCESS, + FAILURE + }; + + // type alias QList to provide shorter lines using OperationVector = QList; 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 releases = QList()); + 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; }; diff --git a/src/helpers/stopreply.cpp b/src/helpers/stopreply.cpp deleted file mode 100644 index 8b3a2a1..0000000 --- a/src/helpers/stopreply.cpp +++ /dev/null @@ -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; -} diff --git a/src/helpers/stopreply.h b/src/helpers/stopreply.h deleted file mode 100644 index 9aa0151..0000000 --- a/src/helpers/stopreply.h +++ /dev/null @@ -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 - -void stopReply(QNetworkReply **reply); -void tidyReply(QNetworkReply **reply);