Updated the code to ignore outdated legacy group control message changes

app/src/main/java/org/thoughtcrime/securesms/database/ConfigDatabase.kt

@@ -3,6 +3,7 @@ package org.thoughtcrime.securesms.database
 import android.content.Context
 import androidx.core.content.contentValuesOf
 import androidx.core.database.getBlobOrNull
+import androidx.core.database.getLongOrNull
 import org.thoughtcrime.securesms.database.helpers.SQLCipherOpenHelper
 
 class ConfigDatabase(context: Context, helper: SQLCipherOpenHelper): Database(context, helper) {
@@ -11,21 +12,23 @@ class ConfigDatabase(context: Context, helper: SQLCipherOpenHelper): Database(co
         private const val VARIANT = "variant"
         private const val PUBKEY = "publicKey"
         private const val DATA = "data"
+        private const val TIMESTAMP = "timestamp"   // Milliseconds
 
         private const val TABLE_NAME = "configs_table"
 
         const val CREATE_CONFIG_TABLE_COMMAND =
-            "CREATE TABLE $TABLE_NAME ($VARIANT TEXT NOT NULL, $PUBKEY TEXT NOT NULL, $DATA BLOB, PRIMARY KEY($VARIANT, $PUBKEY));"
+            "CREATE TABLE $TABLE_NAME ($VARIANT TEXT NOT NULL, $PUBKEY TEXT NOT NULL, $DATA BLOB, $TIMESTAMP INTEGER NOT NULL DEFAULT 0, PRIMARY KEY($VARIANT, $PUBKEY));"
 
         private const val VARIANT_AND_PUBKEY_WHERE = "$VARIANT = ? AND $PUBKEY = ?"
     }
 
-    fun storeConfig(variant: String, publicKey: String, data: ByteArray) {
+    fun storeConfig(variant: String, publicKey: String, data: ByteArray, timestamp: Long) {
         val db = writableDatabase
         val contentValues = contentValuesOf(
             VARIANT to variant,
             PUBKEY to publicKey,
             DATA to data,
+            TIMESTAMP to timestamp
         )
         db.insertOrUpdate(TABLE_NAME, contentValues, VARIANT_AND_PUBKEY_WHERE, arrayOf(variant, publicKey))
     }
@@ -40,4 +43,11 @@ class ConfigDatabase(context: Context, helper: SQLCipherOpenHelper): Database(co
         }
     }
 
+    fun retrieveConfigLastUpdateTimestamp(variant: String, publicKey: String): Long {
+        val db = readableDatabase
+        val cursor = db.query(TABLE_NAME, arrayOf(TIMESTAMP), VARIANT_AND_PUBKEY_WHERE, arrayOf(variant, publicKey),null, null, null)
+        if (cursor == null) return 0
+        if (!cursor.moveToFirst()) return 0
+        return (cursor.getLongOrNull(cursor.getColumnIndex(TIMESTAMP)) ?: 0)
+    }
 }

app/src/main/java/org/thoughtcrime/securesms/database/Storage.kt

@@ -411,6 +411,10 @@ open class Storage(context: Context, helper: SQLCipherOpenHelper, private val co
         notifyUpdates(forConfigObject)
     }
 
