More improvements

8 files changed, 41 insertions, 38 deletions

diff --git a/autotests/testolmsession.cpp b/autotests/testolmsession.cpp
index dba78277..750b804e 100644
--- a/autotests/testolmsession.cpp
+++ b/autotests/testolmsession.cpp
@@ -7,7 +7,7 @@
 
 using namespace Quotient;
 
-std::pair<std::unique_ptr<QOlmSession>, std::unique_ptr<QOlmSession>> createSessionPair()
+std::pair<QOlmSessionPtr, QOlmSessionPtr> createSessionPair()
 {
     QByteArray pickledAccountA("eOBXIKivUT6YYowRH031BNv7zNmzqM5B7CpXdyeaPvala5mt7/OeqrG1qVA7vA1SYloFyvJPIy0QNkD3j1HiPl5vtZHN53rtfZ9exXDok03zjmssqn4IJsqcA7Fbo1FZeKafG0NFcWwCPTdmcV7REqxjqGm3I4K8MQFa45AdTGSUu2C12cWeOcbSMlcINiMral+Uyah1sgPmLJ18h1qcnskXUXQvpffZ5DiUw1Iz5zxnwOQF1GVyowPJD7Zdugvj75RQnDxAn6CzyvrY2k2CuedwqDC3fIXM2xdUNWttW4nC2g4InpBhCVvNwhZYxlUb5BUEjmPI2AB3dAL5ry6o9MFncmbN6x5x");
     QByteArray pickledAccountB("eModTvoFi9oOIkax4j4nuxw9Tcl/J8mOmUctUWI68Q89HSaaPTqR+tdlKQ85v2GOs5NlZCp7EuycypN9GQ4fFbHUCrS7nspa3GFBWsR8PnM8+wez5PWmfFZLg3drOvT0jbMjpDx0MjGYClHBqcrEpKx9oFaIRGBaX6HXzT4lRaWSJkXxuX92q8iGNrLn96PuAWFNcD+2JXpPcNFntslwLUNgqzpZ04aIFYwL80GmzyOgq3Bz1GO6u3TgCQEAmTIYN2QkO0MQeuSfe7UoMumhlAJ6R8GPcdSSPtmXNk4tdyzzlgpVq1hm7ZLKto+g8/5Aq3PvnvA8wCqno2+Pi1duK1pZFTIlActr");
@@ -20,7 +20,7 @@ std::pair<std::unique_ptr<QOlmSession>, std::unique_ptr<QOlmSession>> createSess
     const QByteArray oneTimeKeyA("WzsbsjD85iB1R32iWxfJdwkgmdz29ClMbJSJziECYwk");
     const QByteArray identityKeyB("q/YhJtog/5VHCAS9rM9uUf6AaFk1yPe4GYuyUOXyQCg");
     const QByteArray oneTimeKeyB("oWvzryma+B2onYjo3hM6A3Mgo/Yepm8HvgSvwZMTnjQ");
-    auto outbound = std::get<std::unique_ptr<QOlmSession>>(accountA
+    auto outbound = std::get<QOlmSessionPtr>(accountA
         .createOutboundSession(identityKeyB, oneTimeKeyB));
 
     const auto preKey = outbound->encrypt(""); // Payload does not matter for PreKey
@@ -29,8 +29,8 @@ std::pair<std::unique_ptr<QOlmSession>, std::unique_ptr<QOlmSession>> createSess
         // We can't call QFail here because it's an helper function returning a value
         throw "Wrong first message type received, can't create session";
     }
-    auto inbound = std::get<std::unique_ptr<QOlmSession>>(accountB.createInboundSession(preKey));
-    return std::make_pair<std::unique_ptr<QOlmSession>, std::unique_ptr<QOlmSession>>(std::move(inbound), std::move(outbound));
+    auto inbound = std::get<QOlmSessionPtr>(accountB.createInboundSession(preKey));
+    return std::make_pair<QOlmSessionPtr, QOlmSessionPtr>(std::move(inbound), std::move(outbound));
 }
 
 void TestOlmSession::olmOutboundSessionCreation()
