Show warning when group changes would clear member labels.

This commit is contained in:
jeffrey-signal
2026-03-11 15:34:42 -04:00
parent 35cf24b577
commit 73f5a3398c
13 changed files with 458 additions and 38 deletions

View File

@@ -0,0 +1,137 @@
/*
* Copyright 2025 Signal Messenger, LLC
* SPDX-License-Identifier: AGPL-3.0-only
*/
package org.thoughtcrime.securesms.components.settings.conversation.permissions
import androidx.lifecycle.MutableLiveData
import io.mockk.every
import io.mockk.mockk
import io.mockk.verify
import org.junit.Assert.assertEquals
import org.junit.Rule
import org.junit.Test
import org.thoughtcrime.securesms.groups.GroupAccessControl
import org.thoughtcrime.securesms.groups.GroupId
import org.thoughtcrime.securesms.groups.LiveGroup
import org.thoughtcrime.securesms.util.livedata.LiveDataRule
import org.thoughtcrime.securesms.util.livedata.LiveDataTestUtil
class PermissionsSettingsViewModelTest {
@get:Rule
val liveDataRule = LiveDataRule()
private val groupId = mockk<GroupId.V2>()
private val repository = mockk<PermissionsSettingsRepository>(relaxUnitFun = true)
private fun createViewModel(
memberLabelAccessControl: GroupAccessControl = GroupAccessControl.ONLY_ADMINS,
nonAdminMembersHaveLabels: Boolean = true
): PermissionsSettingsViewModel {
val liveGroup = mockk<LiveGroup> {
every { isSelfAdmin } returns MutableLiveData(false)
every { membershipAdditionAccessControl } returns MutableLiveData(GroupAccessControl.ONLY_ADMINS)
every { attributesAccessControl } returns MutableLiveData(GroupAccessControl.ONLY_ADMINS)
every { isAnnouncementGroup } returns MutableLiveData(false)
every { this@mockk.memberLabelAccessControl } returns MutableLiveData(memberLabelAccessControl)
}
every { repository.hasNonAdminMembersWithLabels(groupId) } returns nonAdminMembersHaveLabels
return PermissionsSettingsViewModel(
groupId = groupId,
repository = repository,
liveGroup = liveGroup
)
}
@Test
fun `onMemberLabelPermissionChangeRequested immediately applies 'only admins' change when there are no non-admin members with labels`() {
val viewModel = createViewModel(
memberLabelAccessControl = GroupAccessControl.ALL_MEMBERS,
nonAdminMembersHaveLabels = false
)
viewModel.onMemberLabelPermissionChangeRequested(nonAdminCanSetMemberLabel = false)
verify {
repository.applyMemberLabelRightsChange(
groupId = groupId,
newRights = GroupAccessControl.ONLY_ADMINS,
errorCallback = any()
)
}
LiveDataTestUtil.assertNoValue(viewModel.events)
}
@Test
fun `onMemberLabelPermissionChangeRequested immediately applies 'all members' change when there are no non-admin members with labels`() {
val viewModel = createViewModel(
memberLabelAccessControl = GroupAccessControl.ONLY_ADMINS,
nonAdminMembersHaveLabels = false
)
viewModel.onMemberLabelPermissionChangeRequested(nonAdminCanSetMemberLabel = true)
verify {
repository.applyMemberLabelRightsChange(
groupId = groupId,
newRights = GroupAccessControl.ALL_MEMBERS,
errorCallback = any()
)
}
LiveDataTestUtil.assertNoValue(viewModel.events)
}
@Test
fun `onMemberLabelPermissionChangeRequested displays warning when restricting to 'only admins' and some non-admin members have labels`() {
val viewModel = createViewModel(
memberLabelAccessControl = GroupAccessControl.ALL_MEMBERS,
nonAdminMembersHaveLabels = true
)
viewModel.onMemberLabelPermissionChangeRequested(nonAdminCanSetMemberLabel = false)
assertEquals(
PermissionsSettingsEvents.ShowMemberLabelsWillBeRemovedWarning,
LiveDataTestUtil.observeAndGetOneValue(viewModel.events)
)
verify(exactly = 0) {
repository.applyMemberLabelRightsChange(any(), any(), any())
}
}
@Test
fun `onMemberLabelPermissionChangeRequested immediately applies 'all members' change when some non-admin members have labels`() {
val viewModel = createViewModel(memberLabelAccessControl = GroupAccessControl.ALL_MEMBERS, nonAdminMembersHaveLabels = true)
viewModel.onMemberLabelPermissionChangeRequested(nonAdminCanSetMemberLabel = true)
verify {
repository.applyMemberLabelRightsChange(
groupId = groupId,
newRights = GroupAccessControl.ALL_MEMBERS,
errorCallback = any()
)
}
LiveDataTestUtil.assertNoValue(viewModel.events)
}
@Test
fun `onRestrictMemberLabelsToAdminsConfirmed applies 'only admins' change`() {
val viewModel = createViewModel(memberLabelAccessControl = GroupAccessControl.ALL_MEMBERS, nonAdminMembersHaveLabels = true)
viewModel.onRestrictMemberLabelsToAdminsConfirmed()
verify {
repository.applyMemberLabelRightsChange(
groupId = groupId,
newRights = GroupAccessControl.ONLY_ADMINS,
errorCallback = any()
)
}
LiveDataTestUtil.assertNoValue(viewModel.events)
}
}

