From 13444136bd7e4aab0775535afcc04ba05c272c2a Mon Sep 17 00:00:00 2001 From: jeffrey-signal Date: Fri, 6 Mar 2026 15:58:00 -0500 Subject: [PATCH] Introduce new add member labels permission. --- .../PermissionsSettingsFragment.kt | 15 +++ .../PermissionsSettingsRepository.kt | 14 +++ .../permissions/PermissionsSettingsState.kt | 3 +- .../PermissionsSettingsViewModel.kt | 13 +++ .../securesms/database/model/GroupRecord.kt | 21 ++++ .../securesms/groups/GroupManager.java | 11 ++ .../securesms/groups/GroupManagerV2.java | 8 ++ .../securesms/groups/LiveGroup.java | 7 +- app/src/main/res/values/strings.xml | 4 + .../api/groupsv2/ChangeSetModifier.java | 2 + ...upChangeActionsBuilderChangeSetModifier.kt | 4 + .../api/groupsv2/DecryptedGroupExtensions.kt | 2 + .../api/groupsv2/DecryptedGroupUtil.java | 15 ++- ...upChangeActionsBuilderChangeSetModifier.kt | 4 + .../api/groupsv2/GroupChangeReconstruct.java | 6 + .../api/groupsv2/GroupChangeUtil.java | 59 ++++++---- .../api/groupsv2/GroupsV2Operations.java | 25 ++-- .../src/main/protowire/DecryptedGroups.proto | 1 + .../src/main/protowire/Groups.proto | 8 +- .../DecryptedGroupUtil_apply_Test.java | 34 +++++- .../DecryptedGroupUtil_empty_Test.java | 12 +- .../groupsv2/GroupChangeReconstructTest.java | 25 ++++ .../GroupChangeUtil_changeIsEmpty_Test.java | 11 +- .../GroupChangeUtil_resolveConflict_Test.java | 109 ++++++++++++++++-- ...il_resolveConflict_decryptedOnly_Test.java | 76 ++++++++++-- ...roupsV2Operations_decrypt_change_Test.java | 12 +- 26 files changed, 441 insertions(+), 60 deletions(-) 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 d9c2123aef..f647a5f671 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 @@ -10,6 +10,7 @@ import org.thoughtcrime.securesms.components.settings.DSLSettingsText import org.thoughtcrime.securesms.components.settings.configure import org.thoughtcrime.securesms.groups.GroupId import org.thoughtcrime.securesms.groups.ui.GroupErrors +import org.thoughtcrime.securesms.util.RemoteConfig import org.thoughtcrime.securesms.util.adapter.mapping.MappingAdapter class PermissionsSettingsFragment : DSLSettingsFragment( @@ -83,6 +84,20 @@ class PermissionsSettingsFragment : DSLSettingsFragment( viewModel.setAnnouncementGroup(it == 0) } ) + + if (RemoteConfig.sendMemberLabels) { + radioListPref( + title = DSLSettingsText.from(R.string.PermissionsSettingsFragment__add_member_labels), + isEnabled = state.selfCanEditSettings, + listItems = permissionsOptions, + dialogTitle = DSLSettingsText.from(R.string.PermissionsSettingsFragment__who_can_add_member_labels), + selected = getSelected(state.nonAdminCanSetMemberLabel), + confirmAction = true, + onSelected = { selectedIndex -> + viewModel.setNonAdminCanSetMemberLabel(selectedIndex == 1) + } + ) + } } } 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 3d39920853..cefa738fa1 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 @@ -56,4 +56,18 @@ class PermissionsSettingsRepository(private val context: Context) { } } } + + fun applyMemberLabelRightsChange(groupId: GroupId, newRights: GroupAccessControl, errorCallback: GroupChangeErrorCallback) { + SignalExecutors.UNBOUNDED.execute { + try { + GroupManager.applyMemberLabelRightsChange(context, groupId.requireV2(), newRights) + } catch (e: GroupChangeException) { + Log.w(TAG, e) + errorCallback.onError(GroupChangeFailureReason.fromException(e)) + } catch (e: IOException) { + Log.w(TAG, e) + errorCallback.onError(GroupChangeFailureReason.fromException(e)) + } + } + } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/settings/conversation/permissions/PermissionsSettingsState.kt b/app/src/main/java/org/thoughtcrime/securesms/components/settings/conversation/permissions/PermissionsSettingsState.kt index ab062a6ced..7b53d088b6 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/settings/conversation/permissions/PermissionsSettingsState.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/components/settings/conversation/permissions/PermissionsSettingsState.kt @@ -4,5 +4,6 @@ data class PermissionsSettingsState( val selfCanEditSettings: Boolean = false, val nonAdminCanAddMembers: Boolean = false, val nonAdminCanEditGroupInfo: Boolean = false, - val announcementGroup: Boolean = false + val announcementGroup: Boolean = false, + val nonAdminCanSetMemberLabel: Boolean = false ) 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 52e02f4e88..03ff270dc1 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 @@ -37,6 +37,10 @@ class PermissionsSettingsViewModel( store.update(liveGroup.isAnnouncementGroup) { isAnnouncementGroup, state -> state.copy(announcementGroup = isAnnouncementGroup) } + + store.update(liveGroup.memberLabelAccessControl) { memberLabelAccessControl, state -> + state.copy(nonAdminCanSetMemberLabel = memberLabelAccessControl == GroupAccessControl.ALL_MEMBERS) + } } fun setNonAdminCanAddMembers(nonAdminCanAddMembers: Boolean) { @@ -57,6 +61,15 @@ class PermissionsSettingsViewModel( } } + fun setNonAdminCanSetMemberLabel(nonAdminCanSetMemberLabel: Boolean) { + repository.applyMemberLabelRightsChange( + groupId = groupId, + newRights = nonAdminCanSetMemberLabel.asGroupAccessControl() + ) { failureReason -> + internalEvents.postValue(PermissionsSettingsEvents.GroupChangeError(failureReason)) + } + } + private fun Boolean.asGroupAccessControl(): GroupAccessControl { return if (this) { GroupAccessControl.ALL_MEMBERS diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/model/GroupRecord.kt b/app/src/main/java/org/thoughtcrime/securesms/database/model/GroupRecord.kt index 7ae261e9b5..5c93936c6f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/model/GroupRecord.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/model/GroupRecord.kt @@ -118,6 +118,27 @@ class GroupRecord( } } + /** + * Who is allowed to add member labels in this group. + * + * Defaults to ALL_MEMBERS for groups created before this permission was added. + */ + val memberLabelAccessControl: GroupAccessControl + get() { + if (!isV2Group) { + return GroupAccessControl.ALL_MEMBERS + } + + return when ((requireV2GroupProperties().decryptedGroup.accessControl ?: AccessControl()).memberLabel) { + AccessControl.AccessRequired.ADMINISTRATOR -> GroupAccessControl.ONLY_ADMINS + + AccessControl.AccessRequired.MEMBER, + AccessControl.AccessRequired.UNKNOWN, // groups predating this permission + AccessControl.AccessRequired.ANY, + AccessControl.AccessRequired.UNSATISFIABLE -> GroupAccessControl.ALL_MEMBERS + } + } + val actionableRequestingMembersCount: Int by lazy { if (isV2Group && memberLevel(Recipient.self()) == GroupTable.MemberLevel.ADMINISTRATOR) { requireV2GroupProperties() diff --git a/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManager.java b/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManager.java index 8946ceb911..24a7154c57 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManager.java +++ b/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManager.java @@ -299,6 +299,17 @@ public final class GroupManager { } } + @WorkerThread + public static void applyMemberLabelRightsChange(@NonNull Context context, + @NonNull GroupId.V2 groupId, + @NonNull GroupAccessControl newRights) + throws GroupChangeFailedException, GroupInsufficientRightsException, IOException, GroupNotAMemberException, GroupChangeBusyException + { + try (GroupManagerV2.GroupEditor editor = new GroupManagerV2(context).edit(groupId.requireV2())) { + editor.updateMemberLabelRights(newRights); + } + } + @WorkerThread public static void applyAnnouncementGroupChange(@NonNull Context context, @NonNull GroupId.V2 groupId, 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 7f8c1840a5..47cdcd0fa1 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManagerV2.java +++ b/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManagerV2.java @@ -321,6 +321,14 @@ final class GroupManagerV2 { return commitChangeWithConflictResolution(selfAci, groupOperations.createChangeMembershipRights(rightsToAccessControl(newRights))); } + @WorkerThread + @NonNull GroupManager.GroupActionResult updateMemberLabelRights(@NonNull GroupAccessControl newRights) + throws GroupChangeFailedException, GroupInsufficientRightsException, IOException, GroupNotAMemberException + { + AccessControl.AccessRequired accessRequired = rightsToAccessControl(newRights); + return commitChangeWithConflictResolution(selfAci, groupOperations.createChangeMemberLabelRights(accessRequired)); + } + @WorkerThread @NonNull GroupManager.GroupActionResult updateAnnouncementGroup(boolean isAnnouncementGroup) throws GroupChangeFailedException, GroupInsufficientRightsException, IOException, GroupNotAMemberException diff --git a/app/src/main/java/org/thoughtcrime/securesms/groups/LiveGroup.java b/app/src/main/java/org/thoughtcrime/securesms/groups/LiveGroup.java index 6023c8b379..79a1bfacfb 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/groups/LiveGroup.java +++ b/app/src/main/java/org/thoughtcrime/securesms/groups/LiveGroup.java @@ -12,6 +12,7 @@ import androidx.lifecycle.Transformations; import com.annimon.stream.ComparatorCompat; import com.annimon.stream.Stream; +import org.signal.core.models.ServiceId; import org.signal.core.util.concurrent.SignalExecutors; import org.signal.storageservice.storage.protos.groups.AccessControl; import org.signal.storageservice.storage.protos.groups.local.DecryptedGroup; @@ -28,7 +29,6 @@ import org.thoughtcrime.securesms.recipients.LiveRecipient; import org.thoughtcrime.securesms.recipients.Recipient; import org.thoughtcrime.securesms.recipients.RecipientId; import org.thoughtcrime.securesms.util.livedata.LiveDataUtil; -import org.signal.core.models.ServiceId; import java.text.Collator; import java.util.Collections; @@ -182,6 +182,11 @@ public final class LiveGroup { return Transformations.map(groupRecord, GroupRecord::getAttributesAccessControl); } + @NonNull + public LiveData getMemberLabelAccessControl() { + return Transformations.map(groupRecord, GroupRecord::getMemberLabelAccessControl); + } + public LiveData> getNonAdminFullMembers() { return Transformations.map(fullMembers, members -> Stream.of(members) diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 99580be949..e42ca83d60 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -6010,6 +6010,10 @@ Who can add new members? Who can edit this group\'s info? Who can send messages and start calls? + + Add member labels + + Who can add member labels? diff --git a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/ChangeSetModifier.java b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/ChangeSetModifier.java index 3088adc76d..2ca413a6fe 100644 --- a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/ChangeSetModifier.java +++ b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/ChangeSetModifier.java @@ -48,4 +48,6 @@ public interface ChangeSetModifier { void removePromotePendingPniAciMembers(int i); void removeModifyMemberLabels(int i); + + void clearModifyMemberLabelAccess(); } diff --git a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupChangeActionsBuilderChangeSetModifier.kt b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupChangeActionsBuilderChangeSetModifier.kt index a4260ddbd7..e8971cb9f0 100644 --- a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupChangeActionsBuilderChangeSetModifier.kt +++ b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupChangeActionsBuilderChangeSetModifier.kt @@ -114,6 +114,10 @@ internal class DecryptedGroupChangeActionsBuilderChangeSetModifier(private val r result.modifyMemberLabels = result.modifyMemberLabels.removeIndex(i) } + override fun clearModifyMemberLabelAccess() { + result.newMemberLabelAccess = AccessControl.AccessRequired.UNKNOWN + } + private fun List.removeIndex(i: Int): List { val modifiedList = this.toMutableList() modifiedList.removeAt(i) diff --git a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupExtensions.kt b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupExtensions.kt index c9be5c56c0..3061d2bbe1 100644 --- a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupExtensions.kt +++ b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupExtensions.kt @@ -74,6 +74,7 @@ fun DecryptedGroupChange.getChangedFields(): Set { if (newInviteLinkAccess != AccessControl.AccessRequired.UNKNOWN) add(GroupChangeField.INVITE_LINK_ACCESS) if (newInviteLinkPassword.size != 0) add(GroupChangeField.INVITE_LINK_PASSWORD) if (newMemberAccess != AccessControl.AccessRequired.UNKNOWN) add(GroupChangeField.MEMBER_ACCESS) + if (newMemberLabelAccess != AccessControl.AccessRequired.UNKNOWN) add(GroupChangeField.MEMBER_LABEL_ACCESS) if (modifyMemberLabels.isNotEmpty()) add(GroupChangeField.MEMBER_LABELS) if (deleteMembers.isNotEmpty()) add(GroupChangeField.MEMBER_REMOVALS) if (modifyMemberRoles.isNotEmpty()) add(GroupChangeField.MEMBER_ROLES) @@ -115,6 +116,7 @@ enum class GroupChangeField(val changeSilently: Boolean = false) { INVITE_LINK_ACCESS, INVITE_LINK_PASSWORD, MEMBER_ACCESS, + MEMBER_LABEL_ACCESS, MEMBER_LABELS(changeSilently = true), MEMBER_REMOVALS, MEMBER_ROLES, diff --git a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupUtil.java b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupUtil.java index efdf050830..5607ce9668 100644 --- a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupUtil.java +++ b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupUtil.java @@ -1,5 +1,7 @@ package org.whispersystems.signalservice.api.groupsv2; +import org.signal.core.models.ServiceId; +import org.signal.core.models.ServiceId.ACI; import org.signal.libsignal.protocol.logging.Log; import org.signal.storageservice.storage.protos.groups.AccessControl; import org.signal.storageservice.storage.protos.groups.Member; @@ -13,8 +15,6 @@ import org.signal.storageservice.storage.protos.groups.local.DecryptedPendingMem import org.signal.storageservice.storage.protos.groups.local.DecryptedPendingMemberRemoval; import org.signal.storageservice.storage.protos.groups.local.DecryptedRequestingMember; import org.signal.storageservice.storage.protos.groups.local.EnabledState; -import org.signal.core.models.ServiceId; -import org.signal.core.models.ServiceId.ACI; import org.whispersystems.signalservice.api.push.ServiceIds; import java.util.ArrayList; @@ -323,6 +323,8 @@ public final class DecryptedGroupUtil { applyModifyAddFromInviteLinkAccessControlAction(builder, change); + applyModifyMemberLabelAccessControlAction(builder, change); + applyAddRequestingMembers(builder, change.newRequestingMembers); applyDeleteRequestingMembers(builder, change.deleteRequestingMembers); @@ -524,6 +526,15 @@ public final class DecryptedGroupUtil { } } + private static void applyModifyMemberLabelAccessControlAction(DecryptedGroup.Builder builder, DecryptedGroupChange change) { + AccessControl.AccessRequired newAccessLevel = change.newMemberLabelAccess; + + if (newAccessLevel != AccessControl.AccessRequired.UNKNOWN) { + AccessControl.Builder accessControlBuilder = builder.accessControl != null ? builder.accessControl.newBuilder() : new AccessControl.Builder(); + builder.accessControl(accessControlBuilder.memberLabel(newAccessLevel).build()); + } + } + private static void applyAddRequestingMembers(DecryptedGroup.Builder builder, List newRequestingMembers) { List requestingMembers = new ArrayList<>(builder.requestingMembers); requestingMembers.addAll(newRequestingMembers); diff --git a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeActionsBuilderChangeSetModifier.kt b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeActionsBuilderChangeSetModifier.kt index eda0d5ec2f..f4a2f827e7 100644 --- a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeActionsBuilderChangeSetModifier.kt +++ b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeActionsBuilderChangeSetModifier.kt @@ -105,6 +105,10 @@ internal class GroupChangeActionsBuilderChangeSetModifier(private val result: Gr result.modifyMemberLabels = result.modifyMemberLabels.removeIndex(i) } + override fun clearModifyMemberLabelAccess() { + result.modifyMemberLabelAccess = null + } + private fun List.removeIndex(i: Int): List { val modifiedList = this.toMutableList() modifiedList.removeAt(i) diff --git a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeReconstruct.java b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeReconstruct.java index 88cb06fd7b..84a63e91ef 100644 --- a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeReconstruct.java +++ b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeReconstruct.java @@ -151,6 +151,12 @@ public final class GroupChangeReconstruct { } } + if (fromState.accessControl == null || (toState.accessControl != null && !fromState.accessControl.memberLabel.equals(toState.accessControl.memberLabel))) { + if (toState.accessControl != null) { + builder.newMemberLabelAccess(toState.accessControl.memberLabel); + } + } + builder.newRequestingMembers(new ArrayList<>(intersectRequestingByAci(toState.requestingMembers, newRequestingMemberAcis))); builder.deleteRequestingMembers(rejectedRequestMembers.stream().map(requestingMember -> requestingMember.aciBytes).collect(Collectors.toList())); diff --git a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil.java b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil.java index b605cfb267..359b35b49e 100644 --- a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil.java +++ b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil.java @@ -29,29 +29,30 @@ public final class GroupChangeUtil { * True iff there are no change actions. */ public static boolean changeIsEmpty(GroupChange.Actions change) { - return change.addMembers.size() == 0 && // field 3 - change.deleteMembers.size() == 0 && // field 4 - change.modifyMemberRoles.size() == 0 && // field 5 - change.modifyMemberProfileKeys.size() == 0 && // field 6 - change.addMembersPendingProfileKey.size() == 0 && // field 7 - change.deleteMembersPendingProfileKey.size() == 0 && // field 8 - change.promoteMembersPendingProfileKey.size() == 0 && // field 9 - change.modifyTitle == null && // field 10 - change.modifyAvatar == null && // field 11 - change.modifyDisappearingMessageTimer == null && // field 12 - change.modifyAttributesAccess == null && // field 13 - change.modifyMemberAccess == null && // field 14 - change.modifyAddFromInviteLinkAccess == null && // field 15 - change.addMembersPendingAdminApproval.size() == 0 && // field 16 - change.deleteMembersPendingAdminApproval.size() == 0 && // field 17 - change.promoteMembersPendingAdminApproval.size() == 0 && // field 18 - change.modifyInviteLinkPassword == null && // field 19 - change.modifyDescription == null && // field 20 - change.modify_announcements_only == null && // field 21 - change.add_members_banned.size() == 0 && // field 22 - change.delete_members_banned.size() == 0 && // field 23 - change.promote_members_pending_pni_aci_profile_key.size() == 0 && // field 24 - change.modifyMemberLabels.isEmpty(); // field 26 + return change.addMembers.isEmpty() && // field 3 + change.deleteMembers.isEmpty() && // field 4 + change.modifyMemberRoles.isEmpty() && // field 5 + change.modifyMemberProfileKeys.isEmpty() && // field 6 + change.addMembersPendingProfileKey.isEmpty() && // field 7 + change.deleteMembersPendingProfileKey.isEmpty() && // field 8 + change.promoteMembersPendingProfileKey.isEmpty() && // field 9 + change.modifyTitle == null && // field 10 + change.modifyAvatar == null && // field 11 + change.modifyDisappearingMessageTimer == null && // field 12 + change.modifyAttributesAccess == null && // field 13 + change.modifyMemberAccess == null && // field 14 + change.modifyAddFromInviteLinkAccess == null && // field 15 + change.addMembersPendingAdminApproval.isEmpty() && // field 16 + change.deleteMembersPendingAdminApproval.isEmpty() && // field 17 + change.promoteMembersPendingAdminApproval.isEmpty() && // field 18 + change.modifyInviteLinkPassword == null && // field 19 + change.modifyDescription == null && // field 20 + change.modify_announcements_only == null && // field 21 + change.add_members_banned.isEmpty() && // field 22 + change.delete_members_banned.isEmpty() && // field 23 + change.promote_members_pending_pni_aci_profile_key.isEmpty() && // field 24 + change.modifyMemberLabels.isEmpty() && // field 26 + change.modifyMemberLabelAccess == null; // field 27 } /** @@ -155,6 +156,7 @@ public final class GroupChangeUtil { resolveField23DeleteBannedMembers (conflictingChange, changeSetModifier, bannedMembersByServiceId); resolveField24PromotePendingPniAciMembers (conflictingChange, changeSetModifier, fullMembersByUuid); resolveField26ModifyMemberLabels (conflictingChange, changeSetModifier, fullMembersByUuid); + resolveField27ModifyMemberLabelAccess (groupState, conflictingChange, changeSetModifier); } private static void resolveField3AddMembers(DecryptedGroupChange conflictingChange, ChangeSetModifier result, HashMap fullMembersByUuid, HashMap pendingMembersByServiceId) { @@ -390,4 +392,15 @@ public final class GroupChangeUtil { } } } + + private static void resolveField27ModifyMemberLabelAccess( + @Nonnull DecryptedGroup groupState, + @Nonnull DecryptedGroupChange conflictingChange, + @Nonnull ChangeSetModifier result + ) + { + if (groupState.accessControl != null && conflictingChange.newMemberLabelAccess == groupState.accessControl.memberLabel) { + result.clearModifyMemberLabelAccess(); + } + } } 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 132bf9b1fc..fffab844b3 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 @@ -1,5 +1,9 @@ package org.whispersystems.signalservice.api.groupsv2; +import org.signal.core.models.ServiceId; +import org.signal.core.models.ServiceId.ACI; +import org.signal.core.models.ServiceId.PNI; +import org.signal.core.util.UuidUtil; import org.signal.libsignal.protocol.logging.Log; import org.signal.libsignal.zkgroup.InvalidInputException; import org.signal.libsignal.zkgroup.NotarySignature; @@ -17,14 +21,14 @@ import org.signal.libsignal.zkgroup.profiles.ExpiringProfileKeyCredential; import org.signal.libsignal.zkgroup.profiles.ProfileKey; import org.signal.libsignal.zkgroup.profiles.ProfileKeyCredentialPresentation; import org.signal.storageservice.storage.protos.groups.AccessControl; -import org.signal.storageservice.storage.protos.groups.MemberBanned; import org.signal.storageservice.storage.protos.groups.Group; import org.signal.storageservice.storage.protos.groups.GroupAttributeBlob; import org.signal.storageservice.storage.protos.groups.GroupChange; import org.signal.storageservice.storage.protos.groups.GroupJoinInfo; import org.signal.storageservice.storage.protos.groups.Member; -import org.signal.storageservice.storage.protos.groups.MemberPendingProfileKey; +import org.signal.storageservice.storage.protos.groups.MemberBanned; import org.signal.storageservice.storage.protos.groups.MemberPendingAdminApproval; +import org.signal.storageservice.storage.protos.groups.MemberPendingProfileKey; import org.signal.storageservice.storage.protos.groups.local.DecryptedApproveMember; import org.signal.storageservice.storage.protos.groups.local.DecryptedBannedMember; import org.signal.storageservice.storage.protos.groups.local.DecryptedGroup; @@ -39,10 +43,6 @@ import org.signal.storageservice.storage.protos.groups.local.DecryptedRequesting import org.signal.storageservice.storage.protos.groups.local.DecryptedString; import org.signal.storageservice.storage.protos.groups.local.DecryptedTimer; import org.signal.storageservice.storage.protos.groups.local.EnabledState; -import org.signal.core.models.ServiceId; -import org.signal.core.models.ServiceId.ACI; -import org.signal.core.models.ServiceId.PNI; -import org.signal.core.util.UuidUtil; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -53,11 +53,11 @@ import java.util.Collections; import java.util.Comparator; import java.util.List; import java.util.Locale; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.UUID; import java.util.stream.Collectors; -import java.util.Objects; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -344,6 +344,12 @@ public final class GroupsV2Operations { ); } + public GroupChange.Actions.Builder createChangeMemberLabelRights(AccessControl.AccessRequired newRights) { + return new GroupChange.Actions.Builder().modifyMemberLabelAccess( + new GroupChange.Actions.ModifyMemberLabelAccessControlAction.Builder().memberLabelAccess(newRights).build() + ); + } + public GroupChange.Actions.Builder createAnnouncementGroupChange(boolean isAnnouncementGroup) { return new GroupChange.Actions.Builder().modify_announcements_only( new GroupChange.Actions.ModifyAnnouncementsOnlyAction.Builder().announcements_only(isAnnouncementGroup).build() @@ -770,6 +776,11 @@ public final class GroupsV2Operations { } builder.modifyMemberLabels(modifyMemberLabels); + // Field 27 + if (actions.modifyMemberLabelAccess != null) { + builder.newMemberLabelAccess(actions.modifyMemberLabelAccess.memberLabelAccess); + } + if (editorServiceId instanceof ServiceId.PNI) { if (actions.addMembers.size() == 1 && builder.newMembers.size() == 1) { GroupChange.Actions.AddMemberAction addMemberAction = actions.addMembers.get(0); diff --git a/lib/libsignal-service/src/main/protowire/DecryptedGroups.proto b/lib/libsignal-service/src/main/protowire/DecryptedGroups.proto index e770621a7c..66993d33c8 100644 --- a/lib/libsignal-service/src/main/protowire/DecryptedGroups.proto +++ b/lib/libsignal-service/src/main/protowire/DecryptedGroups.proto @@ -111,6 +111,7 @@ message DecryptedGroupChange { repeated DecryptedBannedMember deleteBannedMembers = 23; repeated DecryptedMember promotePendingPniAciMembers = 24; repeated DecryptedModifyMemberLabel modifyMemberLabels = 26; + AccessControl.AccessRequired newMemberLabelAccess = 27; } message DecryptedString { diff --git a/lib/libsignal-service/src/main/protowire/Groups.proto b/lib/libsignal-service/src/main/protowire/Groups.proto index a539eda2dc..e15ace6875 100644 --- a/lib/libsignal-service/src/main/protowire/Groups.proto +++ b/lib/libsignal-service/src/main/protowire/Groups.proto @@ -69,6 +69,7 @@ message AccessControl { AccessRequired attributes = 1; AccessRequired members = 2; AccessRequired addFromInviteLink = 3; + AccessRequired memberLabel = 4; } message Group { @@ -225,6 +226,10 @@ message GroupChange { AccessControl.AccessRequired addFromInviteLinkAccess = 1; } + message ModifyMemberLabelAccessControlAction { + AccessControl.AccessRequired memberLabelAccess = 1; + } + message ModifyInviteLinkPasswordAction { bytes inviteLinkPassword = 1; } @@ -262,7 +267,8 @@ message GroupChange { repeated DeleteMemberBannedAction delete_members_banned = 23; // change epoch = 4 repeated PromoteMemberPendingPniAciProfileKeyAction promote_members_pending_pni_aci_profile_key = 24; // change epoch = 5 repeated ModifyMemberLabelAction modifyMemberLabels = 26; // change epoch = 6; - // next: 27 + ModifyMemberLabelAccessControlAction modifyMemberLabelAccess = 27; // change epoch = 6 + // next: 28 } bytes actions = 1; diff --git a/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupUtil_apply_Test.java b/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupUtil_apply_Test.java index b73d98cc0c..58a2352018 100644 --- a/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupUtil_apply_Test.java +++ b/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupUtil_apply_Test.java @@ -51,7 +51,7 @@ public final class DecryptedGroupUtil_apply_Test { int maxFieldFound = getMaxDeclaredFieldNumber(DecryptedGroupChange.class); assertEquals("DecryptedGroupUtil and its tests need updating to account for new fields on " + DecryptedGroupChange.class.getName(), - 26, maxFieldFound); + 27, maxFieldFound); } @Test @@ -1052,4 +1052,36 @@ public final class DecryptedGroupUtil_apply_Test { .modifyMemberLabels(List.of(modifyLabelAction)) .build()); } + + @Test + public void apply_sets_member_label_access() throws NotAbleToApplyGroupV2ChangeException { + DecryptedGroup group = new DecryptedGroup.Builder() + .revision(10) + .accessControl( + new AccessControl.Builder() + .attributes(AccessControl.AccessRequired.ADMINISTRATOR) + .members(AccessControl.AccessRequired.ADMINISTRATOR) + .memberLabel(AccessControl.AccessRequired.ADMINISTRATOR) + .build() + ) + .build(); + + DecryptedGroupChange groupChange = new DecryptedGroupChange.Builder() + .revision(11) + .newMemberLabelAccess(AccessControl.AccessRequired.MEMBER) + .build(); + + DecryptedGroup expectedResult = new DecryptedGroup.Builder() + .revision(11) + .accessControl( + new AccessControl.Builder() + .attributes(AccessControl.AccessRequired.ADMINISTRATOR) + .members(AccessControl.AccessRequired.ADMINISTRATOR) + .memberLabel(AccessControl.AccessRequired.MEMBER) + .build() + ) + .build(); + + assertEquals(expectedResult, DecryptedGroupUtil.apply(group, groupChange)); + } } diff --git a/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupUtil_empty_Test.java b/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupUtil_empty_Test.java index 6a7995bca1..8774309b0d 100644 --- a/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupUtil_empty_Test.java +++ b/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupUtil_empty_Test.java @@ -41,7 +41,7 @@ public final class DecryptedGroupUtil_empty_Test { int maxFieldFound = getMaxDeclaredFieldNumber(DecryptedGroupChange.class); assertEquals("GroupChangeField and getChangedFields() need updating to account for new fields on " + DecryptedGroupChange.class.getName(), - 26, maxFieldFound); + 27, maxFieldFound); } @Test @@ -285,6 +285,16 @@ public final class DecryptedGroupUtil_empty_Test { assertTrue(DecryptedGroupExtensions.isSilent(change)); } + @Test + public void not_empty_with_modify_member_label_access_field_27() { + DecryptedGroupChange change = new DecryptedGroupChange.Builder() + .newMemberLabelAccess(AccessControl.AccessRequired.ADMINISTRATOR) + .build(); + + assertFalse(DecryptedGroupExtensions.getChangedFields(change).isEmpty()); + assertFalse(DecryptedGroupExtensions.isSilent(change)); + } + @Test public void silent_with_profile_keys_and_banned_members() { DecryptedGroupChange change = new DecryptedGroupChange.Builder() diff --git a/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeReconstructTest.java b/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeReconstructTest.java index 8c86985bf2..7c72889603 100644 --- a/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeReconstructTest.java +++ b/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeReconstructTest.java @@ -462,4 +462,29 @@ public final class GroupChangeReconstructTest { assertEquals("", change.modifyMemberLabels.get(0).labelEmoji); assertEquals("", change.modifyMemberLabels.get(0).labelString); } + + @Test + public void new_member_label_access() { + DecryptedGroup from = new DecryptedGroup.Builder() + .accessControl( + new AccessControl.Builder() + .memberLabel(AccessControl.AccessRequired.ADMINISTRATOR) + .build()) + .build(); + + DecryptedGroup to = new DecryptedGroup.Builder() + .accessControl( + new AccessControl.Builder() + .memberLabel(AccessControl.AccessRequired.MEMBER) + .build()) + .build(); + + DecryptedGroupChange decryptedGroupChange = GroupChangeReconstruct.reconstructGroupChange(from, to); + + assertEquals( + new DecryptedGroupChange.Builder() + .newMemberLabelAccess(AccessControl.AccessRequired.MEMBER) + .build(), + decryptedGroupChange); + } } diff --git a/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil_changeIsEmpty_Test.java b/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil_changeIsEmpty_Test.java index 60d7bcfb61..51d1fae5e8 100644 --- a/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil_changeIsEmpty_Test.java +++ b/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil_changeIsEmpty_Test.java @@ -22,7 +22,7 @@ public final class GroupChangeUtil_changeIsEmpty_Test { int maxFieldFound = getMaxDeclaredFieldNumber(GroupChange.Actions.class); assertEquals("GroupChangeUtil and its tests need updating to account for new fields on " + GroupChange.Actions.class.getName(), - 26, maxFieldFound); + 27, maxFieldFound); } @Test @@ -236,4 +236,13 @@ public final class GroupChangeUtil_changeIsEmpty_Test { assertFalse(GroupChangeUtil.changeIsEmpty(actions)); } + + @Test + public void not_empty_with_modify_member_label_access_field_27() { + GroupChange.Actions actions = new GroupChange.Actions.Builder() + .modifyMemberLabelAccess(new GroupChange.Actions.ModifyMemberLabelAccessControlAction()) + .build(); + + assertFalse(GroupChangeUtil.changeIsEmpty(actions)); + } } diff --git a/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil_resolveConflict_Test.java b/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil_resolveConflict_Test.java index 6093ca0e80..da6b7cb414 100644 --- a/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil_resolveConflict_Test.java +++ b/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil_resolveConflict_Test.java @@ -53,7 +53,7 @@ public final class GroupChangeUtil_resolveConflict_Test { int maxFieldFound = getMaxDeclaredFieldNumber(DecryptedGroupChange.class); assertEquals("GroupChangeUtil#resolveConflict and its tests need updating to account for new fields on " + DecryptedGroupChange.class.getName(), - 26, maxFieldFound); + 27, maxFieldFound); } /** @@ -63,10 +63,10 @@ public final class GroupChangeUtil_resolveConflict_Test { */ @Test public void ensure_resolveConflict_knows_about_all_fields_of_GroupChange() { - int maxFieldFound = getMaxDeclaredFieldNumber(DecryptedGroupChange.class); + int maxFieldFound = getMaxDeclaredFieldNumber(GroupChange.Actions.class); assertEquals("GroupChangeUtil#resolveConflict and its tests need updating to account for new fields on " + GroupChange.class.getName(), - 26, maxFieldFound); + 27, maxFieldFound); } /** @@ -857,7 +857,7 @@ public final class GroupChangeUtil_resolveConflict_Test { } @Test - public void field_26__modify_member_label__remove_if_label_already_matches() { + public void field_26__member_label_change_removed_when_same_as_group_state() { UUID memberUuid = UUID.fromString("d1d1d1d1-0000-4000-8000-000000000001"); DecryptedMember existingMember = member(memberUuid) @@ -866,7 +866,7 @@ public final class GroupChangeUtil_resolveConflict_Test { .labelString("matching label") .build(); - DecryptedGroup existingGroup = new DecryptedGroup.Builder() + DecryptedGroup groupState = new DecryptedGroup.Builder() .revision(10) .members(List.of(existingMember)) .build(); @@ -881,21 +881,58 @@ public final class GroupChangeUtil_resolveConflict_Test { .modifyMemberLabels(List.of(modifyLabelAction)) .build(); - DecryptedGroupChange.Builder resolvedActions = GroupChangeUtil.resolveConflict(existingGroup, conflictingChange); - assertTrue(resolvedActions.build().modifyMemberLabels.isEmpty()); + GroupChange.Actions change = new GroupChange.Actions.Builder() + .modifyMemberLabels(List.of(new GroupChange.Actions.ModifyMemberLabelAction())) + .build(); + + GroupChange.Actions resolvedActions = GroupChangeUtil.resolveConflict(groupState, conflictingChange, change).build(); + assertTrue(GroupChangeUtil.changeIsEmpty(resolvedActions)); } @Test - public void field_26__modify_member_label__remove_if_member_not_in_group() { + public void field_26__member_label_change_preserved_when_differs_from_group_state() { + UUID memberUuid = UUID.fromString("d1d1d1d1-0000-4000-8000-000000000001"); + + DecryptedMember existingMember = member(memberUuid) + .newBuilder() + .labelEmoji("🔥") + .labelString("Old Label") + .build(); + + DecryptedGroup groupState = new DecryptedGroup.Builder() + .revision(10) + .members(List.of(existingMember)) + .build(); + + DecryptedModifyMemberLabel modifyLabelAction = new DecryptedModifyMemberLabel.Builder() + .aciBytes(UuidUtil.toByteString(memberUuid)) + .labelEmoji("🎉") + .labelString("New Label") + .build(); + + DecryptedGroupChange conflictingChange = new DecryptedGroupChange.Builder() + .modifyMemberLabels(List.of(modifyLabelAction)) + .build(); + + GroupChange.Actions change = new GroupChange.Actions.Builder() + .modifyMemberLabels(List.of(new GroupChange.Actions.ModifyMemberLabelAction())) + .build(); + + GroupChange.Actions resolvedActions = GroupChangeUtil.resolveConflict(groupState, conflictingChange, change).build(); + assertEquals(change, resolvedActions); + } + + @Test + public void field_26__member_label_change_removed_when_member_not_in_group() { UUID memberUuuid = UUID.fromString("d1d1d1d1-0000-4000-8000-000000000001"); UUID nonMemberUuid = UUID.fromString("d2d2d2d2-0000-4000-8000-000000000002"); - DecryptedGroup existingGroup = new DecryptedGroup.Builder() + DecryptedGroup groupState = new DecryptedGroup.Builder() .revision(10) .members(List.of(member(memberUuuid))) .build(); - DecryptedModifyMemberLabel modifyLabelAction = new org.signal.storageservice.storage.protos.groups.local.DecryptedModifyMemberLabel.Builder() + DecryptedModifyMemberLabel modifyLabelAction = new DecryptedModifyMemberLabel.Builder() .aciBytes(UuidUtil.toByteString(nonMemberUuid)) .labelEmoji("🔥") .labelString("foo bar") @@ -905,7 +942,55 @@ public final class GroupChangeUtil_resolveConflict_Test { .modifyMemberLabels(List.of(modifyLabelAction)) .build(); - DecryptedGroupChange.Builder resolved = GroupChangeUtil.resolveConflict(existingGroup, conflictingChange); - assertTrue(resolved.build().modifyMemberLabels.isEmpty()); + GroupChange.Actions change = new GroupChange.Actions.Builder() + .modifyMemberLabels(List.of(new GroupChange.Actions.ModifyMemberLabelAction())) + .build(); + + GroupChange.Actions resolvedActions = GroupChangeUtil.resolveConflict(groupState, conflictingChange, change).build(); + assertTrue(GroupChangeUtil.changeIsEmpty(resolvedActions)); + } + + @Test + public void field_27__member_label_access_change_preserved_when_differs_from_group_state() { + DecryptedGroup groupState = new DecryptedGroup.Builder() + .accessControl(new AccessControl.Builder().memberLabel(AccessControl.AccessRequired.ADMINISTRATOR).build()) + .build(); + + DecryptedGroupChange decryptedChange = new DecryptedGroupChange.Builder() + .newMemberLabelAccess(AccessControl.AccessRequired.MEMBER) + .build(); + + GroupChange.Actions change = new GroupChange.Actions.Builder() + .modifyMemberLabelAccess( + new GroupChange.Actions.ModifyMemberLabelAccessControlAction.Builder() + .memberLabelAccess(AccessControl.AccessRequired.MEMBER) + .build() + ) + .build(); + + GroupChange.Actions resolvedActions = GroupChangeUtil.resolveConflict(groupState, decryptedChange, change).build(); + assertEquals(change, resolvedActions); + } + + @Test + public void field_27__member_label_access_change_removed_when_same_as_group_state() { + DecryptedGroup groupState = new DecryptedGroup.Builder() + .accessControl(new AccessControl.Builder().memberLabel(AccessControl.AccessRequired.ADMINISTRATOR).build()) + .build(); + + DecryptedGroupChange decryptedChange = new DecryptedGroupChange.Builder() + .newMemberLabelAccess(AccessControl.AccessRequired.ADMINISTRATOR) + .build(); + + GroupChange.Actions change = new GroupChange.Actions.Builder() + .modifyMemberLabelAccess( + new GroupChange.Actions.ModifyMemberLabelAccessControlAction.Builder() + .memberLabelAccess(AccessControl.AccessRequired.ADMINISTRATOR) + .build() + ) + .build(); + + GroupChange.Actions resolvedActions = GroupChangeUtil.resolveConflict(groupState, decryptedChange, change).build(); + assertTrue(GroupChangeUtil.changeIsEmpty(resolvedActions)); } } \ No newline at end of file diff --git a/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil_resolveConflict_decryptedOnly_Test.java b/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil_resolveConflict_decryptedOnly_Test.java index 6ba2073c12..3d56c33425 100644 --- a/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil_resolveConflict_decryptedOnly_Test.java +++ b/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil_resolveConflict_decryptedOnly_Test.java @@ -19,6 +19,7 @@ import java.util.UUID; import okio.ByteString; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.whispersystems.signalservice.api.groupsv2.ProtoTestUtils.admin; import static org.whispersystems.signalservice.api.groupsv2.ProtoTestUtils.approveMember; @@ -45,7 +46,7 @@ public final class GroupChangeUtil_resolveConflict_decryptedOnly_Test { int maxFieldFound = getMaxDeclaredFieldNumber(DecryptedGroupChange.class); assertEquals("GroupChangeUtil#resolveConflict and its tests need updating to account for new fields on " + DecryptedGroupChange.class.getName(), - 26, maxFieldFound); + 27, maxFieldFound); } /** @@ -676,7 +677,7 @@ public final class GroupChangeUtil_resolveConflict_decryptedOnly_Test { } @Test - public void field_26__modify_member_label__remove_if_label_already_matches() { + public void field_26__member_label_change_removed_when_same_as_group_state() { UUID memberUuid = UUID.fromString("d1d1d1d1-0000-4000-8000-000000000001"); DecryptedMember existingMember = member(memberUuid) @@ -691,7 +692,7 @@ public final class GroupChangeUtil_resolveConflict_decryptedOnly_Test { .labelString("Already Set") .build(); - DecryptedGroup existingGroup = new DecryptedGroup.Builder() + DecryptedGroup groupState = new DecryptedGroup.Builder() .revision(10) .members(List.of(existingMember)) .build(); @@ -700,16 +701,16 @@ public final class GroupChangeUtil_resolveConflict_decryptedOnly_Test { .modifyMemberLabels(List.of(modifyLabelAction)) .build(); - DecryptedGroupChange.Builder resolved = GroupChangeUtil.resolveConflict(existingGroup, conflictingChange); - assertTrue(resolved.build().modifyMemberLabels.isEmpty()); + DecryptedGroupChange resolvedChanges = GroupChangeUtil.resolveConflict(groupState, conflictingChange).build(); + assertTrue(DecryptedGroupExtensions.getChangedFields(resolvedChanges).isEmpty()); } @Test - public void field_26__modify_member_label__remove_if_member_not_in_group() { + public void field_26__member_label_change_removed_when_member_not_in_group() { UUID memberUuid = UUID.fromString("d1d1d1d1-0000-4000-8000-000000000001"); UUID notInGroupUuid = UUID.fromString("d2d2d2d2-0000-4000-8000-000000000002"); - DecryptedGroup existingGroup = new DecryptedGroup.Builder() + DecryptedGroup groupState = new DecryptedGroup.Builder() .revision(10) .members(List.of(member(memberUuid))) .build(); @@ -724,7 +725,64 @@ public final class GroupChangeUtil_resolveConflict_decryptedOnly_Test { .modifyMemberLabels(List.of(modifyLabelAction)) .build(); - DecryptedGroupChange.Builder resolved = GroupChangeUtil.resolveConflict(existingGroup, conflictingChange); - assertTrue(resolved.build().modifyMemberLabels.isEmpty()); + DecryptedGroupChange resolvedChanges = GroupChangeUtil.resolveConflict(groupState, conflictingChange).build(); + assertTrue(DecryptedGroupExtensions.getChangedFields(resolvedChanges).isEmpty()); + } + + @Test + public void field_26__member_label_change_preserved_when_label_differs() { + UUID memberUuid = UUID.fromString("d1d1d1d1-0000-4000-8000-000000000001"); + + DecryptedMember existingMember = member(memberUuid) + .newBuilder() + .labelEmoji("🔥") + .labelString("Old Label") + .build(); + + DecryptedGroup groupState = new DecryptedGroup.Builder() + .revision(10) + .members(List.of(existingMember)) + .build(); + + DecryptedModifyMemberLabel modifyLabelAction = new DecryptedModifyMemberLabel.Builder() + .aciBytes(UuidUtil.toByteString(memberUuid)) + .labelEmoji("🎉") + .labelString("New Label") + .build(); + + DecryptedGroupChange conflictingChange = new DecryptedGroupChange.Builder() + .modifyMemberLabels(List.of(modifyLabelAction)) + .build(); + + DecryptedGroupChange resolvedChanges = GroupChangeUtil.resolveConflict(groupState, conflictingChange).build(); + assertFalse(DecryptedGroupExtensions.getChangedFields(resolvedChanges).isEmpty()); + } + + @Test + public void field_27__member_label_access_change_preserved_when_differs_from_group_state() { + DecryptedGroup groupState = new DecryptedGroup.Builder() + .accessControl(new AccessControl.Builder().memberLabel(AccessControl.AccessRequired.ADMINISTRATOR).build()) + .build(); + + DecryptedGroupChange decryptedChange = new DecryptedGroupChange.Builder() + .newMemberLabelAccess(AccessControl.AccessRequired.MEMBER) + .build(); + + DecryptedGroupChange resolvedChanges = GroupChangeUtil.resolveConflict(groupState, decryptedChange).build(); + assertEquals(decryptedChange, resolvedChanges); + } + + @Test + public void field_27__member_label_access_change_removed_when_same_as_group_state() { + DecryptedGroup groupState = new DecryptedGroup.Builder() + .accessControl(new AccessControl.Builder().memberLabel(AccessControl.AccessRequired.ADMINISTRATOR).build()) + .build(); + + DecryptedGroupChange decryptedChange = new DecryptedGroupChange.Builder() + .newMemberLabelAccess(AccessControl.AccessRequired.ADMINISTRATOR) + .build(); + + DecryptedGroupChange resolvedChanges = GroupChangeUtil.resolveConflict(groupState, decryptedChange).build(); + assertTrue(DecryptedGroupExtensions.getChangedFields(resolvedChanges).isEmpty()); } } \ No newline at end of file diff --git a/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupsV2Operations_decrypt_change_Test.java b/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupsV2Operations_decrypt_change_Test.java index 14224ec2c1..82cd0ef634 100644 --- a/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupsV2Operations_decrypt_change_Test.java +++ b/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupsV2Operations_decrypt_change_Test.java @@ -73,7 +73,7 @@ public final class GroupsV2Operations_decrypt_change_Test { int maxFieldFound = getMaxDeclaredFieldNumber(DecryptedGroupChange.class); assertEquals("GroupV2Operations#decryptChange and its tests need updating to account for new fields on " + DecryptedGroupChange.class.getName(), - 26, + 27, maxFieldFound); } @@ -476,6 +476,16 @@ public final class GroupsV2Operations_decrypt_change_Test { ); } + @Test + public void can_pass_through_new_member_label_access_field_27() { + GroupChange.Actions.Builder encryptedChange = groupOperations.createChangeMemberLabelRights(AccessControl.AccessRequired.ADMINISTRATOR); + + DecryptedGroupChange.Builder expectedDecryptedChange = new DecryptedGroupChange.Builder() + .newMemberLabelAccess(AccessControl.AccessRequired.ADMINISTRATOR); + + assertDecryption(encryptedChange, expectedDecryptedChange); + } + private static ProfileKey newProfileKey() { try { return new ProfileKey(Util.getSecretBytes(32));