KeyVerificationSession: cleanup

- Use std::chrono for the timeout (it's more readable and
  less ambiguous) and make it a local variable
- Only pass a Connection object once to constructors
- Ensure buildability even without E2EE (key verification is disabled
  in that case)
- Reorder #includes
- Other cleanup following clang-tidy warnings

6 files changed, 90 insertions, 81 deletions

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 7aebe070..f8667645 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -139,7 +139,6 @@ list(APPEND lib_SRCS
     lib/eventitem.h lib/eventitem.cpp
     lib/accountregistry.h lib/accountregistry.cpp
     lib/mxcreply.h lib/mxcreply.cpp
-    lib/keyverificationsession.h lib/keyverificationsession.cpp
     lib/events/event.h lib/events/event.cpp
     lib/events/eventloader.h
     lib/events/roomevent.h lib/events/roomevent.cpp
@@ -168,7 +167,6 @@ list(APPEND lib_SRCS
     lib/events/encryptedevent.h lib/events/encryptedevent.cpp
     lib/events/roomkeyevent.h lib/events/roomkeyevent.cpp
     lib/events/stickerevent.h
-    lib/events/keyverificationevent.h
     lib/events/filesourceinfo.h lib/events/filesourceinfo.cpp
     lib/jobs/requestdata.h lib/jobs/requestdata.cpp
     lib/jobs/basejob.h lib/jobs/basejob.cpp
@@ -180,6 +178,7 @@ list(APPEND lib_SRCS
 if (${PROJECT_NAME}_ENABLE_E2EE)
     list(APPEND lib_SRCS
         lib/database.h lib/database.cpp
+        lib/keyverificationsession.h lib/keyverificationsession.cpp
         lib/e2ee/qolmaccount.h lib/e2ee/qolmaccount.cpp
         lib/e2ee/qolmsession.h lib/e2ee/qolmsession.cpp
         lib/e2ee/qolminboundsession.h lib/e2ee/qolminboundsession.cpp
@@ -189,6 +188,7 @@ if (${PROJECT_NAME}_ENABLE_E2EE)
         lib/e2ee/qolmerrors.h lib/e2ee/qolmerrors.cpp
         lib/e2ee/qolmsession.h lib/e2ee/qolmsession.cpp
         lib/e2ee/qolmmessage.h lib/e2ee/qolmmessage.cpp
+        lib/events/keyverificationevent.h
     )
 endif()
 
diff --git a/lib/connection.cpp b/lib/connection.cpp
index 3e1e556f..fbe365de 100644
--- a/lib/connection.cpp
+++ b/lib/connection.cpp
@@ -37,15 +37,17 @@
 
 #ifdef Quotient_E2EE_ENABLED
 #    include "database.h"
+#    include "keyverificationsession.h"
+
 #    include "e2ee/qolmaccount.h"
 #    include "e2ee/qolminboundsession.h"
 #    include "e2ee/qolmsession.h"
 #    include "e2ee/qolmutility.h"
 #    include "e2ee/qolmutils.h"
-#    include "events/keyverificationevent.h"
-#    include "keyverificationsession.h"
 
+#    include "events/keyverificationevent.h"
 #endif // Quotient_E2EE_ENABLED
+
 #if QT_VERSION_MAJOR >= 6
 #    include <qt6keychain/keychain.h>
 #else
@@ -988,7 +990,8 @@ void Connection::Private::consumeToDeviceEvents(Events&& toDeviceEvents)
             }
             switchOnType(*tdEvt,
                 [this](const KeyVerificationRequestEvent& event) {
-                    auto session = new KeyVerificationSession(q->userId(), event, q, false, q);
+                    auto session = new KeyVerificationSession(q->userId(),
+                                                              event, q, false);
                     emit q->newKeyVerificationSession(session);
                 }, [this](const KeyVerificationReadyEvent& event) {
                     emit q->incomingKeyVerificationReady(event);
@@ -1028,8 +1031,8 @@ void Connection::Private::handleEncryptedToDeviceEvent(const EncryptedEvent& eve
                     << "is not found at the connection" << q->objectName();
             }
         }, [this](const KeyVerificationRequestEvent& event) {
-            auto session = new KeyVerificationSession(q->userId(), event, q, true, q);
-            emit q->newKeyVerificationSession(session);
+            emit q->newKeyVerificationSession(
+                new KeyVerificationSession(q->userId(), event, q, true));
         }, [this](const KeyVerificationReadyEvent& event) {
             emit q->incomingKeyVerificationReady(event);
         }, [this](const KeyVerificationStartEvent& event) {
@@ -2274,9 +2277,9 @@ QString Connection::Private::curveKeyForUserDevice(const QString& userId,
 }
 
 QString Connection::edKeyForUserDevice(const QString& userId,
-                                       const QString& device) const
+                                       const QString& deviceId) const
 {
-    return d->deviceKeys[userId][device].keys["ed25519:" % device];
+    return d->deviceKeys[userId][deviceId].keys["ed25519:" % deviceId];
 }
 
 bool Connection::Private::isKnownCurveKey(const QString& userId,
@@ -2458,12 +2461,10 @@ void Connection::saveCurrentOutboundMegolmSession(
                                                   session);
 }
 
-#endif
-
 void Connection::startKeyVerificationSession(const QString& deviceId)
 {
-    auto session = new KeyVerificationSession(userId(), deviceId, this, this);
-    Q_EMIT newKeyVerificationSession(session);
+    auto* const session = new KeyVerificationSession(userId(), deviceId, this);
+    emit newKeyVerificationSession(session);
 }
 
 void Connection::sendToDevice(const QString& userId, const QString& deviceId,
@@ -2499,7 +2500,7 @@ void Connection::sendToDevice(const QString& userId, const QString& deviceId,
                       { { userId, { { deviceId, event->contentJson() } } } });
 }
 
-bool Connection::isVerifiedSession(const QString& megolmSessionId)
+bool Connection::isVerifiedSession(const QString& megolmSessionId) const
 {
     auto query = database()->prepareQuery("SELECT olmSessionId FROM inbound_megolm_sessions WHERE sessionId=:sessionId;"_ls);
     query.bindValue(":sessionId", megolmSessionId);
@@ -2520,3 +2521,4 @@ bool Connection::isVerifiedSession(const QString& megolmSessionId)
     database()->execute(query);
     return query.next() && query.value("verified").toBool();
 }
+#endif
diff --git a/lib/connection.h b/lib/connection.h
index b684d16b..3a4ee798 100644
--- a/lib/connection.h
+++ b/lib/connection.h
@@ -24,10 +24,9 @@
 
 #ifdef Quotient_E2EE_ENABLED
 #include "e2ee/e2ee.h"
-#include "e2ee/qolmmessage.h"
 #include "e2ee/qolmoutboundsession.h"
-#include "events/keyverificationevent.h"
 #include "keyverificationsession.h"
+#include "events/keyverificationevent.h"
 #endif
 
 Q_DECLARE_METATYPE(Quotient::GetLoginFlowsJob::LoginFlow)
@@ -332,12 +331,16 @@ public:
     void saveCurrentOutboundMegolmSession(
         const QString& roomId, const QOlmOutboundGroupSession& session) const;
 
-
-    QString edKeyForUserDevice(const QString& user, const QString& device) const;
+    QString edKeyForUserDevice(const QString& userId,
+                               const QString& deviceId) const;
     bool hasOlmSession(const QString& user, const QString& deviceId) const;
 
+    // This assumes that an olm session already exists. If it doesn't, no message is sent.
+    void sendToDevice(const QString& userId, const QString& deviceId,
+                      event_ptr_tt<Event> event, bool encrypted);
+
     /// Returns true if this megolm session comes from a verified device
-    bool isVerifiedSession(const QString& megolmSessionId);
+    bool isVerifiedSession(const QString& megolmSessionId) const;
 
     void sendSessionKeyToDevices(const QString& roomId,
                                  const QByteArray& sessionId,
@@ -528,9 +531,6 @@ public:
     /// Saves the olm account data to disk. Usually doesn't need to be called manually.
     void saveOlmAccount();
 
-    // This assumes that an olm session already exists. If it doesn't, no message is sent.
-    void sendToDevice(const QString& userId, const QString& deviceId, event_ptr_tt<Event> event, bool encrypted);
-
 public Q_SLOTS:
     /// \brief Set the homeserver base URL and retrieve its login flows
     ///
@@ -708,9 +708,9 @@ public Q_SLOTS:
     /** \deprecated Do not use this directly, use Room::leaveRoom() instead */
     virtual LeaveRoomJob* leaveRoom(Room* room);
 
+#ifdef Quotient_E2EE_ENABLED
     void startKeyVerificationSession(const QString& deviceId);
 
-#ifdef Quotient_E2EE_ENABLED
     void encryptionUpdate(Room *room);
 #endif
 
@@ -871,6 +871,8 @@ Q_SIGNALS:
     void lazyLoadingChanged();
     void turnServersChanged(const QJsonObject& servers);
     void devicesListLoaded();
+
+#ifdef Quotient_E2EE_ENABLED
     void incomingKeyVerificationReady(const KeyVerificationReadyEvent& event);
     void incomingKeyVerificationStart(const KeyVerificationStartEvent& event);
     void incomingKeyVerificationAccept(const KeyVerificationAcceptEvent& event);
@@ -881,6 +883,7 @@ Q_SIGNALS:
 
     void newKeyVerificationSession(KeyVerificationSession* session);
     void sessionVerified(const QString& userId, const QString& deviceId);
+#endif
 
 protected:
     /**
diff --git a/lib/database.cpp b/lib/database.cpp
index 79793b9d..4eb82cf5 100644
--- a/lib/database.cpp
+++ b/lib/database.cpp
@@ -9,7 +9,6 @@
 #include <QtCore/QStandardPaths>
 #include <QtCore/QDebug>
 #include <QtCore/QDir>
-#include <ctime>
 
 #include "e2ee/e2ee.h"
 #include "e2ee/qolmsession.h"
diff --git a/lib/keyverificationsession.cpp b/lib/keyverificationsession.cpp
index d889b465..1ee489ea 100644
--- a/lib/keyverificationsession.cpp
+++ b/lib/keyverificationsession.cpp
@@ -1,53 +1,59 @@
 // SPDX-FileCopyrightText: 2022 Tobias Fella <fella@posteo.de>
 // SPDX-License-Identifier: LGPL-2.1-or-later
 
-#include "connection.h"
 #include "keyverificationsession.h"
-#include "olm/sas.h"
+
+#include "connection.h"
+#include "database.h"
 #include "e2ee/qolmaccount.h"
 #include "e2ee/qolmutils.h"
+#include "olm/sas.h"
+
 #include "events/event.h"
+
 #include <QtCore/QCryptographicHash>
-#include <QtCore/QUuid>
 #include <QtCore/QTimer>
-#include "database.h"
+#include <QtCore/QUuid>
+
+#include <chrono>
 
 using namespace Quotient;
+using namespace std::chrono;
 
-KeyVerificationSession::KeyVerificationSession(const QString& remoteUserId, const KeyVerificationRequestEvent& event, Connection *connection, bool encrypted, QObject* parent)
-    : QObject(parent)
-    , m_remoteUserId(remoteUserId)
+KeyVerificationSession::KeyVerificationSession(
+    QString remoteUserId, const KeyVerificationRequestEvent& event,
+    Connection* connection, bool encrypted)
+    : QObject(connection)
+    , m_remoteUserId(std::move(remoteUserId))
     , m_remoteDeviceId(event.fromDevice())
     , m_transactionId(event.transactionId())
     , m_connection(connection)
     , m_encrypted(encrypted)
     , m_remoteSupportedMethods(event.methods())
 {
-    auto timeoutTime = std::min(event.timestamp().addSecs(600),
-                                QDateTime::currentDateTime().addSecs(120));
-    m_timeout =
-        timeoutTime.toMSecsSinceEpoch() - QDateTime::currentMSecsSinceEpoch();
-    if (m_timeout <= 5000) {
-        return;
-    }
-    init();
-    setState(INCOMING);
+    const auto& currentTime = QDateTime::currentDateTime();
+    const auto timeoutTime =
+        std::min(event.timestamp().addSecs(600), currentTime.addSecs(120));
+    const milliseconds timeout{ currentTime.msecsTo(timeoutTime) };
+    if (timeout > 5s)
+        init(timeout);
+    // Otherwise don't even bother starting up
 }
 
-KeyVerificationSession::KeyVerificationSession(const QString& userId, const QString& deviceId, Connection* connection, QObject* parent)
-    : QObject(parent)
-    , m_remoteUserId(userId)
-    , m_remoteDeviceId(deviceId)
+KeyVerificationSession::KeyVerificationSession(QString userId, QString deviceId,
+                                               Connection* connection)
+    : QObject(connection)
+    , m_remoteUserId(std::move(userId))
+    , m_remoteDeviceId(std::move(deviceId))
     , m_transactionId(QUuid::createUuid().toString())
     , m_connection(connection)
     , m_encrypted(false)
 {
-    m_timeout = 600000;
-    init();
+    init(600s);
     QMetaObject::invokeMethod(this, &KeyVerificationSession::sendRequest);
 }
 
-void KeyVerificationSession::init()
+void KeyVerificationSession::init(milliseconds timeout)
 {
     connect(m_connection, &Connection::incomingKeyVerificationReady, this, [this](const KeyVerificationReadyEvent& event) {
         if (event.transactionId() == m_transactionId && event.fromDevice() == m_remoteDeviceId) {
@@ -85,10 +91,7 @@ void KeyVerificationSession::init()
         }
     });
 
-    QTimer::singleShot(m_timeout, this, [this] {
-        cancelVerification(TIMEOUT);
-    });
-
+    QTimer::singleShot(timeout, this, [this] { cancelVerification(TIMEOUT); });
 
     m_sas = olm_sas(new uint8_t[olm_sas_size()]);
     auto randomSize = olm_create_sas_random_length(m_sas);
@@ -380,7 +383,7 @@ void KeyVerificationSession::handleMac(const KeyVerificationMacEvent& event)
     }
 }
 
-void KeyVerificationSession::handleDone(const KeyVerificationDoneEvent& event)
+void KeyVerificationSession::handleDone(const KeyVerificationDoneEvent&)
 {
     if (state() != DONE) {
         cancelVerification(UNEXPECTED_MESSAGE);
diff --git a/lib/keyverificationsession.h b/lib/keyverificationsession.h
index cb7a99e9..2756fa0a 100644
--- a/lib/keyverificationsession.h
+++ b/lib/keyverificationsession.h
@@ -4,10 +4,10 @@
 #pragma once
 
 #include "events/keyverificationevent.h"
+
 #include <QtCore/QObject>
-#include <qchar.h>
 
-class OlmSAS;
+struct OlmSAS;
 
 namespace Quotient {
 class Connection;
@@ -22,16 +22,20 @@ class QUOTIENT_API KeyVerificationSession : public QObject
 
 public:
     enum State {
-        INCOMING, // There is a request for verification incoming
-        WAITINGFORREADY, // We sent a request for verification and are waiting for ready
-        READY, // Either party sent a ready as a response to a request; The user selects a method
-        WAITINGFORACCEPT, // We sent a start and are waiting for an accept
-        ACCEPTED, // The other party sent an accept and is waiting for a key
-        WAITINGFORKEY, // We're waiting for a key
-        WAITINGFORVERIFICATION, // We're waiting for the *user* to verify the emojis
-        WAITINGFORMAC, // We're waiting for the mac
-        CANCELED, // The session has been canceled
-        DONE, // The verification is done
+        INCOMING, ///< There is a request for verification incoming
+        //! We sent a request for verification and are waiting for ready
+        WAITINGFORREADY,
+        //! Either party sent a ready as a response to a request; the user
+        //! selects a method
+        READY,
+        WAITINGFORACCEPT, ///< We sent a start and are waiting for an accept
+        ACCEPTED, ///< The other party sent an accept and is waiting for a key
+        WAITINGFORKEY, ///< We're waiting for a key
+        //! We're waiting for the *user* to verify the emojis
+        WAITINGFORVERIFICATION,
+        WAITINGFORMAC, ///< We're waiting for the mac
+        CANCELED, ///< The session has been canceled
+        DONE, ///< The verification is done
     };
     Q_ENUM(State)
 
@@ -60,19 +64,21 @@ public:
         MISMATCHED_SAS,
         REMOTE_MISMATCHED_SAS,
     };
-    Q_ENUM(Error);
+    Q_ENUM(Error)
 
-    //Q_PROPERTY(int timeLeft READ timeLeft NOTIFY timeLeftChanged)
     Q_PROPERTY(QString remoteDeviceId MEMBER m_remoteDeviceId CONSTANT)
     Q_PROPERTY(QList<QVariantMap> sasEmojis READ sasEmojis NOTIFY sasEmojisChanged)
     Q_PROPERTY(State state READ state NOTIFY stateChanged)
     Q_PROPERTY(Error error READ error NOTIFY errorChanged)
 
-    KeyVerificationSession(const QString& remoteUserId, const KeyVerificationRequestEvent& event, Connection* connection, bool encrypted, QObject* parent = nullptr);
-    KeyVerificationSession(const QString& userId, const QString& deviceId, Connection* connection, QObject* parent = nullptr);
-    ~KeyVerificationSession();
+    KeyVerificationSession(QString remoteUserId,
+                           const KeyVerificationRequestEvent& event,
+                           Connection* connection, bool encrypted);
+    KeyVerificationSession(QString userId, QString deviceId,
+                           Connection* connection);
+    ~KeyVerificationSession() override;
+    Q_DISABLE_COPY_MOVE(KeyVerificationSession)
 
-    int timeLeft() const;
     QList<QVariantMap> sasEmojis() const;
     State state() const;
 
@@ -88,8 +94,6 @@ public Q_SLOTS:
     void cancelVerification(Error error);
 
 Q_SIGNALS:
-
-    void timeLeftChanged();
     void startReceived();
     void keyReceived();
     void sasEmojisChanged();
@@ -98,20 +102,18 @@ Q_SIGNALS:
     void finished();
 
 private:
-    QString m_remoteUserId;
-    QString m_remoteDeviceId;
-    QString m_transactionId;
+    const QString m_remoteUserId;
+    const QString m_remoteDeviceId;
+    const QString m_transactionId;
     Connection* m_connection;
     OlmSAS* m_sas = nullptr;
-    int timeleft = 0;
     QList<QVariantMap> m_sasEmojis;
     bool startSentByUs = false;
-    State m_state;
-    Error m_error;
+    State m_state = INCOMING;
+    Error m_error = NONE;
     QString m_startEvent;
     QString m_commitment;
     QString m_language;
-    int m_timeout;
     bool macReceived = false;
     bool m_encrypted;
     QStringList m_remoteSupportedMethods;
@@ -121,9 +123,9 @@ private:
     void handleAccept(const KeyVerificationAcceptEvent& event);
     void handleKey(const KeyVerificationKeyEvent& event);
     void handleMac(const KeyVerificationMacEvent& event);
-    void handleDone(const KeyVerificationDoneEvent& event);
+    void handleDone(const KeyVerificationDoneEvent&);
     void handleCancel(const KeyVerificationCancelEvent& event);
-    void init();
+    void init(std::chrono::milliseconds timeout);
     void setState(State state);
     void setError(Error error);
     QStringList commonSupportedMethods(const QStringList& remoteSupportedMethods) const;