Merge pull request #1633 from vector-im/feature/keys_upload

Upload device keys only once to the homeserver and fix crash when no network (#1629)

CHANGES.md

@@ -5,7 +5,7 @@ Features ✨:
  -
 
 Improvements 🙌:
- -
+ - Upload device keys only once to the homeserver and fix crash when no network (#1629)
 
 Bugfix 🐛:
  - Fix crash when coming from a notification (#1601)

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/DefaultCryptoService.kt

@@ -70,7 +70,6 @@ import im.vector.matrix.android.internal.crypto.model.event.RoomKeyWithHeldConte
 import im.vector.matrix.android.internal.crypto.model.event.SecretSendEventContent
 import im.vector.matrix.android.internal.crypto.model.rest.DeviceInfo
 import im.vector.matrix.android.internal.crypto.model.rest.DevicesListResponse
-import im.vector.matrix.android.internal.crypto.model.rest.KeysUploadResponse
 import im.vector.matrix.android.internal.crypto.model.rest.RoomKeyRequestBody
 import im.vector.matrix.android.internal.crypto.model.toRest
 import im.vector.matrix.android.internal.crypto.repository.WarnOnUnknownDeviceRepository
@@ -344,8 +343,11 @@ internal class DefaultCryptoService @Inject constructor(
         cryptoCoroutineScope.launchToCallback(coroutineDispatchers.crypto, NoOpMatrixCallback()) {
             // Open the store
             cryptoStore.open()
-            // TODO why do that everytime? we should mark that it was done
+            // this can throw if no network
+            tryThis {
                 uploadDeviceKeys()
+            }
+
             oneTimeKeysUploader.maybeUploadOneTimeKeys()
             // this can throw if no backup
             tryThis {
@@ -981,7 +983,11 @@ internal class DefaultCryptoService @Inject constructor(
     /**
      * Upload my user's device keys.
      */
-    private suspend fun uploadDeviceKeys(): KeysUploadResponse {
+    private suspend fun uploadDeviceKeys() {
+        if (cryptoStore.getDeviceKeysUploaded()) {
+            Timber.d("Keys already uploaded, nothing to do")
+            return
+        }
         // Prepare the device keys data to send
         // Sign it
         val canonicalJson = JsonCanonicalizer.getCanonicalJson(Map::class.java, getMyDevice().signalableJSONDictionary())
@@ -992,7 +998,9 @@ internal class DefaultCryptoService @Inject constructor(
         )
 
         val uploadDeviceKeysParams = UploadKeysTask.Params(rest, null)
-        return uploadKeysTask.execute(uploadDeviceKeysParams)
+        uploadKeysTask.execute(uploadDeviceKeysParams)
+
+        cryptoStore.setDeviceKeysUploaded(true)
     }
 
     /**

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/IMXCryptoStore.kt

@@ -433,4 +433,7 @@ internal interface IMXCryptoStore {
     fun getOutgoingSecretRequest(secretName: String): OutgoingSecretRequest?
     fun getIncomingRoomKeyRequests(): List<IncomingRoomKeyRequest>
     fun getGossipingEventsTrail(): List<Event>
+
+    fun setDeviceKeysUploaded(uploaded: Boolean)
+    fun getDeviceKeysUploaded(): Boolean
 }

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/RealmCryptoStore.kt

@@ -842,6 +842,18 @@ internal class RealmCryptoStore @Inject constructor(
         } ?: false
     }
 
+    override fun setDeviceKeysUploaded(uploaded: Boolean) {
+        doRealmTransaction(realmConfiguration) {
+            it.where<CryptoMetadataEntity>().findFirst()?.deviceKeysSentToServer = uploaded
+        }
+    }
+
+    override fun getDeviceKeysUploaded(): Boolean {
+        return doWithRealm(realmConfiguration) {
+            it.where<CryptoMetadataEntity>().findFirst()?.deviceKeysSentToServer
+        } ?: false
+    }
+
     override fun setRoomsListBlacklistUnverifiedDevices(roomIds: List<String>) {
         doRealmTransaction(realmConfiguration) {
             // Reset all

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/RealmCryptoStoreMigration.kt

@@ -54,7 +54,7 @@ internal class RealmCryptoStoreMigration @Inject constructor(private val crossSi
         // 0, 1, 2: legacy Riot-Android
         // 3: migrate to RiotX schema
         // 4, 5, 6, 7, 8, 9: migrations from RiotX (which was previously 1, 2, 3, 4, 5, 6)
-        const val CRYPTO_STORE_SCHEMA_VERSION = 10L
+        const val CRYPTO_STORE_SCHEMA_VERSION = 11L
     }
 
     override fun migrate(realm: DynamicRealm, oldVersion: Long, newVersion: Long) {
@@ -70,6 +70,7 @@ internal class RealmCryptoStoreMigration @Inject constructor(private val crossSi
         if (oldVersion <= 7) migrateTo8(realm)
         if (oldVersion <= 8) migrateTo9(realm)
         if (oldVersion <= 9) migrateTo10(realm)
+        if (oldVersion <= 10) migrateTo11(realm)
     }
 
     private fun migrateTo1Legacy(realm: DynamicRealm) {
@@ -446,4 +447,11 @@ internal class RealmCryptoStoreMigration @Inject constructor(private val crossSi
                 .addField(SharedSessionEntityFields.CHAIN_INDEX, Long::class.java)
                 .setNullable(SharedSessionEntityFields.CHAIN_INDEX, true)
     }
+
+    // Version 11L added deviceKeysSentToServer boolean to CryptoMetadataEntity
+    private fun migrateTo11(realm: DynamicRealm) {
+        Timber.d("Step 10 -> 11")
+        realm.schema.get("CryptoMetadataEntity")
+                ?.addField(CryptoMetadataEntityFields.DEVICE_KEYS_SENT_TO_SERVER, Boolean::class.java)
+    }
 }

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/model/CryptoMetadataEntity.kt

@@ -36,6 +36,9 @@ internal open class CryptoMetadataEntity(
         // The keys backup version currently used. Null means no backup.
         var backupVersion: String? = null,
 
+        // The device keys has been sent to the homeserver
+        var deviceKeysSentToServer: Boolean = false,
+
         var xSignMasterPrivateKey: String? = null,
         var xSignUserPrivateKey: String? = null,
         var xSignSelfSignedPrivateKey: String? = null,