View File

@@ -95,6 +95,18 @@ class GroupChangeData(private val revision: Int, private val groupOperations: Gr
fun modifyRole(serviceId: ServiceId, role: Member.Role) {
actionsBuilder.modifyMemberRoles += GroupChange.Actions.ModifyMemberRoleAction(userId = groupOperations.encryptServiceId(serviceId), role = role)
}
fun clearMemberLabel(serviceId: ServiceId) {
actionsBuilder.modifyMemberLabels += GroupChange.Actions.ModifyMemberLabelAction(
userId = groupOperations.encryptServiceId(serviceId),
labelEmoji = okio.ByteString.EMPTY,
labelString = okio.ByteString.EMPTY
)
}
fun changeMemberLabelAccess(access: AccessControl.AccessRequired) {
actionsBuilder.modifyMemberLabelAccess = GroupChange.Actions.ModifyMemberLabelAccessControlAction(memberLabelAccess = access)
}
}
class GroupStateTestData(private val masterKey: GroupMasterKey, private val groupOperations: GroupsV2Operations.GroupOperations? = null) {

View File

@@ -28,17 +28,20 @@ import org.signal.libsignal.protocol.logging.SignalProtocolLogger
import org.signal.libsignal.protocol.logging.SignalProtocolLoggerProvider
import org.signal.libsignal.zkgroup.groups.GroupMasterKey
import org.signal.libsignal.zkgroup.groups.GroupSecretParams
import org.signal.storageservice.storage.protos.groups.AccessControl
import org.signal.storageservice.storage.protos.groups.GroupChangeResponse
import org.signal.storageservice.storage.protos.groups.Member
import org.signal.storageservice.storage.protos.groups.local.DecryptedGroup
import org.thoughtcrime.securesms.TestZkGroupServer
import org.thoughtcrime.securesms.database.GroupStateTestData
import org.thoughtcrime.securesms.database.GroupTable
import org.thoughtcrime.securesms.database.RecipientDatabaseTestUtils
import org.thoughtcrime.securesms.database.model.databaseprotos.member
import org.thoughtcrime.securesms.groups.v2.GroupCandidateHelper
import org.thoughtcrime.securesms.keyvalue.SignalStore
import org.thoughtcrime.securesms.logging.CustomSignalProtocolLogger
import org.thoughtcrime.securesms.recipients.Recipient
import org.thoughtcrime.securesms.recipients.RecipientId
import org.thoughtcrime.securesms.testutil.MockAppDependenciesRule
import org.thoughtcrime.securesms.testutil.SystemOutLogger
import org.thoughtcrime.securesms.util.RemoteConfig
@@ -62,6 +65,7 @@ class GroupManagerV2Test_edit {
val selfPni: PNI = PNI.from(UUID.randomUUID())
val serviceIds: ServiceIds = ServiceIds(selfAci, selfPni)
val otherAci: ACI = ACI.from(UUID.randomUUID())
val otherAci2: ACI = ACI.from(UUID.randomUUID())
}
@get:Rule
@@ -83,6 +87,8 @@ class GroupManagerV2Test_edit {
mockkObject(RemoteConfig)
mockkStatic(RemoteConfig::class)
mockkObject(SignalStore)
mockkObject(Recipient)
mockkStatic(Recipient::class)
every { RemoteConfig.internalUser } returns false
ThreadUtil.enforceAssertions = false
@@ -126,6 +132,7 @@ class GroupManagerV2Test_edit {
every { groupTable.update(any<GroupId.V2>(), any(), any()) } returns Unit
every { sendGroupUpdateHelper.sendGroupUpdate(masterKey, any(), any(), any()) } returns GroupManagerV2.RecipientAndThread(Recipient.UNKNOWN, 1)
every { groupsV2API.patchGroup(any(), any(), any()) } returns GroupChangeResponse(group_change = data.groupChange!!)
every { Recipient.externalGroupExact(groupId) } returns RecipientDatabaseTestUtils.createRecipient(resolved = true)
}
private fun editGroup(perform: GroupManagerV2.GroupEditor.() -> Unit) {
@@ -165,4 +172,142 @@ class GroupManagerV2Test_edit {
assertThat(patchedGroup.members.find { it.aciBytes == otherAci.toByteString() }?.role, "Other is now an admin in the group").isEqualTo(Member.Role.ADMINISTRATOR)
}
}
@Test
fun `demoting an admin who has a label clears the label when label access is restricted to only admins`() {
val otherRecipientId = RecipientId.from(200)
val otherRecipient = RecipientDatabaseTestUtils.createRecipient(recipientId = otherRecipientId, serviceId = otherAci, resolved = true)
every { Recipient.resolved(otherRecipientId) } returns otherRecipient
given {
localState(
revision = 5,
accessControl = AccessControl(memberLabel = AccessControl.AccessRequired.ADMINISTRATOR),
members = listOf(
member(selfAci, role = Member.Role.ADMINISTRATOR),
member(otherAci, role = Member.Role.ADMINISTRATOR, labelString = "Team Lead")
)
)
groupChange(6) {
source(selfAci)
modifyRole(otherAci, Member.Role.DEFAULT)
clearMemberLabel(otherAci)
}
}
editGroup {
setMemberAdmin(otherRecipientId, false)
}
then { patchedGroup ->
val other = patchedGroup.members.find { it.aciBytes == otherAci.toByteString() }!!
assertThat(other.role, "Other is demoted to default").isEqualTo(Member.Role.DEFAULT)
assertThat(other.labelString, "Label text is cleared").isEqualTo("")
assertThat(other.labelEmoji, "Label emoji is cleared").isEqualTo("")
}
}
@Test
fun `demoting an admin who has a label does not clear the label when label access is open to all members`() {
val otherRecipientId = RecipientId.from(201)
val otherRecipient = RecipientDatabaseTestUtils.createRecipient(recipientId = otherRecipientId, serviceId = otherAci, resolved = true)
every { Recipient.resolved(otherRecipientId) } returns otherRecipient
given {
localState(
revision = 5,
accessControl = AccessControl(memberLabel = AccessControl.AccessRequired.MEMBER),
members = listOf(
member(selfAci, role = Member.Role.ADMINISTRATOR),
member(otherAci, role = Member.Role.ADMINISTRATOR, labelString = "Team Lead")
)
)
groupChange(6) {
source(selfAci)
modifyRole(otherAci, Member.Role.DEFAULT)
}
}
editGroup {
setMemberAdmin(otherRecipientId, false)
}
then { patchedGroup ->
val other = patchedGroup.members.find { it.aciBytes == otherAci.toByteString() }!!
assertThat(other.role, "Other is demoted to default").isEqualTo(Member.Role.DEFAULT)
assertThat(other.labelString, "Label text is preserved").isEqualTo("Team Lead")
}
}
@Test
fun `restricting label access to admins only clears non-admins that have labels set`() {
given {
localState(
revision = 5,
accessControl = AccessControl(memberLabel = AccessControl.AccessRequired.MEMBER),
members = listOf(
member(selfAci, role = Member.Role.ADMINISTRATOR),
member(otherAci, role = Member.Role.DEFAULT, labelString = "Foo"),
member(otherAci2, role = Member.Role.DEFAULT, labelString = "Bar")
)
)
groupChange(6) {
source(selfAci)
changeMemberLabelAccess(AccessControl.AccessRequired.ADMINISTRATOR)
clearMemberLabel(otherAci)
clearMemberLabel(otherAci2)
}
}
editGroup {
updateMemberLabelRights(GroupAccessControl.ONLY_ADMINS)
}
then { patchedGroup ->
val other = patchedGroup.members.find { it.aciBytes == otherAci.toByteString() }!!
assertThat(other.labelString, "Other's label text is cleared").isEqualTo("")
assertThat(other.labelEmoji, "Other's label emoji is cleared").isEqualTo("")
val other2 = patchedGroup.members.find { it.aciBytes == otherAci2.toByteString() }!!
assertThat(other2.labelString, "Other2's label text is cleared").isEqualTo("")
assertThat(other2.labelEmoji, "Other2's label emoji is cleared").isEqualTo("")
}
}
@Test
fun `restricting label access to admins only does not modify labels of admins that have labels set`() {
given {
localState(
revision = 5,
accessControl = AccessControl(memberLabel = AccessControl.AccessRequired.MEMBER),
members = listOf(
member(selfAci, role = Member.Role.ADMINISTRATOR),
member(otherAci, role = Member.Role.ADMINISTRATOR, labelString = "Foo"),
member(otherAci2, role = Member.Role.ADMINISTRATOR, labelString = "Bar")
)
)
groupChange(6) {
source(selfAci)
changeMemberLabelAccess(AccessControl.AccessRequired.ADMINISTRATOR)
}
}
editGroup {
updateMemberLabelRights(GroupAccessControl.ONLY_ADMINS)
}
then { patchedGroup ->
val other = patchedGroup.members.find { it.aciBytes == otherAci.toByteString() }!!
assertThat(other.labelString, "Other's label text is preserved").isEqualTo("Foo")
val other2 = patchedGroup.members.find { it.aciBytes == otherAci2.toByteString() }!!
assertThat(other2.labelString, "Other2's label text is preserved").isEqualTo("Bar")
}
}
}