diff --git a/app/src/main/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessor.kt b/app/src/main/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessor.kt index 37823ffdbe..1ec42c2258 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessor.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessor.kt @@ -204,10 +204,10 @@ class GroupsV2StateProcessor private constructor( Log.w(TAG, "$logPrefix Unable to query server for group, says we're not in group, trying to resolve locally") if (currentLocalState != null && signedGroupChange != null) { - if (notInGroupAndNotBeingAdded(groupRecord, signedGroupChange)) { - Log.w(TAG, "$logPrefix Server says we're not a member. Ignoring P2P group change because we're not currently in the group and this change doesn't add us in.") + if (!isAddingOrRemovingSelf(signedGroupChange)) { + Log.w(TAG, "$logPrefix Server says we're not a member. Ignoring P2P group change because this change doesn't add or remove us.") } else { - Log.i(TAG, "$logPrefix Server says we're not a member. Force applying P2P group change.") + Log.i(TAG, "$logPrefix Server says we're not a member. Force applying P2P group change because it adds or removes us.") when (val forcedP2pUpdateResult = updateViaPeerGroupChange(timestamp, serverGuid, signedGroupChange, currentLocalState, forceApply = true)) { is InternalUpdateResult.Updated -> return GroupUpdateResult.updated(forcedP2pUpdateResult.updatedLocalState) InternalUpdateResult.NoUpdateNeeded -> return GroupUpdateResult.CONSISTENT_OR_AHEAD @@ -294,7 +294,8 @@ class GroupsV2StateProcessor private constructor( serverGuid = serverGuid, groupStateDiff = groupStateDiff, groupSendEndorsements = null, - forceSave = forceApply + forceSave = forceApply, + persistProfileKeys = !forceApply ) } @@ -488,6 +489,51 @@ class GroupsV2StateProcessor private constructor( return !currentlyInGroup && !addedAsMember && !addedAsPendingMember && !addedAsRequestingMember } + private fun isAddingOrRemovingSelf(signedGroupChange: DecryptedGroupChange): Boolean { + val addedAsMember = signedGroupChange + .newMembers + .asSequence() + .mapNotNull { ACI.parseOrNull(it.aciBytes) } + .any { serviceIds.matches(it) } + + val addedAsPendingMember = signedGroupChange + .newPendingMembers + .asSequence() + .map { it.serviceIdBytes } + .any { serviceIds.matches(it) } + + val addedAsRequestingMember = signedGroupChange + .newRequestingMembers + .asSequence() + .mapNotNull { ACI.parseOrNull(it.aciBytes) } + .any { serviceIds.matches(it) } + + val removedAsMember = signedGroupChange + .deleteMembers + .asSequence() + .mapNotNull { ACI.parseOrNull(it) } + .any { serviceIds.matches(it) } + + val removedAsPendingMember = signedGroupChange + .deletePendingMembers + .asSequence() + .map { it.serviceIdBytes } + .any { serviceIds.matches(it) } + + val removedAsRequestingMember = signedGroupChange + .deleteRequestingMembers + .asSequence() + .mapNotNull { ACI.parseOrNull(it) } + .any { serviceIds.matches(it) } + + return addedAsMember || + addedAsPendingMember || + addedAsRequestingMember || + removedAsMember || + removedAsPendingMember || + removedAsRequestingMember + } + private fun notHavingInviteRevoked(signedGroupChange: DecryptedGroupChange): Boolean { val havingInviteRevoked = signedGroupChange .deletePendingMembers @@ -523,7 +569,8 @@ class GroupsV2StateProcessor private constructor( serverGuid: String?, groupStateDiff: GroupStateDiff, groupSendEndorsements: ReceivedGroupSendEndorsements?, - forceSave: Boolean + forceSave: Boolean, + persistProfileKeys: Boolean = true ): InternalUpdateResult { val currentLocalState: DecryptedGroup? = groupStateDiff.previousGroupState val applyGroupStateDiffResult = GroupStatePatcher.applyGroupStateDiff(groupStateDiff, GroupStatePatcher.LATEST) @@ -573,7 +620,12 @@ class GroupsV2StateProcessor private constructor( } else { profileAndMessageHelper.insertUpdateMessages(timestamp, currentLocalState, applyGroupStateDiffResult.processedLogEntries, serverGuid) } - profileAndMessageHelper.persistLearnedProfileKeys(groupStateDiff) + + if (persistProfileKeys) { + profileAndMessageHelper.persistLearnedProfileKeys(groupStateDiff) + } else { + Log.w(TAG, "$logPrefix Skipping profile key persistence for force-applied P2P change") + } val performCdsLookup = groupStateDiff .serverHistory diff --git a/app/src/test/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessorTest.kt b/app/src/test/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessorTest.kt index c52ae027b1..280428ce0f 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessorTest.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessorTest.kt @@ -1242,4 +1242,78 @@ class GroupsV2StateProcessorTest { fail("No exception thrown") } + + @Test(expected = GroupNotAMemberException::class) + fun skipP2PChangeAfterServerNotAMemberWhenChangeDoesNotTouchSelf() { + given { + localState( + revision = 1, + members = selfAndOthers, + active = true + ) + } + + every { groupsV2API.getGroupJoinedAt(any()) } returns NetworkResult.StatusCodeError(NotInGroupException()) + every { groupsV2API.getGroupAsResult(any(), any()) } returns NetworkResult.StatusCodeError(NotInGroupException()) + justRun { profileAndMessageHelper.leaveGroupLocally(any()) } + + val signedChange = DecryptedGroupChange( + revision = 3, + newTitle = DecryptedString("Breaking Signal for Science"), + newDescription = DecryptedString("We break stuff, because we must.") + ) + + try { + processor.updateLocalGroupToRevision( + targetRevision = 3, + timestamp = 0, + signedGroupChange = signedChange, + serverGuid = UUID.randomUUID().toString() + ) + } finally { + verify(exactly = 0) { groupTable.update(any(), any(), any(), any()) } + verify { profileAndMessageHelper.leaveGroupLocally(serviceIds) } + } + } + + @Test + fun applyP2PChangeThatRemovesSelfAfterServerNotAMember() { + given { + localState( + revision = 1, + members = selfAndOthers, + active = true + ) + expectTableUpdate = true + } + + every { groupsV2API.getGroupJoinedAt(any()) } returns NetworkResult.StatusCodeError(NotInGroupException()) + every { groupsV2API.getGroupAsResult(any(), any()) } returns NetworkResult.StatusCodeError(NotInGroupException()) + + val signedChange = DecryptedGroupChange( + revision = 3, + editorServiceIdBytes = otherAci.toByteString(), + deleteMembers = listOf(selfAci.toByteString()) + ) + + val result = processor.updateLocalGroupToRevision( + targetRevision = 3, + timestamp = 0, + signedGroupChange = signedChange, + serverGuid = UUID.randomUUID().toString() + ) + + assertThat(result.updateStatus, "local should force-apply the kick") + .isEqualTo(GroupUpdateResult.UpdateStatus.GROUP_UPDATED) + assertThat(result.latestServer) + .isNotNull() + .transform { + assertThat(it.revision, "revision matches peer change").isEqualTo(3) + val memberBytes = it.members.map { member -> member.aciBytes } + assertThat(memberBytes, "self removed from members").transform { bytes -> bytes.none { it == selfAci.toByteString() } }.isEqualTo(true) + } + + verify { groupTable.update(masterKey, result.latestServer!!, null) } + verify(exactly = 0) { profileAndMessageHelper.persistLearnedProfileKeys(any()) } + } }