Add autotest for key verification and fix several edge-cases

7 files changed, 122 insertions, 23 deletions

diff --git a/autotests/CMakeLists.txt b/autotests/CMakeLists.txt
index c11901bf..48edb168 100644
--- a/autotests/CMakeLists.txt
+++ b/autotests/CMakeLists.txt
@@ -19,4 +19,5 @@ if(${PROJECT_NAME}_ENABLE_E2EE)
     quotient_add_test(NAME testolmsession)
     quotient_add_test(NAME testolmutility)
     quotient_add_test(NAME testfilecrypto)
+    quotient_add_test(NAME testkeyverification)
 endif()
diff --git a/autotests/testkeyverification.cpp b/autotests/testkeyverification.cpp
new file mode 100644
index 00000000..fbbb7614
--- /dev/null
+++ b/autotests/testkeyverification.cpp
@@ -0,0 +1,50 @@
+// SPDX-FileCopyrightText: 2022 Tobias Fella <fella@posteo.de>
+//
+// SPDX-License-Identifier: LGPL-2.1-or-later
+
+#include "testkeyverification.h"
+#include "qt_connection_util.h"
+
+void TestKeyVerificationSession::testVerification()
+{
+    CREATE_CONNECTION(a, "alice1", "secret", "AliceDesktop")
+    CREATE_CONNECTION(b, "alice1", "secret", "AlicePhone")
+
+    KeyVerificationSession *aSession = nullptr;
+    connect(a.get(), &Connection::newKeyVerificationSession, this, [&](KeyVerificationSession* session) {
+        aSession = session;
+        QVERIFY(session->remoteDeviceId() == b->deviceId());
+        QVERIFY(session->state() == KeyVerificationSession::WAITINGFORREADY);
+        connectSingleShot(session, &KeyVerificationSession::stateChanged, this, [=](){
+            QVERIFY(session->state() == KeyVerificationSession::ACCEPTED);
+            connectSingleShot(session, &KeyVerificationSession::stateChanged, this, [=](){
+                QVERIFY(session->state() == KeyVerificationSession::WAITINGFORVERIFICATION);
+                session->sendMac();
+            });
+        });
+    });
+    a->startKeyVerificationSession(b->deviceId());
+    connect(b.get(), &Connection::newKeyVerificationSession, this, [=](KeyVerificationSession* session) {
+        QVERIFY(session->remoteDeviceId() == a->deviceId());
+        QVERIFY(session->state() == KeyVerificationSession::INCOMING);
+        session->sendReady();
+        // KeyVerificationSession::READY is skipped because we have only one method
+        QVERIFY(session->state() == KeyVerificationSession::WAITINGFORACCEPT);
+        connectSingleShot(session, &KeyVerificationSession::stateChanged, this, [=](){
+            QVERIFY(session->state() == KeyVerificationSession::WAITINGFORKEY || session->state() == KeyVerificationSession::ACCEPTED);
+            connectSingleShot(session, &KeyVerificationSession::stateChanged, this, [=]() {
+                QVERIFY(session->state() == KeyVerificationSession::WAITINGFORVERIFICATION);
+                QVERIFY(aSession);
+                QVERIFY(aSession->sasEmojis() == session->sasEmojis());
+                session->sendMac();
+                QVERIFY(session->state() == KeyVerificationSession::WAITINGFORMAC);
+            });
+        });
+
+    });
+    b->syncLoop();
+    a->syncLoop();
+    QSignalSpy spy(b.get(), &Connection::incomingKeyVerificationDone);
+    spy.wait(10000);
+}
+QTEST_GUILESS_MAIN(TestKeyVerificationSession)
diff --git a/autotests/testkeyverification.h b/autotests/testkeyverification.h
new file mode 100644
index 00000000..28e00e11
--- /dev/null
+++ b/autotests/testkeyverification.h
@@ -0,0 +1,18 @@
+// SPDX-FileCopyrightText: 2022 Tobias Fella <fella@posteo.de>
+//
+// SPDX-License-Identifier: LGPL-2.1-or-later
+
+#pragma once
+
+#include <QTest>
+#include "testutils.h"
+
+class TestKeyVerificationSession : public QObject
+{
+    Q_OBJECT
+
+private Q_SLOTS:
+    void testVerification();
+
+};
+
diff --git a/autotests/testolmaccount.cpp b/autotests/testolmaccount.cpp
index 4b32393d..280705d0 100644
--- a/autotests/testolmaccount.cpp
+++ b/autotests/testolmaccount.cpp
@@ -14,6 +14,8 @@
 #include <networkaccessmanager.h>
 #include <room.h>
 
