🐛(global) allow thread viewers to post internal comments

src/backend/core/api/permissions.py

@@ -545,6 +545,98 @@ def initial(self, request, *args, **kwargs):
             raise PermissionDenied()
 
 
+def _user_can_comment_on_thread(user, thread_id):
+    """True if ``user`` can post/read internal comments on ``thread_id``.
+
+    Requires any ``ThreadAccess`` (viewer or editor) on a mailbox where the
+    user has ``MAILBOX_ROLES_CAN_EDIT``. Writing an internal comment is a
+    personal authoring act that does not mutate the thread's shared state,
+    so VIEWER ThreadAccess is enough as long as the user is at least editor
+    on the mailbox.
+    """
+    return models.ThreadAccess.objects.filter(
+        thread_id=thread_id,
+        mailbox__accesses__user=user,
+        mailbox__accesses__role__in=enums.MAILBOX_ROLES_CAN_EDIT,
+    ).exists()
+
+
+class HasThreadCommentAccess(IsAuthenticated):
+    """Allows users who can author internal comments on the thread.
+
+    Used for endpoints that serve the comment authoring flow (posting an
+    ``im`` ThreadEvent, listing thread users to pick mention targets): both
+    must stay accessible to Thread VIEWERs as long as they are at least
+    editor on one of the mailboxes sharing the thread.
+    """
+
+    def has_permission(self, request, view):
+        if not super().has_permission(request, view):
+            return False
+
+        thread_id = view.kwargs.get("thread_id")
+        if not thread_id:
+            return False
+
+        return _user_can_comment_on_thread(request.user, thread_id)
+
+
+class HasThreadEventWriteAccess(IsAuthenticated):
+    """Gates write actions on ThreadEvent — rules depend on the event type.
+
+    Only ``im`` events (internal comments) relax the thread-level role: a
+    Thread VIEWER with mailbox edit rights can create/update/destroy their
+    own ``im`` events. Any other event type is considered a shared-state
+    mutation (system event, status change, …) and keeps the stricter
+    full-edit-rights policy (``MailboxAccess`` in ``MAILBOX_ROLES_CAN_EDIT``
+    AND ``ThreadAccess.role == EDITOR``).
+
+    Author check for update/destroy is enforced here regardless of type —
+    no user can edit or delete another user's event.
+    """
+
+    message = "You do not have permission to perform this action on this thread."
+
+    def has_permission(self, request, view):
+        if not super().has_permission(request, view):
+            return False
+
+        thread_id = view.kwargs.get("thread_id")
+        if thread_id is None:
+            return True
+
+        if view.action == "create":
+            event_type = request.data.get("type") if hasattr(request, "data") else None
+            if event_type == enums.ThreadEventTypeChoices.IM:
+                return _user_can_comment_on_thread(request.user, thread_id)
+            # Non-IM (or missing) types require full edit rights. Using the
+            # stricter path as the default keeps unknown types safe.
+            return (
+                models.ThreadAccess.objects.editable_by(request.user)
+                .filter(thread_id=thread_id)
+                .exists()
+            )
+
+        # update / partial_update / destroy: defer to object-level check
+        # (type is resolved from the stored instance, not the payload).
+        return True
+
+    def has_object_permission(self, request, view, obj):
+        if not isinstance(obj, models.ThreadEvent):
+            return False
+
+        if (
+            view.action in ("update", "partial_update", "destroy")
+            and obj.author_id != request.user.id
+        ):
+            return False
+
+        if obj.type == enums.ThreadEventTypeChoices.IM:
+            return _user_can_comment_on_thread(request.user, obj.thread_id)
+
+        return obj.thread.get_abilities(request.user)[enums.ThreadAbilities.CAN_EDIT]
+
+
 class HasThreadEditAccess(IsAuthenticated):
     """Allows access only to users with full edit rights on the thread.
 

src/backend/core/api/viewsets/thread_event.py

@@ -36,16 +36,18 @@ class ThreadEventViewSet(
     lookup_url_kwarg = "id"
 
     def get_permissions(self):
-        """Use HasThreadEditAccess for write actions.
+        """Route write actions through the type-aware permission class.
 
-        ThreadEvent write operations require editor access, except
-        ``read_mention`` which is a personal acknowledgement and only
-        requires read access on the thread.
+        Reads (``list``, ``retrieve``) and the personal ``read_mention``
+        acknowledgement only need read access on the thread. Writes are
+        gated by :class:`HasThreadEventWriteAccess`, which relaxes the
+        ThreadAccess role for ``im`` (comment) events while keeping the
+        stricter full-edit-rights rule for every other event type.
         """
         if self.action in ["list", "retrieve", "read_mention"]:
             return [permissions.IsAuthenticated(), permissions.IsAllowedToAccess()]
 
