From 44ec15c0e07d946553e470ffb44f9d8e1ac34cd6 Mon Sep 17 00:00:00 2001 From: Michelle Tang Date: Tue, 25 Nov 2025 11:19:20 -0500 Subject: [PATCH] Enforce correct permissions when pinning messages. --- .../conversation/ConversationReactionDelegate.java | 5 +++-- .../conversation/ConversationReactionOverlay.java | 7 +++++-- .../securesms/conversation/MenuState.java | 7 ++++--- .../conversation/PinnedMessagesBottomSheet.kt | 8 ++++++-- .../conversation/v2/ConversationBannerView.kt | 3 ++- .../conversation/v2/ConversationFragment.kt | 12 +++++++----- .../conversation/v2/PinnedMessagesComponent.kt | 5 ++++- .../v2/groups/ConversationGroupMemberLevel.kt | 3 ++- .../v2/groups/ConversationGroupViewModel.kt | 8 +++++++- .../securesms/jobs/PushGroupSendJob.java | 5 ++++- .../thoughtcrime/securesms/jobs/UnpinMessageJob.kt | 10 ++++++++++ .../securesms/messages/DataMessageProcessor.kt | 9 +++++++++ 12 files changed, 63 insertions(+), 19 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationReactionDelegate.java b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationReactionDelegate.java index 612148b9f6..5899300588 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationReactionDelegate.java +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationReactionDelegate.java @@ -39,9 +39,10 @@ public final class ConversationReactionDelegate { @NonNull Recipient conversationRecipient, @NonNull ConversationMessage conversationMessage, boolean isNonAdminInAnnouncementGroup, - @NonNull SelectedConversationModel selectedConversationModel) + @NonNull SelectedConversationModel selectedConversationModel, + boolean canEditGroupInfo) { - resolveOverlay().show(activity, conversationRecipient, conversationMessage, lastSeenDownPoint, isNonAdminInAnnouncementGroup, selectedConversationModel); + resolveOverlay().show(activity, conversationRecipient, conversationMessage, lastSeenDownPoint, isNonAdminInAnnouncementGroup, selectedConversationModel, canEditGroupInfo); } public void hide() { diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationReactionOverlay.java b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationReactionOverlay.java index f8d1b8397e..834bed6d96 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationReactionOverlay.java +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationReactionOverlay.java @@ -69,6 +69,7 @@ public final class ConversationReactionOverlay extends FrameLayout { private SelectedConversationModel selectedConversationModel; private OverlayState overlayState = OverlayState.HIDDEN; private boolean isNonAdminInAnnouncementGroup; + private boolean canEditGroupInfo; private boolean downIsOurs; private int selected = -1; @@ -144,7 +145,8 @@ public final class ConversationReactionOverlay extends FrameLayout { @NonNull ConversationMessage conversationMessage, @NonNull PointF lastSeenDownPoint, boolean isNonAdminInAnnouncementGroup, - @NonNull SelectedConversationModel selectedConversationModel) + @NonNull SelectedConversationModel selectedConversationModel, + boolean canEditGroupInfo) { if (overlayState != OverlayState.HIDDEN) { return; @@ -154,6 +156,7 @@ public final class ConversationReactionOverlay extends FrameLayout { this.conversationRecipient = conversationRecipient; this.selectedConversationModel = selectedConversationModel; this.isNonAdminInAnnouncementGroup = isNonAdminInAnnouncementGroup; + this.canEditGroupInfo = canEditGroupInfo; overlayState = OverlayState.UNINITAILIZED; selected = -1; @@ -674,7 +677,7 @@ public final class ConversationReactionOverlay extends FrameLayout { } private @NonNull List getMenuActionItems(@NonNull ConversationMessage conversationMessage) { - MenuState menuState = MenuState.getMenuState(conversationRecipient, conversationMessage.getMultiselectCollection().toSet(), false, isNonAdminInAnnouncementGroup); + MenuState menuState = MenuState.getMenuState(conversationRecipient, conversationMessage.getMultiselectCollection().toSet(), false, isNonAdminInAnnouncementGroup, canEditGroupInfo); List items = new ArrayList<>(); diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/MenuState.java b/app/src/main/java/org/thoughtcrime/securesms/conversation/MenuState.java index ef8954c02d..cc41b9b4c8 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/MenuState.java +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/MenuState.java @@ -102,7 +102,8 @@ public final class MenuState { public static MenuState getMenuState(@NonNull Recipient conversationRecipient, @NonNull Set selectedParts, boolean shouldShowMessageRequest, - boolean isNonAdminInAnnouncementGroup) + boolean isNonAdminInAnnouncementGroup, + boolean canEditGroupInfo) { Builder builder = new Builder(); @@ -170,11 +171,11 @@ public final class MenuState { hasPollTerminate = true; } - if (RemoteConfig.sendPinnedMessages() && !messageRecord.isPending() && messageRecord.getPinnedUntil() == 0 && !conversationRecipient.isReleaseNotes()) { // TODO(michelle): Also check against group permissions + if (RemoteConfig.sendPinnedMessages() && !messageRecord.isPending() && messageRecord.getPinnedUntil() == 0 && !conversationRecipient.isReleaseNotes() && canEditGroupInfo) { canPinMessage = true; } - if (RemoteConfig.sendPinnedMessages() && messageRecord.getPinnedUntil() != 0 && !conversationRecipient.isReleaseNotes()) { // TODO(michelle): Also check against group permissions + if (RemoteConfig.sendPinnedMessages() && messageRecord.getPinnedUntil() != 0 && !conversationRecipient.isReleaseNotes() && canEditGroupInfo) { canUnpinMessage = true; } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/PinnedMessagesBottomSheet.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/PinnedMessagesBottomSheet.kt index 598aad72e2..2ab944940b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/PinnedMessagesBottomSheet.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/PinnedMessagesBottomSheet.kt @@ -38,6 +38,7 @@ import org.thoughtcrime.securesms.recipients.RecipientId import org.thoughtcrime.securesms.util.BottomSheetUtil import org.thoughtcrime.securesms.util.StickyHeaderDecoration import org.thoughtcrime.securesms.util.fragments.findListener +import org.thoughtcrime.securesms.util.visible import java.util.Locale /** @@ -110,12 +111,13 @@ class PinnedMessagesBottomSheet : FixedRoundedCornerBottomSheetDialogFragment() initializeGiphyMp4(view.findViewById(R.id.video_container)!!, list) - // TODO(michelle): Hide if not allowed to unpin / Check with design about a confirmation dialog here + // TODO(michelle): Check with design about a confirmation dialog here val unpinAll = view.findViewById(R.id.unpin_all) unpinAll.setOnClickListener { viewModel.unpinMessage() dismissAllowingStateLoss() } + unpinAll.visible = requireArguments().getBoolean(KEY_CAN_UNPIN) } private fun initializeGiphyMp4(videoContainer: ViewGroup, list: RecyclerView): GiphyMp4ProjectionRecycler { @@ -271,12 +273,14 @@ class PinnedMessagesBottomSheet : FixedRoundedCornerBottomSheetDialogFragment() private const val KEY_THREAD_ID = "thread_id" private const val KEY_CONVERSATION_RECIPIENT_ID = "conversation_recipient_id" + private const val KEY_CAN_UNPIN = "can_unpin" @JvmStatic - fun show(fragmentManager: FragmentManager, threadId: Long, conversationRecipientId: RecipientId) { + fun show(fragmentManager: FragmentManager, threadId: Long, conversationRecipientId: RecipientId, canUnpin: Boolean) { val args = Bundle().apply { putLong(KEY_THREAD_ID, threadId) putString(KEY_CONVERSATION_RECIPIENT_ID, conversationRecipientId.serialize()) + putBoolean(KEY_CAN_UNPIN, canUnpin) } val fragment = PinnedMessagesBottomSheet().apply { diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationBannerView.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationBannerView.kt index ff0a61065d..8c0ac42a40 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationBannerView.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationBannerView.kt @@ -133,7 +133,7 @@ class ConversationBannerView @JvmOverloads constructor( hide(voiceNotePlayerStub) } - fun showPinnedMessageStub(messages: List) { + fun showPinnedMessageStub(messages: List, canUnpin: Boolean) { show( stub = pinnedMessageStub ) { @@ -142,6 +142,7 @@ class ConversationBannerView @JvmOverloads constructor( SignalTheme(isDarkMode = DynamicTheme.isDarkTheme(context)) { PinnedMessagesBanner( messages = messages, + canUnpin = canUnpin, onUnpinMessage = { messageId -> listener?.onUnpinMessage(messageId) }, onGoToMessage = { messageId -> listener?.onGoToMessage(messageId) }, onViewAllMessages = { listener?.onViewAllMessages() } diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationFragment.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationFragment.kt index 5b31f15190..325aac5251 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationFragment.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationFragment.kt @@ -1336,7 +1336,7 @@ class ConversationFragment : private fun presentPinnedMessage(pinnedMessages: List) { if (pinnedMessages.isNotEmpty()) { - binding.conversationBanner.showPinnedMessageStub(pinnedMessages) + binding.conversationBanner.showPinnedMessageStub(messages = pinnedMessages, canUnpin = conversationGroupViewModel.canEditGroupInfo()) } else { binding.conversationBanner.hidePinnedMessageStub() } @@ -2311,7 +2311,8 @@ class ConversationFragment : recipient, selectedParts, viewModel.hasMessageRequestState, - conversationGroupViewModel.isNonAdminInAnnouncementGroup() + conversationGroupViewModel.isNonAdminInAnnouncementGroup(), + conversationGroupViewModel.canEditGroupInfo() ) val items = arrayListOf() @@ -2458,7 +2459,7 @@ class ConversationFragment : ) { reactionDelegate.setOnActionSelectedListener(onActionSelectedListener) reactionDelegate.setOnHideListener(onHideListener) - reactionDelegate.show(requireActivity(), viewModel.recipientSnapshot!!, conversationMessage, conversationGroupViewModel.isNonAdminInAnnouncementGroup(), selectedConversationModel) + reactionDelegate.show(requireActivity(), viewModel.recipientSnapshot!!, conversationMessage, conversationGroupViewModel.isNonAdminInAnnouncementGroup(), selectedConversationModel, conversationGroupViewModel.canEditGroupInfo()) viewModel.setIsReactionDelegateShowing(true) composeText.clearFocus() } @@ -4232,8 +4233,9 @@ class ConversationFragment : override fun onViewAllMessages() { PinnedMessagesBottomSheet.show( childFragmentManager, - args.threadId, - viewModel.recipientSnapshot?.id!! + threadId = args.threadId, + conversationRecipientId = viewModel.recipientSnapshot?.id!!, + canUnpin = conversationGroupViewModel.canEditGroupInfo() ) } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/PinnedMessagesComponent.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/PinnedMessagesComponent.kt index 8ed88c5c41..178f1ecfbb 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/PinnedMessagesComponent.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/PinnedMessagesComponent.kt @@ -63,6 +63,7 @@ import kotlin.jvm.optionals.getOrDefault @Composable fun PinnedMessagesBanner( messages: List = emptyList(), + canUnpin: Boolean, onUnpinMessage: (Long) -> Unit = {}, onGoToMessage: (Long) -> Unit = {}, onViewAllMessages: () -> Unit = {} @@ -150,7 +151,9 @@ fun PinnedMessagesBanner( DropdownMenus.Menu(controller = menuController, offsetX = 2.dp, offsetY = 16.dp) { menuController -> Column { - DropdownMenus.ItemWithIcon(menuController, R.drawable.symbol_pin_slash_24, R.string.PinnedMessage__unpin_message) { onUnpinMessage(message.id) } + if (canUnpin) { + DropdownMenus.ItemWithIcon(menuController, R.drawable.symbol_pin_slash_24, R.string.PinnedMessage__unpin_message) { onUnpinMessage(message.id) } + } DropdownMenus.ItemWithIcon(menuController, R.drawable.symbol_chat_24, R.string.PinnedMessage__go_to_message) { onGoToMessage(message.id) } DropdownMenus.ItemWithIcon(menuController, R.drawable.symbol_list_bullet_24, R.string.PinnedMessage__view_all_messages) { onViewAllMessages() } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/groups/ConversationGroupMemberLevel.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/groups/ConversationGroupMemberLevel.kt index b9f47aab6a..662ba890f0 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/groups/ConversationGroupMemberLevel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/groups/ConversationGroupMemberLevel.kt @@ -8,5 +8,6 @@ import org.thoughtcrime.securesms.database.GroupTable */ data class ConversationGroupMemberLevel( val groupTableMemberLevel: GroupTable.MemberLevel, - val isAnnouncementGroup: Boolean + val isAnnouncementGroup: Boolean, + val allMembersCanEditGroupInfo: Boolean ) diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/groups/ConversationGroupViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/groups/ConversationGroupViewModel.kt index 688ec2a40d..172b513c58 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/groups/ConversationGroupViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/groups/ConversationGroupViewModel.kt @@ -17,6 +17,7 @@ import org.thoughtcrime.securesms.conversation.v2.ConversationRecipientRepositor import org.thoughtcrime.securesms.database.GroupTable import org.thoughtcrime.securesms.database.model.GroupRecord import org.thoughtcrime.securesms.dependencies.AppDependencies +import org.thoughtcrime.securesms.groups.GroupAccessControl import org.thoughtcrime.securesms.groups.GroupId import org.thoughtcrime.securesms.groups.ui.GroupChangeFailureReason import org.thoughtcrime.securesms.groups.v2.GroupBlockJoinRequestResult @@ -53,7 +54,7 @@ class ConversationGroupViewModel( init { disposables += _groupRecord.subscribe { groupRecord -> _groupActiveState.onNext(ConversationGroupActiveState(groupRecord.isActive, groupRecord.isV2Group)) - _memberLevel.onNext(ConversationGroupMemberLevel(groupRecord.memberLevel(Recipient.self()), groupRecord.isAnnouncementGroup)) + _memberLevel.onNext(ConversationGroupMemberLevel(groupRecord.memberLevel(Recipient.self()), groupRecord.isAnnouncementGroup, groupRecord.attributesAccessControl == GroupAccessControl.ALL_MEMBERS)) } } @@ -66,6 +67,11 @@ class ConversationGroupViewModel( return memberLevel.groupTableMemberLevel != GroupTable.MemberLevel.ADMINISTRATOR && memberLevel.isAnnouncementGroup } + fun canEditGroupInfo(): Boolean { + val memberLevel = _memberLevel.value ?: return true + return memberLevel.groupTableMemberLevel == GroupTable.MemberLevel.ADMINISTRATOR || memberLevel.allMembersCanEditGroupInfo + } + fun blockJoinRequests(recipient: Recipient): Single { return _groupRecord .firstOrError() diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/PushGroupSendJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/PushGroupSendJob.java index 6f314e2b10..e9b743d4a9 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/PushGroupSendJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/PushGroupSendJob.java @@ -25,6 +25,7 @@ import org.thoughtcrime.securesms.database.model.GroupRecord; import org.thoughtcrime.securesms.database.model.MessageId; import org.thoughtcrime.securesms.database.model.MessageRecord; import org.thoughtcrime.securesms.dependencies.AppDependencies; +import org.thoughtcrime.securesms.groups.GroupAccessControl; import org.thoughtcrime.securesms.groups.GroupId; import org.thoughtcrime.securesms.jobmanager.Job; import org.thoughtcrime.securesms.jobmanager.JobLogger; @@ -348,7 +349,9 @@ public final class PushGroupSendJob extends PushSendJob { } else { Optional groupRecord = SignalDatabase.groups().getGroup(groupRecipient.requireGroupId()); - if (groupRecord.isPresent() && groupRecord.get().isAnnouncementGroup() && !groupRecord.get().isAdmin(Recipient.self())) { + if (pinnedMessage != null && groupRecord.isPresent() && groupRecord.get().getAttributesAccessControl() == GroupAccessControl.ONLY_ADMINS && !groupRecord.get().isAdmin(Recipient.self())) { + throw new UndeliverableMessageException("Non-admins cannot pin messages in this group!"); + } else if (pinnedMessage == null && groupRecord.isPresent() && groupRecord.get().isAnnouncementGroup() && !groupRecord.get().isAdmin(Recipient.self())) { throw new UndeliverableMessageException("Non-admins cannot send messages in announcement groups!"); } diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/UnpinMessageJob.kt b/app/src/main/java/org/thoughtcrime/securesms/jobs/UnpinMessageJob.kt index 23bd8815ba..1f1357096f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/UnpinMessageJob.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/UnpinMessageJob.kt @@ -3,6 +3,7 @@ package org.thoughtcrime.securesms.jobs import org.signal.core.util.logging.Log import org.thoughtcrime.securesms.database.SignalDatabase import org.thoughtcrime.securesms.database.model.MessageId +import org.thoughtcrime.securesms.groups.GroupAccessControl import org.thoughtcrime.securesms.groups.GroupId import org.thoughtcrime.securesms.jobmanager.Job import org.thoughtcrime.securesms.jobmanager.impl.NetworkConstraint @@ -10,6 +11,7 @@ import org.thoughtcrime.securesms.jobs.protos.UnpinJobData import org.thoughtcrime.securesms.keyvalue.SignalStore import org.thoughtcrime.securesms.messages.GroupSendUtil import org.thoughtcrime.securesms.recipients.Recipient +import org.thoughtcrime.securesms.recipients.Recipient.Companion.self import org.thoughtcrime.securesms.recipients.RecipientId import org.thoughtcrime.securesms.recipients.RecipientUtil import org.thoughtcrime.securesms.util.GroupUtil @@ -97,6 +99,14 @@ class UnpinMessageJob( return Result.failure() } + if (conversationRecipient.isPushV2Group) { + val groupRecord = SignalDatabase.groups.getGroup(conversationRecipient.id) + if (groupRecord.isPresent && groupRecord.get().attributesAccessControl == GroupAccessControl.ONLY_ADMINS && !groupRecord.get().isAdmin(self())) { + Log.w(TAG, "Non-admins cannot send unpin messages to group.") + return Result.failure() + } + } + val targetSentTimestamp = message.dateSent val recipients = Recipient.resolvedList(recipientIds.filter { it != Recipient.self().id.toLong() }.map { RecipientId.from(it) }) diff --git a/app/src/main/java/org/thoughtcrime/securesms/messages/DataMessageProcessor.kt b/app/src/main/java/org/thoughtcrime/securesms/messages/DataMessageProcessor.kt index 108f7f63e4..c6f3f9ce51 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/messages/DataMessageProcessor.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/messages/DataMessageProcessor.kt @@ -48,6 +48,7 @@ import org.thoughtcrime.securesms.database.model.databaseprotos.PollTerminate import org.thoughtcrime.securesms.database.model.toBodyRangeList import org.thoughtcrime.securesms.dependencies.AppDependencies import org.thoughtcrime.securesms.groups.BadGroupIdException +import org.thoughtcrime.securesms.groups.GroupAccessControl import org.thoughtcrime.securesms.groups.GroupId import org.thoughtcrime.securesms.jobs.AttachmentDownloadJob import org.thoughtcrime.securesms.jobs.DirectoryRefreshJob @@ -1305,6 +1306,10 @@ object DataMessageProcessor { warn(envelope.timestamp!!, "[handlePinMessage] Sender is not in the group! timestamp: ${pinMessage.targetSentTimestamp}") return null } + if (groupRecord != null && groupRecord.attributesAccessControl == GroupAccessControl.ONLY_ADMINS && !groupRecord.isAdmin(senderRecipient)) { + warn(envelope.timestamp!!, "[handlePinMessage] Sender needs to be an admin! timestamp: ${pinMessage.targetSentTimestamp}") + return null + } if (groupRecord == null && senderRecipient.id != threadRecipient.id && Recipient.self().id != senderRecipient.id) { warn(envelope.timestamp!!, "[handlePinMessage] Sender is not a part of the 1:1 thread! timestamp: ${pinMessage.targetSentTimestamp}") @@ -1384,6 +1389,10 @@ object DataMessageProcessor { warn(envelope.timestamp!!, "[handleUnpinMessage] Sender is not in the group! timestamp: ${unpinMessage.targetSentTimestamp}") return null } + if (groupRecord != null && groupRecord.attributesAccessControl == GroupAccessControl.ONLY_ADMINS && !groupRecord.isAdmin(senderRecipient)) { + warn(envelope.timestamp!!, "[handleUnpinMessage] Sender needs to be an admin! timestamp: ${unpinMessage.targetSentTimestamp}") + return null + } if (groupRecord == null && senderRecipient.id != threadRecipient.id && Recipient.self().id != senderRecipient.id) { warn(envelope.timestamp!!, "[handleUnpinMessage] Sender is not a part of the 1:1 thread! timestamp: ${unpinMessage.targetSentTimestamp}")