+    override fun canPerformConfigChange(variant: String, publicKey: String, changeTimestampMs: Long): Boolean {
+        return configFactory.canPerformChange(variant, publicKey, changeTimestampMs)
+    }
+
     fun notifyUpdates(forConfigObject: ConfigBase) {
         when (forConfigObject) {
             is UserProfile -> updateUser(forConfigObject)
@@ -869,7 +873,8 @@ open class Storage(context: Context, helper: SQLCipherOpenHelper, private val co
             encPubKey = (latestKeyPair.publicKey as DjbECPublicKey).publicKey,  // 'serialize()' inserts an extra byte
             encSecKey = latestKeyPair.privateKey.serialize(),
             priority = if (isPinned(threadID)) PRIORITY_PINNED else ConfigBase.PRIORITY_VISIBLE,
-            disappearingTimer = recipientSettings.expireMessages.toLong()
+            disappearingTimer = recipientSettings.expireMessages.toLong(),
+            joinedAt = (existingGroup.formationTimestamp / 1000L)
         )
         userGroups.set(groupInfo)
     }
@@ -1263,6 +1268,7 @@ open class Storage(context: Context, helper: SQLCipherOpenHelper, private val co
     override fun deleteConversation(threadID: Long) {
         val recipient = getRecipientForThread(threadID)
         val threadDB = DatabaseComponent.get(context).threadDatabase()
+        val groupDB = DatabaseComponent.get(context).groupDatabase()
         threadDB.deleteConversation(threadID)
         if (recipient != null) {
             if (recipient.isContactRecipient) {
@@ -1276,9 +1282,11 @@ open class Storage(context: Context, helper: SQLCipherOpenHelper, private val co
                 // TODO: handle closed group
                 val volatile = configFactory.convoVolatile ?: return
                 val groups = configFactory.userGroups ?: return
-                val closedGroup = getGroup(recipient.address.toGroupString())
+                val groupID = recipient.address.toGroupString()
+                val closedGroup = getGroup(groupID)
                 val groupPublicKey = GroupUtil.doubleDecodeGroupId(recipient.address.serialize())
                 if (closedGroup != null) {
+                    groupDB.delete(groupID) // TODO: Should we delete the group? (seems odd to leave it)
                     volatile.eraseLegacyClosedGroup(groupPublicKey)
                     groups.eraseLegacyGroup(groupPublicKey)
                 } else {

app/src/main/java/org/thoughtcrime/securesms/dependencies/ConfigFactory.kt

@@ -21,6 +21,13 @@ class ConfigFactory(
     private val maybeGetUserInfo: () -> Pair<ByteArray, String>?
 ) :
     ConfigFactoryProtocol {
+    companion object {
+        // This is a buffer period within which we will process messages which would result in a
+        // config change, any message which would normally result in a config change which was sent
+        // before `lastConfigMessage.timestamp - configChangeBufferPeriod` will not  actually have
+        // it's changes applied (control text will still be added though)
+        val configChangeBufferPeriod: Long = (2 * 60 * 1000)
+    }
 
     fun keyPairChanged() { // this should only happen restoring or clearing data
         _userConfig?.free()
@@ -136,48 +143,58 @@ class ConfigFactory(
         listOfNotNull(user, contacts, convoVolatile, userGroups)
 
 
-    private fun persistUserConfigDump() = synchronized(userLock) {
+    private fun persistUserConfigDump(timestamp: Long) = synchronized(userLock) {
         val dumped = user?.dump() ?: return
         val (_, publicKey) = maybeGetUserInfo() ?: return
-        configDatabase.storeConfig(SharedConfigMessage.Kind.USER_PROFILE.name, publicKey, dumped)
+        configDatabase.storeConfig(SharedConfigMessage.Kind.USER_PROFILE.name, publicKey, dumped, timestamp)
     }
 
-    private fun persistContactsConfigDump() = synchronized(contactsLock) {
+    private fun persistContactsConfigDump(timestamp: Long) = synchronized(contactsLock) {
         val dumped = contacts?.dump() ?: return
         val (_, publicKey) = maybeGetUserInfo() ?: return
-        configDatabase.storeConfig(SharedConfigMessage.Kind.CONTACTS.name, publicKey, dumped)
+        configDatabase.storeConfig(SharedConfigMessage.Kind.CONTACTS.name, publicKey, dumped, timestamp)
     }
 
-    private fun persistConvoVolatileConfigDump() = synchronized(convoVolatileLock) {
+    private fun persistConvoVolatileConfigDump(timestamp: Long) = synchronized(convoVolatileLock) {
         val dumped = convoVolatile?.dump() ?: return
         val (_, publicKey) = maybeGetUserInfo() ?: return
         configDatabase.storeConfig(
             SharedConfigMessage.Kind.CONVO_INFO_VOLATILE.name,
             publicKey,
-            dumped
+            dumped,
+            timestamp
         )
     }
 
-    private fun persistUserGroupsConfigDump() = synchronized(userGroupsLock) {
+    private fun persistUserGroupsConfigDump(timestamp: Long) = synchronized(userGroupsLock) {
         val dumped = userGroups?.dump() ?: return
         val (_, publicKey) = maybeGetUserInfo() ?: return
-        configDatabase.storeConfig(SharedConfigMessage.Kind.GROUPS.name, publicKey, dumped)
+        configDatabase.storeConfig(SharedConfigMessage.Kind.GROUPS.name, publicKey, dumped, timestamp)
     }
 
-    override fun persist(forConfigObject: ConfigBase) {
+    override fun persist(forConfigObject: ConfigBase, timestamp: Long) {
         try {
             listeners.forEach { listener ->
                 listener.notifyUpdates(forConfigObject)
             }
             when (forConfigObject) {
-                is UserProfile -> persistUserConfigDump()
+                is UserProfile -> persistUserConfigDump(timestamp)
-                is Contacts -> persistContactsConfigDump()
+                is Contacts -> persistContactsConfigDump(timestamp)
-                is ConversationVolatileConfig -> persistConvoVolatileConfigDump()
+                is ConversationVolatileConfig -> persistConvoVolatileConfigDump(timestamp)
-                is UserGroupsConfig -> persistUserGroupsConfigDump()
+                is UserGroupsConfig -> persistUserGroupsConfigDump(timestamp)
                 else -> throw UnsupportedOperationException("Can't support type of ${forConfigObject::class.simpleName} yet")
             }
         } catch (e: Exception) {
             Log.e("Loki", "failed to persist ${forConfigObject.javaClass.simpleName}", e)
         }
     }
+
+    override fun canPerformChange(variant: String, publicKey: String, changeTimestampMs: Long): Boolean {
+        if (!ConfigBase.isNewConfigEnabled(isConfigForcedOn, SnodeAPI.nowWithOffset)) return true
+
+        val lastUpdateTimestampMs = configDatabase.retrieveConfigLastUpdateTimestamp(variant, publicKey)
+
+        // Ensure the change occurred after the last config message was handled (minus the buffer period)
+        return (changeTimestampMs >= (lastUpdateTimestampMs - ConfigFactory.configChangeBufferPeriod))
+    }
 }

app/src/main/java/org/thoughtcrime/securesms/preferences/SettingsActivity.kt

@@ -32,6 +32,7 @@ import nl.komponents.kovenant.ui.alwaysUi
 import nl.komponents.kovenant.ui.successUi
 import org.session.libsession.avatars.AvatarHelper
 import org.session.libsession.messaging.MessagingModuleConfiguration
+import org.session.libsession.snode.SnodeAPI
 import org.session.libsession.utilities.*
 import org.session.libsession.utilities.SSKEnvironment.ProfileManagerProtocol
 import org.thoughtcrime.securesms.PassphraseRequiredActionBarActivity
@@ -242,7 +243,7 @@ class SettingsActivity : PassphraseRequiredActionBarActivity() {
                 }
             }
             if (userConfig != null && userConfig.needsDump()) {
-                configFactory.persist(userConfig)
+                configFactory.persist(userConfig, SnodeAPI.nowWithOffset)
             }
             ConfigurationMessageUtilities.forceSyncConfigurationNowIfNeeded(this@SettingsActivity)
         }

libsession-util/src/main/cpp/user_groups.h

@@ -123,7 +123,7 @@ inline jobject serialize_legacy_group_info(JNIEnv *env, session::config::legacy_
 
     jclass legacy_group_class = env->FindClass("network/loki/messenger/libsession_util/util/GroupInfo$LegacyGroupInfo");
     jmethodID constructor = env->GetMethodID(legacy_group_class, "<init>", "(Ljava/lang/String;Ljava/lang/String;Ljava/util/Map;[B[BIJJ)V");
-    jobject serialized = env->NewObject(legacy_group_class, constructor, session_id, name, members, enc_pubkey, enc_seckey, priority, joined_at, (jlong) info.disappearing_timer.count());
+    jobject serialized = env->NewObject(legacy_group_class, constructor, session_id, name, members, enc_pubkey, enc_seckey, priority, (jlong) info.disappearing_timer.count(), joined_at);
     return serialized;
 }
 

libsession/src/main/java/org/session/libsession/database/StorageProtocol.kt

@@ -223,4 +223,5 @@ interface StorageProtocol {
 
     // Shared configs
     fun notifyConfigUpdates(forConfigObject: ConfigBase)
+    fun canPerformConfigChange(variant: String, publicKey: String, changeTimestampMs: Long): Boolean
 }

libsession/src/main/java/org/session/libsession/messaging/jobs/ConfigurationSyncJob.kt

@@ -2,6 +2,7 @@ package org.session.libsession.messaging.jobs
 
 import network.loki.messenger.libsession_util.ConfigBase
 import network.loki.messenger.libsession_util.ConfigBase.Companion.protoKindFor
+import network.loki.messenger.libsession_util.UserGroupsConfig
 import nl.komponents.kovenant.functional.bind
 import org.session.libsession.messaging.MessagingModuleConfiguration
 import org.session.libsession.messaging.messages.Destination
@@ -60,6 +61,7 @@ data class ConfigurationSyncJob(val destination: Destination): Job {
         val toDeleteHashes = mutableListOf<String>()
 
         // allow null results here so the list index matches configsRequiringPush
+        val sentTimestamp: Long = SnodeAPI.nowWithOffset
         val batchObjects: List<Pair<SharedConfigurationMessage, SnodeAPI.SnodeBatchRequestInfo>?> = configsRequiringPush.map { config ->
             val (data, seqNo, obsoleteHashes) = config.push()
             toDeleteHashes += obsoleteHashes
@@ -140,7 +142,7 @@ data class ConfigurationSyncJob(val destination: Destination): Job {
                 Log.d(TAG, "Successfully removed the deleted hashes from ${config.javaClass.simpleName}")
                 // dump and write config after successful
                 if (config.needsDump()) { // usually this will be true?
-                    configFactory.persist(config)
+                    configFactory.persist(config, toPushMessage.sentTimestamp ?: sentTimestamp)
                 }
             }
         } catch (e: Exception) {

libsession/src/main/java/org/session/libsession/messaging/sending_receiving/ReceivedMessageHandler.kt

@@ -42,6 +42,7 @@ import org.session.libsignal.crypto.ecc.DjbECPublicKey
 import org.session.libsignal.crypto.ecc.ECKeyPair
 import org.session.libsignal.messages.SignalServiceGroup
 import org.session.libsignal.protos.SignalServiceProtos
+import org.session.libsignal.protos.SignalServiceProtos.SharedConfigMessage
 import org.session.libsignal.utilities.Base64
 import org.session.libsignal.utilities.IdPrefix
 import org.session.libsignal.utilities.Log
@@ -440,7 +441,14 @@ private fun MessageReceiver.handleClosedGroupControlMessage(message: ClosedGroup
         is ClosedGroupControlMessage.Kind.MembersRemoved -> handleClosedGroupMembersRemoved(message)
         is ClosedGroupControlMessage.Kind.MemberLeft -> handleClosedGroupMemberLeft(message)
     }
-    if (message.kind !is ClosedGroupControlMessage.Kind.New) {
+    if (
+            message.kind !is ClosedGroupControlMessage.Kind.New &&
+            MessagingModuleConfiguration.shared.storage.canPerformConfigChange(
+                    SharedConfigMessage.Kind.GROUPS.name,
+                    MessagingModuleConfiguration.shared.storage.getUserPublicKey()!!,
+                    message.sentTimestamp!!
+            )
+    ) {
         // update the config
         val closedGroupPublicKey = message.getPublicKey()
         val storage = MessagingModuleConfiguration.shared.storage
@@ -471,10 +479,24 @@ private fun MessageReceiver.handleNewClosedGroup(message: ClosedGroupControlMess
 private fun handleNewClosedGroup(sender: String, sentTimestamp: Long, groupPublicKey: String, name: String, encryptionKeyPair: ECKeyPair, members: List<String>, admins: List<String>, formationTimestamp: Long, expireTimer: Int) {
     val context = MessagingModuleConfiguration.shared.context
     val storage = MessagingModuleConfiguration.shared.storage
-    val userPublicKey = TextSecurePreferences.getLocalNumber(context)
+    val userPublicKey = storage.getUserPublicKey()!!
-    // Create the group
     val groupID = GroupUtil.doubleEncodeGroupID(groupPublicKey)
     val groupExists = storage.getGroup(groupID) != null
+
+    if (!storage.canPerformConfigChange(SharedConfigMessage.Kind.GROUPS.name, userPublicKey, sentTimestamp)) {
+        // If the closed group already exists then store the encryption keys (since the config only stores
+        // the latest key we won't be able to decrypt older messages if we were added to the group within
+        // the last two weeks and the key has been rotated - unfortunately if the user was added more than
+        // two weeks ago and the keys were rotated within the last two weeks then we won't be able to decrypt
+        // messages received before the key rotation)
+        if (groupExists) {
+            storage.addClosedGroupEncryptionKeyPair(encryptionKeyPair, groupPublicKey, sentTimestamp)
+            storage.updateGroupConfig(groupPublicKey)
+        }
+        return
+    }
+
+    // Create the group
     if (groupExists) {
         // Update the group
         if (!storage.isGroupActive(groupPublicKey)) {
@@ -498,7 +520,7 @@ private fun handleNewClosedGroup(sender: String, sentTimestamp: Long, groupPubli
     // Set expiration timer
     storage.setExpirationTimer(groupID, expireTimer)
     // Notify the PN server
-    PushNotificationAPI.performOperation(PushNotificationAPI.ClosedGroupOperation.Subscribe, groupPublicKey, storage.getUserPublicKey()!!)
+    PushNotificationAPI.performOperation(PushNotificationAPI.ClosedGroupOperation.Subscribe, groupPublicKey, userPublicKey)
     // Create thread
     storage.getOrCreateThreadIdFor(Address.fromSerialized(groupID))
     // Start polling
@@ -569,7 +591,12 @@ private fun MessageReceiver.handleClosedGroupNameChanged(message: ClosedGroupCon
     val members = group.members.map { it.serialize() }
     val admins = group.admins.map { it.serialize() }
     val name = kind.name
-    storage.updateTitle(groupID, name)
+
+    // Only update the group in storage if it isn't invalidated by the config state
+    if (storage.canPerformConfigChange(SharedConfigMessage.Kind.GROUPS.name, userPublicKey!!, message.sentTimestamp!!)) {
+        storage.updateTitle(groupID, name)
+    }
+
     // Notify the user
     if (userPublicKey == senderPublicKey) {
         val threadID = storage.getOrCreateThreadIdFor(Address.fromSerialized(groupID))
@@ -603,12 +630,16 @@ private fun MessageReceiver.handleClosedGroupMembersAdded(message: ClosedGroupCo
 
     val updateMembers = kind.members.map { it.toByteArray().toHexString() }
     val newMembers = members + updateMembers
-    storage.updateMembers(groupID, newMembers.map { Address.fromSerialized(it) })
 
-    // Update zombie members in case the added members are zombies
+    // Only update the group in storage if it isn't invalidated by the config state
-    val zombies = storage.getZombieMembers(groupID)
+    if (storage.canPerformConfigChange(SharedConfigMessage.Kind.GROUPS.name, userPublicKey, message.sentTimestamp!!)) {
-    if (zombies.intersect(updateMembers).isNotEmpty()) {
+        storage.updateMembers(groupID, newMembers.map { Address.fromSerialized(it) })
-        storage.setZombieMembers(groupID, zombies.minus(updateMembers).map { Address.fromSerialized(it) })
+
+        // Update zombie members in case the added members are zombies
+        val zombies = storage.getZombieMembers(groupID)
+        if (zombies.intersect(updateMembers).isNotEmpty()) {
+            storage.setZombieMembers(groupID, zombies.minus(updateMembers).map { Address.fromSerialized(it) })
+        }
     }
 
     // Notify the user
@@ -690,14 +721,18 @@ private fun MessageReceiver.handleClosedGroupMembersRemoved(message: ClosedGroup
         Log.d("Loki", "Received a MEMBERS_REMOVED instead of a MEMBERS_LEFT from sender: $senderPublicKey.")
     }
     val wasCurrentUserRemoved = userPublicKey in removedMembers
-    // Admin should send a MEMBERS_LEFT message but handled here just in case
+
-    if (didAdminLeave || wasCurrentUserRemoved) {
+    // Only update the group in storage if it isn't invalidated by the config state
-        disableLocalGroupAndUnsubscribe(groupPublicKey, groupID, userPublicKey, true)
+    if (storage.canPerformConfigChange(SharedConfigMessage.Kind.GROUPS.name, userPublicKey, message.sentTimestamp!!)) {
-        return
+        // Admin should send a MEMBERS_LEFT message but handled here just in case
-    } else {
+        if (didAdminLeave || wasCurrentUserRemoved) {
-        storage.updateMembers(groupID, newMembers.map { Address.fromSerialized(it) })
+            disableLocalGroupAndUnsubscribe(groupPublicKey, groupID, userPublicKey, true)
-        // Update zombie members
+            return
-        storage.setZombieMembers(groupID, zombies.minus(removedMembers).map { Address.fromSerialized(it) })
+        } else {
+            storage.updateMembers(groupID, newMembers.map { Address.fromSerialized(it) })
+            // Update zombie members
+            storage.setZombieMembers(groupID, zombies.minus(removedMembers).map { Address.fromSerialized(it) })
+        }
     }
 
     // Notify the user
@@ -746,18 +781,23 @@ private fun MessageReceiver.handleClosedGroupMemberLeft(message: ClosedGroupCont
     val didAdminLeave = admins.contains(senderPublicKey)
     val updatedMemberList = members - senderPublicKey
     val userLeft = (userPublicKey == senderPublicKey)
-    if (didAdminLeave || userLeft) {
-        disableLocalGroupAndUnsubscribe(groupPublicKey, groupID, userPublicKey, delete = userLeft)
 
-        if (userLeft) {
+    // Only update the group in storage if it isn't invalidated by the config state
-            return
+    if (storage.canPerformConfigChange(SharedConfigMessage.Kind.GROUPS.name, userPublicKey, message.sentTimestamp!!)) {
+        if (didAdminLeave || userLeft) {
+            disableLocalGroupAndUnsubscribe(groupPublicKey, groupID, userPublicKey, delete = userLeft)
+
+            if (userLeft) {
+                return
+            }
+        } else {
+            storage.updateMembers(groupID, updatedMemberList.map { Address.fromSerialized(it) })
+            // Update zombie members
+            val zombies = storage.getZombieMembers(groupID)
+            storage.setZombieMembers(groupID, zombies.plus(senderPublicKey).map { Address.fromSerialized(it) })
         }
-    } else {
-        storage.updateMembers(groupID, updatedMemberList.map { Address.fromSerialized(it) })
-        // Update zombie members
-        val zombies = storage.getZombieMembers(groupID)
-        storage.setZombieMembers(groupID, zombies.plus(senderPublicKey).map { Address.fromSerialized(it) })
     }
+
     // Notify the user
     if (!userLeft) {
         storage.insertIncomingInfoMessage(context, senderPublicKey, groupID, SignalServiceGroup.Type.QUIT, name, members, admins, message.sentTimestamp!!)

libsession/src/main/java/org/session/libsession/messaging/sending_receiving/pollers/Poller.kt

@@ -146,6 +146,7 @@ class Poller(private val configFactory: ConfigFactoryProtocol, debounceTimer: Ti
             return
         }
 
+        var latestMessageTimestamp: Long? = null
         messages.forEach { (envelope, hash) ->
             try {
                 val (message, _) = MessageReceiver.parse(data = envelope.toByteArray(), openGroupServerID = null)
@@ -155,13 +156,14 @@ class Poller(private val configFactory: ConfigFactoryProtocol, debounceTimer: Ti
                     return@forEach
                 }
                 forConfigObject.merge(hash!! to message.data)
+                latestMessageTimestamp = if ((message.sentTimestamp ?: 0L) > (latestMessageTimestamp ?: 0L)) { message.sentTimestamp } else { latestMessageTimestamp }
             } catch (e: Exception) {
                 Log.e("Loki", e)
             }
         }
         // process new results
         if (forConfigObject.needsDump()) {
-            configFactory.persist(forConfigObject)
+            configFactory.persist(forConfigObject, latestMessageTimestamp ?: SnodeAPI.nowWithOffset)
         }
     }
 

libsession/src/main/java/org/session/libsession/utilities/ConfigFactoryProtocol.kt

@@ -12,7 +12,8 @@ interface ConfigFactoryProtocol {
     val convoVolatile: ConversationVolatileConfig?
     val userGroups: UserGroupsConfig?
     fun getUserConfigs(): List<ConfigBase>
-    fun persist(forConfigObject: ConfigBase)
+    fun persist(forConfigObject: ConfigBase, timestamp: Long)
+    fun canPerformChange(variant: String, publicKey: String, changeTimestampMs: Long): Boolean
 }
 
 interface ConfigFactoryUpdateListener {