From 6531ba6a13b41a2744a26a253e39b1b142453162 Mon Sep 17 00:00:00 2001 From: Onuray Sahin Date: Fri, 16 Oct 2020 15:18:35 +0300 Subject: [PATCH 1/2] Fix crpyto store migration from RiotX. Fixes #2252 --- CHANGES.md | 1 + .../store/db/RealmCryptoStoreMigration.kt | 73 +++++++++++++------ 2 files changed, 53 insertions(+), 21 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 3441af2f71..8ac86ec451 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -35,6 +35,7 @@ Bugfix 🐛: - Don't set presence when handling a push notification or polling (#2156) - Be robust against `StrandHogg` task injection - Clear alerts if user sign out + - Messages encrypted with no way to decrypt after SDK update from 0.18 to 1.0.0 (#2252) Translations 🗣: - Move store data to `/fastlane/metadata/android` (#812) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStoreMigration.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStoreMigration.kt index c106c82538..d8758194b5 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStoreMigration.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStoreMigration.kt @@ -43,6 +43,7 @@ import org.matrix.android.sdk.internal.crypto.store.db.model.WithHeldSessionEnti import org.matrix.android.sdk.internal.di.SerializeNulls import io.realm.DynamicRealm import io.realm.RealmMigration +import io.realm.RealmObjectSchema import org.matrix.androidsdk.crypto.data.MXOlmInboundGroupSession2 import timber.log.Timber import javax.inject.Inject @@ -57,6 +58,29 @@ internal class RealmCryptoStoreMigration @Inject constructor(private val crossSi const val CRYPTO_STORE_SCHEMA_VERSION = 11L } + private fun RealmObjectSchema.addFieldIfNotExists(fieldName: String, fieldType: Class<*>): RealmObjectSchema { + if (!hasField(fieldName)) { + addField(fieldName, fieldType) + } + return this + } + + private fun RealmObjectSchema.removeFieldIfExists(fieldName: String): RealmObjectSchema { + if (hasField(fieldName)) { + removeField(fieldName) + } + return this + } + + private fun RealmObjectSchema.setRequiredIfNotAlready(fieldName: String, isRequired: Boolean): RealmObjectSchema { + if (isRequired && !isRequired(fieldName)) { + setRequired(fieldName, true) + } else if (!isRequired && isRequired(fieldName)) { + setRequired(fieldName, false) + } + return this + } + override fun migrate(realm: DynamicRealm, oldVersion: Long, newVersion: Long) { Timber.v("Migrating Realm Crypto from $oldVersion to $newVersion") @@ -89,13 +113,13 @@ internal class RealmCryptoStoreMigration @Inject constructor(private val crossSi Timber.d("Update IncomingRoomKeyRequestEntity format: requestBodyString field is exploded into several fields") realm.schema.get("IncomingRoomKeyRequestEntity") - ?.addField("requestBodyAlgorithm", String::class.java) - ?.addField("requestBodyRoomId", String::class.java) - ?.addField("requestBodySenderKey", String::class.java) - ?.addField("requestBodySessionId", String::class.java) + ?.addFieldIfNotExists("requestBodyAlgorithm", String::class.java) + ?.addFieldIfNotExists("requestBodyRoomId", String::class.java) + ?.addFieldIfNotExists("requestBodySenderKey", String::class.java) + ?.addFieldIfNotExists("requestBodySessionId", String::class.java) ?.transform { dynamicObject -> - val requestBodyString = dynamicObject.getString("requestBodyString") try { + val requestBodyString = dynamicObject.getString("requestBodyString") // It was a map before val map: Map? = deserializeFromRealm(requestBodyString) @@ -109,18 +133,18 @@ internal class RealmCryptoStoreMigration @Inject constructor(private val crossSi Timber.e(e, "Error") } } - ?.removeField("requestBodyString") + ?.removeFieldIfExists("requestBodyString") Timber.d("Update IncomingRoomKeyRequestEntity format: requestBodyString field is exploded into several fields") realm.schema.get("OutgoingRoomKeyRequestEntity") - ?.addField("requestBodyAlgorithm", String::class.java) - ?.addField("requestBodyRoomId", String::class.java) - ?.addField("requestBodySenderKey", String::class.java) - ?.addField("requestBodySessionId", String::class.java) + ?.addFieldIfNotExists("requestBodyAlgorithm", String::class.java) + ?.addFieldIfNotExists("requestBodyRoomId", String::class.java) + ?.addFieldIfNotExists("requestBodySenderKey", String::class.java) + ?.addFieldIfNotExists("requestBodySessionId", String::class.java) ?.transform { dynamicObject -> - val requestBodyString = dynamicObject.getString("requestBodyString") try { + val requestBodyString = dynamicObject.getString("requestBodyString") // It was a map before val map: Map? = deserializeFromRealm(requestBodyString) @@ -134,16 +158,18 @@ internal class RealmCryptoStoreMigration @Inject constructor(private val crossSi Timber.e(e, "Error") } } - ?.removeField("requestBodyString") + ?.removeFieldIfExists("requestBodyString") Timber.d("Create KeysBackupDataEntity") - realm.schema.create("KeysBackupDataEntity") - .addField(KeysBackupDataEntityFields.PRIMARY_KEY, Integer::class.java) - .addPrimaryKey(KeysBackupDataEntityFields.PRIMARY_KEY) - .setRequired(KeysBackupDataEntityFields.PRIMARY_KEY, true) - .addField(KeysBackupDataEntityFields.BACKUP_LAST_SERVER_HASH, String::class.java) - .addField(KeysBackupDataEntityFields.BACKUP_LAST_SERVER_NUMBER_OF_KEYS, Integer::class.java) + if (!realm.schema.contains("KeysBackupDataEntity")) { + realm.schema.create("KeysBackupDataEntity") + .addField(KeysBackupDataEntityFields.PRIMARY_KEY, Integer::class.java) + .addPrimaryKey(KeysBackupDataEntityFields.PRIMARY_KEY) + .setRequired(KeysBackupDataEntityFields.PRIMARY_KEY, true) + .addField(KeysBackupDataEntityFields.BACKUP_LAST_SERVER_HASH, String::class.java) + .addField(KeysBackupDataEntityFields.BACKUP_LAST_SERVER_NUMBER_OF_KEYS, Integer::class.java) + } } private fun migrateTo3RiotX(realm: DynamicRealm) { @@ -151,8 +177,8 @@ internal class RealmCryptoStoreMigration @Inject constructor(private val crossSi Timber.d("Migrate to RiotX model") realm.schema.get("CryptoRoomEntity") - ?.addField(CryptoRoomEntityFields.SHOULD_ENCRYPT_FOR_INVITED_MEMBERS, Boolean::class.java) - ?.setRequired(CryptoRoomEntityFields.SHOULD_ENCRYPT_FOR_INVITED_MEMBERS, false) + ?.addFieldIfNotExists(CryptoRoomEntityFields.SHOULD_ENCRYPT_FOR_INVITED_MEMBERS, Boolean::class.java) + ?.setRequiredIfNotAlready(CryptoRoomEntityFields.SHOULD_ENCRYPT_FOR_INVITED_MEMBERS, false) // Convert format of MXDeviceInfo, package has to be the same. realm.schema.get("DeviceInfoEntity") @@ -204,8 +230,13 @@ internal class RealmCryptoStoreMigration @Inject constructor(private val crossSi // Version 4L added Cross Signing info persistence private fun migrateTo4(realm: DynamicRealm) { Timber.d("Step 3 -> 4") - Timber.d("Create KeyInfoEntity") + if (realm.schema.contains("TrustLevelEntity")) { + Timber.d("Skipping Step 3 -> 4 because entities already exist") + return + } + + Timber.d("Create KeyInfoEntity") val trustLevelEntityEntitySchema = realm.schema.create("TrustLevelEntity") .addField(TrustLevelEntityFields.CROSS_SIGNED_VERIFIED, Boolean::class.java) .setNullable(TrustLevelEntityFields.CROSS_SIGNED_VERIFIED, true) From 6d3a659362e2fe8948e0416499e8244e17e419c6 Mon Sep 17 00:00:00 2001 From: Onuray Sahin Date: Mon, 19 Oct 2020 11:14:15 +0300 Subject: [PATCH 2/2] Benoit code review fixes. --- .../internal/crypto/store/db/RealmCryptoStoreMigration.kt | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStoreMigration.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStoreMigration.kt index d8758194b5..08806b0627 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStoreMigration.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStoreMigration.kt @@ -73,10 +73,8 @@ internal class RealmCryptoStoreMigration @Inject constructor(private val crossSi } private fun RealmObjectSchema.setRequiredIfNotAlready(fieldName: String, isRequired: Boolean): RealmObjectSchema { - if (isRequired && !isRequired(fieldName)) { - setRequired(fieldName, true) - } else if (!isRequired && isRequired(fieldName)) { - setRequired(fieldName, false) + if (isRequired != isRequired(fieldName)) { + setRequired(fieldName, isRequired) } return this }