diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/settings/conversation/permissions/PermissionsSettingsEvents.kt b/app/src/main/java/org/thoughtcrime/securesms/components/settings/conversation/permissions/PermissionsSettingsEvents.kt index e721632ae5..af0faa9a95 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/settings/conversation/permissions/PermissionsSettingsEvents.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/components/settings/conversation/permissions/PermissionsSettingsEvents.kt @@ -4,4 +4,5 @@ import org.thoughtcrime.securesms.groups.ui.GroupChangeFailureReason sealed class PermissionsSettingsEvents { class GroupChangeError(val reason: GroupChangeFailureReason) : PermissionsSettingsEvents() + object ShowMemberLabelsWillBeRemovedWarning : PermissionsSettingsEvents() } diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/settings/conversation/permissions/PermissionsSettingsFragment.kt b/app/src/main/java/org/thoughtcrime/securesms/components/settings/conversation/permissions/PermissionsSettingsFragment.kt index f647a5f671..4d4d93289c 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/settings/conversation/permissions/PermissionsSettingsFragment.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/components/settings/conversation/permissions/PermissionsSettingsFragment.kt @@ -3,6 +3,7 @@ package org.thoughtcrime.securesms.components.settings.conversation.permissions import android.widget.Toast import androidx.annotation.StringRes import androidx.fragment.app.viewModels +import com.google.android.material.dialog.MaterialAlertDialogBuilder import org.thoughtcrime.securesms.R import org.thoughtcrime.securesms.components.settings.DSLConfiguration import org.thoughtcrime.securesms.components.settings.DSLSettingsFragment @@ -39,6 +40,7 @@ class PermissionsSettingsFragment : DSLSettingsFragment( viewModel.events.observe(viewLifecycleOwner) { event -> when (event) { is PermissionsSettingsEvents.GroupChangeError -> handleGroupChangeError(event) + is PermissionsSettingsEvents.ShowMemberLabelsWillBeRemovedWarning -> showMemberLabelsWillBeRemovedDialog() } } } @@ -94,13 +96,26 @@ class PermissionsSettingsFragment : DSLSettingsFragment( selected = getSelected(state.nonAdminCanSetMemberLabel), confirmAction = true, onSelected = { selectedIndex -> - viewModel.setNonAdminCanSetMemberLabel(selectedIndex == 1) + if (selectedIndex >= 0) { + viewModel.onMemberLabelPermissionChangeRequested(nonAdminCanSetMemberLabel = selectedIndex == 1) + } } ) } } } + private fun showMemberLabelsWillBeRemovedDialog() { + MaterialAlertDialogBuilder(requireContext()) + .setTitle(R.string.PermissionsSettingsFragment__member_labels_will_be_cleared_title) + .setMessage(R.string.PermissionsSettingsFragment__member_labels_will_be_cleared_body) + .setPositiveButton(R.string.PermissionsSettingsFragment__change_permission) { _, _ -> + viewModel.onRestrictMemberLabelsToAdminsConfirmed() + } + .setNegativeButton(android.R.string.cancel, null) + .show() + } + @StringRes private fun getSelected(isNonAdminAllowed: Boolean): Int { return if (isNonAdminAllowed) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/settings/conversation/permissions/PermissionsSettingsRepository.kt b/app/src/main/java/org/thoughtcrime/securesms/components/settings/conversation/permissions/PermissionsSettingsRepository.kt index cefa738fa1..6d1a04ad85 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/settings/conversation/permissions/PermissionsSettingsRepository.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/components/settings/conversation/permissions/PermissionsSettingsRepository.kt @@ -3,6 +3,9 @@ package org.thoughtcrime.securesms.components.settings.conversation.permissions import android.content.Context import org.signal.core.util.concurrent.SignalExecutors import org.signal.core.util.logging.Log +import org.signal.core.util.orNull +import org.thoughtcrime.securesms.database.GroupTable +import org.thoughtcrime.securesms.database.SignalDatabase import org.thoughtcrime.securesms.groups.GroupAccessControl import org.thoughtcrime.securesms.groups.GroupChangeException import org.thoughtcrime.securesms.groups.GroupId @@ -13,7 +16,10 @@ import java.io.IOException private val TAG = Log.tag(PermissionsSettingsRepository::class.java) -class PermissionsSettingsRepository(private val context: Context) { +class PermissionsSettingsRepository( + private val context: Context, + private val groupTable: GroupTable = SignalDatabase.groups +) { fun applyMembershipRightsChange(groupId: GroupId, newRights: GroupAccessControl, error: GroupChangeErrorCallback) { SignalExecutors.UNBOUNDED.execute { @@ -57,6 +63,12 @@ class PermissionsSettingsRepository(private val context: Context) { } } + fun hasNonAdminMembersWithLabels(groupId: GroupId): Boolean { + val v2GroupId = groupId.v2OrNull() ?: return false + val group = groupTable.getGroup(v2GroupId).orNull() ?: return false + return group.requireV2GroupProperties().nonAdminMembersWithLabels().isNotEmpty() + } + fun applyMemberLabelRightsChange(groupId: GroupId, newRights: GroupAccessControl, errorCallback: GroupChangeErrorCallback) { SignalExecutors.UNBOUNDED.execute { try { diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/settings/conversation/permissions/PermissionsSettingsViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/components/settings/conversation/permissions/PermissionsSettingsViewModel.kt index 03ff270dc1..1064ac4b68 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/settings/conversation/permissions/PermissionsSettingsViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/components/settings/conversation/permissions/PermissionsSettingsViewModel.kt @@ -11,11 +11,11 @@ import org.thoughtcrime.securesms.util.livedata.Store class PermissionsSettingsViewModel( private val groupId: GroupId, - private val repository: PermissionsSettingsRepository + private val repository: PermissionsSettingsRepository, + liveGroup: LiveGroup = LiveGroup(groupId) ) : ViewModel() { private val store = Store(PermissionsSettingsState()) - private val liveGroup = LiveGroup(groupId) private val internalEvents = SingleLiveEvent() val state: LiveData = store.stateLiveData @@ -61,7 +61,17 @@ class PermissionsSettingsViewModel( } } - fun setNonAdminCanSetMemberLabel(nonAdminCanSetMemberLabel: Boolean) { + fun onMemberLabelPermissionChangeRequested(nonAdminCanSetMemberLabel: Boolean) { + if (!nonAdminCanSetMemberLabel && repository.hasNonAdminMembersWithLabels(groupId)) { + internalEvents.postValue(PermissionsSettingsEvents.ShowMemberLabelsWillBeRemovedWarning) + } else { + setNonAdminCanSetMemberLabel(nonAdminCanSetMemberLabel) + } + } + + fun onRestrictMemberLabelsToAdminsConfirmed() = setNonAdminCanSetMemberLabel(false) + + private fun setNonAdminCanSetMemberLabel(nonAdminCanSetMemberLabel: Boolean) { repository.applyMemberLabelRightsChange( groupId = groupId, newRights = nonAdminCanSetMemberLabel.asGroupAccessControl() diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/GroupTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/GroupTable.kt index e67cae4c5f..f582fc05b1 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/GroupTable.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/GroupTable.kt @@ -42,8 +42,10 @@ import org.signal.libsignal.zkgroup.groups.GroupMasterKey import org.signal.libsignal.zkgroup.groups.GroupSecretParams import org.signal.libsignal.zkgroup.groupsend.GroupSendEndorsement import org.signal.libsignal.zkgroup.groupsend.GroupSendFullToken +import org.signal.storageservice.storage.protos.groups.AccessControl import org.signal.storageservice.storage.protos.groups.Member import org.signal.storageservice.storage.protos.groups.local.DecryptedGroup +import org.signal.storageservice.storage.protos.groups.local.DecryptedMember import org.signal.storageservice.storage.protos.groups.local.DecryptedPendingMember import org.thoughtcrime.securesms.contacts.paged.ContactSearchSortOrder import org.thoughtcrime.securesms.contacts.paged.collections.ContactSearchIterator @@ -1295,6 +1297,24 @@ class GroupTable(context: Context?, databaseHelper: SignalDatabase?) : } } } + + fun nonAdminMembersWithLabels(): List { + return decryptedGroup.members + .filter { it.role != Member.Role.ADMINISTRATOR && it.hasLabel() } + } + + /** + * Returns true if demoting [aci] from admin should cause their member label to be cleared. + */ + fun adminDemotionClearsLabel(aci: ACI): Boolean { + val accessRequired = decryptedGroup.accessControl?.memberLabel ?: AccessControl.AccessRequired.UNKNOWN + return when { + accessRequired != AccessControl.AccessRequired.ADMINISTRATOR -> false + else -> decryptedGroup.members.findMemberByAci(aci).orNull()?.hasLabel() == true + } + } + + private fun DecryptedMember.hasLabel(): Boolean = labelString.isNotBlank() || labelEmoji.isNotBlank() } @Throws(BadGroupIdException::class) diff --git a/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManagerV2.java b/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManagerV2.java index 47cdcd0fa1..63540d2dbb 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManagerV2.java +++ b/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManagerV2.java @@ -322,11 +322,27 @@ final class GroupManagerV2 { } @WorkerThread - @NonNull GroupManager.GroupActionResult updateMemberLabelRights(@NonNull GroupAccessControl newRights) + @NonNull + GroupManager.GroupActionResult updateMemberLabelRights(@NonNull GroupAccessControl newRights) throws GroupChangeFailedException, GroupInsufficientRightsException, IOException, GroupNotAMemberException { - AccessControl.AccessRequired accessRequired = rightsToAccessControl(newRights); - return commitChangeWithConflictResolution(selfAci, groupOperations.createChangeMemberLabelRights(accessRequired)); + AccessControl.AccessRequired newAccess = rightsToAccessControl(newRights); + GroupChange.Actions.Builder change = groupOperations.createChangeMemberLabelRights(newAccess); + DecryptedGroup decryptedGroup = v2GroupProperties.getDecryptedGroup(); + AccessControl.AccessRequired currentAccess = decryptedGroup.accessControl != null ? decryptedGroup.accessControl.memberLabel : AccessControl.AccessRequired.UNKNOWN; + + if (newAccess == AccessControl.AccessRequired.ADMINISTRATOR && currentAccess != AccessControl.AccessRequired.ADMINISTRATOR) { + List membersWithLabelsToClear = v2GroupProperties.nonAdminMembersWithLabels() + .stream() + .map(member -> ACI.parseOrNull(member.aciBytes)) + .filter(Objects::nonNull) + .collect(Collectors.toList()); + + if (!membersWithLabelsToClear.isEmpty()) { + change.modifyMemberLabels(groupOperations.createRemoveMemberLabelsChange(membersWithLabelsToClear).modifyMemberLabels); + } + } + return commitChangeWithConflictResolution(selfAci, change); } @WorkerThread @@ -338,7 +354,7 @@ final class GroupManagerV2 { @WorkerThread @NonNull GroupManager.GroupActionResult updateGroupTitleDescriptionAndAvatar(@Nullable String title, @Nullable String description, @Nullable byte[] avatarBytes, boolean avatarChanged) - throws GroupChangeFailedException, GroupInsufficientRightsException, IOException, GroupNotAMemberException + throws GroupChangeFailedException, GroupInsufficientRightsException, IOException, GroupNotAMemberException { try { GroupChange.Actions.Builder change = title != null ? groupOperations.createModifyGroupTitle(title) @@ -409,8 +425,14 @@ final class GroupManagerV2 { boolean admin) throws GroupChangeFailedException, GroupInsufficientRightsException, IOException, GroupNotAMemberException { - Recipient recipient = Recipient.resolved(recipientId); - return commitChangeWithConflictResolution(selfAci, groupOperations.createChangeMemberRole(recipient.requireAci(), admin ? Member.Role.ADMINISTRATOR : Member.Role.DEFAULT)); + Recipient recipient = Recipient.resolved(recipientId); + ACI recipientAci = recipient.requireAci(); + + GroupChange.Actions.Builder change = groupOperations.createChangeMemberRole(recipientAci, admin ? Member.Role.ADMINISTRATOR : Member.Role.DEFAULT); + if (!admin && v2GroupProperties.adminDemotionClearsLabel(recipientAci)) { + change.modifyMemberLabels(groupOperations.createRemoveMemberLabelsChange(Collections.singletonList(recipientAci)).modifyMemberLabels); + } + return commitChangeWithConflictResolution(selfAci, change); } @WorkerThread @@ -1261,7 +1283,7 @@ final class GroupManagerV2 { throw new GroupChangeFailedException("Unable to cancel group join request after conflicts"); } -} + } private abstract static class LockOwner implements Closeable { final Closeable lock; @@ -1339,16 +1361,11 @@ final class GroupManagerV2 { } private static @NonNull AccessControl.AccessRequired rightsToAccessControl(@NonNull GroupAccessControl rights) { - switch (rights){ - case ALL_MEMBERS: - return AccessControl.AccessRequired.MEMBER; - case ONLY_ADMINS: - return AccessControl.AccessRequired.ADMINISTRATOR; - case NO_ONE: - return AccessControl.AccessRequired.UNSATISFIABLE; - default: - throw new AssertionError(); - } + return switch (rights) { + case ALL_MEMBERS -> AccessControl.AccessRequired.MEMBER; + case ONLY_ADMINS -> AccessControl.AccessRequired.ADMINISTRATOR; + case NO_ONE -> AccessControl.AccessRequired.UNSATISFIABLE; + }; } static class RecipientAndThread { diff --git a/app/src/main/java/org/thoughtcrime/securesms/recipients/ui/bottomsheet/RecipientDialogRepository.java b/app/src/main/java/org/thoughtcrime/securesms/recipients/ui/bottomsheet/RecipientDialogRepository.java index e2528c3232..808da67248 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/recipients/ui/bottomsheet/RecipientDialogRepository.java +++ b/app/src/main/java/org/thoughtcrime/securesms/recipients/ui/bottomsheet/RecipientDialogRepository.java @@ -6,7 +6,9 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.core.util.Consumer; +import org.signal.core.models.ServiceId; import org.signal.core.util.concurrent.SignalExecutors; +import org.signal.core.util.concurrent.SimpleTask; import org.signal.core.util.logging.Log; import org.thoughtcrime.securesms.contacts.sync.ContactDiscovery; import org.thoughtcrime.securesms.database.GroupTable; @@ -21,7 +23,6 @@ import org.thoughtcrime.securesms.groups.ui.GroupChangeErrorCallback; import org.thoughtcrime.securesms.groups.ui.GroupChangeFailureReason; import org.thoughtcrime.securesms.recipients.Recipient; import org.thoughtcrime.securesms.recipients.RecipientId; -import org.signal.core.util.concurrent.SimpleTask; import java.io.IOException; import java.util.ArrayList; @@ -104,6 +105,24 @@ final class RecipientDialogRepository { onComplete::accept); } + void willAdminDemotionClearLabel(@NonNull Consumer onComplete) { + SimpleTask.BackgroundTask hasLabelToClear = () -> { + if (groupId == null || !groupId.isV2()) { + return false; + } + + GroupRecord groupRecord = SignalDatabase.groups().getGroup(groupId.requireV2()).orElse(null); + ServiceId.ACI aci = Recipient.resolved(recipientId).getAci().orElse(null); + + if (groupRecord != null && aci != null) { + return groupRecord.requireV2GroupProperties().adminDemotionClearsLabel(aci); + } + return false; + }; + + SimpleTask.run(SignalExecutors.UNBOUNDED, hasLabelToClear, onComplete::accept); + } + void getGroupMembership(@NonNull Consumer> onComplete) { SimpleTask.run(SignalExecutors.UNBOUNDED, () -> { diff --git a/app/src/main/java/org/thoughtcrime/securesms/recipients/ui/bottomsheet/RecipientDialogViewModel.java b/app/src/main/java/org/thoughtcrime/securesms/recipients/ui/bottomsheet/RecipientDialogViewModel.java index d1650f706b..abe455f2c0 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/recipients/ui/bottomsheet/RecipientDialogViewModel.java +++ b/app/src/main/java/org/thoughtcrime/securesms/recipients/ui/bottomsheet/RecipientDialogViewModel.java @@ -4,6 +4,7 @@ import android.app.Activity; import android.content.Context; import android.widget.Toast; +import androidx.annotation.MainThread; import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.annotation.WorkerThread; @@ -263,22 +264,30 @@ final class RecipientDialogViewModel extends ViewModel { .show(); } + @MainThread void onRemoveGroupAdminClicked(@NonNull Activity activity) { - new MaterialAlertDialogBuilder(activity) - .setMessage(context.getString(R.string.RecipientBottomSheet_remove_s_as_group_admin, Objects.requireNonNull(recipient.getValue()).getDisplayName(context))) - .setPositiveButton(R.string.RecipientBottomSheet_remove_as_admin, - (dialog, which) -> { - adminActionBusy.setValue(true); - recipientDialogRepository.setMemberAdmin(false, result -> { - adminActionBusy.setValue(false); - if (!result) { - Toast.makeText(activity, R.string.ManageGroupActivity_failed_to_update_the_group, Toast.LENGTH_SHORT).show(); - } - }, - this::showErrorToast); - }) - .setNegativeButton(android.R.string.cancel, (dialog, which) -> {}) - .show(); + Recipient groupMember = Objects.requireNonNull(recipient.getValue()); + + recipientDialogRepository.willAdminDemotionClearLabel(willDemotionClearLabel -> { + int messageRes = willDemotionClearLabel ? R.string.RecipientBottomSheet_remove_s_as_group_admin_and_clear_member_label + : R.string.RecipientBottomSheet_remove_s_as_group_admin; + + new MaterialAlertDialogBuilder(activity) + .setMessage(context.getString(messageRes, groupMember.getDisplayName(context))) + .setPositiveButton(R.string.RecipientBottomSheet_remove_as_admin, + (dialog, which) -> { + adminActionBusy.setValue(true); + recipientDialogRepository.setMemberAdmin(false, result -> { + adminActionBusy.setValue(false); + if (!result) { + Toast.makeText(activity, R.string.ManageGroupActivity_failed_to_update_the_group, Toast.LENGTH_SHORT).show(); + } + }, + this::showErrorToast); + }) + .setNegativeButton(android.R.string.cancel, (dialog, which) -> {}) + .show(); + }); } void onRemoveFromGroupClicked(@NonNull Activity activity, boolean isLinkActive, @NonNull Runnable onSuccess) { diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 0bc053b2f3..35578b86cb 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -5191,6 +5191,8 @@ Remove from group Remove %1$s as group admin? + + Remove %1$s as group admin? This will also clear their member label. "%1$s" will be able to edit this group and its members. Remove %1$s from the group? @@ -6032,6 +6034,12 @@ Add member labels Who can add member labels? + + Member labels will be cleared + + Changing this permission to "Only admins" will clear member labels set by non-admins in this group. + + Change permission diff --git a/app/src/test/java/org/thoughtcrime/securesms/components/settings/conversation/permissions/PermissionsSettingsViewModelTest.kt b/app/src/test/java/org/thoughtcrime/securesms/components/settings/conversation/permissions/PermissionsSettingsViewModelTest.kt new file mode 100644 index 0000000000..93e60f3ed7 --- /dev/null +++ b/app/src/test/java/org/thoughtcrime/securesms/components/settings/conversation/permissions/PermissionsSettingsViewModelTest.kt @@ -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() + private val repository = mockk(relaxUnitFun = true) + + private fun createViewModel( + memberLabelAccessControl: GroupAccessControl = GroupAccessControl.ONLY_ADMINS, + nonAdminMembersHaveLabels: Boolean = true + ): PermissionsSettingsViewModel { + val liveGroup = mockk { + 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) + } +} diff --git a/app/src/test/java/org/thoughtcrime/securesms/database/GroupTestUtil.kt b/app/src/test/java/org/thoughtcrime/securesms/database/GroupTestUtil.kt index fa63b5f123..fc63193870 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/database/GroupTestUtil.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/database/GroupTestUtil.kt @@ -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) { diff --git a/app/src/test/java/org/thoughtcrime/securesms/groups/GroupManagerV2Test_edit.kt b/app/src/test/java/org/thoughtcrime/securesms/groups/GroupManagerV2Test_edit.kt index baa0b143d8..504e2de9c0 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/groups/GroupManagerV2Test_edit.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/groups/GroupManagerV2Test_edit.kt @@ -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(), 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") + } + } } diff --git a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/GroupsV2Operations.java b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/GroupsV2Operations.java index fffab844b3..79da821de5 100644 --- a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/GroupsV2Operations.java +++ b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/GroupsV2Operations.java @@ -1120,6 +1120,21 @@ public final class GroupsV2Operations { ); } + public GroupChange.Actions.Builder createRemoveMemberLabelsChange(@Nonnull List acis) { + List actions = acis + .stream() + .map(memberAci -> + new GroupChange.Actions.ModifyMemberLabelAction.Builder() + .userId(encryptServiceId(memberAci)) + .labelEmoji(ByteString.EMPTY) + .labelString(ByteString.EMPTY) + .build() + ) + .collect(Collectors.toList()); + + return new GroupChange.Actions.Builder().modifyMemberLabels(actions); + } + public List decryptAddMembers(List addMembers) throws InvalidGroupStateException, InvalidInputException, VerificationFailedException { List ids = new ArrayList<>(addMembers.size()); for (GroupChange.Actions.AddMemberAction addMember : addMembers) {