6 files changed, 102 insertions, 170 deletions

diff --git a/autotests/testkeyverification.cpp b/autotests/testkeyverification.cpp
index 59a1c934..96aad6c7 100644
--- a/autotests/testkeyverification.cpp
+++ b/autotests/testkeyverification.cpp
@@ -17,7 +17,7 @@ private Q_SLOTS:
         CREATE_CONNECTION(a, "alice1", "secret", "AliceDesktop")
         CREATE_CONNECTION(b, "alice1", "secret", "AlicePhone")
 
-        KeyVerificationSession *aSession = nullptr;
+        QPointer<KeyVerificationSession> aSession{};
         connect(a.get(), &Connection::newKeyVerificationSession, this, [&](KeyVerificationSession* session) {
             aSession = session;
             QVERIFY(session->remoteDeviceId() == b->deviceId());
@@ -51,7 +51,7 @@ private Q_SLOTS:
         });
         b->syncLoop();
         a->syncLoop();
-        QSignalSpy spy(b.get(), &Connection::incomingKeyVerificationDone);
+        QSignalSpy spy(aSession, &KeyVerificationSession::finished);
         spy.wait(10000);
     }
 };
diff --git a/lib/connection.cpp b/lib/connection.cpp
index 53c99969..5003f40c 100644
--- a/lib/connection.cpp
+++ b/lib/connection.cpp
@@ -124,6 +124,7 @@ public:
     // A map from SenderKey to vector of InboundSession
     UnorderedMap<QString, std::vector<QOlmSessionPtr>> olmSessions;
 
+    QHash<QString, KeyVerificationSession*> verificationSessions;
 #endif
 
     GetCapabilitiesJob* capabilitiesJob = nullptr;
@@ -971,6 +972,8 @@ void Connection::Private::consumeToDeviceEvents(Events&& toDeviceEvents)
         qCDebug(E2EE) << "Consuming" << toDeviceEvents.size()
                       << "to-device events";
         for (auto&& tdEvt : toDeviceEvents) {
+            if (processIfVerificationEvent(*tdEvt, false))
+                continue;
             if (auto&& event = eventCast<EncryptedEvent>(std::move(tdEvt))) {
                 if (event->algorithm() != OlmV1Curve25519AesSha2AlgoKey) {
                     qCDebug(E2EE) << "Unsupported algorithm" << event->id()
@@ -985,9 +988,7 @@ void Connection::Private::consumeToDeviceEvents(Events&& toDeviceEvents)
                 outdatedUsers += event->senderId();
                 encryptionUpdateRequired = true;
                 pendingEncryptedEvents.push_back(std::move(event));
-                continue;
             }
-            processIfVerificationEvent(*tdEvt, false);
         }
     }
 #endif
@@ -998,33 +999,22 @@ bool Connection::Private::processIfVerificationEvent(const Event& evt,
                                                      bool encrypted)
 {
     return switchOnType(evt,
-        [this, encrypted](const KeyVerificationRequestEvent& event) {
-            auto session =
-                new KeyVerificationSession(q->userId(), event, q, encrypted);
-            emit q->newKeyVerificationSession(session);
-            return true;
-        }, [this](const KeyVerificationReadyEvent& event) {
-            emit q->incomingKeyVerificationReady(event);
-            return true;
-        }, [this](const KeyVerificationStartEvent& event) {
-            emit q->incomingKeyVerificationStart(event);
-            return true;
-        }, [this](const KeyVerificationAcceptEvent& event) {
-            emit q->incomingKeyVerificationAccept(event);
-            return true;
-        }, [this](const KeyVerificationKeyEvent& event) {
-            emit q->incomingKeyVerificationKey(event);
+        [this, encrypted](const KeyVerificationRequestEvent& reqEvt) {
+            const auto sessionIter = verificationSessions.insert(
+                reqEvt.transactionId(),
+                new KeyVerificationSession(q->userId(), reqEvt, q, encrypted));
+            emit q->newKeyVerificationSession(*sessionIter);
             return true;
-        }, [this](const KeyVerificationMacEvent& event) {
-            emit q->incomingKeyVerificationMac(event);
-            return true;
-        }, [this](const KeyVerificationDoneEvent& event) {
-            emit q->incomingKeyVerificationDone(event);
-            return true;
-        }, [this](const KeyVerificationCancelEvent& event) {
-            emit q->incomingKeyVerificationCancel(event);
+        },
+        [this](const KeyVerificationEvent& kvEvt) {
+            if (auto* const session =
+                verificationSessions.value(kvEvt.transactionId())) {
+                session->handleEvent(kvEvt);
+                emit q->keyVerificationStateChanged(session, session->state());
+            }
             return true;
-        }, false);
+        },
+        false);
 }
 
 void Connection::Private::handleEncryptedToDeviceEvent(const EncryptedEvent& event)