@@ -56,17 +56,17 @@ void TestOlmSession::olmEncryptDecrypt()
 void TestOlmSession::correctSessionOrdering()
 {
     // n0W5IJ2ZmaI9FxKRj/wohUQ6WEU0SfoKsgKKHsr4VbM
-    auto session1 = std::get<std::unique_ptr<QOlmSession>>(QOlmSession::unpickle("7g5cfQRsDk2ROXf9S01n2leZiFRon+EbvXcMOADU0UGvlaV6t/0ihD2/0QGckDIvbmE1aV+PxB0zUtHXh99bI/60N+PWkCLA84jEY4sz3d45ui/TVoFGLDHlymKxvlj7XngXrbtlxSkVntsPzDiNpKEXCa26N2ubKpQ0fbjrV5gbBTYWfU04DXHPXFDTksxpNALYt/h0eVMVhf6hB0ZzpLBsOG0mpwkLufwub0CuDEDGGmRddz3TcNCLq5NnI8R9udDWvHAkTS1UTbHuIf/y6cZg875nJyXpAvd8/XhL8TOo8ot2sE1fElBa4vrH/m9rBQMC1GPkhLBIizmY44C+Sq9PQRnF+uCZ", Unencrypted{}));
+    auto session1 = std::get<QOlmSessionPtr>(QOlmSession::unpickle("7g5cfQRsDk2ROXf9S01n2leZiFRon+EbvXcMOADU0UGvlaV6t/0ihD2/0QGckDIvbmE1aV+PxB0zUtHXh99bI/60N+PWkCLA84jEY4sz3d45ui/TVoFGLDHlymKxvlj7XngXrbtlxSkVntsPzDiNpKEXCa26N2ubKpQ0fbjrV5gbBTYWfU04DXHPXFDTksxpNALYt/h0eVMVhf6hB0ZzpLBsOG0mpwkLufwub0CuDEDGGmRddz3TcNCLq5NnI8R9udDWvHAkTS1UTbHuIf/y6cZg875nJyXpAvd8/XhL8TOo8ot2sE1fElBa4vrH/m9rBQMC1GPkhLBIizmY44C+Sq9PQRnF+uCZ", Unencrypted{}));
     // +9pHJhP3K4E5/2m8PYBPLh8pS9CJodwUOh8yz3mnmw0
-    auto session2 = std::get<std::unique_ptr<QOlmSession>>(QOlmSession::unpickle("7g5cfQRsDk2ROXf9S01n2leZiFRon+EbvXcMOADU0UFD+q37/WlfTAzQsSjCdD07FcErZ4siEy5vpiB+pyO8i53ptZvb2qRvqNKFzPaXuu33PS2PBTmmnR+kJt+DgDNqWadyaj/WqEAejc7ALqSs5GuhbZtpoLe+lRSRK0rwVX3gzz4qrl8pm0pD5pSZAUWRXDRlieGWMclz68VUvnSaQH7ElTo4S634CJk+xQfFFCD26v0yONPSN6rwouS1cWPuG5jTlnV8vCFVTU2+lduKh54Ko6FUJ/ei4xR8Nk2duBGSc/TdllX9e2lDYHSUkWoD4ti5xsFioB8Blus7JK9BZfcmRmdlxIOD", Unencrypted {}));
+    auto session2 = std::get<QOlmSessionPtr>(QOlmSession::unpickle("7g5cfQRsDk2ROXf9S01n2leZiFRon+EbvXcMOADU0UFD+q37/WlfTAzQsSjCdD07FcErZ4siEy5vpiB+pyO8i53ptZvb2qRvqNKFzPaXuu33PS2PBTmmnR+kJt+DgDNqWadyaj/WqEAejc7ALqSs5GuhbZtpoLe+lRSRK0rwVX3gzz4qrl8pm0pD5pSZAUWRXDRlieGWMclz68VUvnSaQH7ElTo4S634CJk+xQfFFCD26v0yONPSN6rwouS1cWPuG5jTlnV8vCFVTU2+lduKh54Ko6FUJ/ei4xR8Nk2duBGSc/TdllX9e2lDYHSUkWoD4ti5xsFioB8Blus7JK9BZfcmRmdlxIOD", Unencrypted {}));
     // MC7n8hX1l7WlC2/WJGHZinMocgiBZa4vwGAOredb/ME
-    auto session3 = std::get<std::unique_ptr<QOlmSession>>(QOlmSession::unpickle("7g5cfQRsDk2ROXf9S01n2leZiFRon+EbvXcMOADU0UGNk2TmVDJ95K0Nywf24FNklNVtXtFDiFPHFwNSmCbHNCp3hsGtZlt0AHUkMmL48XklLqzwtVk5/v2RRmSKR5LqYdIakrtuK/fY0ENhBZIbI1sRetaJ2KMbY9l6rCJNfFg8VhpZ4KTVvEZVuP9g/eZkCnP5NxzXiBRF6nfY3O/zhcKxa3acIqs6BMhyLsfuJ80t+hQ1HvVyuhBerGujdSDzV9tJ9SPidOwfYATk81LVF9hTmnI0KaZa7qCtFzhG0dU/Z3hIWH9HOaw1aSB/IPmughbwdJOwERyhuo3YHoznlQnJ7X252BlI", Unencrypted{}));
+    auto session3 = std::get<QOlmSessionPtr>(QOlmSession::unpickle("7g5cfQRsDk2ROXf9S01n2leZiFRon+EbvXcMOADU0UGNk2TmVDJ95K0Nywf24FNklNVtXtFDiFPHFwNSmCbHNCp3hsGtZlt0AHUkMmL48XklLqzwtVk5/v2RRmSKR5LqYdIakrtuK/fY0ENhBZIbI1sRetaJ2KMbY9l6rCJNfFg8VhpZ4KTVvEZVuP9g/eZkCnP5NxzXiBRF6nfY3O/zhcKxa3acIqs6BMhyLsfuJ80t+hQ1HvVyuhBerGujdSDzV9tJ9SPidOwfYATk81LVF9hTmnI0KaZa7qCtFzhG0dU/Z3hIWH9HOaw1aSB/IPmughbwdJOwERyhuo3YHoznlQnJ7X252BlI", Unencrypted{}));
 
     const auto session1Id = session1->sessionId();
     const auto session2Id = session2->sessionId();
     const auto session3Id = session3->sessionId();
 
-    std::vector<std::unique_ptr<QOlmSession>> sessionList;
+    std::vector<QOlmSessionPtr> sessionList;
     sessionList.push_back(std::move(session1));
     sessionList.push_back(std::move(session2));
     sessionList.push_back(std::move(session3));
diff --git a/lib/connection.cpp b/lib/connection.cpp
index df9ff445..a7af1477 100644
--- a/lib/connection.cpp
+++ b/lib/connection.cpp
@@ -509,7 +509,7 @@ void Connection::Private::completeSetup(const QString& mxId)
         // create new account and save unpickle data
         olmAccount->createNewAccount();
         auto job = q->callApi<UploadKeysJob>(olmAccount->deviceKeys());
-        connect(job, &BaseJob::failure, q, [=]{
+        connect(job, &BaseJob::failure, q, [job]{
             qCWarning(E2EE) << "Failed to upload device keys:" << job->errorString();
         });
     } else {
@@ -677,10 +677,10 @@ void Connection::onSyncSuccess(SyncData&& data, bool fromCache)
         auto keys = d->olmAccount->oneTimeKeys();
         auto job = d->olmAccount->createUploadKeyRequest(keys);
         run(job, ForegroundRequest);
-        connect(job, &BaseJob::success, this, [=](){
+        connect(job, &BaseJob::success, this, [this](){
             d->olmAccount->markKeysAsPublished();
         });
-        connect(job, &BaseJob::result, this, [=](){
+        connect(job, &BaseJob::result, this, [this](){
             d->isUploadingKeys = false;
         });
     }
@@ -1903,7 +1903,7 @@ void Connection::Private::loadOutdatedUserDevices()
     }
     auto queryKeysJob = q->callApi<QueryKeysJob>(users);
     currentQueryKeysJob = queryKeysJob;
-    connect(queryKeysJob, &BaseJob::success, q, [=](){
+    connect(queryKeysJob, &BaseJob::success, q, [this, queryKeysJob](){
         currentQueryKeysJob = nullptr;
         const auto data = queryKeysJob->deviceKeys();
         for(const auto &[user, keys] : asKeyValueRange(data)) {
@@ -2024,7 +2024,7 @@ void Connection::Private::loadDevicesList()
     deviceKeys = fromJson<QHash<QString, QHash<QString, QueryKeysJob::DeviceInformation>>>(json["devices_list"].toObject());
     auto oldToken = json["sync_token"].toString();
     auto changesJob = q->callApi<GetKeysChangesJob>(oldToken, q->nextBatchToken());
-    connect(changesJob, &BaseJob::success, q, [=](){
+    connect(changesJob, &BaseJob::success, q, [this, changesJob](){
         bool hasNewOutdatedUser = false;
         for(const auto &user : changesJob->changed()) {
             outdatedUsers += user;
diff --git a/lib/crypto/qolmaccount.cpp b/lib/crypto/qolmaccount.cpp
index 1de8a0dc..5c9f5db4 100644
--- a/lib/crypto/qolmaccount.cpp
+++ b/lib/crypto/qolmaccount.cpp
@@ -179,13 +179,13 @@ OneTimeKeys QOlmAccount::oneTimeKeys() const
     const auto json = QJsonDocument::fromJson(oneTimeKeysBuffer).object();
     OneTimeKeys oneTimeKeys;
 
-    for (const QJsonValue &key1 : json.keys()) {
-        auto oneTimeKeyObject = json[key1.toString()].toObject();
+    for (const QString& key1 : json.keys()) {
+        auto oneTimeKeyObject = json[key1].toObject();
         auto keyMap = QMap<QString, QString>();
         for (const QString &key2 : oneTimeKeyObject.keys()) {
             keyMap[key2] = oneTimeKeyObject[key2].toString();
         }
-        oneTimeKeys.keys[key1.toString()] = keyMap;
+        oneTimeKeys.keys[key1] = keyMap;
     }
     return oneTimeKeys;
 }
@@ -215,7 +215,7 @@ QByteArray QOlmAccount::signOneTimeKey(const QString &key) const
     return sign(j.toJson(QJsonDocument::Compact));
 }
 
-std::optional<QOlmError> QOlmAccount::removeOneTimeKeys(const std::unique_ptr<QOlmSession> &session) const
+std::optional<QOlmError> QOlmAccount::removeOneTimeKeys(const QOlmSessionPtr &session) const
 {
     const auto error = olm_remove_one_time_keys(m_account, session->raw());
 
@@ -266,19 +266,19 @@ UploadKeysJob *QOlmAccount::createUploadKeyRequest(const OneTimeKeys &oneTimeKey
     return new UploadKeysJob(keys, oneTimeKeysSigned);
 }
 
-std::variant<std::unique_ptr<QOlmSession>, QOlmError> QOlmAccount::createInboundSession(const QOlmMessage &preKeyMessage)
+std::variant<QOlmSessionPtr, QOlmError> QOlmAccount::createInboundSession(const QOlmMessage &preKeyMessage)
 {
     Q_ASSERT(preKeyMessage.type() == QOlmMessage::PreKey);
     return QOlmSession::createInboundSession(this, preKeyMessage);
 }
 
-std::variant<std::unique_ptr<QOlmSession>, QOlmError> QOlmAccount::createInboundSessionFrom(const QByteArray &theirIdentityKey, const QOlmMessage &preKeyMessage)
+std::variant<QOlmSessionPtr, QOlmError> QOlmAccount::createInboundSessionFrom(const QByteArray &theirIdentityKey, const QOlmMessage &preKeyMessage)
 {
     Q_ASSERT(preKeyMessage.type() == QOlmMessage::PreKey);
     return QOlmSession::createInboundSessionFrom(this, theirIdentityKey, preKeyMessage);
 }
 
-std::variant<std::unique_ptr<QOlmSession>, QOlmError> QOlmAccount::createOutboundSession(const QByteArray &theirIdentityKey, const QByteArray &theirOneTimeKey)
+std::variant<QOlmSessionPtr, QOlmError> QOlmAccount::createOutboundSession(const QByteArray &theirIdentityKey, const QByteArray &theirOneTimeKey)
 {
     return QOlmSession::createOutboundSession(this, theirIdentityKey, theirOneTimeKey);
 }
diff --git a/lib/crypto/qolmaccount.h b/lib/crypto/qolmaccount.h
index 1f94ab2b..dd461e8b 100644
--- a/lib/crypto/qolmaccount.h
+++ b/lib/crypto/qolmaccount.h
@@ -19,6 +19,8 @@ namespace Quotient {
 class QOlmSession;
 class Connection;
 
+using QOlmSessionPtr = std::unique_ptr<QOlmSession>;
+
 //! An olm account manages all cryptographic keys used on a device.
 //! \code{.cpp}
 //! const auto olmAccount = new QOlmAccount(this);
@@ -77,22 +79,22 @@ public:
     DeviceKeys deviceKeys() const;
 
     //! Remove the one time key used to create the supplied session.
-    [[nodiscard]] std::optional<QOlmError> removeOneTimeKeys(const std::unique_ptr<QOlmSession> &session) const;
+    [[nodiscard]] std::optional<QOlmError> removeOneTimeKeys(const QOlmSessionPtr &session) const;
 
     //! Creates an inbound session for sending/receiving messages from a received 'prekey' message.
     //!
     //! \param message An Olm pre-key message that was encrypted for this account.
-    std::variant<std::unique_ptr<QOlmSession>, QOlmError> createInboundSession(const QOlmMessage &preKeyMessage);
+    std::variant<QOlmSessionPtr, QOlmError> createInboundSession(const QOlmMessage &preKeyMessage);
 
     //! Creates an inbound session for sending/receiving messages from a received 'prekey' message.
     //!
     //! \param theirIdentityKey - The identity key of the Olm account that
     //! encrypted this Olm message.
-    std::variant<std::unique_ptr<QOlmSession>, QOlmError> createInboundSessionFrom(const QByteArray &theirIdentityKey, const QOlmMessage &preKeyMessage);
+    std::variant<QOlmSessionPtr, QOlmError> createInboundSessionFrom(const QByteArray &theirIdentityKey, const QOlmMessage &preKeyMessage);
 
     //! Creates an outbound session for sending messages to a specific
     /// identity and one time key.
-    std::variant<std::unique_ptr<QOlmSession>, QOlmError> createOutboundSession(const QByteArray &theirIdentityKey, const QByteArray &theirOneTimeKey);
+    std::variant<QOlmSessionPtr, QOlmError> createOutboundSession(const QByteArray &theirIdentityKey, const QByteArray &theirOneTimeKey);
 
     void markKeysAsPublished();
 
diff --git a/lib/crypto/qolmsession.cpp b/lib/crypto/qolmsession.cpp
index a327a643..a0386613 100644
--- a/lib/crypto/qolmsession.cpp
+++ b/lib/crypto/qolmsession.cpp
@@ -27,7 +27,7 @@ OlmSession* QOlmSession::create()
     return olm_session(new uint8_t[olm_session_size()]);
 }
 
-std::variant<std::unique_ptr<QOlmSession>, QOlmError> QOlmSession::createInbound(QOlmAccount *account, const QOlmMessage &preKeyMessage, bool from, const QString &theirIdentityKey)
+std::variant<QOlmSessionPtr, QOlmError> QOlmSession::createInbound(QOlmAccount *account, const QOlmMessage &preKeyMessage, bool from, const QString &theirIdentityKey)
 {
     if (preKeyMessage.type() != QOlmMessage::PreKey) {
         qCCritical(E2EE) << "The message is not a pre-key in when creating inbound session" << BadMessageFormat;
@@ -53,17 +53,17 @@ std::variant<std::unique_ptr<QOlmSession>, QOlmError> QOlmSession::createInbound
     return std::make_unique<QOlmSession>(olmSession);
 }
 
-std::variant<std::unique_ptr<QOlmSession>, QOlmError> QOlmSession::createInboundSession(QOlmAccount *account, const QOlmMessage &preKeyMessage)
+std::variant<QOlmSessionPtr, QOlmError> QOlmSession::createInboundSession(QOlmAccount *account, const QOlmMessage &preKeyMessage)
 {
     return createInbound(account, preKeyMessage);
 }
 
-std::variant<std::unique_ptr<QOlmSession>, QOlmError> QOlmSession::createInboundSessionFrom(QOlmAccount *account, const QString &theirIdentityKey, const QOlmMessage &preKeyMessage)
+std::variant<QOlmSessionPtr, QOlmError> QOlmSession::createInboundSessionFrom(QOlmAccount *account, const QString &theirIdentityKey, const QOlmMessage &preKeyMessage)
 {
     return createInbound(account, preKeyMessage, true, theirIdentityKey);
 }
 
-std::variant<std::unique_ptr<QOlmSession>, QOlmError> QOlmSession::createOutboundSession(QOlmAccount *account, const QString &theirIdentityKey, const QString &theirOneTimeKey)
+std::variant<QOlmSessionPtr, QOlmError> QOlmSession::createOutboundSession(QOlmAccount *account, const QString &theirIdentityKey, const QString &theirOneTimeKey)
 {
     auto *olmOutboundSession = create();
     const auto randomLen = olm_create_outbound_session_random_length(olmOutboundSession);
@@ -105,7 +105,7 @@ std::variant<QByteArray, QOlmError> QOlmSession::pickle(const PicklingMode &mode
     return pickledBuf;
 }
 
-std::variant<std::unique_ptr<QOlmSession>, QOlmError> QOlmSession::unpickle(const QByteArray &pickled, const PicklingMode &mode)
+std::variant<QOlmSessionPtr, QOlmError> QOlmSession::unpickle(const QByteArray &pickled, const PicklingMode &mode)
 {
     QByteArray pickledBuf = pickled;
     auto *olmSession = create();
diff --git a/lib/crypto/qolmsession.h b/lib/crypto/qolmsession.h
index 959c77d0..7a040b3d 100644
--- a/lib/crypto/qolmsession.h
+++ b/lib/crypto/qolmsession.h
@@ -74,7 +74,6 @@ private:
     OlmSession* m_session;
 };
 
-
-//using QOlmSessionPtr = std::unique_ptr<QOlmSession>;
+using QOlmSessionPtr = std::unique_ptr<QOlmSession>;
 
 } //namespace Quotient
diff --git a/lib/encryptionmanager.cpp b/lib/encryptionmanager.cpp
index 84282dbf..ed6ad20b 100644
--- a/lib/encryptionmanager.cpp
+++ b/lib/encryptionmanager.cpp
@@ -38,7 +38,7 @@ public:
     EncryptionManager* q;
 
     // A map from senderKey to InboundSession
-    UnorderedMap<QString, std::unique_ptr<QOlmSession>> sessions; // TODO: cache
+    UnorderedMap<QString, QOlmSessionPtr> sessions;
     void updateDeviceKeys(
         const QHash<QString,
                     QHash<QString, QueryKeysJob::DeviceInformation>>& deviceKeys)
@@ -76,7 +76,7 @@ public:
                 qCWarning(E2EE) << "Failed to unpickle olm session";
                 continue;
             }
-            sessions[senderKey] = std::move(std::get<std::unique_ptr<QOlmSession>>(sessionResult));
+            sessions[senderKey] = std::move(std::get<QOlmSessionPtr>(sessionResult));
         }
     }
     void saveSessions() {
@@ -135,9 +135,11 @@ public:
             qCWarning(E2EE) << "Failed to create inbound session for" << senderKey << std::get<QOlmError>(newSessionResult);
             return {};
         }
-        std::unique_ptr<QOlmSession> newSession = std::move(std::get<std::unique_ptr<QOlmSession>>(newSessionResult));
-        // TODO Error handling?
-        olmAccount->removeOneTimeKeys(newSession);
+        auto newSession = std::move(std::get<QOlmSessionPtr>(newSessionResult));
+        auto error = olmAccount->removeOneTimeKeys(newSession);
+        if (error) {
+            qWarning(E2EE) << "Failed to remove one time key for session" << newSession->sessionId();
+        }
         const auto result = newSession->decrypt(message);
         sessions[senderKey] = std::move(newSession);
         saveSessions();
diff --git a/lib/room.cpp b/lib/room.cpp
index 07ffd0cd..e4fe2fb8 100644
--- a/lib/room.cpp
+++ b/lib/room.cpp
@@ -514,10 +514,10 @@ Room::Room(Connection* connection, QString id, JoinState initialJoinState)
         return this == r; // loadedRoomState fires only once per room
     });
 #ifdef Quotient_E2EE_ENABLED
-    connectSingleShot(this, &Room::encryption, this, [=](){
+    connectSingleShot(this, &Room::encryption, this, [this, connection](){
         connection->encryptionUpdate(this);
     });
-    connect(this, &Room::userAdded, this, [=](){
+    connect(this, &Room::userAdded, this, [this, connection](){
         if(usesEncryption()) {
             connection->encryptionUpdate(this);
         }