From ffa61095b8be7c61c3b4cdd693e59239ab668516 Mon Sep 17 00:00:00 2001 From: CH Chethan Reddy <40890937+Chethan2k1@users.noreply.github.com> Date: Fri, 26 Jun 2020 15:10:37 +0530 Subject: [PATCH] Error Handling and some fixes --- resources/qml/UserProfile.qml | 16 ++- .../DeviceVerification.qml | 52 ++------- src/DeviceVerificationFlow.cpp | 109 +++++++++++------- src/DeviceVerificationFlow.h | 12 +- src/timeline/TimelineViewManager.cpp | 45 +++++++- 5 files changed, 138 insertions(+), 96 deletions(-) diff --git a/resources/qml/UserProfile.qml b/resources/qml/UserProfile.qml index 36c8586..6ef7503 100644 --- a/resources/qml/UserProfile.qml +++ b/resources/qml/UserProfile.qml @@ -108,30 +108,28 @@ ApplicationWindow{ // userProfileList.ignoreUser() // } // } - ImageButton{ - image:":/icons/icons/ui/round-remove-button.png" + image:":/icons/icons/ui/black-bubble-speech.png" Layout.margins: { left: 5 right: 5 } ToolTip.visible: hovered - ToolTip.text: qsTr("Kick the user") + ToolTip.text: qsTr("Start a private chat") onClicked : { - userProfileList.kickUser() + userProfileList.startChat() } } - ImageButton{ - image:":/icons/icons/ui/black-bubble-speech.png" + image:":/icons/icons/ui/round-remove-button.png" Layout.margins: { left: 5 right: 5 } ToolTip.visible: hovered - ToolTip.text: qsTr("Start a conversation") + ToolTip.text: qsTr("Kick the user") onClicked : { - userProfileList.startChat() + userProfileList.kickUser() } } } @@ -205,7 +203,7 @@ ApplicationWindow{ text:"OK" onClicked: userProfileDialog.close() - Layout.alignment: Qt.AlignRight + Layout.alignment: Qt.AlignRight | Qt.AlignBottom Layout.margins : { right : 10 diff --git a/resources/qml/device-verification/DeviceVerification.qml b/resources/qml/device-verification/DeviceVerification.qml index ca21f48..15c2d7a 100644 --- a/resources/qml/device-verification/DeviceVerification.qml +++ b/resources/qml/device-verification/DeviceVerification.qml @@ -22,12 +22,6 @@ ApplicationWindow { implicitHeight: currentItem.implicitHeight } - onClosing: { - flow.cancelVerification(); - deviceVerificationList.remove(flow.tranId); - delete flow; - } - property var flow Connections { target: flow @@ -123,33 +117,6 @@ ApplicationWindow { color:colors.text verticalAlignment: Text.AlignVCenter } - - RowLayout { - RadioButton { - id: decimalRadio - Layout.alignment: Qt.AlignLeft - text: qsTr("Decimal") - contentItem: Text { - text: decimalRadio.text - color: colors.text - } - onClicked: { flow.method = DeviceVerificationFlow.Decimal } - } - Item { - Layout.fillWidth: true - } - RadioButton { - id: emojiRadio - Layout.alignment: Qt.AlignRight - text: qsTr("Emoji") - contentItem: Text { - text: emojiRadio.text - color: colors.text - } - onClicked: { flow.method = DeviceVerificationFlow.Emoji } - } - } - RowLayout { Button { Layout.alignment: Qt.AlignLeft @@ -165,9 +132,8 @@ ApplicationWindow { } onClicked: { dialog.close(); - flow.cancelVerification(); + flow.cancelVerification(DeviceVerificationFlow.User); deviceVerificationList.remove(flow.tranId); - delete flow; } } Item { @@ -227,9 +193,8 @@ ApplicationWindow { } onClicked: { dialog.close(); - flow.cancelVerification(); + flow.cancelVerification(DeviceVerificationFlow.User); deviceVerificationList.remove(flow.tranId); - delete flow; } } Item { @@ -261,14 +226,17 @@ ApplicationWindow { Label { font.pixelSize: Qt.application.font.pixelSize * 2 text: flow.sasList[0] + color:colors.text } Label { font.pixelSize: Qt.application.font.pixelSize * 2 text: flow.sasList[1] + color:colors.text } Label { font.pixelSize: Qt.application.font.pixelSize * 2 text: flow.sasList[2] + color:colors.text } } @@ -287,9 +255,8 @@ ApplicationWindow { } onClicked: { dialog.close(); - flow.cancelVerification(); + flow.cancelVerification(DeviceVerificationFlow.MismatchedSAS); deviceVerificationList.remove(flow.tranId); - delete flow; } } Item { @@ -411,6 +378,7 @@ ApplicationWindow { implicitWidth: col.width ColumnLayout { id: col + Layout.fillWidth: true anchors.bottom: parent.bottom property var emoji: emojis.mapping[flow.sasList[index]] Label { @@ -424,6 +392,7 @@ ApplicationWindow { Label { Layout.alignment: Qt.AlignHCenter | Qt.AlignBottom text: col.emoji.description + color:colors.text } } } @@ -445,9 +414,8 @@ ApplicationWindow { } onClicked: { dialog.close(); - flow.cancelVerification(); + flow.cancelVerification(DeviceVerificationFlow.MismatchedSAS); deviceVerificationList.remove(flow.tranId); - delete flow; } } Item { @@ -507,7 +475,7 @@ ApplicationWindow { } onClicked: { dialog.close(); - flow.cancelVerification(); + flow.cancelVerification(DeviceVerificationFlow.User); deviceVerificationList.remove(flow.tranId); delete flow; } diff --git a/src/DeviceVerificationFlow.cpp b/src/DeviceVerificationFlow.cpp index 2c6e9c1..b5134a3 100644 --- a/src/DeviceVerificationFlow.cpp +++ b/src/DeviceVerificationFlow.cpp @@ -17,42 +17,53 @@ DeviceVerificationFlow::DeviceVerificationFlow(QObject *) this->isMacVerified = false; connect(timeout, &QTimer::timeout, this, [this]() { emit timedout(); + this->cancelVerification(DeviceVerificationFlow::Error::Timeout); this->deleteLater(); }); - connect(ChatPage::instance(), - &ChatPage::recievedDeviceVerificationStart, - this, - [this](const mtx::events::collections::DeviceEvents &message) { - auto msg = - std::get>(message); - if (msg.content.transaction_id == this->transaction_id) { - if ((std::find(msg.content.key_agreement_protocols.begin(), - msg.content.key_agreement_protocols.end(), - "curve25519-hkdf-sha256") != - msg.content.key_agreement_protocols.end()) && - (std::find(msg.content.hashes.begin(), - msg.content.hashes.end(), - "sha256") != msg.content.hashes.end()) && - (std::find(msg.content.message_authentication_codes.begin(), - msg.content.message_authentication_codes.end(), - "hmac-sha256") != - msg.content.message_authentication_codes.end()) && - ((std::find(msg.content.short_authentication_string.begin(), + connect( + ChatPage::instance(), + &ChatPage::recievedDeviceVerificationStart, + this, + [this](const mtx::events::collections::DeviceEvents &message) { + auto msg = + std::get>(message); + if (msg.content.transaction_id == this->transaction_id) { + if ((std::find(msg.content.key_agreement_protocols.begin(), + msg.content.key_agreement_protocols.end(), + "curve25519-hkdf-sha256") != + msg.content.key_agreement_protocols.end()) && + (std::find(msg.content.hashes.begin(), + msg.content.hashes.end(), + "sha256") != msg.content.hashes.end()) && + (std::find(msg.content.message_authentication_codes.begin(), + msg.content.message_authentication_codes.end(), + "hmac-sha256") != + msg.content.message_authentication_codes.end())) { + if (std::find(msg.content.short_authentication_string.begin(), msg.content.short_authentication_string.end(), mtx::events::msg::SASMethods::Decimal) != - msg.content.short_authentication_string.end()) || - (std::find(msg.content.short_authentication_string.begin(), - msg.content.short_authentication_string.end(), - mtx::events::msg::SASMethods::Emoji) != - msg.content.short_authentication_string.end()))) { - this->acceptVerificationRequest(); - this->canonical_json = nlohmann::json(msg.content); - } else { - this->cancelVerification(); - } - } - }); + msg.content.short_authentication_string.end()) { + this->method = DeviceVerificationFlow::Method::Emoji; + } else if (std::find( + msg.content.short_authentication_string.begin(), + msg.content.short_authentication_string.end(), + mtx::events::msg::SASMethods::Emoji) != + msg.content.short_authentication_string.end()) { + this->method = DeviceVerificationFlow::Method::Decimal; + } else { + this->cancelVerification( + DeviceVerificationFlow::Error::UnknownMethod); + return; + } + this->acceptVerificationRequest(); + this->canonical_json = nlohmann::json(msg.content); + } else { + this->cancelVerification( + DeviceVerificationFlow::Error::UnknownMethod); + } + } + }); connect( ChatPage::instance(), &ChatPage::recievedDeviceVerificationAccept, @@ -76,7 +87,8 @@ DeviceVerificationFlow::DeviceVerificationFlow(QObject *) this->mac_method = msg.content.message_authentication_code; this->sendVerificationKey(); } else { - this->cancelVerification(); + this->cancelVerification( + DeviceVerificationFlow::Error::UnknownMethod); } } }); @@ -130,7 +142,8 @@ DeviceVerificationFlow::DeviceVerificationFlow(QObject *) msg.content.key + this->canonical_json.dump()))) { emit this->verificationRequestAccepted(this->method); } else { - this->cancelVerification(); + this->cancelVerification( + DeviceVerificationFlow::Error::MismatchedCommitment); } } } @@ -156,7 +169,8 @@ DeviceVerificationFlow::DeviceVerificationFlow(QObject *) this->sas->calculate_mac(this->device_keys[mac.first], info + mac.first)) { } else { - this->cancelVerification(); + this->cancelVerification( + DeviceVerificationFlow::Error::KeyMismatch); return; } } @@ -172,7 +186,8 @@ DeviceVerificationFlow::DeviceVerificationFlow(QObject *) else this->isMacVerified = true; } else { - this->cancelVerification(); + this->cancelVerification( + DeviceVerificationFlow::Error::KeyMismatch); } } }); @@ -429,15 +444,31 @@ DeviceVerificationFlow::sendVerificationRequest() } //! cancels a verification flow void -DeviceVerificationFlow::cancelVerification() +DeviceVerificationFlow::cancelVerification(DeviceVerificationFlow::Error error_code) { mtx::requests::ToDeviceMessages body; mtx::events::msg::KeyVerificationCancel req; req.transaction_id = this->transaction_id; - // TODO: Add Proper Error Messages and Code - req.reason = "Device Verification Cancelled"; - req.code = "400"; + if (error_code == DeviceVerificationFlow::Error::UnknownMethod) { + req.code = "m.unknown_method"; + req.reason = "unknown method recieved"; + } else if (error_code == DeviceVerificationFlow::Error::MismatchedCommitment) { + req.code = "m.mismatched_commitment"; + req.reason = "commitment didn't match"; + } else if (error_code == DeviceVerificationFlow::Error::MismatchedSAS) { + req.code = "m.mismatched_sas"; + req.reason = "sas didn't match"; + } else if (error_code == DeviceVerificationFlow::Error::KeyMismatch) { + req.code = "m.key_match"; + req.reason = "keys did not match"; + } else if (error_code == DeviceVerificationFlow::Error::Timeout) { + req.code = "m.timeout"; + req.reason = "timed out"; + } else if (error_code == DeviceVerificationFlow::Error::User) { + req.code = "m.user"; + req.reason = "user cancelled the verification"; + } body[this->toClient][deviceId.toStdString()] = req; diff --git a/src/DeviceVerificationFlow.h b/src/DeviceVerificationFlow.h index ea86a10..891c6ae 100644 --- a/src/DeviceVerificationFlow.h +++ b/src/DeviceVerificationFlow.h @@ -28,6 +28,16 @@ public: Emoji }; Q_ENUM(Method) + enum Error + { + UnknownMethod, + MismatchedCommitment, + MismatchedSAS, + KeyMismatch, + Timeout, + User + }; + Q_ENUM(Error) DeviceVerificationFlow(QObject *parent = nullptr); QString getTransactionId(); @@ -56,7 +66,7 @@ public slots: //! starts the verification flow void startVerificationRequest(); //! cancels a verification flow - void cancelVerification(); + void cancelVerification(DeviceVerificationFlow::Error error_code); //! sends the verification key void sendVerificationKey(); //! sends the mac of the keys diff --git a/src/timeline/TimelineViewManager.cpp b/src/timeline/TimelineViewManager.cpp index 14c6695..0b73223 100644 --- a/src/timeline/TimelineViewManager.cpp +++ b/src/timeline/TimelineViewManager.cpp @@ -184,6 +184,9 @@ TimelineViewManager::TimelineViewManager(QSharedPointer userSettin QString::fromStdString(msg.content.transaction_id), QString::fromStdString(msg.sender), QString::fromStdString(msg.content.from_device)); + } else { + flow->cancelVerification( + DeviceVerificationFlow::Error::UnknownMethod); } } }); @@ -197,11 +200,43 @@ TimelineViewManager::TimelineViewManager(QSharedPointer userSettin auto flow = new DeviceVerificationFlow(this); flow->canonical_json = nlohmann::json(msg.content); if (!(this->dvList->exist(QString::fromStdString(msg.content.transaction_id)))) { - emit newDeviceVerificationRequest( - std::move(flow), - QString::fromStdString(msg.content.transaction_id), - QString::fromStdString(msg.sender), - QString::fromStdString(msg.content.from_device)); + if ((std::find(msg.content.key_agreement_protocols.begin(), + msg.content.key_agreement_protocols.end(), + "curve25519-hkdf-sha256") != + msg.content.key_agreement_protocols.end()) && + (std::find(msg.content.hashes.begin(), + msg.content.hashes.end(), + "sha256") != msg.content.hashes.end()) && + (std::find(msg.content.message_authentication_codes.begin(), + msg.content.message_authentication_codes.end(), + "hmac-sha256") != + msg.content.message_authentication_codes.end())) { + if (std::find(msg.content.short_authentication_string.begin(), + msg.content.short_authentication_string.end(), + mtx::events::msg::SASMethods::Emoji) != + msg.content.short_authentication_string.end()) { + flow->setMethod(DeviceVerificationFlow::Method::Emoji); + } else if (std::find( + msg.content.short_authentication_string.begin(), + msg.content.short_authentication_string.end(), + mtx::events::msg::SASMethods::Decimal) != + msg.content.short_authentication_string.end()) { + flow->setMethod(DeviceVerificationFlow::Method::Decimal); + } else { + flow->cancelVerification( + DeviceVerificationFlow::Error::UnknownMethod); + return; + } + emit newDeviceVerificationRequest( + std::move(flow), + QString::fromStdString(msg.content.transaction_id), + QString::fromStdString(msg.sender), + QString::fromStdString(msg.content.from_device)); + flow->canonical_json = nlohmann::json(msg.content); + } else { + flow->cancelVerification( + DeviceVerificationFlow::Error::UnknownMethod); + } } }); }