diff --git a/lib/connection.h b/lib/connection.h
index 79c23c9a..75faf370 100644
--- a/lib/connection.h
+++ b/lib/connection.h
@@ -864,15 +864,10 @@ Q_SIGNALS:
     void devicesListLoaded();
 
 #ifdef Quotient_E2EE_ENABLED
-    void incomingKeyVerificationReady(const KeyVerificationReadyEvent& event);
-    void incomingKeyVerificationStart(const KeyVerificationStartEvent& event);
-    void incomingKeyVerificationAccept(const KeyVerificationAcceptEvent& event);
-    void incomingKeyVerificationKey(const KeyVerificationKeyEvent& event);
-    void incomingKeyVerificationMac(const KeyVerificationMacEvent& event);
-    void incomingKeyVerificationDone(const KeyVerificationDoneEvent& event);
-    void incomingKeyVerificationCancel(const KeyVerificationCancelEvent& event);
-
     void newKeyVerificationSession(KeyVerificationSession* session);
+    void keyVerificationStateChanged(
+        const KeyVerificationSession* session,
+        Quotient::KeyVerificationSession::State state);
     void sessionVerified(const QString& userId, const QString& deviceId);
 #endif
 
diff --git a/lib/events/keyverificationevent.h b/lib/events/keyverificationevent.h
index 0ffd8b2c..0e939508 100644
--- a/lib/events/keyverificationevent.h
+++ b/lib/events/keyverificationevent.h
@@ -9,13 +9,24 @@ namespace Quotient {
 
 static constexpr auto SasV1Method = "m.sas.v1"_ls;
 
+class QUOTIENT_API KeyVerificationEvent : public Event {
+public:
+    QUO_BASE_EVENT(KeyVerificationEvent, "m.key.*"_ls, Event::BaseMetaType)
+
+    using Event::Event;
+
+    /// An opaque identifier for the verification request. Must
+    /// be unique with respect to the devices involved.
+    QUO_CONTENT_GETTER(QString, transactionId)
+};
+
 /// Requests a key verification with another user's devices.
 /// Typically sent as a to-device event.
-class QUOTIENT_API KeyVerificationRequestEvent : public Event {
+class QUOTIENT_API KeyVerificationRequestEvent : public KeyVerificationEvent {
 public:
     QUO_EVENT(KeyVerificationRequestEvent, "m.key.verification.request")
 
-    using Event::Event;
+    using KeyVerificationEvent::KeyVerificationEvent;
     KeyVerificationRequestEvent(const QString& transactionId,
                                 const QString& fromDevice,
                                 const QStringList& methods,
@@ -30,10 +41,6 @@ public:
     /// The device ID which is initiating the request.
     QUO_CONTENT_GETTER(QString, fromDevice)
 
-    /// An opaque identifier for the verification request. Must
-    /// be unique with respect to the devices involved.
-    QUO_CONTENT_GETTER(QString, transactionId)
-
     /// The verification methods supported by the sender.
     QUO_CONTENT_GETTER(QStringList, methods)
 
@@ -44,11 +51,11 @@ public:
     QUO_CONTENT_GETTER(QDateTime, timestamp)
 };
 
-class QUOTIENT_API KeyVerificationReadyEvent : public Event {
+class QUOTIENT_API KeyVerificationReadyEvent : public KeyVerificationEvent {
 public:
     QUO_EVENT(KeyVerificationReadyEvent, "m.key.verification.ready")
 
-    using Event::Event;
+    using KeyVerificationEvent::KeyVerificationEvent;
     KeyVerificationReadyEvent(const QString& transactionId,
                               const QString& fromDevice,
                               const QStringList& methods)
@@ -61,19 +68,16 @@ public:
     /// The device ID which is accepting the request.
     QUO_CONTENT_GETTER(QString, fromDevice)
 
-    /// The transaction id of the verification request
-    QUO_CONTENT_GETTER(QString, transactionId)
-
     /// The verification methods supported by the sender.
     QUO_CONTENT_GETTER(QStringList, methods)
 };
 
 /// Begins a key verification process.
-class QUOTIENT_API KeyVerificationStartEvent : public Event {
+class QUOTIENT_API KeyVerificationStartEvent : public KeyVerificationEvent {
 public:
     QUO_EVENT(KeyVerificationStartEvent, "m.key.verification.start")
 
-    using Event::Event;
+    using KeyVerificationEvent::KeyVerificationEvent;
     KeyVerificationStartEvent(const QString& transactionId,
                               const QString& fromDevice)
         : KeyVerificationStartEvent(
@@ -92,10 +96,6 @@ public:
     /// The device ID which is initiating the process.
     QUO_CONTENT_GETTER(QString, fromDevice)
 
-    /// An opaque identifier for the verification request. Must
-    /// be unique with respect to the devices involved.
-    QUO_CONTENT_GETTER(QString, transactionId)
-
     /// The verification method to use.
     QUO_CONTENT_GETTER(QString, method)
 
@@ -140,11 +140,11 @@ public:
 
 /// Accepts a previously sent m.key.verification.start message.
 /// Typically sent as a to-device event.
-class QUOTIENT_API KeyVerificationAcceptEvent : public Event {
+class QUOTIENT_API KeyVerificationAcceptEvent : public KeyVerificationEvent {
 public:
     QUO_EVENT(KeyVerificationAcceptEvent, "m.key.verification.accept")
 
-    using Event::Event;
+    using KeyVerificationEvent::KeyVerificationEvent;
     KeyVerificationAcceptEvent(const QString& transactionId,
                                const QString& commitment)
         : KeyVerificationAcceptEvent(basicJson(
@@ -158,9 +158,6 @@ public:
                       { "commitment"_ls, commitment } }))
     {}
 
-    /// An opaque identifier for the verification process.
-    QUO_CONTENT_GETTER(QString, transactionId)
-
     /// The verification method to use. Must be 'm.sas.v1'.
     QUO_CONTENT_GETTER(QString, method)
 
@@ -170,10 +167,7 @@ public:
 
     /// The hash method the device is choosing to use, out of the
     /// options in the m.key.verification.start message.
-    QString hashData() const
-    {
-        return contentPart<QString>("hash"_ls);
-    }
+    QUO_CONTENT_GETTER_X(QString, hashData, "hash"_ls)
 
     /// The message authentication code the device is choosing to use, out
     /// of the options in the m.key.verification.start message.
@@ -188,11 +182,11 @@ public:
     QUO_CONTENT_GETTER(QString, commitment)
 };
 
-class QUOTIENT_API KeyVerificationCancelEvent : public Event {
+class QUOTIENT_API KeyVerificationCancelEvent : public KeyVerificationEvent {
 public:
     QUO_EVENT(KeyVerificationCancelEvent, "m.key.verification.cancel")
 
-    using Event::Event;
+    using KeyVerificationEvent::KeyVerificationEvent;
     KeyVerificationCancelEvent(const QString& transactionId,
                                const QString& reason)
         : KeyVerificationCancelEvent(
@@ -203,9 +197,6 @@ public:
                               }))
     {}
 
-    /// An opaque identifier for the verification process.
-    QUO_CONTENT_GETTER(QString, transactionId)
-
     /// A human readable description of the code. The client should only
     /// rely on this string if it does not understand the code.
     QUO_CONTENT_GETTER(QString, reason)
@@ -216,30 +207,27 @@ public:
 
 /// Sends the ephemeral public key for a device to the partner device.
 /// Typically sent as a to-device event.
-class QUOTIENT_API KeyVerificationKeyEvent : public Event {
+class KeyVerificationKeyEvent : public KeyVerificationEvent {
 public:
     QUO_EVENT(KeyVerificationKeyEvent, "m.key.verification.key")
 
-    using Event::Event;
+    using KeyVerificationEvent::KeyVerificationEvent;
     KeyVerificationKeyEvent(const QString& transactionId, const QString& key)
         : KeyVerificationKeyEvent(
             basicJson(TypeId, { { "transaction_id"_ls, transactionId },
                                 { "key"_ls, key } }))
     {}
 
-    /// An opaque identifier for the verification process.
-    QUO_CONTENT_GETTER(QString, transactionId)
-
     /// The device's ephemeral public key, encoded as unpadded base64.
     QUO_CONTENT_GETTER(QString, key)
 };
 
 /// Sends the MAC of a device's key to the partner device.
-class QUOTIENT_API KeyVerificationMacEvent : public Event {
+class QUOTIENT_API KeyVerificationMacEvent : public KeyVerificationEvent {
 public:
     QUO_EVENT(KeyVerificationMacEvent, "m.key.verification.mac")
 
-    using Event::Event;
+    using KeyVerificationEvent::KeyVerificationEvent;
     KeyVerificationMacEvent(const QString& transactionId, const QString& keys,
                             const QJsonObject& mac)
         : KeyVerificationMacEvent(
@@ -248,9 +236,6 @@ public:
                                 { "mac"_ls, mac } }))
     {}
 
-    /// An opaque identifier for the verification process.
-    QUO_CONTENT_GETTER(QString, transactionId)
-
     /// The device's ephemeral public key, encoded as unpadded base64.
     QUO_CONTENT_GETTER(QString, keys)
 
@@ -260,17 +245,14 @@ public:
     }
 };
 
-class QUOTIENT_API KeyVerificationDoneEvent : public Event {
+class QUOTIENT_API KeyVerificationDoneEvent : public KeyVerificationEvent {
 public:
     QUO_EVENT(KeyVerificationDoneEvent, "m.key.verification.done")
 
-    using Event::Event;
+    using KeyVerificationEvent::KeyVerificationEvent;
     explicit KeyVerificationDoneEvent(const QString& transactionId)
         : KeyVerificationDoneEvent(
             basicJson(TypeId, { { "transaction_id"_ls, transactionId } }))
     {}
-
-    /// The same transactionId as before
-    QUO_CONTENT_GETTER(QString, transactionId)
 };
 } // namespace Quotient
diff --git a/lib/keyverificationsession.cpp b/lib/keyverificationsession.cpp
index 24fc08e1..cc4428d7 100644
--- a/lib/keyverificationsession.cpp
+++ b/lib/keyverificationsession.cpp
@@ -68,42 +68,6 @@ KeyVerificationSession::KeyVerificationSession(QString userId, QString deviceId,
 
 void KeyVerificationSession::init(milliseconds timeout)
 {
-    connect(m_connection, &Connection::incomingKeyVerificationReady, this, [this](const KeyVerificationReadyEvent& event) {
-        if (event.transactionId() == m_transactionId && event.fromDevice() == m_remoteDeviceId) {
-            handleReady(event);
-        }
-    });
-    connect(m_connection, &Connection::incomingKeyVerificationStart, this, [this](const KeyVerificationStartEvent& event) {
-        if (event.transactionId() == m_transactionId && event.fromDevice() == m_remoteDeviceId) {
-            handleStart(event);
-        }
-    });
-    connect(m_connection, &Connection::incomingKeyVerificationAccept, this, [this](const KeyVerificationAcceptEvent& event) {
-        if (event.transactionId() == m_transactionId) {
-            handleAccept(event);
-        }
-    });
-    connect(m_connection, &Connection::incomingKeyVerificationKey, this, [this](const KeyVerificationKeyEvent& event) {
-        if (event.transactionId() == m_transactionId) {
-            handleKey(event);
-        }
-    });
-    connect(m_connection, &Connection::incomingKeyVerificationMac, this, [this](const KeyVerificationMacEvent& event) {
-        if (event.transactionId() == m_transactionId) {
-            handleMac(event);
-        }
-    });
-    connect(m_connection, &Connection::incomingKeyVerificationDone, this, [this](const KeyVerificationDoneEvent& event) {
-        if (event.transactionId() == m_transactionId) {
-            handleDone(event);
-        }
-    });
-    connect(m_connection, &Connection::incomingKeyVerificationCancel, this, [this](const KeyVerificationCancelEvent& event) {
-        if (event.transactionId() == m_transactionId) {
-            handleCancel(event);
-        }
-    });
-
     QTimer::singleShot(timeout, this, [this] { cancelVerification(TIMEOUT); });
 
     m_sas = olm_sas(new std::byte[olm_sas_size()]);
@@ -118,6 +82,53 @@ KeyVerificationSession::~KeyVerificationSession()
     delete[] reinterpret_cast<std::byte*>(m_sas);
 }
 
+void KeyVerificationSession::handleEvent(const KeyVerificationEvent& baseEvent)
+{
+    if (!switchOnType(
+            baseEvent,
+            [this](const KeyVerificationCancelEvent& event) {
+                setError(stringToError(event.code()));
+                setState(CANCELED);
+                return true;
+            },
+            [this](const KeyVerificationStartEvent& event) {
+                if (state() != WAITINGFORREADY && state() != READY)
+                    return false;
+                handleStart(event);
+                return true;
+            },
+            [this](const KeyVerificationReadyEvent& event) {
+                if (state() == WAITINGFORREADY)
+                    handleReady(event);
+                // ACCEPTED is also fine here because it's possible to receive
+                // ready and start in the same sync, in which case start might
+                // be handled before ready.
+                return state() == WAITINGFORREADY || state() == ACCEPTED;
+            },
+            [this](const KeyVerificationAcceptEvent& event) {
+                if (state() != WAITINGFORACCEPT)
+                    return false;
+                m_commitment = event.commitment();
+                sendKey();
+                setState(WAITINGFORKEY);
+                return true;
+            },
+            [this](const KeyVerificationKeyEvent& event) {
+                if (state() != ACCEPTED && state() != WAITINGFORKEY)
+                    return false;
+                handleKey(event);
+                return true;
+            },
+            [this](const KeyVerificationMacEvent& event) {
+                if (state() != WAITINGFORMAC)
+                    return false;
+                handleMac(event);
+                return true;
+            },
+            [this](const KeyVerificationDoneEvent&) { return state() == DONE; }))
+        cancelVerification(UNEXPECTED_MESSAGE);
+}
+
 struct EmojiStoreEntry : EmojiEntry {
     QHash<QString, QString> translatedDescriptions;
 
@@ -154,10 +165,6 @@ EmojiEntry emojiForCode(int code, const QString& language)
 
 void KeyVerificationSession::handleKey(const KeyVerificationKeyEvent& event)
 {
-    if (state() != WAITINGFORKEY && state() != ACCEPTED) {
-        cancelVerification(UNEXPECTED_MESSAGE);
-        return;
-    }
     auto eventKey = event.key().toLatin1();
     olm_sas_set_their_key(m_sas, eventKey.data(), eventKey.size());
 
@@ -314,34 +321,18 @@ 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;
-    }
     setState(READY);
     m_remoteSupportedMethods = event.methods();
     auto methods = commonSupportedMethods(m_remoteSupportedMethods);
 
-    if (methods.isEmpty()) {
+    if (methods.isEmpty())
         cancelVerification(UNKNOWN_METHOD);
-        return;
-    }
-
-    if (methods.size() == 1) {
-        sendStartSas();
-    }
+    else if (methods.size() == 1)
+        sendStartSas(); // -> WAITINGFORACCEPT
 }
 
 void KeyVerificationSession::handleStart(const KeyVerificationStartEvent& event)
 {
-    if (state() != READY && state() != WAITINGFORREADY) {
-        cancelVerification(UNEXPECTED_MESSAGE);
-        return;
-    }
     if (startSentByUs) {
         if (m_remoteUserId > m_connection->userId() || (m_remoteUserId == m_connection->userId() && m_remoteDeviceId > m_connection->deviceId())) {
             return;
@@ -362,17 +353,6 @@ void KeyVerificationSession::handleStart(const KeyVerificationStartEvent& event)
     setState(ACCEPTED);
 }
 
-void KeyVerificationSession::handleAccept(const KeyVerificationAcceptEvent& event)
-{
-    if(state() != WAITINGFORACCEPT) {
-        cancelVerification(UNEXPECTED_MESSAGE);
-        return;
-    }
-    m_commitment = event.commitment();
-    sendKey();
-    setState(WAITINGFORKEY);
-}
-
 void KeyVerificationSession::handleMac(const KeyVerificationMacEvent& event)
 {
     QStringList keys = event.mac().keys();
@@ -402,19 +382,6 @@ void KeyVerificationSession::handleMac(const KeyVerificationMacEvent& event)
     }
 }
 
-void KeyVerificationSession::handleDone(const KeyVerificationDoneEvent&)
-{
-    if (state() != DONE) {
-        cancelVerification(UNEXPECTED_MESSAGE);
-    }
-}
-
-void KeyVerificationSession::handleCancel(const KeyVerificationCancelEvent& event)
-{
-    setError(stringToError(event.code()));
-    setState(CANCELED);
-}
-
 QVector<EmojiEntry> KeyVerificationSession::sasEmojis() const
 {
     return m_sasEmojis;
diff --git a/lib/keyverificationsession.h b/lib/keyverificationsession.h
index b2e3b36d..9cac1184 100644
--- a/lib/keyverificationsession.h
+++ b/lib/keyverificationsession.h
@@ -91,6 +91,8 @@ public:
     ~KeyVerificationSession() override;
     Q_DISABLE_COPY_MOVE(KeyVerificationSession)
 
+    void handleEvent(const KeyVerificationEvent& baseEvent);
+
     QVector<EmojiEntry> sasEmojis() const;
     State state() const;
 
@@ -108,7 +110,6 @@ public Q_SLOTS:
     void cancelVerification(Error error);
 
 Q_SIGNALS:
-    void startReceived();
     void keyReceived();
     void sasEmojisChanged();
     void stateChanged();
@@ -133,11 +134,8 @@ private:
 
     void handleReady(const KeyVerificationReadyEvent& event);
     void handleStart(const KeyVerificationStartEvent& event);
-    void handleAccept(const KeyVerificationAcceptEvent& event);
     void handleKey(const KeyVerificationKeyEvent& event);
     void handleMac(const KeyVerificationMacEvent& event);
-    void handleDone(const KeyVerificationDoneEvent&);
-    void handleCancel(const KeyVerificationCancelEvent& event);
     void init(std::chrono::milliseconds timeout);
     void setState(State state);
     void setError(Error error);