+#include "testutils.h"
+
 using namespace Quotient;
 
 void TestOlmAccount::pickleUnpickledTest()
@@ -168,26 +170,6 @@ void TestOlmAccount::encryptedFile()
     QCOMPARE(file.key.kty, "oct");
 }
 
-#define CREATE_CONNECTION(VAR, USERNAME, SECRET, DEVICE_NAME)                  \
-    NetworkAccessManager::instance()->ignoreSslErrors(true);                   \
-    auto VAR = std::make_shared<Connection>();                                 \
-    (VAR)->resolveServer("@" USERNAME ":localhost:1234");                      \
-    connect((VAR).get(), &Connection::loginFlowsChanged, this, [=] {           \
-        (VAR)->loginWithPassword((USERNAME), SECRET, DEVICE_NAME, "");         \
-    });                                                                        \
-    connect((VAR).get(), &Connection::networkError, [](const QString& error) { \
-        QWARN(qUtf8Printable(error));                                          \
-        QFAIL("Network error: make sure synapse is running");                  \
-    });                                                                        \
-    connect((VAR).get(), &Connection::loginError, [](const QString& error) {   \
-        QWARN(qUtf8Printable(error));                                          \
-        QFAIL("Login failed");                                                 \
-    });                                                                        \
-    QSignalSpy spy##VAR((VAR).get(), &Connection::loginFlowsChanged);          \
-    QSignalSpy spy2##VAR((VAR).get(), &Connection::connected);                 \
-    QVERIFY(spy##VAR.wait(10000));                                             \
-    QVERIFY(spy2##VAR.wait(10000));
-
 void TestOlmAccount::uploadIdentityKey()
 {
     CREATE_CONNECTION(conn, "alice1", "secret", "AlicePhone")
diff --git a/autotests/testutils.h b/autotests/testutils.h
new file mode 100644
index 00000000..7d016a34
--- /dev/null
+++ b/autotests/testutils.h
@@ -0,0 +1,32 @@
+// SPDX-FileCopyrightText: 2021 Carl Schwan <carlschwan@kde.org>
+//
+// SPDX-License-Identifier: LGPL-2.1-or-later
+
+#pragma once
+
+#include <connection.h>
+#include <networkaccessmanager.h>
+
+#include <QtTest/QSignalSpy>
+
+using namespace Quotient;
+
+#define CREATE_CONNECTION(VAR, USERNAME, SECRET, DEVICE_NAME)                  \
+    NetworkAccessManager::instance()->ignoreSslErrors(true);                   \
+    auto VAR = std::make_shared<Connection>();                                 \
+    (VAR)->resolveServer("@" USERNAME ":localhost:1234");                      \
+    connect((VAR).get(), &Connection::loginFlowsChanged, this, [=] {           \
+        (VAR)->loginWithPassword((USERNAME), SECRET, DEVICE_NAME, "");         \
+    });                                                                        \
+    connect((VAR).get(), &Connection::networkError, [](const QString& error) { \
+        QWARN(qUtf8Printable(error));                                          \
+        QFAIL("Network error: make sure synapse is running");                  \
+    });                                                                        \
+    connect((VAR).get(), &Connection::loginError, [](const QString& error) {   \
+        QWARN(qUtf8Printable(error));                                          \
+        QFAIL("Login failed");                                                 \
+    });                                                                        \
+    QSignalSpy spy##VAR((VAR).get(), &Connection::loginFlowsChanged);          \
+    QSignalSpy spy2##VAR((VAR).get(), &Connection::connected);                 \
+    QVERIFY(spy##VAR.wait(10000));                                             \
+    QVERIFY(spy2##VAR.wait(10000));
diff --git a/lib/keyverificationsession.cpp b/lib/keyverificationsession.cpp
index 44a085fe..24fc08e1 100644
--- a/lib/keyverificationsession.cpp
+++ b/lib/keyverificationsession.cpp
@@ -154,7 +154,7 @@ EmojiEntry emojiForCode(int code, const QString& language)
 
 void KeyVerificationSession::handleKey(const KeyVerificationKeyEvent& event)
 {
-    if (state() != WAITINGFORKEY && state() != WAITINGFORVERIFICATION) {
+    if (state() != WAITINGFORKEY && state() != ACCEPTED) {
         cancelVerification(UNEXPECTED_MESSAGE);
         return;
     }
@@ -176,7 +176,6 @@ void KeyVerificationSession::handleKey(const KeyVerificationKeyEvent& event)
     } else {
         sendKey();
     }
-    setState(WAITINGFORVERIFICATION);
 
     std::string key(olm_sas_pubkey_length(m_sas), '\0');
     olm_sas_get_pubkey(m_sas, key.data(), key.size());
@@ -214,6 +213,7 @@ void KeyVerificationSession::handleKey(const KeyVerificationKeyEvent& event)
 
     emit sasEmojisChanged();
     emit keyReceived();
+    setState(WAITINGFORVERIFICATION);
 }
 
 QString KeyVerificationSession::calculateMac(const QString& input,
@@ -314,6 +314,10 @@ void KeyVerificationSession::sendStartSas()
 
 void KeyVerificationSession::handleReady(const KeyVerificationReadyEvent& event)
 {
+    if (state() == ACCEPTED) {
+        // It's possible to receive ready and start in the same sync, in which case start might be handled before ready.
+        return;
+    }
     if (state() != WAITINGFORREADY) {
         cancelVerification(UNEXPECTED_MESSAGE);
         return;
@@ -334,7 +338,7 @@ void KeyVerificationSession::handleReady(const KeyVerificationReadyEvent& event)
 
 void KeyVerificationSession::handleStart(const KeyVerificationStartEvent& event)
 {
-    if (state() != READY) {
+    if (state() != READY && state() != WAITINGFORREADY) {
         cancelVerification(UNEXPECTED_MESSAGE);
         return;
     }
@@ -510,3 +514,8 @@ KeyVerificationSession::Error KeyVerificationSession::stringToError(const QStrin
     }
     return NONE;
 }
+
+QString KeyVerificationSession::remoteDeviceId() const
+{
+    return m_remoteDeviceId;
+}
diff --git a/lib/keyverificationsession.h b/lib/keyverificationsession.h
index aa0295cb..31f63453 100644
--- a/lib/keyverificationsession.h
+++ b/lib/keyverificationsession.h
@@ -19,6 +19,11 @@ struct QUOTIENT_API EmojiEntry {
     Q_GADGET
     Q_PROPERTY(QString emoji MEMBER emoji CONSTANT)
     Q_PROPERTY(QString description MEMBER description CONSTANT)
+
+public:
+    bool operator==(const EmojiEntry& rhs) const {
+        return emoji == rhs.emoji && description == rhs.description;
+    }
 };
 
 /** A key verification session. Listen for incoming sessions by connecting to Connection::newKeyVerificationSession.
@@ -93,6 +98,8 @@ public:
 
     Error error() const;
 
+    QString remoteDeviceId() const;
+
 public Q_SLOTS:
     void sendRequest();
     void sendReady();