From 2fc83b87ad2e0ac67c1980bc047cb6324e08eee5 Mon Sep 17 00:00:00 2001 From: 0x330a <92654767+0x330a@users.noreply.github.com> Date: Fri, 27 Oct 2023 16:10:26 +1100 Subject: [PATCH] fix: group keys wasn't populating the config pushes for non keys objects in sync job --- .../securesms/database/Storage.kt | 30 +++++++----- .../securesms/dependencies/ConfigFactory.kt | 34 ++++++++----- .../securesms/groups/CreateGroupFragment.kt | 25 ++++++++-- .../thoughtcrime/securesms/ui/Components.kt | 5 +- .../org/thoughtcrime/securesms/ui/Themes.kt | 4 +- .../util/ConfigurationMessageUtilities.kt | 6 +++ gradle.properties | 2 +- .../libsession_util/util/GroupInfo.kt | 2 + .../libsession/database/StorageProtocol.kt | 2 +- .../messaging/jobs/ConfigurationSyncJob.kt | 48 ++++++++++--------- .../ReceivedMessageHandler.kt | 10 +++- .../pollers/ClosedGroupPoller.kt | 7 ++- .../org/session/libsession/snode/SnodeAPI.kt | 4 ++ .../utilities/ConfigFactoryProtocol.kt | 5 +- 14 files changed, 124 insertions(+), 60 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/Storage.kt b/app/src/main/java/org/thoughtcrime/securesms/database/Storage.kt index e324413d3..2323b4997 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/Storage.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/Storage.kt @@ -1231,20 +1231,24 @@ open class Storage( MessageSender.send(responseMessage, fromSerialized(groupId.hexString())) } - override fun setGroupInviteComplete(approved: Boolean, invitee: String, closedGroup: SessionId) { - val groupMembers = configFactory.getGroupMemberConfig(closedGroup) ?: return - val member = groupMembers.get(invitee) ?: run { - Log.e("ClosedGroup", "User wasn't in the group membership to add!") - return + override fun setGroupInviteCompleteIfNeeded(approved: Boolean, invitee: String, closedGroup: SessionId) { + // don't try to process invitee acceptance if we aren't admin + if (configFactory.userGroups?.getClosedGroup(closedGroup.hexString())?.hasAdminKey() != true) return + + configFactory.getGroupMemberConfig(closedGroup)?.use { groupMembers -> + val member = groupMembers.get(invitee) ?: run { + Log.e("ClosedGroup", "User wasn't in the group membership to add!") + return + } + if (!member.invitePending) return groupMembers.close() + if (approved) { + groupMembers.set(member.copy(invitePending = false)) + } else { + groupMembers.erase(member) + } + configFactory.persistGroupConfigDump(groupMembers, closedGroup, SnodeAPI.nowWithOffset) + ConfigurationMessageUtilities.forceSyncConfigurationNowIfNeeded(Destination.ClosedGroup(closedGroup.hexString())) } - if (approved) { - groupMembers.set(member.copy(invitePending = false)) - } else { - groupMembers.erase(member) - } - configFactory.persistGroupConfigDump(groupMembers, closedGroup, SnodeAPI.nowWithOffset) - ConfigurationMessageUtilities.forceSyncConfigurationNowIfNeeded(Destination.ClosedGroup(closedGroup.hexString())) - groupMembers.close() } override fun setServerCapabilities(server: String, capabilities: List) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/dependencies/ConfigFactory.kt b/app/src/main/java/org/thoughtcrime/securesms/dependencies/ConfigFactory.kt index 83967f9a4..879119f30 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/dependencies/ConfigFactory.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/dependencies/ConfigFactory.kt @@ -182,26 +182,29 @@ class ConfigFactory( override fun getGroupInfoConfig(groupSessionId: SessionId): GroupInfoConfig? = getGroupAuthInfo(groupSessionId)?.let { (sk, _) -> // get any potential initial dumps val dump = configDatabase.retrieveConfigAndHashes( - SharedConfigMessage.Kind.CLOSED_GROUP_INFO.name, + ConfigDatabase.INFO_VARIANT, groupSessionId.hexString() ) ?: byteArrayOf() GroupInfoConfig.newInstance(Hex.fromStringCondensed(groupSessionId.publicKey), sk, dump) } - override fun getGroupKeysConfig(groupSessionId: SessionId): GroupKeysConfig? = getGroupAuthInfo(groupSessionId)?.let { (sk, _) -> + override fun getGroupKeysConfig(groupSessionId: SessionId, + info: GroupInfoConfig?, + members: GroupMembersConfig?, + free: Boolean): GroupKeysConfig? = getGroupAuthInfo(groupSessionId)?.let { (sk, _) -> // Get the user info or return early val (userSk, _) = maybeGetUserInfo() ?: return@let null // Get the group info or return early - val info = getGroupInfoConfig(groupSessionId) ?: return@let null + val usedInfo = info ?: getGroupInfoConfig(groupSessionId) ?: return@let null // Get the group members or return early - val members = getGroupMemberConfig(groupSessionId) ?: return@let null + val usedMembers = members ?: getGroupMemberConfig(groupSessionId) ?: return@let null // Get the dump or empty val dump = configDatabase.retrieveConfigAndHashes( - SharedConfigMessage.Kind.ENCRYPTION_KEYS.name, + ConfigDatabase.KEYS_VARIANT, groupSessionId.hexString() ) ?: byteArrayOf() @@ -211,18 +214,20 @@ class ConfigFactory( Hex.fromStringCondensed(groupSessionId.publicKey), sk, dump, - info, - members + usedInfo, + usedMembers ) - info.free() - members.free() + if (free) { + info?.free() + members?.free() + } keys } override fun getGroupMemberConfig(groupSessionId: SessionId): GroupMembersConfig? = getGroupAuthInfo(groupSessionId)?.let { (sk, auth) -> // Get initial dump if we have one val dump = configDatabase.retrieveConfigAndHashes( - SharedConfigMessage.Kind.CLOSED_GROUP_MEMBERS.name, + ConfigDatabase.MEMBER_VARIANT, groupSessionId.hexString() ) ?: byteArrayOf() @@ -298,8 +303,13 @@ class ConfigFactory( fun persistGroupConfigDump(forConfigObject: ConfigBase, groupSessionId: SessionId, timestamp: Long) = synchronized(userGroupsLock) { val dumped = forConfigObject.dump() + val variant = when (forConfigObject) { + is GroupMembersConfig -> ConfigDatabase.MEMBER_VARIANT + is GroupInfoConfig -> ConfigDatabase.INFO_VARIANT + else -> throw Exception("Shouldn't be called") + } configDatabase.storeConfig( - ConfigDatabase.KEYS_VARIANT, + variant, groupSessionId.hexString(), dumped, timestamp @@ -375,7 +385,7 @@ class ConfigFactory( configDatabase.retrieveConfigLastUpdateTimestamp(variant, publicKey) // Ensure the change occurred after the last config message was handled (minus the buffer period) - return (changeTimestampMs >= (lastUpdateTimestampMs - ConfigFactory.configChangeBufferPeriod)) + return (changeTimestampMs >= (lastUpdateTimestampMs - configChangeBufferPeriod)) } override fun saveGroupConfigs( diff --git a/app/src/main/java/org/thoughtcrime/securesms/groups/CreateGroupFragment.kt b/app/src/main/java/org/thoughtcrime/securesms/groups/CreateGroupFragment.kt index 4f8459a34..428e90819 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/groups/CreateGroupFragment.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/groups/CreateGroupFragment.kt @@ -10,6 +10,7 @@ import androidx.annotation.StringRes import androidx.compose.foundation.background import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding @@ -35,6 +36,7 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color import androidx.compose.ui.platform.ComposeView +import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.res.stringResource import androidx.compose.ui.semantics.contentDescription import androidx.compose.ui.semantics.semantics @@ -50,13 +52,17 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import network.loki.messenger.R import network.loki.messenger.databinding.FragmentCreateGroupBinding +import org.session.libsession.avatars.ContactPhoto import org.session.libsession.messaging.contacts.Contact +import org.session.libsession.utilities.Address import org.session.libsession.utilities.Device import org.session.libsession.utilities.recipients.Recipient import org.thoughtcrime.securesms.conversation.start.NewConversationDelegate import org.thoughtcrime.securesms.conversation.v2.ConversationActivityV2 import org.thoughtcrime.securesms.ui.AppTheme +import org.thoughtcrime.securesms.ui.Avatar import org.thoughtcrime.securesms.ui.EditableAvatar +import org.thoughtcrime.securesms.ui.LocalPreviewMode import org.thoughtcrime.securesms.ui.NavigationBar import org.thoughtcrime.securesms.ui.PreviewTheme import org.thoughtcrime.securesms.ui.ThemeResPreviewParameterProvider @@ -252,9 +258,11 @@ fun LazyListScope.memberList(contacts: List, modifier: Modifier = Modif EmptyPlaceholder(modifier.fillParentMaxWidth()) } } else { - items(contacts) { - // TODO - + items(contacts) { contact -> + Row(modifier) { + val context = LocalContext.current + Avatar(Recipient.from(context, Address.fromSerialized(contact.sessionID), false)) + } } } } @@ -282,11 +290,10 @@ fun EmptyPlaceholder(modifier: Modifier = Modifier) { fun ClosedGroupPreview( @PreviewParameter(ThemeResPreviewParameterProvider::class) themeResId: Int ) { - val random = "05abcd1234" + val random = "05abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234" val previewMembers = setOf( Contact(random).apply { name = "Person" - } ) PreviewTheme(themeResId) { @@ -298,4 +305,12 @@ fun ClosedGroupPreview( onBack = {}, ) } +} + +@Composable +fun Contact.contactPhoto(): ContactPhoto { + if (LocalPreviewMode.current) { + // + } + TODO() } \ No newline at end of file diff --git a/app/src/main/java/org/thoughtcrime/securesms/ui/Components.kt b/app/src/main/java/org/thoughtcrime/securesms/ui/Components.kt index 0f6b184cf..c3290fa18 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/ui/Components.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/ui/Components.kt @@ -256,7 +256,10 @@ fun NavigationBar( } } //Main title - Box(modifier = Modifier.fillMaxHeight().weight(1f).padding(8.dp)) { + Box(modifier = Modifier + .fillMaxHeight() + .weight(1f) + .padding(8.dp)) { Text( text = title, Modifier.align(titleAlignment), diff --git a/app/src/main/java/org/thoughtcrime/securesms/ui/Themes.kt b/app/src/main/java/org/thoughtcrime/securesms/ui/Themes.kt index 64bbd21d8..5a9ece992 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/ui/Themes.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/ui/Themes.kt @@ -18,6 +18,7 @@ import com.google.android.material.color.MaterialColors import network.loki.messenger.R val LocalExtraColors = staticCompositionLocalOf { error("No Custom Attribute value provided") } +val LocalPreviewMode = staticCompositionLocalOf { false } data class ExtraColors( @@ -56,7 +57,8 @@ fun PreviewTheme( content: @Composable () -> Unit ) { CompositionLocalProvider( - LocalContext provides ContextThemeWrapper(LocalContext.current, themeResId) + LocalContext provides ContextThemeWrapper(LocalContext.current, themeResId), + LocalPreviewMode provides true ) { AppTheme { Box(modifier = Modifier.background(color = MaterialTheme.colors.background)) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/util/ConfigurationMessageUtilities.kt b/app/src/main/java/org/thoughtcrime/securesms/util/ConfigurationMessageUtilities.kt index 7d2041495..bd2358c8b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/util/ConfigurationMessageUtilities.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/util/ConfigurationMessageUtilities.kt @@ -47,12 +47,18 @@ object ConfigurationMessageUtilities { debouncer.publish { // don't schedule job if we already have one val storage = MessagingModuleConfiguration.shared.storage + val configFactory = MessagingModuleConfiguration.shared.configFactory val destinations = synchronized(destinationUpdater) { val objects = pendingDestinations.toList() pendingDestinations.clear() objects } destinations.forEach { destination -> + if (destination is Destination.ClosedGroup) { + // ensure we have the appropriate admin keys, skip this destination otherwise + val group = configFactory.userGroups?.getClosedGroup(destination.publicKey) ?: return@forEach + if (group.adminKey.isEmpty()) return@forEach Log.w("ConfigurationSync", "Trying to schedule config sync for group we aren't an admin of") + } val currentStorageJob = storage.getConfigSyncJob(destination) if (currentStorageJob != null) { (currentStorageJob as ConfigurationSyncJob).shouldRunAgain.set(true) diff --git a/gradle.properties b/gradle.properties index 1d7bc62d4..9bf8bccd5 100644 --- a/gradle.properties +++ b/gradle.properties @@ -25,7 +25,7 @@ coreVersion=1.8.0 coroutinesVersion=1.6.4 curve25519Version=0.6.0 daggerVersion=2.46.1 -glideVersion=4.11.0 +glideVersion=4.16.0 jacksonDatabindVersion=2.9.8 junitVersion=4.13.2 kotlinxJsonVersion=1.3.3 diff --git a/libsession-util/src/main/java/network/loki/messenger/libsession_util/util/GroupInfo.kt b/libsession-util/src/main/java/network/loki/messenger/libsession_util/util/GroupInfo.kt index fc4ad1615..e72876290 100644 --- a/libsession-util/src/main/java/network/loki/messenger/libsession_util/util/GroupInfo.kt +++ b/libsession-util/src/main/java/network/loki/messenger/libsession_util/util/GroupInfo.kt @@ -42,6 +42,8 @@ sealed class GroupInfo { return if (adminKey.isNotEmpty()) adminKey else authData } + fun hasAdminKey() = adminKey.isNotEmpty() + } data class LegacyGroupInfo( diff --git a/libsession/src/main/java/org/session/libsession/database/StorageProtocol.kt b/libsession/src/main/java/org/session/libsession/database/StorageProtocol.kt index d520c6717..fbda3a406 100644 --- a/libsession/src/main/java/org/session/libsession/database/StorageProtocol.kt +++ b/libsession/src/main/java/org/session/libsession/database/StorageProtocol.kt @@ -160,7 +160,7 @@ interface StorageProtocol { fun createNewGroup(groupName: String, groupDescription: String, members: Set): Optional fun getMembers(groupPublicKey: String): List fun acceptClosedGroupInvite(groupId: SessionId, name: String, authData: ByteArray, invitingAdmin: SessionId) - fun setGroupInviteComplete(approved: Boolean, invitee: String, closedGroup: SessionId) + fun setGroupInviteCompleteIfNeeded(approved: Boolean, invitee: String, closedGroup: SessionId) // Groups fun getAllGroups(includeInactive: Boolean): List diff --git a/libsession/src/main/java/org/session/libsession/messaging/jobs/ConfigurationSyncJob.kt b/libsession/src/main/java/org/session/libsession/messaging/jobs/ConfigurationSyncJob.kt index 43228ded5..a881b3155 100644 --- a/libsession/src/main/java/org/session/libsession/messaging/jobs/ConfigurationSyncJob.kt +++ b/libsession/src/main/java/org/session/libsession/messaging/jobs/ConfigurationSyncJob.kt @@ -55,18 +55,19 @@ data class ConfigurationSyncJob(val destination: Destination) : Job { val groupId = SessionId.from(destination.publicKey) // Get the signing key for pushing configs - // TODO: do nothing if we don't have the keys / aren't admin - val signingKey = - configFactoryProtocol.userGroups!!.getClosedGroup( - destination.publicKey - )!! - .signingKey() + val signingKey = configFactoryProtocol + .userGroups?.getClosedGroup(destination.publicKey)?.adminKey + if (signingKey?.isNotEmpty() == true) { + val info = configFactoryProtocol.getGroupInfoConfig(groupId)!! + val members = configFactoryProtocol.getGroupMemberConfig(groupId)!! + val keys = configFactoryProtocol.getGroupKeysConfig( + groupId, + info, + members, + false + )!! - val keys = configFactoryProtocol.getGroupKeysConfig(groupId)!! - val info = configFactoryProtocol.getGroupInfoConfig(groupId)!! - val members = configFactoryProtocol.getGroupMemberConfig(groupId)!! - - val requiringPush = + val requiringPush = listOf(keys, info, members).filter { when (it) { is GroupKeysConfig -> it.pendingConfig()?.isNotEmpty() == true @@ -75,20 +76,21 @@ data class ConfigurationSyncJob(val destination: Destination) : Job { } } - // free the objects that were created but won't be used after this point - // in case any of the configs don't need pushing, they won't be freed later - (listOf(keys, info, members) subtract requiringPush).forEach(Config::free) + // free the objects that were created but won't be used after this point + // in case any of the configs don't need pushing, they won't be freed later + (listOf(keys, info, members) subtract requiringPush).forEach(Config::free) - requiringPush.mapNotNull { config -> - if (config is GroupKeysConfig) { - config.messageInformation(destination.publicKey, signingKey) - } else if (config is ConfigBase) { - config.messageInformation(toDelete, destination.publicKey, signingKey, groupId.publicKey) - } else { - Log.e("ConfigurationSyncJob", "Tried to create a message from an unknown config") - null + requiringPush.mapNotNull { config -> + if (config is GroupKeysConfig) { + config.messageInformation(destination.publicKey, signingKey) + } else if (config is ConfigBase) { + config.messageInformation(toDelete, destination.publicKey, signingKey, groupId.publicKey) + } else { + Log.e("ConfigurationSyncJob", "Tried to create a message from an unknown config") + null + } } - } + } else emptyList() } else if (destination is Destination.Contact) { // assume our own user as check already takes place in `execute` for our own key // if contact diff --git a/libsession/src/main/java/org/session/libsession/messaging/sending_receiving/ReceivedMessageHandler.kt b/libsession/src/main/java/org/session/libsession/messaging/sending_receiving/ReceivedMessageHandler.kt index 104da3fd8..4e7970848 100644 --- a/libsession/src/main/java/org/session/libsession/messaging/sending_receiving/ReceivedMessageHandler.kt +++ b/libsession/src/main/java/org/session/libsession/messaging/sending_receiving/ReceivedMessageHandler.kt @@ -313,6 +313,14 @@ fun MessageReceiver.handleVisibleMessage( storage.setBlocksCommunityMessageRequests(recipient, message.blocksMessageRequests) } } + // Handle group invite response if new closed group + if (threadRecipient?.isClosedGroupRecipient == true) { + storage.setGroupInviteCompleteIfNeeded( + approved = true, + recipient.address.serialize(), + SessionId.from(threadRecipient.address.serialize()) + ) + } // Parse quote if needed var quoteModel: QuoteModel? = null var quoteMessageBody: String? = null @@ -534,7 +542,7 @@ private fun MessageReceiver.handleInviteResponse(message: GroupUpdated, closedGr // val profile = message // maybe we do need data to be the inner so we can access profile val storage = MessagingModuleConfiguration.shared.storage val approved = message.inner.inviteResponse.isApproved - storage.setGroupInviteComplete(approved, sender, closedGroup) + storage.setGroupInviteCompleteIfNeeded(approved, sender, closedGroup) } private fun MessageReceiver.handleNewLibSessionClosedGroupMessage(message: GroupUpdated) { diff --git a/libsession/src/main/java/org/session/libsession/messaging/sending_receiving/pollers/ClosedGroupPoller.kt b/libsession/src/main/java/org/session/libsession/messaging/sending_receiving/pollers/ClosedGroupPoller.kt index 76185745c..a2e2e03ed 100644 --- a/libsession/src/main/java/org/session/libsession/messaging/sending_receiving/pollers/ClosedGroupPoller.kt +++ b/libsession/src/main/java/org/session/libsession/messaging/sending_receiving/pollers/ClosedGroupPoller.kt @@ -98,7 +98,12 @@ class ClosedGroupPoller(private val executor: CoroutineScope, val info = configFactoryProtocol.getGroupInfoConfig(closedGroupSessionId) ?: return null val members = configFactoryProtocol.getGroupMemberConfig(closedGroupSessionId) ?: return null - val keys = configFactoryProtocol.getGroupKeysConfig(closedGroupSessionId) ?: return null + val keys = configFactoryProtocol.getGroupKeysConfig( + closedGroupSessionId, + info, + members, + free = false + ) ?: return null val hashesToExtend = mutableSetOf() diff --git a/libsession/src/main/java/org/session/libsession/snode/SnodeAPI.kt b/libsession/src/main/java/org/session/libsession/snode/SnodeAPI.kt index c30e10abc..4ff3ebfd2 100644 --- a/libsession/src/main/java/org/session/libsession/snode/SnodeAPI.kt +++ b/libsession/src/main/java/org/session/libsession/snode/SnodeAPI.kt @@ -1014,6 +1014,10 @@ object SnodeAPI { Log.d("Loki", "404, probably no file found") return Error.Generic } + 401 -> { + Log.d("Loki", "401, check you're doing a sign properly") + return Error.SigningFailed + } else -> { handleBadSnode() Log.d("Loki", "Unhandled response code: ${statusCode}.") diff --git a/libsession/src/main/java/org/session/libsession/utilities/ConfigFactoryProtocol.kt b/libsession/src/main/java/org/session/libsession/utilities/ConfigFactoryProtocol.kt index cda8e6731..37584c9f9 100644 --- a/libsession/src/main/java/org/session/libsession/utilities/ConfigFactoryProtocol.kt +++ b/libsession/src/main/java/org/session/libsession/utilities/ConfigFactoryProtocol.kt @@ -20,7 +20,10 @@ interface ConfigFactoryProtocol { fun getGroupInfoConfig(groupSessionId: SessionId): GroupInfoConfig? fun getGroupMemberConfig(groupSessionId: SessionId): GroupMembersConfig? - fun getGroupKeysConfig(groupSessionId: SessionId): GroupKeysConfig? + fun getGroupKeysConfig(groupSessionId: SessionId, + info: GroupInfoConfig? = null, + members: GroupMembersConfig? = null, + free: Boolean = true): GroupKeysConfig? fun getUserConfigs(): List fun persist(forConfigObject: Config, timestamp: Long)