Signal/Android
1
0
Fork 0
mirror of https://github.com/signalapp/Signal-Android.git synced 2026-07-11 16:27:04 +01:00
Code Issues Packages Projects Releases Wiki Activity

Do not force-apply P2P group changes unless the change adds or removes us.

Browse Source 
Co-authored-by: Greyson Parrelli <greyson@signal.org>
This commit is contained in:
Cody Henthorne
2026-04-15 14:33:11 -04:00
committed by jeffrey-signal
parent a797bbf850
commit ab0102a372
2 changed files with 132 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/src/main/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessor.kt
+58 -6
View File
@@ -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
app/src/test/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessorTest.kt
+74
View File
@@ -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<GroupMasterKey>(), any<DecryptedGroup>(), 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<ProfileKeySet>()) }
}
}