From 7705b7dec0ac2c8c77e9f0f34116049a8c30f3f2 Mon Sep 17 00:00:00 2001 From: Alex Hart Date: Mon, 3 Aug 2026 16:43:31 -0300 Subject: [PATCH] Show all similarly-named recipients in one-to-one spoofing reviews. Co-authored-by: Greyson Parrelli --- .../spoofing/ReviewCardViewModel.java | 20 +-- .../spoofing/ReviewCardViewModelTest.kt | 121 ++++++++++++++++++ build.gradle.kts | 3 + 3 files changed, 134 insertions(+), 10 deletions(-) create mode 100644 app/src/test/java/org/thoughtcrime/securesms/profiles/spoofing/ReviewCardViewModelTest.kt diff --git a/app/src/main/java/org/thoughtcrime/securesms/profiles/spoofing/ReviewCardViewModel.java b/app/src/main/java/org/thoughtcrime/securesms/profiles/spoofing/ReviewCardViewModel.java index 21c92eff98..d7a961b5a4 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/profiles/spoofing/ReviewCardViewModel.java +++ b/app/src/main/java/org/thoughtcrime/securesms/profiles/spoofing/ReviewCardViewModel.java @@ -4,6 +4,7 @@ import kotlin.Pair; import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import androidx.annotation.VisibleForTesting; import androidx.annotation.WorkerThread; import androidx.lifecycle.LiveData; import androidx.lifecycle.MutableLiveData; @@ -31,16 +32,15 @@ public class ReviewCardViewModel extends ViewModel { private final boolean isGroupThread; public ReviewCardViewModel(@NonNull ReviewCardRepository repository, @Nullable GroupId groupId) { - final LiveData isSelfGroupAdmin; + this(repository, + groupId != null, + groupId != null ? new LiveGroup(groupId).getRecipientIsAdmin(Recipient.self().getId()) + : new DefaultValueLiveData<>(false)); + } - if (groupId != null) { - LiveGroup liveGroup = new LiveGroup(groupId); - isSelfGroupAdmin = liveGroup.getRecipientIsAdmin(Recipient.self().getId()); - } else { - isSelfGroupAdmin = new DefaultValueLiveData<>(false); - } - - this.isGroupThread = groupId != null; + @VisibleForTesting + ReviewCardViewModel(@NonNull ReviewCardRepository repository, boolean isGroupThread, @NonNull LiveData isSelfGroupAdmin) { + this.isGroupThread = isGroupThread; this.repository = repository; this.reviewRecipients = new MutableLiveData<>(); @@ -87,7 +87,7 @@ public class ReviewCardViewModel extends ViewModel { @WorkerThread private @NonNull List transformReviewRecipients(boolean isSelfGroupAdmin, @NonNull List reviewRecipients) { return reviewRecipients.stream() - .filter(r -> repository.loadGroupsInCommonCount(r) > 0) + .filter(r -> !isGroupThread || repository.loadGroupsInCommonCount(r) > 0) .map(r -> new ReviewCard(r, repository.loadGroupsInCommonCount(r) - (isGroupThread ? 1 : 0), getCardType(r), diff --git a/app/src/test/java/org/thoughtcrime/securesms/profiles/spoofing/ReviewCardViewModelTest.kt b/app/src/test/java/org/thoughtcrime/securesms/profiles/spoofing/ReviewCardViewModelTest.kt new file mode 100644 index 0000000000..ae9cc94fbb --- /dev/null +++ b/app/src/test/java/org/thoughtcrime/securesms/profiles/spoofing/ReviewCardViewModelTest.kt @@ -0,0 +1,121 @@ +package org.thoughtcrime.securesms.profiles.spoofing + +import android.app.Application +import android.content.Context +import androidx.lifecycle.LiveData +import androidx.lifecycle.Observer +import assertk.assertThat +import assertk.assertions.containsExactly +import assertk.assertions.isEqualTo +import assertk.assertions.isTrue +import io.mockk.every +import io.mockk.mockk +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import org.thoughtcrime.securesms.recipients.Recipient +import org.thoughtcrime.securesms.recipients.RecipientId +import org.thoughtcrime.securesms.util.DefaultValueLiveData +import org.thoughtcrime.securesms.util.livedata.LiveDataRule +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicReference + +@RunWith(RobolectricTestRunner::class) +@Config(manifest = Config.NONE, application = Application::class) +class ReviewCardViewModelTest { + + @get:Rule + val liveDataRule = LiveDataRule() + + private val repository = FakeReviewCardRepository() + + /** + * Regression test for AND-9680: an impersonator sending a one-to-one message request has no groups in + * common with us by construction, and must not be filtered out of the review. + */ + @Test + fun `given a one-to-one thread, when a similarly named recipient has no groups in common, then it still gets a card`() { + val impersonator = repository.addReviewRecipient(groupsInCommon = 0) + val contact = repository.addReviewRecipient(groupsInCommon = 4, systemContact = true) + + val cards = loadCards(isGroupThread = false) + + assertThat(cards.map { it.reviewRecipient }).containsExactly(impersonator.recipient, contact.recipient) + assertThat(cards[0].inCommonGroupsCount).isEqualTo(0) + assertThat(cards[0].cardType).isEqualTo(ReviewCard.CardType.REQUEST) + assertThat(cards[1].inCommonGroupsCount).isEqualTo(4) + assertThat(cards[1].cardType).isEqualTo(ReviewCard.CardType.YOUR_CONTACT) + } + + /** + * The card count excludes the group being reviewed, so a member with no groups in common would render as + * "-1 other groups in common". Those members stay filtered out. + */ + @Test + fun `given a group thread, when a similarly named member has no groups in common, then it does not get a card`() { + repository.addReviewRecipient(groupsInCommon = 0) + val member = repository.addReviewRecipient(groupsInCommon = 2) + + val cards = loadCards(isGroupThread = true) + + assertThat(cards.map { it.reviewRecipient }).containsExactly(member.recipient) + assertThat(cards[0].inCommonGroupsCount).isEqualTo(1) + assertThat(cards[0].cardType).isEqualTo(ReviewCard.CardType.MEMBER) + } + + private fun loadCards(isGroupThread: Boolean): List { + val viewModel = ReviewCardViewModel(repository, isGroupThread, DefaultValueLiveData(false)) + + return viewModel.reviewCards.observeNextValue { repository.emitLoadedRecipients() } + } + + private fun LiveData.observeNextValue(trigger: () -> Unit): T { + val value = AtomicReference() + val latch = CountDownLatch(1) + val observer = Observer { + value.set(it) + latch.countDown() + } + + observeForever(observer) + trigger() + + assertThat(latch.await(10, TimeUnit.SECONDS), "Timed out waiting for a value").isTrue() + removeObserver(observer) + + return value.get() + } + + private class FakeReviewCardRepository : ReviewCardRepository(mockk(relaxed = true), RecipientId.from(1)) { + + private val groupsInCommonCounts = LinkedHashMap() + private var loadedListener: OnRecipientsLoadedListener? = null + + fun addReviewRecipient(groupsInCommon: Int, systemContact: Boolean = false): ReviewRecipient { + val recipient = ReviewRecipient( + mockk { + every { isSystemContact } returns systemContact + } + ) + + groupsInCommonCounts[recipient] = groupsInCommon + + return recipient + } + + fun emitLoadedRecipients() { + loadedListener!!.onRecipientsLoaded(groupsInCommonCounts.keys.toList()) + } + + public override fun loadRecipients(onRecipientsLoadedListener: OnRecipientsLoadedListener) { + loadedListener = onRecipientsLoadedListener + } + + public override fun loadGroupsInCommonCount(reviewRecipient: ReviewRecipient): Int { + return groupsInCommonCounts.getValue(reviewRecipient) + } + } +} diff --git a/build.gradle.kts b/build.gradle.kts index 11c9af79e5..b82220cbde 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -59,6 +59,9 @@ subprojects { tasks.withType().configureEach { maxParallelForks = (Runtime.getRuntime().availableProcessors() / 4).coerceAtLeast(1) + + // Raised for robolectric + maxHeapSize = "2g" } }