Enforce correct permissions when pinning messages.

2025-12-20 02:58:45 +00:00 · 2025-11-25 11:19:20 -05:00
parent 40008bddc7
commit 44ec15c0e0
12 changed files with 63 additions and 19 deletions
--- 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() {
--- 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<ActionItem> 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<ActionItem> items = new ArrayList<>();

--- 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<MultiselectPart> 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;
      }
    }
--- 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<TextView>(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 {
--- 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<ConversationMessage>) {
+  fun showPinnedMessageStub(messages: List<ConversationMessage>, 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() }
--- 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<ConversationMessage>) {
    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<ActionItem>()
@@ -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()
      )
    }
  }
--- 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<ConversationMessage> = 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 {
+            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() }
          }
--- 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
 )
--- 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<GroupBlockJoinRequestResult> {
    return _groupRecord
      .firstOrError()
--- 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> 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!");
        }

--- 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) })
--- 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}")