-        return [permissions.HasThreadEditAccess()]
+        return [permissions.HasThreadEventWriteAccess()]
 
     def get_queryset(self):
         """Restrict results to events for the specified thread."""

src/backend/core/api/viewsets/thread_user.py

@@ -19,7 +19,7 @@ class ThreadUserViewSet(
     pagination_class = None
     permission_classes = [
         permissions.IsAuthenticated,
-        permissions.IsAllowedToManageThreadAccess,
+        permissions.HasThreadCommentAccess,
     ]
 
     def get_queryset(self):

src/backend/core/tests/api/test_thread_event_permissions.py

@@ -280,22 +280,27 @@ def test_author_can_delete_own_event(self, api_client):
         assert response.status_code == status.HTTP_204_NO_CONTENT
 
 
-class TestViewerCannotCreateEvents:
-    """Test that VIEWER-role users cannot create events (only EDITOR+)."""
+class TestImEventCreationPermissions:
+    """Permission matrix for creating ``im`` ThreadEvents (internal comments).
 
-    def test_viewer_cannot_create_event(self, api_client):
-        """Users with only VIEWER role on a thread should not create events."""
+    ``im`` events are personal authoring acts (not shared-state mutations),
+    so Thread VIEWERs may post them as long as they are at least editor on
+    the mailbox. The mailbox role is the hard floor.
+    """
+
+    def test_thread_viewer_mailbox_editor_can_create_im(self, api_client):
+        """Thread VIEWER + mailbox editor: must be allowed to comment."""
         user, _mailbox, thread = setup_user_with_thread_access(
             role=enums.ThreadAccessRoleChoices.VIEWER
         )
         api_client.force_authenticate(user=user)
 
-        data = {"type": "im", "data": {"content": "from a viewer"}}
+        data = {"type": "im", "data": {"content": "from a thread viewer"}}
         response = api_client.post(get_thread_event_url(thread.id), data, format="json")
-        assert response.status_code == status.HTTP_403_FORBIDDEN
+        assert response.status_code == status.HTTP_201_CREATED
 
-    def test_editor_can_create_event(self, api_client):
-        """Users with EDITOR role should be able to create events (sanity check)."""
+    def test_thread_editor_can_create_im(self, api_client):
+        """Thread EDITOR + mailbox admin: sanity check."""
         user, _mailbox, thread = setup_user_with_thread_access(
             role=enums.ThreadAccessRoleChoices.EDITOR
         )
@@ -305,20 +310,18 @@ def test_editor_can_create_event(self, api_client):
         response = api_client.post(get_thread_event_url(thread.id), data, format="json")
         assert response.status_code == status.HTTP_201_CREATED
 
-    def test_viewer_mailbox_with_editor_thread_access_cannot_create_event(
-        self, api_client
-    ):
-        """Previously-missed scenario: a user with only VIEWER MailboxAccess
-        cannot post events even when the mailbox has EDITOR ThreadAccess.
-        Creating a ThreadEvent mutates shared state, so both role checks
-        must pass.
+    def test_viewer_mailbox_cannot_create_im(self, api_client):
+        """Mailbox VIEWER cannot post comments even with EDITOR thread access.
+
+        Authoring a comment requires at least editor-level mailbox role,
+        otherwise a read-only teammate could inject content into threads.
         """
         user = factories.UserFactory()
         mailbox = factories.MailboxFactory()
         factories.MailboxAccessFactory(
             mailbox=mailbox,
             user=user,
-            role=enums.MailboxRoleChoices.VIEWER,  # VIEWER mailbox role
+            role=enums.MailboxRoleChoices.VIEWER,
         )
         thread = factories.ThreadFactory()
         factories.ThreadAccessFactory(
@@ -332,6 +335,87 @@ def test_viewer_mailbox_with_editor_thread_access_cannot_create_event(
         response = api_client.post(get_thread_event_url(thread.id), data, format="json")
         assert response.status_code == status.HTTP_403_FORBIDDEN
 
+    def test_no_thread_access_cannot_create_im(self, api_client):
+        """A user without any ThreadAccess on the thread is denied."""
+        user = factories.UserFactory()
+        mailbox = factories.MailboxFactory()
+        factories.MailboxAccessFactory(
+            mailbox=mailbox,
+            user=user,
+            role=enums.MailboxRoleChoices.ADMIN,
+        )
+        thread = factories.ThreadFactory()  # no ThreadAccess for user's mailbox
+        api_client.force_authenticate(user=user)
+
+        data = {"type": "im", "data": {"content": "from outsider"}}
+        response = api_client.post(get_thread_event_url(thread.id), data, format="json")
+        assert response.status_code == status.HTTP_403_FORBIDDEN
+
+
+class TestImEventAuthorEditsOwnComment:
+    """Authors of ``im`` events must be able to edit/delete their own
+    comment even if they are only VIEWER on the thread.
+
+    If we allowed them to create the comment, we must let them take it
+    back — no orphan comments the author can't retract.
+    """
+
+    def test_thread_viewer_author_can_update_own_im(self, api_client):
+        """
+        A user with VIEWER access to a thread can update their own ``im`` event.
+        """
+        user, _mailbox, thread = setup_user_with_thread_access(
+            role=enums.ThreadAccessRoleChoices.VIEWER
+        )
+        api_client.force_authenticate(user=user)
+        event = factories.ThreadEventFactory(thread=thread, author=user, type="im")
+
+        response = api_client.patch(
+            get_thread_event_url(thread.id, event.id),
+            {"data": {"content": "edited from viewer"}},
+            format="json",
+        )
+        assert response.status_code == status.HTTP_200_OK
+        event.refresh_from_db()
+        assert event.data["content"] == "edited from viewer"
+
+    def test_thread_viewer_author_can_destroy_own_im(self, api_client):
+        """
+        A user with VIEWER access to a thread can destroy their own ``im`` event.
+        """
+        user, _mailbox, thread = setup_user_with_thread_access(
+            role=enums.ThreadAccessRoleChoices.VIEWER
+        )
+        api_client.force_authenticate(user=user)
+        event = factories.ThreadEventFactory(thread=thread, author=user, type="im")
+
+        response = api_client.delete(get_thread_event_url(thread.id, event.id))
+        assert response.status_code == status.HTTP_204_NO_CONTENT
+        assert not models.ThreadEvent.objects.filter(id=event.id).exists()
+
+    def test_mailbox_viewer_author_cannot_update_own_im(self, api_client):
+        """Mailbox role dropped to VIEWER after the event was created:
+        the author has lost comment rights and can no longer edit.
+        """
+        user = factories.UserFactory()
+        mailbox = factories.MailboxFactory()
+        factories.MailboxAccessFactory(
+            mailbox=mailbox, user=user, role=enums.MailboxRoleChoices.VIEWER
+        )
+        thread = factories.ThreadFactory()
+        factories.ThreadAccessFactory(
+            mailbox=mailbox, thread=thread, role=enums.ThreadAccessRoleChoices.EDITOR
+        )
+        event = factories.ThreadEventFactory(thread=thread, author=user, type="im")
+
+        api_client.force_authenticate(user=user)
+        response = api_client.patch(
+            get_thread_event_url(thread.id, event.id),
+            {"data": {"content": "nope"}},
+            format="json",
+        )
+        assert response.status_code == status.HTTP_403_FORBIDDEN
+
 
 class TestParameterConfusionAttack:
     """Test that conflicting thread_id in URL path vs query params can't bypass permissions."""

src/backend/core/tests/api/test_thread_user.py

@@ -62,9 +62,10 @@ def test_list_thread_users_nonexistent_thread(self, api_client):
 class TestThreadUserListPermissions:
     """Test permission matrix for GET /threads/{thread_id}/users/.
 
-    The endpoint uses IsAllowedToManageThreadAccess which requires:
-    - ThreadAccess role = EDITOR on the thread
-    - MailboxAccess role in MAILBOX_ROLES_CAN_EDIT (EDITOR, SENDER, ADMIN)
+    The endpoint backs the mention picker in the comment composer, so it
+    must stay reachable to anyone who can post an ``im`` comment: mailbox
+    role in MAILBOX_ROLES_CAN_EDIT (EDITOR, SENDER, ADMIN) + any
+    ThreadAccess (VIEWER or EDITOR).
     """
 
     @pytest.mark.parametrize(
@@ -73,12 +74,17 @@ class TestThreadUserListPermissions:
             (enums.ThreadAccessRoleChoices.EDITOR, enums.MailboxRoleChoices.ADMIN),
             (enums.ThreadAccessRoleChoices.EDITOR, enums.MailboxRoleChoices.EDITOR),
             (enums.ThreadAccessRoleChoices.EDITOR, enums.MailboxRoleChoices.SENDER),
+            # Thread VIEWER with editor-level mailbox role: allowed because
+            # the user can still author comments and needs to pick mentions.
+            (enums.ThreadAccessRoleChoices.VIEWER, enums.MailboxRoleChoices.ADMIN),
+            (enums.ThreadAccessRoleChoices.VIEWER, enums.MailboxRoleChoices.EDITOR),
+            (enums.ThreadAccessRoleChoices.VIEWER, enums.MailboxRoleChoices.SENDER),
         ],
     )
     def test_list_thread_users_allowed(
         self, api_client, thread_access_role, mailbox_access_role
     ):
-        """Users with EDITOR thread access + EDITOR/SENDER/ADMIN mailbox role can list."""
+        """Any ThreadAccess + editor-level MailboxAccess grants list access."""
         user, _mailbox, thread = setup_user_with_thread_access(
             thread_role=thread_access_role,
             mailbox_role=mailbox_access_role,
@@ -91,12 +97,9 @@ def test_list_thread_users_allowed(
     @pytest.mark.parametrize(
         "thread_access_role, mailbox_access_role",
         [
-            # VIEWER on thread — regardless of mailbox role
-            (enums.ThreadAccessRoleChoices.VIEWER, enums.MailboxRoleChoices.ADMIN),
-            (enums.ThreadAccessRoleChoices.VIEWER, enums.MailboxRoleChoices.EDITOR),
-            (enums.ThreadAccessRoleChoices.VIEWER, enums.MailboxRoleChoices.SENDER),
-            # EDITOR on thread but only VIEWER on mailbox
+            # Mailbox VIEWER: mailbox role too low regardless of thread role.
             (enums.ThreadAccessRoleChoices.EDITOR, enums.MailboxRoleChoices.VIEWER),
+            (enums.ThreadAccessRoleChoices.VIEWER, enums.MailboxRoleChoices.VIEWER),
         ],
     )
     def test_list_thread_users_forbidden(

src/frontend/src/features/forms/components/combobox/_index.scss

@@ -197,6 +197,7 @@
   &::after {
     content: attr(data-value) ' ';
     white-space: pre-wrap;
+    visibility: hidden;
     pointer-events: none;
   }
 

src/frontend/src/features/forms/components/message-form/attachment-uploader.tsx

@@ -69,7 +69,7 @@ export const AttachmentUploader = ({
                 >
                     {t("Add attachments")}
                 </Button>
-                <DriveAttachmentPicker onPick={onDriveAttachmentPick} />
+                <DriveAttachmentPicker onPick={onDriveAttachmentPick} disabled={disabled} />
                 <p className="attachment-uploader__input__helper-text">
                     {t("or drag and drop some files")}
                 </p>

src/frontend/src/features/forms/components/message-form/index.tsx

@@ -97,7 +97,7 @@ export const MessageForm = ({
     const autoSaveTimerRef = useRef<NodeJS.Timeout | null>(null);
     const saveDraftRef = useRef<() => void>(() => {});
     const quoteType: QuoteType | undefined = mode !== "new" ? (mode === "forward" ? "forward" : "reply") : undefined;
-    const { selectedMailbox, mailboxes, invalidateThreadMessages, invalidateThreadsStats, unselectThread } = useMailboxContext();
+    const { selectedMailbox, selectedThread, mailboxes, invalidateThreadMessages, invalidateThreadsStats, unselectThread } = useMailboxContext();
     const hideSubjectField = Boolean(draftMessage?.parent_id ?? parentMessage);
     const defaultSenderId = mailboxes?.find((mailbox) => {
         if (draft?.sender) return draft.sender.email === mailbox.email;
@@ -221,8 +221,10 @@ export const MessageForm = ({
         name: "from",
     });
     const currentSender = mailboxes?.find((mailbox) => mailbox.id === currentSenderId);
-    const canSendMessages = useAbility(Abilities.CAN_SEND_MESSAGES, currentSender!);
-    const canWriteMessages = useAbility(Abilities.CAN_WRITE_MESSAGES, currentSender!);
+    const canEditCurrentThread = useAbility(Abilities.CAN_EDIT_THREAD, selectedThread);
+    const canEditThread = mode === "new" ? true : canEditCurrentThread;
+    const canSendMessages = useAbility(Abilities.CAN_SEND_MESSAGES, currentSender!) && canEditThread;
+    const canWriteMessages = useAbility(Abilities.CAN_WRITE_MESSAGES, currentSender!) && canEditThread;
     const canChangeSender = !draft || canWriteMessages;
 
     const initialAttachments = useMemo((): (Attachment | DriveFile)[] => {

src/frontend/src/features/layouts/components/mailbox-panel/components/mailbox-actions/index.tsx

@@ -66,4 +66,3 @@ export const MailboxPanelActions = () => {
         </div>
     )
 }
-