From 01b45a69fbb99bbd9e45896ff6d4f47b0b89d3b1 Mon Sep 17 00:00:00 2001 From: jbpenrath Date: Thu, 9 Apr 2026 11:23:56 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B(global)=20enforce=20full=20edit=20?= =?UTF-8?q?rights=20on=20thread=20mutations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A user with VIEWER MailboxAccess on a shared mailbox could still mutate threads that the mailbox had EDITOR ThreadAccess to: the permission check only looked at ThreadAccess.role, never at MailboxAccess.role. Both roles must now be satisfied (EDITOR on ThreadAccess AND a role in MAILBOX_ROLES_CAN_EDIT on MailboxAccess) for archive, spam, trash, label, split, refresh_summary and thread-event writes. Personal actions (unread, starred) intentionally stay open to any mailbox access since they only mutate the caller's own ThreadAccess row. The rule is centralised in ThreadAccessQuerySet.editable_by(user, mailbox_id) so viewsets and permission classes share a single source of truth, and exposed to the frontend via a new Thread.abilities.edit field consumed by use-ability, which gates the matching UI controls. --- src/backend/core/api/openapi.json | 8 + src/backend/core/api/permissions.py | 50 ++-- src/backend/core/api/serializers.py | 25 ++ src/backend/core/api/viewsets/flag.py | 33 ++- src/backend/core/api/viewsets/label.py | 30 ++- src/backend/core/api/viewsets/thread.py | 24 +- .../core/api/viewsets/thread_access.py | 24 +- src/backend/core/enums.py | 6 + src/backend/core/models.py | 58 +++++ src/backend/core/tests/api/test_labels.py | 112 +++++++- .../core/tests/api/test_messages_flag.py | 65 ++++- .../core/tests/api/test_thread_access.py | 243 +++++++++++++++++- .../api/test_thread_event_permissions.py | 27 ++ .../core/tests/api/test_thread_split.py | 22 ++ .../core/tests/api/test_threads_flag.py | 113 +++++++- .../core/tests/api/test_threads_list.py | 63 ++++- .../e2e/management/commands/e2e_demo.py | 2 +- .../src/__tests__/thread-starred-read.spec.ts | 10 +- src/frontend/public/locales/common/en-US.json | 31 +++ src/frontend/public/locales/common/fr-FR.json | 39 +++ .../src/features/api/gen/models/index.ts | 1 + .../src/features/api/gen/models/thread.ts | 2 + .../api/gen/models/thread_abilities.ts | 9 + .../components/thread-panel-header.tsx | 57 ++-- .../thread-accesses-widget/index.tsx | 24 +- .../components/thread-action-bar/index.tsx | 62 ++++- .../components/thread-event-input/index.tsx | 2 +- .../components/thread-labels-widget/index.tsx | 11 +- .../components/thread-message/index.tsx | 7 +- .../thread-message/thread-message-actions.tsx | 21 +- .../thread-message/thread-message-header.tsx | 10 +- .../src/features/message/use-archive.tsx | 12 +- .../src/features/message/use-flag.tsx | 53 +++- .../src/features/message/use-spam.tsx | 12 +- .../src/features/message/use-starred.tsx | 12 +- .../src/features/message/use-trash.tsx | 12 +- src/frontend/src/hooks/use-ability.ts | 13 + 37 files changed, 1155 insertions(+), 150 deletions(-) create mode 100644 src/frontend/src/features/api/gen/models/thread_abilities.ts diff --git a/src/backend/core/api/openapi.json b/src/backend/core/api/openapi.json index 1776f2ad..e7f17dd4 100644 --- a/src/backend/core/api/openapi.json +++ b/src/backend/core/api/openapi.json @@ -9055,9 +9055,17 @@ "events_count": { "type": "integer", "readOnly": true + }, + "abilities": { + "type": "object", + "additionalProperties": { + "type": "boolean" + }, + "readOnly": true } }, "required": [ + "abilities", "accesses", "active_messaged_at", "archived_messaged_at", diff --git a/src/backend/core/api/permissions.py b/src/backend/core/api/permissions.py index b5e3f9cb..39a29821 100644 --- a/src/backend/core/api/permissions.py +++ b/src/backend/core/api/permissions.py @@ -123,17 +123,18 @@ class IsAllowedToAccess(IsAuthenticated): is_list_action = hasattr(view, "action") and view.action == "list" if not is_list_action: - # For create action on nested routes, check thread access with edit role + # For create action on nested routes, check full edit rights + # on the thread: EDITOR ThreadAccess role AND CAN_EDIT MailboxAccess role. if ( thread_id_from_url and hasattr(view, "action") and view.action == "create" ): - return models.ThreadAccess.objects.filter( - thread_id=thread_id_from_url, - role__in=enums.THREAD_ROLES_CAN_EDIT, - mailbox__accesses__user=request.user, - ).exists() + return ( + models.ThreadAccess.objects.editable_by(request.user) + .filter(thread_id=thread_id_from_url) + .exists() + ) # Allow non-list actions (like detail views or specific APIViews like SendMessageView) # to proceed to object-level checks or handle permissions within the view. return True @@ -323,6 +324,17 @@ class IsAllowedToManageThreadAccess(IsAuthenticated): if obj.thread.id != view.kwargs.get("thread_id"): return False + # Destroying a ThreadAccess removes the thread for every member of + # `obj.mailbox` (the row is unique per (thread, mailbox)). Require + # editor-level rights on that mailbox so a viewer cannot revoke + # access on behalf of the whole team. Thread role is irrelevant: + # a viewer on the thread but editor on the mailbox can still leave. + if view.action == "destroy": + return obj.mailbox.accesses.filter( + user=request.user, + role__in=enums.MAILBOX_ROLES_CAN_EDIT, + ).exists() + return ( models.ThreadAccess.objects.select_related("mailbox") .filter( @@ -534,7 +546,15 @@ class IsGlobalChannelMixin: class HasThreadEditAccess(IsAuthenticated): - """Allows access only to users with EDITOR role on the thread via ThreadAccess.""" + """Allows access only to users with full edit rights on the thread. + + Full edit rights require BOTH: + - `ThreadAccess.role == EDITOR` on the thread + - A `MailboxAccess.role` in `MAILBOX_ROLES_CAN_EDIT` on the same mailbox + + This prevents a user with VIEWER MailboxAccess on a shared mailbox + from mutating threads that the mailbox has EDITOR ThreadAccess to. + """ message = "You do not have permission to perform this action on this thread." @@ -554,11 +574,11 @@ class HasThreadEditAccess(IsAuthenticated): if thread_id_from_url is None: return True - return models.ThreadAccess.objects.filter( - thread_id=thread_id_from_url, - role__in=enums.THREAD_ROLES_CAN_EDIT, - mailbox__accesses__user=request.user, - ).exists() + return ( + models.ThreadAccess.objects.editable_by(request.user) + .filter(thread_id=thread_id_from_url) + .exists() + ) def has_object_permission(self, request, view, obj): """Check editor access on the thread the object belongs to. @@ -577,11 +597,7 @@ class HasThreadEditAccess(IsAuthenticated): else: thread = obj - return models.ThreadAccess.objects.filter( - thread=thread, - mailbox__accesses__user=request.user, - role__in=enums.THREAD_ROLES_CAN_EDIT, - ).exists() + return thread.get_abilities(request.user)[enums.ThreadAbilities.CAN_EDIT] class HasAccessToMailbox(IsAuthenticated): diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index 6906f684..1e7b3f27 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -651,6 +651,30 @@ class ThreadSerializer(serializers.ModelSerializer): labels = serializers.SerializerMethodField() summary = serializers.CharField(read_only=True) events_count = serializers.IntegerField(read_only=True) + abilities = serializers.SerializerMethodField(read_only=True) + + @extend_schema_field(serializers.DictField(child=serializers.BooleanField())) + def get_abilities(self, instance): + """Return the current user's abilities on the thread, scoped to + the mailbox context when provided. Frontend components use this + to hide mutating actions (archive, spam, delete, reply...) from + users with read-only access. + + Prefers the ``_can_edit`` annotation set by the viewset queryset + (single SQL subquery for the whole page) and falls back to a + per-instance query for code paths that build threads outside the + annotated queryset (e.g. the split action). + """ + can_edit = getattr(instance, "_can_edit", None) + if can_edit is not None: + return {enums.ThreadAbilities.CAN_EDIT: can_edit} + + request = self.context.get("request") + if request is None: + return {} + return instance.get_abilities( + request.user, mailbox_id=self.context.get("mailbox_id") + ) @extend_schema_field(serializers.BooleanField()) def get_has_unread(self, instance): @@ -750,6 +774,7 @@ class ThreadSerializer(serializers.ModelSerializer): "labels", "summary", "events_count", + "abilities", ] read_only_fields = fields # Mark all as read-only for safety diff --git a/src/backend/core/api/viewsets/flag.py b/src/backend/core/api/viewsets/flag.py index be1580ae..437e6b67 100644 --- a/src/backend/core/api/viewsets/flag.py +++ b/src/backend/core/api/viewsets/flag.py @@ -13,7 +13,7 @@ from drf_spectacular.utils import ( from rest_framework import serializers as drf_serializers from rest_framework.views import APIView -from core import enums, models +from core import models from core.services.search.tasks import update_threads_mailbox_flags_task from .. import permissions @@ -188,15 +188,24 @@ class ChangeFlagView(APIView): current_time = timezone.now() updated_threads = set() - # Get IDs of threads the user has access to - accessible_thread_ids_qs = models.ThreadAccess.objects.filter( - mailbox__accesses__user=request.user, - ).values_list("thread_id", flat=True) - - # Unread and starred are personal actions that don't require EDITOR access. - if flag not in ("unread", "starred"): - accessible_thread_ids_qs = accessible_thread_ids_qs.filter( - role__in=enums.THREAD_ROLES_CAN_EDIT + # Get IDs of threads the user has access to. + # + # Unread and starred are personal actions: they only mutate the + # user's own ThreadAccess row (read_at / starred_at) and are + # intentionally allowed for viewers. Any MailboxAccess is enough. + # + # All other flags (trashed / archived / spam) mutate shared + # Message state, so they require full edit rights on the thread: + # EDITOR ThreadAccess role AND CAN_EDIT MailboxAccess role. We + # delegate to ThreadAccess.objects.editable_by to keep the + # permission check consistent across the codebase. + if flag in ("unread", "starred"): + accessible_thread_ids_qs = models.ThreadAccess.objects.filter( + mailbox__accesses__user=request.user, + ) + else: + accessible_thread_ids_qs = models.ThreadAccess.objects.editable_by( + request.user ) if mailbox_id: @@ -204,6 +213,10 @@ class ChangeFlagView(APIView): mailbox_id=mailbox_id ) + accessible_thread_ids_qs = accessible_thread_ids_qs.values_list( + "thread_id", flat=True + ) + if flag in ("unread", "starred") and not thread_ids and message_ids: # If no thread_ids but we have message_ids, we need to get the thread_ids from the messages thread_ids = ( diff --git a/src/backend/core/api/viewsets/label.py b/src/backend/core/api/viewsets/label.py index 23656616..6b4e5b95 100644 --- a/src/backend/core/api/viewsets/label.py +++ b/src/backend/core/api/viewsets/label.py @@ -291,12 +291,22 @@ class LabelViewSet( {"detail": "No thread IDs provided"}, status=status.HTTP_400_BAD_REQUEST, ) + # Require full edit rights on each thread **scoped to the label's + # own mailbox**. The mailbox here is intrinsic to the label (a + # property of the resource), not ambient from the caller's view, + # so we always pass `label.mailbox_id`. This enforces two things + # at once: + # - User must have full edit rights (EDITOR ThreadAccess role + + # CAN_EDIT MailboxAccess role) on the thread — blocks viewer + # access to the thread. + # - The thread must actually exist under label.mailbox — blocks + # attaching a label to a thread that is only visible from a + # different mailbox, even when the user has edit rights there. accessible_threads = models.Thread.objects.filter( Exists( - models.ThreadAccess.objects.filter( - mailbox__accesses__user=request.user, - thread=OuterRef("pk"), - ) + models.ThreadAccess.objects.editable_by( + request.user, mailbox_id=label.mailbox_id + ).filter(thread=OuterRef("pk")) ), id__in=thread_ids, ) @@ -350,12 +360,16 @@ class LabelViewSet( {"detail": "No thread IDs provided"}, status=status.HTTP_400_BAD_REQUEST, ) + # Removing a label from a thread is a mutation on shared state, + # so it requires full edit rights (EDITOR ThreadAccess role + + # CAN_EDIT MailboxAccess role) scoped to the label's own mailbox + # — same check as add_threads. See the comment there for the + # rationale on using `label.mailbox_id` (intrinsic to the label). accessible_threads = models.Thread.objects.filter( Exists( - models.ThreadAccess.objects.filter( - mailbox__accesses__user=request.user, - thread=OuterRef("pk"), - ) + models.ThreadAccess.objects.editable_by( + request.user, mailbox_id=label.mailbox_id + ).filter(thread=OuterRef("pk")) ), id__in=thread_ids, ) diff --git a/src/backend/core/api/viewsets/thread.py b/src/backend/core/api/viewsets/thread.py index 3dee1820..de3c7ad7 100644 --- a/src/backend/core/api/viewsets/thread.py +++ b/src/backend/core/api/viewsets/thread.py @@ -38,8 +38,12 @@ class ThreadViewSet( lookup_url_kwarg = "pk" def get_permissions(self): - """Use HasThreadEditAccess for actions that require EDITOR role.""" - if self.action in ("destroy", "split"): + """Use HasThreadEditAccess for actions that require EDITOR role. + + `refresh_summary` mutates thread state (writes `thread.summary`) + so it must also gate on full edit rights, not just authentication. + """ + if self.action in ("destroy", "split", "refresh_summary"): return [permissions.HasThreadEditAccess()] return super().get_permissions() @@ -143,6 +147,15 @@ class ThreadViewSet( both code paths always expose the same fields (e.g. ``events_count``), avoiding silent divergence in the serialized payload. """ + can_edit_qs = models.ThreadAccess.objects.filter( + thread=OuterRef("pk"), + role=enums.ThreadAccessRoleChoices.EDITOR, + mailbox__accesses__user=user, + mailbox__accesses__role__in=enums.MAILBOX_ROLES_CAN_EDIT, + ) + if mailbox_id: + can_edit_qs = can_edit_qs.filter(mailbox_id=mailbox_id) + return queryset.annotate( _has_unread=models.ThreadAccess.thread_unread_filter(user, mailbox_id), _has_starred=models.ThreadAccess.thread_starred_filter(user, mailbox_id), @@ -162,6 +175,7 @@ class ThreadViewSet( ) ), events_count=Count("events", distinct=True), + _can_edit=Exists(can_edit_qs), ) @staticmethod @@ -867,7 +881,11 @@ class ThreadViewSet( ) serializer = serializers.ThreadSerializer( - new_thread, context={"request": request} + new_thread, + context={ + "request": request, + "mailbox_id": request.query_params.get("mailbox_id"), + }, ) return drf.response.Response(serializer.data, status=status.HTTP_201_CREATED) diff --git a/src/backend/core/api/viewsets/thread_access.py b/src/backend/core/api/viewsets/thread_access.py index fc16a6e9..54f72cb8 100644 --- a/src/backend/core/api/viewsets/thread_access.py +++ b/src/backend/core/api/viewsets/thread_access.py @@ -1,13 +1,16 @@ """API ViewSet for ThreadAccess model.""" +from django.db import transaction + from drf_spectacular.types import OpenApiTypes from drf_spectacular.utils import ( OpenApiParameter, extend_schema, ) from rest_framework import mixins, viewsets +from rest_framework.exceptions import ValidationError -from core import models +from core import enums, models from .. import permissions, serializers @@ -62,3 +65,22 @@ class ThreadAccessViewSet( """Create a new thread access.""" request.data["thread"] = self.kwargs.get("thread_id") return super().create(request, *args, **kwargs) + + @transaction.atomic + def perform_destroy(self, instance): + """Prevent deletion of the last editor access on a thread.""" + if instance.role == enums.ThreadAccessRoleChoices.EDITOR: + remaining_editors = ( + models.ThreadAccess.objects.select_for_update() + .filter( + thread=instance.thread, + role=enums.ThreadAccessRoleChoices.EDITOR, + ) + .exclude(id=instance.id) + .exists() + ) + if not remaining_editors: + raise ValidationError( + "Cannot delete the last editor access of a thread." + ) + super().perform_destroy(instance) diff --git a/src/backend/core/enums.py b/src/backend/core/enums.py index 7caffe5d..b6b90dab 100644 --- a/src/backend/core/enums.py +++ b/src/backend/core/enums.py @@ -139,6 +139,12 @@ class MailboxAbilities(models.TextChoices): CAN_IMPORT_MESSAGES = "import_messages", "Can import messages" +class ThreadAbilities(models.TextChoices): + """Defines specific abilities a user can have on a Thread.""" + + CAN_EDIT = "edit", "Can edit the thread (archive, spam, trash, labels, events)" + + class ThreadEventTypeChoices(models.TextChoices): """Defines the possible types of thread events.""" diff --git a/src/backend/core/models.py b/src/backend/core/models.py index fbbcf0f8..50d2c93d 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -34,6 +34,7 @@ from encrypted_fields.fields import EncryptedJSONField, EncryptedTextField from timezone_field import TimeZoneField from core.enums import ( + MAILBOX_ROLES_CAN_EDIT, ChannelScopeLevel, CompressionTypeChoices, CRUDAbilities, @@ -45,6 +46,7 @@ from core.enums import ( MessageDeliveryStatusChoices, MessageRecipientTypeChoices, MessageTemplateTypeChoices, + ThreadAbilities, ThreadAccessRoleChoices, ThreadEventTypeChoices, UserAbilities, @@ -1221,6 +1223,25 @@ class Thread(BaseModel): ] ) + def get_abilities(self, user, mailbox_id=None): + """ + Compute and return abilities for a given user on this thread. + + Scoped to an optional mailbox context: when provided, the check + is restricted to the ThreadAccess of that mailbox. Otherwise the + check looks across any of the user's mailboxes. + """ + if not user.is_authenticated: + return {ThreadAbilities.CAN_EDIT: False} + + can_edit = ( + ThreadAccess.objects.editable_by(user, mailbox_id=mailbox_id) + .filter(thread=self) + .exists() + ) + + return {ThreadAbilities.CAN_EDIT: can_edit} + class Label(BaseModel): """Label model to organize threads into folders using slash-based naming.""" @@ -1425,6 +1446,41 @@ class Label(BaseModel): super().delete(*args, **kwargs) +class ThreadAccessQuerySet(models.QuerySet): + """Custom queryset exposing reusable access-scoped filters.""" + + def editable_by(self, user, mailbox_id=None): + """Return ThreadAccess rows granting full edit rights to `user`. + + Combines BOTH conditions, which are the single source of truth + for "can this user edit this thread?": + + - `ThreadAccess.role == EDITOR` + - The user's `MailboxAccess.role` on that mailbox is in + `MAILBOX_ROLES_CAN_EDIT` + + When `mailbox_id` is provided, the result is also scoped to that + mailbox. Used by the flag, label, thread and thread-event + endpoints to replace ad-hoc permission checks. + + IMPORTANT: the two `mailbox__accesses__*` conditions MUST stay + inside the same `.filter()` call. If split, Django generates two + independent JOINs and the query matches any pair of mailbox + accesses satisfying either condition — a false positive. + """ + qs = self.filter( + role=ThreadAccessRoleChoices.EDITOR, + mailbox__accesses__user=user, + mailbox__accesses__role__in=MAILBOX_ROLES_CAN_EDIT, + ) + if mailbox_id is not None: + qs = qs.filter(mailbox_id=mailbox_id) + return qs + + +ThreadAccessManager = models.Manager.from_queryset(ThreadAccessQuerySet) + + class ThreadAccess(BaseModel): """Thread access model to store thread access information for a mailbox.""" @@ -1442,6 +1498,8 @@ class ThreadAccess(BaseModel): read_at = models.DateTimeField("read at", null=True, blank=True) starred_at = models.DateTimeField("starred at", null=True, blank=True) + objects = ThreadAccessManager() + class Meta: db_table = "messages_threadaccess" verbose_name = "thread access" diff --git a/src/backend/core/tests/api/test_labels.py b/src/backend/core/tests/api/test_labels.py index 829d5307..39e7d7d7 100644 --- a/src/backend/core/tests/api/test_labels.py +++ b/src/backend/core/tests/api/test_labels.py @@ -549,10 +549,16 @@ class TestLabelViewSet: models.MailboxRoleChoices.SENDER, ], ) - def test_add_threads_to_label(self, api_client, label, role): + def test_add_threads_to_label(self, api_client, user, role): """Test adding threads to a label.""" + # The label and the threads must live in the same mailbox: + # that's the real flow (a user manages a label from inside the + # mailbox that owns it). Parametrising the role on that same + # mailbox also matches production usage, where any role in + # MAILBOX_ROLES_CAN_EDIT can manage labels. mailbox = MailboxFactory() - mailbox.accesses.create(user=label.mailbox.accesses.first().user, role=role) + mailbox.accesses.create(user=user, role=role) + label = LabelFactory(mailbox=mailbox) threads = ThreadFactory.create_batch(3) for thread in threads: thread.accesses.create( @@ -616,12 +622,14 @@ class TestLabelViewSet: models.MailboxRoleChoices.SENDER, ], ) - def test_remove_threads_from_label(self, api_client, label, mailbox_role): + def test_remove_threads_from_label(self, api_client, user, mailbox_role): """Test removing threads from a label.""" + # Mirror of test_add_threads_to_label — see that test for the + # rationale on putting the label and the threads in the same + # mailbox where the role is granted. mailbox = MailboxFactory() - mailbox.accesses.create( - user=label.mailbox.accesses.first().user, role=mailbox_role - ) + mailbox.accesses.create(user=user, role=mailbox_role) + label = LabelFactory(mailbox=mailbox) threads = ThreadFactory.create_batch(3) for thread in threads: thread.accesses.create( @@ -654,6 +662,98 @@ class TestLabelViewSet: assert response.status_code == status.HTTP_200_OK assert label.threads.count() == 1 # Thread not removed + def test_add_threads_cross_mailbox_viewer_thread_access_forbidden( + self, api_client, label, user + ): + """User is EDITOR/ADMIN on label.mailbox (can manage labels) but + only has VIEWER ThreadAccess to the thread (via a different + mailbox). Labelling the thread is a shared-state mutation on the + thread, so it must be blocked. + """ + other_mailbox = MailboxFactory() + other_mailbox.accesses.create(user=user, role=models.MailboxRoleChoices.VIEWER) + thread = ThreadFactory() + thread.accesses.create( + mailbox=other_mailbox, + role=enums.ThreadAccessRoleChoices.EDITOR, # Mailbox has edit access + ) + + url = reverse("labels-add-threads", args=[label.pk]) + response = api_client.post(url, {"thread_ids": [str(thread.id)]}, format="json") + + # The endpoint returns 200 but silently skips the thread. + assert response.status_code == status.HTTP_200_OK + assert label.threads.count() == 0 + + def test_remove_threads_cross_mailbox_viewer_thread_access_forbidden( + self, api_client, label, user + ): + """Mirror of add_threads above — removing a label from a thread + also requires full edit rights on the thread. + """ + other_mailbox = MailboxFactory() + other_mailbox.accesses.create(user=user, role=models.MailboxRoleChoices.VIEWER) + thread = ThreadFactory() + thread.accesses.create( + mailbox=other_mailbox, + role=enums.ThreadAccessRoleChoices.EDITOR, + ) + label.threads.add(thread) # Pre-existing association + + url = reverse("labels-remove-threads", args=[label.pk]) + response = api_client.post(url, {"thread_ids": [str(thread.id)]}, format="json") + + assert response.status_code == status.HTTP_200_OK + assert label.threads.count() == 1 # Not removed + + def test_add_threads_thread_not_in_label_mailbox_forbidden( + self, api_client, label, user + ): + """User is EDITOR on both label.mailbox and another mailbox, but + the thread is only accessible from the other mailbox. Attaching + a label from label.mailbox to a thread that is not visible from + that mailbox would leak the label outside its owning mailbox, + so it must be blocked even though the user technically has full + edit rights on the thread elsewhere. + """ + other_mailbox = MailboxFactory() + other_mailbox.accesses.create(user=user, role=models.MailboxRoleChoices.EDITOR) + thread = ThreadFactory() + thread.accesses.create( + mailbox=other_mailbox, + role=enums.ThreadAccessRoleChoices.EDITOR, + ) + + url = reverse("labels-add-threads", args=[label.pk]) + response = api_client.post(url, {"thread_ids": [str(thread.id)]}, format="json") + + # Endpoint returns 200 but silently skips the out-of-mailbox thread. + assert response.status_code == status.HTTP_200_OK + assert label.threads.count() == 0 + + def test_remove_threads_thread_not_in_label_mailbox_forbidden( + self, api_client, label, user + ): + """Mirror of add_threads above — removing a label from a thread + must also be scoped to the label's own mailbox, so a pre-existing + (inconsistent) association to an out-of-mailbox thread cannot be + unwound via the endpoint either. + """ + other_mailbox = MailboxFactory() + other_mailbox.accesses.create(user=user, role=models.MailboxRoleChoices.EDITOR) + thread = ThreadFactory() + thread.accesses.create( + mailbox=other_mailbox, + role=enums.ThreadAccessRoleChoices.EDITOR, + ) + label.threads.add(thread) # Pre-existing (inconsistent) association + + url = reverse("labels-remove-threads", args=[label.pk]) + response = api_client.post(url, {"thread_ids": [str(thread.id)]}, format="json") + + assert response.status_code == status.HTTP_200_OK + assert label.threads.count() == 1 # Not removed + def test_label_hierarchy(self, mailbox): """Test label hierarchy with slash-based naming.""" parent_label = LabelFactory(name="Work", mailbox=mailbox) diff --git a/src/backend/core/tests/api/test_messages_flag.py b/src/backend/core/tests/api/test_messages_flag.py index 959e97a8..0528862c 100644 --- a/src/backend/core/tests/api/test_messages_flag.py +++ b/src/backend/core/tests/api/test_messages_flag.py @@ -1,6 +1,6 @@ """Test changing flags on messages or threads.""" -# pylint: disable=redefined-outer-name +# pylint: disable=redefined-outer-name,too-many-lines import json from datetime import timedelta @@ -12,7 +12,7 @@ from django.utils import timezone import pytest from rest_framework import status -from core import enums +from core import enums, models from core.factories import ( MailboxFactory, MessageFactory, @@ -677,6 +677,49 @@ def test_api_flag_viewer_can_star_thread(api_client): assert access.starred_at is not None +@pytest.mark.parametrize( + "flag,field", + [ + ("trashed", "is_trashed"), + ("archived", "is_archived"), + ("spam", "is_spam"), + ], +) +def test_api_flag_viewer_mailbox_with_editor_thread_access_forbidden( + api_client, flag, field +): + """A user with VIEWER MailboxAccess cannot mutate shared-state flags + via message_ids even when the mailbox has EDITOR ThreadAccess on the thread. + """ + user = UserFactory() + api_client.force_authenticate(user=user) + mailbox = MailboxFactory(users_read=[user]) # VIEWER mailbox role + thread = ThreadFactory() + ThreadAccessFactory( + mailbox=mailbox, + thread=thread, + role=enums.ThreadAccessRoleChoices.EDITOR, # Mailbox has edit on thread + ) + message = MessageFactory(thread=thread) + + data = {"flag": flag, "value": True, "message_ids": [str(message.id)]} + response = api_client.post(API_URL, data=data, format="json") + + assert response.status_code == status.HTTP_200_OK + assert response.data["updated_threads"] == 0 + + message.refresh_from_db() + assert getattr(message, field) is False + + # Elevating MailboxAccess to EDITOR lets the same request succeed. + models.MailboxAccess.objects.filter(user=user, mailbox=mailbox).update( + role=enums.MailboxRoleChoices.EDITOR + ) + response = api_client.post(API_URL, data=data, format="json") + assert response.status_code == status.HTTP_200_OK + assert response.data["updated_threads"] == 1 + + # --- Tests for Trashed Flag --- @@ -684,7 +727,7 @@ def test_api_flag_mark_messages_trashed_success(api_client): """Test marking messages as trashed successfully.""" user = UserFactory() api_client.force_authenticate(user=user) - mailbox = MailboxFactory(users_read=[user]) + mailbox = MailboxFactory(users_admin=[user]) thread = ThreadFactory() ThreadAccessFactory( mailbox=mailbox, thread=thread, role=enums.ThreadAccessRoleChoices.EDITOR @@ -717,7 +760,7 @@ def test_api_flag_mark_messages_untrashed_success(api_client): """Test marking messages as untrashed successfully.""" user = UserFactory() api_client.force_authenticate(user=user) - mailbox = MailboxFactory(users_read=[user]) + mailbox = MailboxFactory(users_admin=[user]) thread = ThreadFactory() ThreadAccessFactory( mailbox=mailbox, thread=thread, role=enums.ThreadAccessRoleChoices.EDITOR @@ -753,7 +796,7 @@ def test_api_flag_mark_messages_archived_success(api_client): """Test marking messages as archived successfully.""" user = UserFactory() api_client.force_authenticate(user=user) - mailbox = MailboxFactory(users_read=[user]) + mailbox = MailboxFactory(users_admin=[user]) thread = ThreadFactory() ThreadAccessFactory( mailbox=mailbox, thread=thread, role=enums.ThreadAccessRoleChoices.EDITOR @@ -786,7 +829,7 @@ def test_api_flag_mark_messages_unarchived_success(api_client): """Test marking messages as unarchived successfully.""" user = UserFactory() api_client.force_authenticate(user=user) - mailbox = MailboxFactory(users_read=[user]) + mailbox = MailboxFactory(users_admin=[user]) thread = ThreadFactory() ThreadAccessFactory( mailbox=mailbox, thread=thread, role=enums.ThreadAccessRoleChoices.EDITOR @@ -820,7 +863,7 @@ def test_api_flag_mark_messages_spam_success(api_client): """Test marking messages as spam successfully.""" user = UserFactory() api_client.force_authenticate(user=user) - mailbox = MailboxFactory(users_read=[user]) + mailbox = MailboxFactory(users_admin=[user]) thread = ThreadFactory() ThreadAccessFactory( mailbox=mailbox, thread=thread, role=enums.ThreadAccessRoleChoices.EDITOR @@ -853,7 +896,7 @@ def test_api_flag_mark_messages_not_spam_success(api_client): """Test marking messages as not spam successfully.""" user = UserFactory() api_client.force_authenticate(user=user) - mailbox = MailboxFactory(users_read=[user]) + mailbox = MailboxFactory(users_admin=[user]) thread = ThreadFactory() ThreadAccessFactory( mailbox=mailbox, thread=thread, role=enums.ThreadAccessRoleChoices.EDITOR @@ -896,7 +939,7 @@ def test_api_flag_cascade_to_draft_children(api_client, flag, field, date_field) """Flagging a message by message_id cascades to its draft children.""" user = UserFactory() api_client.force_authenticate(user=user) - mailbox = MailboxFactory(users_read=[user]) + mailbox = MailboxFactory(users_admin=[user]) thread = ThreadFactory() ThreadAccessFactory( mailbox=mailbox, thread=thread, role=enums.ThreadAccessRoleChoices.EDITOR @@ -932,7 +975,7 @@ def test_api_flag_cascade_unflag_to_draft_children(api_client, flag, field, date """Unflagging a message by message_id cascades to its draft children.""" user = UserFactory() api_client.force_authenticate(user=user) - mailbox = MailboxFactory(users_read=[user]) + mailbox = MailboxFactory(users_admin=[user]) thread = ThreadFactory() ThreadAccessFactory( mailbox=mailbox, thread=thread, role=enums.ThreadAccessRoleChoices.EDITOR @@ -965,7 +1008,7 @@ def test_api_flag_cascade_does_not_affect_non_draft_children(api_client): """Trashing a message does NOT cascade to non-draft children.""" user = UserFactory() api_client.force_authenticate(user=user) - mailbox = MailboxFactory(users_read=[user]) + mailbox = MailboxFactory(users_admin=[user]) thread = ThreadFactory() ThreadAccessFactory( mailbox=mailbox, thread=thread, role=enums.ThreadAccessRoleChoices.EDITOR diff --git a/src/backend/core/tests/api/test_thread_access.py b/src/backend/core/tests/api/test_thread_access.py index 7f2f6c22..0566f14f 100644 --- a/src/backend/core/tests/api/test_thread_access.py +++ b/src/backend/core/tests/api/test_thread_access.py @@ -1,11 +1,14 @@ """Tests for the ThreadAccess API endpoints.""" +import threading import uuid +from django.db import connection from django.urls import reverse import pytest from rest_framework import status +from rest_framework.test import APIClient from core import enums, factories, models @@ -572,6 +575,10 @@ class TestThreadAccessDelete: thread=thread, role=thread_access_role, ) + # Ensure another editor exists so the last-editor guard doesn't block + factories.ThreadAccessFactory( + thread=thread, role=enums.ThreadAccessRoleChoices.EDITOR + ) api_client.force_authenticate(user=user) url = get_thread_access_url(thread.id, thread_access.id) @@ -584,20 +591,22 @@ class TestThreadAccessDelete: @pytest.mark.parametrize( "thread_access_role, mailbox_access_role", [ + # A user with viewer rights on the thread but edit-level rights on + # the mailbox can still leave: destroying the shared ThreadAccess + # affects the whole mailbox, so mailbox authority is what matters. (enums.ThreadAccessRoleChoices.VIEWER, enums.MailboxRoleChoices.ADMIN), (enums.ThreadAccessRoleChoices.VIEWER, enums.MailboxRoleChoices.EDITOR), (enums.ThreadAccessRoleChoices.VIEWER, enums.MailboxRoleChoices.SENDER), - (enums.ThreadAccessRoleChoices.EDITOR, enums.MailboxRoleChoices.VIEWER), ], ) - def test_delete_thread_access_forbidden( + def test_delete_thread_access_self_removal( self, api_client, thread_access_role, mailbox_access_role ): - """Test deleting a thread access without permission.""" + """A user with edit-level rights on the mailbox can remove its own + ThreadAccess regardless of the thread role.""" user = factories.UserFactory() api_client.force_authenticate(user=user) - # Create a thread access that the user doesn't have any role to delete mailbox = factories.MailboxFactory() factories.MailboxAccessFactory( mailbox=mailbox, @@ -610,14 +619,93 @@ class TestThreadAccessDelete: thread=thread, role=thread_access_role, ) + # Ensure another editor exists so the last-editor guard doesn't block + factories.ThreadAccessFactory( + thread=thread, + role=enums.ThreadAccessRoleChoices.EDITOR, + ) + + url = get_thread_access_url(thread.id, thread_access.id) + response = api_client.delete(url) + assert response.status_code == status.HTTP_204_NO_CONTENT + assert not models.ThreadAccess.objects.filter(id=thread_access.id).exists() + + @pytest.mark.parametrize( + "thread_access_role", + [ + enums.ThreadAccessRoleChoices.VIEWER, + enums.ThreadAccessRoleChoices.EDITOR, + ], + ) + def test_delete_thread_access_mailbox_viewer_forbidden( + self, api_client, thread_access_role + ): + """A mailbox viewer cannot destroy the mailbox's ThreadAccess — that + would revoke the thread for every other member of the mailbox.""" + user = factories.UserFactory() + api_client.force_authenticate(user=user) + + mailbox = factories.MailboxFactory() + factories.MailboxAccessFactory( + mailbox=mailbox, + user=user, + role=enums.MailboxRoleChoices.VIEWER, + ) + thread = factories.ThreadFactory() + thread_access = factories.ThreadAccessFactory( + mailbox=mailbox, + thread=thread, + role=thread_access_role, + ) + factories.ThreadAccessFactory( + thread=thread, + role=enums.ThreadAccessRoleChoices.EDITOR, + ) url = get_thread_access_url(thread.id, thread_access.id) response = api_client.delete(url) assert response.status_code == status.HTTP_403_FORBIDDEN + assert models.ThreadAccess.objects.filter(id=thread_access.id).exists() - # Create a thread access that the user doesn't have any role to delete - thread_access = factories.ThreadAccessFactory() - url = get_thread_access_url(thread.id, thread_access.id) + @pytest.mark.parametrize( + "thread_access_role, mailbox_access_role", + [ + (enums.ThreadAccessRoleChoices.VIEWER, enums.MailboxRoleChoices.ADMIN), + (enums.ThreadAccessRoleChoices.VIEWER, enums.MailboxRoleChoices.EDITOR), + (enums.ThreadAccessRoleChoices.VIEWER, enums.MailboxRoleChoices.SENDER), + (enums.ThreadAccessRoleChoices.EDITOR, enums.MailboxRoleChoices.VIEWER), + ], + ) + def test_delete_thread_access_other_forbidden( + self, api_client, thread_access_role, mailbox_access_role + ): + """A user without full edit rights cannot delete another user's thread access.""" + user = factories.UserFactory() + api_client.force_authenticate(user=user) + + mailbox = factories.MailboxFactory() + factories.MailboxAccessFactory( + mailbox=mailbox, + user=user, + role=mailbox_access_role, + ) + thread = factories.ThreadFactory() + factories.ThreadAccessFactory( + mailbox=mailbox, + thread=thread, + role=thread_access_role, + ) + + # Another user's thread access on the same thread + other_thread_access = factories.ThreadAccessFactory(thread=thread) + + url = get_thread_access_url(thread.id, other_thread_access.id) + response = api_client.delete(url) + assert response.status_code == status.HTTP_403_FORBIDDEN + assert models.ThreadAccess.objects.filter(id=other_thread_access.id).exists() + + # Non-existent thread access + url = get_thread_access_url(thread.id, uuid.uuid4()) response = api_client.delete(url) assert response.status_code == status.HTTP_404_NOT_FOUND @@ -631,9 +719,150 @@ class TestThreadAccessDelete: response = api_client.delete(url) assert response.status_code == status.HTTP_404_NOT_FOUND + def test_delete_thread_access_last_editor_rejected(self, api_client): + """Deleting the last editor access on a thread must be rejected.""" + user = factories.UserFactory() + mailbox = factories.MailboxFactory() + factories.MailboxAccessFactory( + mailbox=mailbox, + user=user, + role=enums.MailboxRoleChoices.ADMIN, + ) + thread = factories.ThreadFactory() + thread_access = factories.ThreadAccessFactory( + mailbox=mailbox, + thread=thread, + role=enums.ThreadAccessRoleChoices.EDITOR, + ) + api_client.force_authenticate(user=user) + + url = get_thread_access_url(thread.id, thread_access.id) + response = api_client.delete(url) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert models.ThreadAccess.objects.filter(id=thread_access.id).exists() + + def test_delete_thread_access_last_editor_with_viewers_rejected(self, api_client): + """Deleting the last editor is rejected even if viewers remain.""" + user = factories.UserFactory() + mailbox = factories.MailboxFactory() + factories.MailboxAccessFactory( + mailbox=mailbox, + user=user, + role=enums.MailboxRoleChoices.ADMIN, + ) + thread = factories.ThreadFactory() + editor_access = factories.ThreadAccessFactory( + mailbox=mailbox, + thread=thread, + role=enums.ThreadAccessRoleChoices.EDITOR, + ) + # Add viewers — they don't count as editors + factories.ThreadAccessFactory( + thread=thread, role=enums.ThreadAccessRoleChoices.VIEWER + ) + api_client.force_authenticate(user=user) + + url = get_thread_access_url(thread.id, editor_access.id) + response = api_client.delete(url) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert models.ThreadAccess.objects.filter(id=editor_access.id).exists() + + def test_delete_thread_access_editor_allowed_when_others_remain(self, api_client): + """Deleting an editor access is allowed when other editors remain.""" + user = factories.UserFactory() + mailbox = factories.MailboxFactory() + factories.MailboxAccessFactory( + mailbox=mailbox, + user=user, + role=enums.MailboxRoleChoices.ADMIN, + ) + thread = factories.ThreadFactory() + editor_access = factories.ThreadAccessFactory( + mailbox=mailbox, + thread=thread, + role=enums.ThreadAccessRoleChoices.EDITOR, + ) + # Another editor on the thread + factories.ThreadAccessFactory( + thread=thread, role=enums.ThreadAccessRoleChoices.EDITOR + ) + api_client.force_authenticate(user=user) + + url = get_thread_access_url(thread.id, editor_access.id) + response = api_client.delete(url) + assert response.status_code == status.HTTP_204_NO_CONTENT + assert not models.ThreadAccess.objects.filter(id=editor_access.id).exists() + def test_delete_thread_access_unauthorized(self, api_client): """Test deleting a thread access without authentication.""" thread = factories.ThreadFactory() url = get_thread_access_url(thread.id, uuid.uuid4()) response = api_client.delete(url) assert response.status_code == status.HTTP_401_UNAUTHORIZED + + @pytest.mark.django_db(transaction=True) + def test_delete_thread_access_concurrent_last_editors(self): + """Two concurrent deletes of the last two editors must not orphan the thread. + + Without select_for_update, both requests can see the other editor still + present and both proceed, leaving zero editors. + """ + user = factories.UserFactory() + mailbox = factories.MailboxFactory() + factories.MailboxAccessFactory( + mailbox=mailbox, + user=user, + role=enums.MailboxRoleChoices.ADMIN, + ) + thread = factories.ThreadFactory() + editor_a = factories.ThreadAccessFactory( + mailbox=mailbox, + thread=thread, + role=enums.ThreadAccessRoleChoices.EDITOR, + ) + other_mailbox = factories.MailboxFactory() + factories.MailboxAccessFactory( + mailbox=other_mailbox, + user=user, + role=enums.MailboxRoleChoices.ADMIN, + ) + editor_b = factories.ThreadAccessFactory( + mailbox=other_mailbox, + thread=thread, + role=enums.ThreadAccessRoleChoices.EDITOR, + ) + + url_a = get_thread_access_url(thread.id, editor_a.id) + url_b = get_thread_access_url(thread.id, editor_b.id) + + results = {} + + def delete_access(name, url): + try: + client = APIClient() + client.force_authenticate(user=user) + results[name] = client.delete(url) + finally: + connection.close() + + t1 = threading.Thread(target=delete_access, args=("a", url_a)) + t2 = threading.Thread(target=delete_access, args=("b", url_b)) + t1.start() + t2.start() + t1.join(timeout=10) + t2.join(timeout=10) + + status_codes = {results["a"].status_code, results["b"].status_code} + + # One must succeed (204), the other must be rejected (400) + assert status_codes == { + status.HTTP_204_NO_CONTENT, + status.HTTP_400_BAD_REQUEST, + }, f"Expected one 204 and one 400, got {status_codes}" + + # At least one editor must remain + remaining = models.ThreadAccess.objects.filter( + thread=thread, + role=enums.ThreadAccessRoleChoices.EDITOR, + ).count() + assert remaining == 1 diff --git a/src/backend/core/tests/api/test_thread_event_permissions.py b/src/backend/core/tests/api/test_thread_event_permissions.py index 34880c66..9805c0a7 100644 --- a/src/backend/core/tests/api/test_thread_event_permissions.py +++ b/src/backend/core/tests/api/test_thread_event_permissions.py @@ -305,6 +305,33 @@ class TestViewerCannotCreateEvents: 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. + """ + user = factories.UserFactory() + mailbox = factories.MailboxFactory() + factories.MailboxAccessFactory( + mailbox=mailbox, + user=user, + role=enums.MailboxRoleChoices.VIEWER, # VIEWER mailbox role + ) + thread = factories.ThreadFactory() + factories.ThreadAccessFactory( + mailbox=mailbox, + thread=thread, + role=enums.ThreadAccessRoleChoices.EDITOR, + ) + api_client.force_authenticate(user=user) + + data = {"type": "im", "data": {"content": "from a mailbox viewer"}} + response = api_client.post(get_thread_event_url(thread.id), data, 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.""" diff --git a/src/backend/core/tests/api/test_thread_split.py b/src/backend/core/tests/api/test_thread_split.py index 49cd943c..63ead677 100644 --- a/src/backend/core/tests/api/test_thread_split.py +++ b/src/backend/core/tests/api/test_thread_split.py @@ -130,6 +130,28 @@ def test_split_thread_feature_disabled(api_client): assert Thread.objects.filter(id=thread.id).count() == 1 +def test_split_thread_viewer_mailbox_with_editor_thread_access_forbidden(api_client): + """Previously-missed scenario: a user with VIEWER MailboxAccess on a + shared inbox cannot split a thread even when the mailbox itself has + EDITOR ThreadAccess on the thread — the user's own mailbox role must + be honoured. + """ + user = UserFactory() + api_client.force_authenticate(user=user) + + mailbox = MailboxFactory(users_read=[user]) # VIEWER mailbox role + thread, messages = _create_thread_with_messages(mailbox, count=3) + ThreadAccessFactory( + mailbox=mailbox, + thread=thread, + role=enums.ThreadAccessRoleChoices.EDITOR, + ) + + url = _get_split_url(thread.id) + response = api_client.post(url, {"message_id": str(messages[1].id)}) + assert response.status_code == status.HTTP_403_FORBIDDEN + + # --- Validation tests --- diff --git a/src/backend/core/tests/api/test_threads_flag.py b/src/backend/core/tests/api/test_threads_flag.py index 920f0da4..dbacb512 100644 --- a/src/backend/core/tests/api/test_threads_flag.py +++ b/src/backend/core/tests/api/test_threads_flag.py @@ -107,9 +107,14 @@ def test_api_flag_thread_viewer_should_not_update(api_client): assert thread.has_trashed is False assert thread.messages.first().is_trashed is False - # Elevate to EDITOR and verify flag change succeeds + # Elevate BOTH ThreadAccess and MailboxAccess to an edit role. Edit + # rights require both — a VIEWER mailbox role on a shared inbox + # cannot mutate thread state even when ThreadAccess is EDITOR. thread_access.role = enums.ThreadAccessRoleChoices.EDITOR thread_access.save() + models.MailboxAccess.objects.filter(user=user, mailbox=mailbox).update( + role=enums.MailboxRoleChoices.EDITOR + ) response = api_client.post(FLAG_API_URL, data=data, format="json") assert response.status_code == status.HTTP_200_OK assert response.data["updated_threads"] == 1 @@ -124,7 +129,7 @@ def test_api_flag_trash_single_thread_success(api_client): """Test marking a single thread as trashed successfully via flag endpoint.""" user = factories.UserFactory() api_client.force_authenticate(user=user) - mailbox = factories.MailboxFactory(users_read=[user]) + mailbox = factories.MailboxFactory(users_admin=[user]) thread = factories.ThreadFactory() factories.ThreadAccessFactory( mailbox=mailbox, @@ -164,7 +169,7 @@ def test_api_flag_untrash_single_thread_success(api_client): """Test marking a single thread as untrashed successfully via flag endpoint.""" user = factories.UserFactory() api_client.force_authenticate(user=user) - mailbox = factories.MailboxFactory(users_read=[user]) + mailbox = factories.MailboxFactory(users_admin=[user]) thread = factories.ThreadFactory() factories.ThreadAccessFactory( mailbox=mailbox, @@ -208,7 +213,7 @@ def test_api_flag_trash_multiple_threads_success(api_client): """Test marking multiple threads as trashed successfully.""" user = factories.UserFactory() api_client.force_authenticate(user=user) - mailbox = factories.MailboxFactory(users_read=[user]) + mailbox = factories.MailboxFactory(users_admin=[user]) thread1 = factories.ThreadFactory() factories.ThreadAccessFactory( mailbox=mailbox, @@ -275,7 +280,7 @@ def test_api_flag_spam_single_thread_success(api_client): """Test marking a single thread as spam successfully via flag endpoint.""" user = factories.UserFactory() api_client.force_authenticate(user=user) - mailbox = factories.MailboxFactory(users_read=[user]) + mailbox = factories.MailboxFactory(users_admin=[user]) thread = factories.ThreadFactory() factories.ThreadAccessFactory( mailbox=mailbox, @@ -313,7 +318,7 @@ def test_api_flag_not_spam_single_thread_success(api_client): """Test marking a single thread as not spam successfully via flag endpoint.""" user = factories.UserFactory() api_client.force_authenticate(user=user) - mailbox = factories.MailboxFactory(users_read=[user]) + mailbox = factories.MailboxFactory(users_admin=[user]) thread = factories.ThreadFactory() factories.ThreadAccessFactory( mailbox=mailbox, @@ -350,7 +355,7 @@ def test_api_flag_spam_multiple_threads_success(api_client): """Test marking multiple threads as spam successfully.""" user = factories.UserFactory() api_client.force_authenticate(user=user) - mailbox = factories.MailboxFactory(users_read=[user]) + mailbox = factories.MailboxFactory(users_admin=[user]) thread1 = factories.ThreadFactory() factories.ThreadAccessFactory( mailbox=mailbox, @@ -406,3 +411,97 @@ def test_api_flag_spam_multiple_threads_success(api_client): assert thread2.messages.first().is_spam is True msg3.refresh_from_db() assert msg3.is_spam is True # Remained spam + + +# --- Tests for VIEWER MailboxAccess + EDITOR ThreadAccess combination --- +# +# These tests cover the previously-missed scenario where a user is a VIEWER +# on a shared mailbox that has EDITOR ThreadAccess to threads that land in +# it. The pre-fix code checked only the ThreadAccess role, so a mailbox +# viewer could mutate threads despite having no edit right on the mailbox. + + +@pytest.mark.parametrize( + "flag,field", + [ + ("archived", "is_archived"), + ("spam", "is_spam"), + ("trashed", "is_trashed"), + ], +) +def test_api_flag_viewer_mailbox_with_editor_thread_access_forbidden( + api_client, flag, field +): + """A user with VIEWER MailboxAccess cannot mutate shared-state flags + even when the mailbox itself has EDITOR ThreadAccess on the thread. + """ + user = factories.UserFactory() + api_client.force_authenticate(user=user) + mailbox = factories.MailboxFactory(users_read=[user]) # VIEWER mailbox role + thread = factories.ThreadFactory() + factories.ThreadAccessFactory( + mailbox=mailbox, + thread=thread, + role=enums.ThreadAccessRoleChoices.EDITOR, # Mailbox has edit on thread + ) + factories.MessageFactory(thread=thread) + + data = {"flag": flag, "value": True, "thread_ids": [str(thread.id)]} + response = api_client.post(FLAG_API_URL, data=data, format="json") + + # The user should not be able to mutate the flag — 200 + 0 updates. + assert response.status_code == status.HTTP_200_OK + assert response.data["updated_threads"] == 0 + + thread.refresh_from_db() + message = thread.messages.first() + assert getattr(message, field) is False + + # Elevating MailboxAccess to EDITOR lets the same request succeed. + models.MailboxAccess.objects.filter(user=user, mailbox=mailbox).update( + role=enums.MailboxRoleChoices.EDITOR + ) + response = api_client.post(FLAG_API_URL, data=data, format="json") + assert response.status_code == status.HTTP_200_OK + assert response.data["updated_threads"] == 1 + + +def test_api_flag_viewer_mailbox_can_still_star_and_mark_unread(api_client): + """Regression guard: star and unread are personal-state actions and + remain available to viewers (both mailbox-viewer and thread-viewer). + """ + user = factories.UserFactory() + api_client.force_authenticate(user=user) + mailbox = factories.MailboxFactory(users_read=[user]) # VIEWER mailbox role + thread = factories.ThreadFactory() + thread_access = factories.ThreadAccessFactory( + mailbox=mailbox, + thread=thread, + role=enums.ThreadAccessRoleChoices.VIEWER, # VIEWER thread role too + ) + factories.MessageFactory(thread=thread) + + starred_data = { + "flag": "starred", + "value": True, + "thread_ids": [str(thread.id)], + "mailbox_id": str(mailbox.id), + } + response = api_client.post(FLAG_API_URL, data=starred_data, format="json") + assert response.status_code == status.HTTP_200_OK + assert response.data["updated_threads"] == 1 + thread_access.refresh_from_db() + assert thread_access.starred_at is not None + + unread_data = { + "flag": "unread", + "value": True, + "thread_ids": [str(thread.id)], + "mailbox_id": str(mailbox.id), + "read_at": None, + } + response = api_client.post(FLAG_API_URL, data=unread_data, format="json") + assert response.status_code == status.HTTP_200_OK + assert response.data["updated_threads"] == 1 + thread_access.refresh_from_db() + assert thread_access.read_at is None diff --git a/src/backend/core/tests/api/test_threads_list.py b/src/backend/core/tests/api/test_threads_list.py index e80c68e4..69d9dd94 100644 --- a/src/backend/core/tests/api/test_threads_list.py +++ b/src/backend/core/tests/api/test_threads_list.py @@ -254,14 +254,75 @@ def test_delete_thread_viewer_should_be_forbidden(api_client): assert response.status_code == status.HTTP_403_FORBIDDEN assert Thread.objects.filter(pk=thread.pk).exists() - # Elevate to EDITOR and verify delete succeeds + # Elevate BOTH ThreadAccess and MailboxAccess to EDITOR — full edit + # rights on a thread require both roles. thread_access.role = enums.ThreadAccessRoleChoices.EDITOR thread_access.save() + MailboxAccess.objects.filter(user=user, mailbox=mailbox).update( + role=enums.MailboxRoleChoices.EDITOR + ) response = api_client.delete(url) assert response.status_code == status.HTTP_204_NO_CONTENT assert not Thread.objects.filter(pk=thread.pk).exists() +def test_delete_thread_viewer_mailbox_with_editor_thread_access_forbidden(api_client): + """Previously-missed scenario: VIEWER MailboxAccess + EDITOR ThreadAccess. + + A shared inbox grants EDITOR ThreadAccess to every incoming thread, + but a user with only VIEWER MailboxAccess on that inbox must not be + able to delete those threads. `HasThreadEditAccess` previously + checked only the ThreadAccess role and missed this case. + """ + user = UserFactory() + api_client.force_authenticate(user=user) + mailbox = MailboxFactory(users_read=[user]) # VIEWER mailbox role + thread = ThreadFactory() + ThreadAccessFactory( + mailbox=mailbox, + thread=thread, + role=enums.ThreadAccessRoleChoices.EDITOR, # Mailbox has edit on thread + ) + MessageFactory(thread=thread) + + url = reverse("threads-detail", kwargs={"pk": str(thread.id)}) + response = api_client.delete(url) + assert response.status_code == status.HTTP_403_FORBIDDEN + assert Thread.objects.filter(pk=thread.pk).exists() + + # Elevating MailboxAccess to EDITOR unblocks the destroy action. + MailboxAccess.objects.filter(user=user, mailbox=mailbox).update( + role=enums.MailboxRoleChoices.EDITOR + ) + response = api_client.delete(url) + assert response.status_code == status.HTTP_204_NO_CONTENT + assert not Thread.objects.filter(pk=thread.pk).exists() + + +def test_refresh_summary_viewer_forbidden(api_client): + """A viewer cannot trigger the AI summary regeneration — it mutates + `thread.summary`. The endpoint is covered by `HasThreadEditAccess` + via `ThreadViewSet.get_permissions`. + """ + user = UserFactory() + api_client.force_authenticate(user=user) + mailbox = MailboxFactory(users_read=[user]) # VIEWER mailbox role + thread = ThreadFactory(summary="initial") + ThreadAccessFactory( + mailbox=mailbox, + thread=thread, + role=enums.ThreadAccessRoleChoices.EDITOR, + ) + MessageFactory(thread=thread) + + url = reverse("threads-refresh-summary", kwargs={"pk": str(thread.id)}) + response = api_client.post(url) + assert response.status_code == status.HTTP_403_FORBIDDEN + + thread.refresh_from_db() + assert thread.summary == "initial" # untouched + + def test_list_threads_success(api_client): """Test listing threads successfully.""" user = UserFactory() diff --git a/src/backend/e2e/management/commands/e2e_demo.py b/src/backend/e2e/management/commands/e2e_demo.py index 04dc4d47..a91273dc 100644 --- a/src/backend/e2e/management/commands/e2e_demo.py +++ b/src/backend/e2e/management/commands/e2e_demo.py @@ -407,7 +407,7 @@ class Command(BaseCommand): models.ThreadAccess.objects.create( thread=thread, mailbox=mailbox, - role=ThreadAccessRoleChoices.VIEWER, + role=ThreadAccessRoleChoices.EDITOR, ) models.Message.objects.create( thread=thread, diff --git a/src/e2e/src/__tests__/thread-starred-read.spec.ts b/src/e2e/src/__tests__/thread-starred-read.spec.ts index 36a9de80..32d1049b 100644 --- a/src/e2e/src/__tests__/thread-starred-read.spec.ts +++ b/src/e2e/src/__tests__/thread-starred-read.spec.ts @@ -215,18 +215,18 @@ test.describe("Thread read / unread", () => { await page .getByRole("heading", { name: "Inbox thread alpha", level: 2 }) .waitFor({ state: "visible" }); - await page.waitForLoadState("networkidle"); - // Close the thread view to go back to the list - await page.getByRole("button", { name: "Close this thread" }).click(); - - // Verify the thread is now read (no unread indicator) + // Wait for auto-read to propagate: the sidebar reflects the read state + // once the flag mutation succeeds and the thread list cache is updated. await expect( page.locator('[data-unread="false"]', { hasText: "Inbox thread alpha", }), ).toBeVisible(); + // Close the thread view to go back to the list + await page.getByRole("button", { name: "Close this thread" }).click(); + // Click the filter button to apply unread filter (default selected filter) await page.getByRole("button", { name: "Filter threads" }).click(); await page.waitForLoadState("networkidle"); diff --git a/src/frontend/public/locales/common/en-US.json b/src/frontend/public/locales/common/en-US.json index 3c36d648..e407a719 100755 --- a/src/frontend/public/locales/common/en-US.json +++ b/src/frontend/public/locales/common/en-US.json @@ -31,6 +31,22 @@ "{{count}} months ago_other": "{{count}} months ago", "{{count}} occurrences_one": "{{count}} occurrence", "{{count}} occurrences_other": "{{count}} occurrences", + "{{count}} out of {{total}} messages are now starred._one": "{{count}} out of {{total}} message is now starred.", + "{{count}} out of {{total}} messages are now starred._other": "{{count}} out of {{total}} messages are now starred.", + "{{count}} out of {{total}} messages have been archived._one": "{{count}} out of {{total}} message has been archived.", + "{{count}} out of {{total}} messages have been archived._other": "{{count}} out of {{total}} messages have been archived.", + "{{count}} out of {{total}} messages have been deleted._one": "{{count}} out of {{total}} message has been deleted.", + "{{count}} out of {{total}} messages have been deleted._other": "{{count}} out of {{total}} messages have been deleted.", + "{{count}} out of {{total}} messages have been reported as spam._one": "{{count}} out of {{total}} message has been reported as spam.", + "{{count}} out of {{total}} messages have been reported as spam._other": "{{count}} out of {{total}} messages have been reported as spam.", + "{{count}} out of {{total}} threads are now starred._one": "{{count}} out of {{total}} thread is now starred.", + "{{count}} out of {{total}} threads are now starred._other": "{{count}} out of {{total}} threads are now starred.", + "{{count}} out of {{total}} threads have been archived._one": "{{count}} out of {{total}} thread has been archived.", + "{{count}} out of {{total}} threads have been archived._other": "{{count}} out of {{total}} threads have been archived.", + "{{count}} out of {{total}} threads have been deleted._one": "{{count}} out of {{total}} thread has been deleted.", + "{{count}} out of {{total}} threads have been deleted._other": "{{count}} out of {{total}} threads have been deleted.", + "{{count}} out of {{total}} threads have been reported as spam._one": "{{count}} out of {{total}} thread has been reported as spam.", + "{{count}} out of {{total}} threads have been reported as spam._other": "{{count}} out of {{total}} threads have been reported as spam.", "{{count}} results_one": "{{count}} result", "{{count}} results_other": "{{count}} results", "{{count}} results mentioning you_one": "{{count}} result mentioning you", @@ -104,6 +120,7 @@ "Addresses": "Addresses", "After creating the widget, you will receive the installation code to add to your website.": "After creating the widget, you will receive the installation code to add to your website.", "All messages": "All messages", + "All users with access to the mailbox \"{{mailboxName}}\" will no longer see this thread.": "All users with access to the mailbox \"{{mailboxName}}\" will no longer see this thread.", "Always": "Always", "An address with this prefix already exists in this domain.": "An address with this prefix already exists in this domain.", "An archive is uploading": "An archive is uploading", @@ -359,6 +376,8 @@ "Last saved {{relativeTime}}": "Last saved {{relativeTime}}", "Last update: {{timestamp}}": "Last update: {{timestamp}}", "Layout": "Layout", + "Leave this thread": "Leave this thread", + "Leave this thread?": "Leave this thread?", "less than a minute ago": "less than a minute ago", "Loading addresses...": "Loading addresses...", "Loading auto-replies...": "Loading auto-replies...", @@ -421,12 +440,20 @@ "No event found in calendar invite": "No event found in calendar invite", "No integration found": "No integration found", "No mailbox": "No mailbox", + "No message could be archived.": "No message could be archived.", + "No message could be deleted.": "No message could be deleted.", + "No message could be reported as spam.": "No message could be reported as spam.", + "No message could be starred.": "No message could be starred.", "No results": "No results", "No signature": "No signature", "No signatures found": "No signatures found", "No subject": "No subject", "No summary available.": "No summary available.", "No template found": "No template found", + "No thread could be archived.": "No thread could be archived.", + "No thread could be deleted.": "No thread could be deleted.", + "No thread could be reported as spam.": "No thread could be reported as spam.", + "No thread could be starred.": "No thread could be starred.", "No threads": "No threads", "No threads match the active filters": "No threads match the active filters", "On going": "On going", @@ -454,6 +481,7 @@ "Refresh": "Refresh", "Refresh summary": "Refresh summary", "Remove": "Remove", + "Remove access?": "Remove access?", "Remove report": "Remove report", "Remove spam report": "Remove spam report", "Remove tag": "Remove tag", @@ -632,6 +660,7 @@ "Yearly": "Yearly", "Yesterday": "Yesterday", "You": "You", + "You and all users with access to the mailbox \"{{mailboxName}}\" will no longer see this thread.": "You and all users with access to the mailbox \"{{mailboxName}}\" will no longer see this thread.", "You are the last editor of this thread, you cannot therefore modify your access.": "You are the last editor of this thread, you cannot therefore modify your access.", "You can close this window and continue using the app.": "You can close this window and continue using the app.", "You can now inform the person that their mailbox is ready to be used and communicate the instructions for authentication.": "You can now inform the person that their mailbox is ready to be used and communicate the instructions for authentication.", @@ -641,6 +670,8 @@ "You have {{count}} recipients, which exceeds the maximum of {{max}} recipients per message. The message cannot be sent until you reduce the number of recipients._other": "You have {{count}} recipients, which exceeds the maximum of {{max}} recipients per message. The message cannot be sent until you reduce the number of recipients.", "You have aborted the upload.": "You have aborted the upload.", "You have unsaved changes. Are you sure you want to close?": "You have unsaved changes. Are you sure you want to close?", + "You left the thread": "You left the thread", + "You may not have sufficient permissions for all selected threads.": "You may not have sufficient permissions for all selected threads.", "You must confirm this statement.": "You must confirm this statement.", "Your email...": "Your email...", "Your messages have been imported successfully!": "Your messages have been imported successfully!", diff --git a/src/frontend/public/locales/common/fr-FR.json b/src/frontend/public/locales/common/fr-FR.json index e28827ae..f7ba3d62 100755 --- a/src/frontend/public/locales/common/fr-FR.json +++ b/src/frontend/public/locales/common/fr-FR.json @@ -47,6 +47,30 @@ "{{count}} occurrences_one": "{{count}} événement", "{{count}} occurrences_many": "{{count}} événements", "{{count}} occurrences_other": "{{count}} événements", + "{{count}} out of {{total}} messages are now starred._one": "{{count}} message sur {{total}} est maintenant suivi.", + "{{count}} out of {{total}} messages are now starred._many": "{{count}} messages sur {{total}} sont maintenant suivis.", + "{{count}} out of {{total}} messages are now starred._other": "{{count}} messages sur {{total}} sont maintenant suivis.", + "{{count}} out of {{total}} messages have been archived._one": "{{count}} message sur {{total}} a été archivé.", + "{{count}} out of {{total}} messages have been archived._many": "{{count}} messages sur {{total}} ont été archivés.", + "{{count}} out of {{total}} messages have been archived._other": "{{count}} messages sur {{total}} ont été archivés.", + "{{count}} out of {{total}} messages have been deleted._one": "{{count}} message sur {{total}} a été supprimé.", + "{{count}} out of {{total}} messages have been deleted._many": "{{count}} messages sur {{total}} ont été supprimés.", + "{{count}} out of {{total}} messages have been deleted._other": "{{count}} messages sur {{total}} ont été supprimés.", + "{{count}} out of {{total}} messages have been reported as spam._one": "{{count}} message sur {{total}} a été signalé comme spam.", + "{{count}} out of {{total}} messages have been reported as spam._many": "{{count}} messages sur {{total}} ont été signalés comme spam.", + "{{count}} out of {{total}} messages have been reported as spam._other": "{{count}} messages sur {{total}} ont été signalés comme spam.", + "{{count}} out of {{total}} threads are now starred._one": "{{count}} conversation sur {{total}} est maintenant suivie.", + "{{count}} out of {{total}} threads are now starred._many": "{{count}} conversations sur {{total}} sont maintenant suivies.", + "{{count}} out of {{total}} threads are now starred._other": "{{count}} conversations sur {{total}} sont maintenant suivies.", + "{{count}} out of {{total}} threads have been archived._one": "{{count}} conversation sur {{total}} a été archivée.", + "{{count}} out of {{total}} threads have been archived._many": "{{count}} conversations sur {{total}} ont été archivées.", + "{{count}} out of {{total}} threads have been archived._other": "{{count}} conversations sur {{total}} ont été archivées.", + "{{count}} out of {{total}} threads have been deleted._one": "{{count}} conversation sur {{total}} a été supprimée.", + "{{count}} out of {{total}} threads have been deleted._many": "{{count}} conversations sur {{total}} ont été supprimées.", + "{{count}} out of {{total}} threads have been deleted._other": "{{count}} conversations sur {{total}} ont été supprimées.", + "{{count}} out of {{total}} threads have been reported as spam._one": "{{count}} conversation sur {{total}} a été signalée comme spam.", + "{{count}} out of {{total}} threads have been reported as spam._many": "{{count}} conversations sur {{total}} ont été signalées comme spam.", + "{{count}} out of {{total}} threads have been reported as spam._other": "{{count}} conversations sur {{total}} ont été signalées comme spam.", "{{count}} results_one": "{{count}} résultat", "{{count}} results_many": "{{count}} résultats", "{{count}} results_other": "{{count}} résultats", @@ -145,6 +169,7 @@ "Addresses": "Adresses", "After creating the widget, you will receive the installation code to add to your website.": "Après avoir créé le widget, vous recevrez le code d'installation à ajouter à votre site web.", "All messages": "Tous les messages", + "All users with access to the mailbox \"{{mailboxName}}\" will no longer see this thread.": "Tous les utilisateurs avec un accès à la boîte \"{{mailboxName}}\" ne verront plus ce thread.", "Always": "Toujours", "An address with this prefix already exists in this domain.": "Une adresse avec ce préfixe existe déjà dans ce domaine.", "An archive is uploading": "Une archive est en cours de téléversement", @@ -406,6 +431,8 @@ "Last saved {{relativeTime}}": "Dernière sauvegarde {{relativeTime}}", "Last update: {{timestamp}}": "Dernière mise à jour : {{timestamp}}", "Layout": "Mise en page", + "Leave this thread": "Quitter cette conversation", + "Leave this thread?": "Quitter cette conversation ?", "less than a minute ago": "il y a moins d'une minute", "Loading addresses...": "Chargement des adresses...", "Loading auto-replies...": "Chargement des réponses automatiques...", @@ -468,12 +495,20 @@ "No event found in calendar invite": "Aucun événement trouvé dans l'invitation calendrier", "No integration found": "Aucune intégration trouvée", "No mailbox": "Aucune boîte aux lettres", + "No message could be archived.": "Aucun message n'a pu être archivé.", + "No message could be deleted.": "Aucun message n'a pu être supprimé.", + "No message could be reported as spam.": "Aucun message n'a pu être signalé comme spam.", + "No message could be starred.": "Aucun message n'a pu être suivi.", "No results": "Aucun résultat", "No signature": "Aucune signature", "No signatures found": "Aucune signature trouvée", "No subject": "Aucun objet", "No summary available.": "Aucun résumé disponible.", "No template found": "Aucun modèle trouvé", + "No thread could be archived.": "Aucune conversation n'a pu être archivée.", + "No thread could be deleted.": "Aucune conversation n'a pu être supprimée.", + "No thread could be reported as spam.": "Aucune conversation n'a pu être signalée comme spam.", + "No thread could be starred.": "Aucune conversation n'a pu être suivie.", "No threads": "Aucune conversation", "No threads match the active filters": "Aucune conversation ne correspond aux filtres actifs", "On going": "En cours", @@ -501,6 +536,7 @@ "Refresh": "Actualiser", "Refresh summary": "Actualiser le résumé", "Remove": "Supprimer", + "Remove access?": "Retirer l'accès ?", "Remove report": "Annuler le signalement", "Remove spam report": "Annuler le signalement spam", "Remove tag": "Supprimer le tag", @@ -683,6 +719,7 @@ "Yearly": "Annuel", "Yesterday": "Hier", "You": "Vous", + "You and all users with access to the mailbox \"{{mailboxName}}\" will no longer see this thread.": "Vous et tous les utilisateurs avec un accès à la boîte « {{mailboxName}} » ne pourront plus voir cette conversation.", "You are the last editor of this thread, you cannot therefore modify your access.": "Vous êtes le dernier éditeur de cette conversation, vous ne pouvez donc pas modifier votre accès.", "You can close this window and continue using the app.": "Vous pouvez fermer cette fenêtre et continuer à utiliser l'application.", "You can now inform the person that their mailbox is ready to be used and communicate the instructions for authentication.": "Vous pouvez désormais prévenir la personne que sa boîte aux lettres est prête à être utilisée et lui communiquer les instructions pour s'authentifier.", @@ -693,6 +730,8 @@ "You have {{count}} recipients, which exceeds the maximum of {{max}} recipients per message. The message cannot be sent until you reduce the number of recipients._other": "Vous avez {{count}} destinataires, ce qui dépasse le maximum de {{max}} destinataires autorisés par message. Le message ne peut pas être envoyé tant que vous n'avez pas réduit le nombre de destinataires.", "You have aborted the upload.": "Vous avez annulé le téléversement.", "You have unsaved changes. Are you sure you want to close?": "Vous avez des modifications non enregistrées. Êtes-vous sûr de vouloir fermer ?", + "You left the thread": "Vous avez quitté le fil", + "You may not have sufficient permissions for all selected threads.": "Vous n'avez peut-être pas les droits suffisants sur toutes les conversations sélectionnées.", "You must confirm this statement.": "Vous devez confirmer cette déclaration.", "Your email...": "Renseigner votre email...", "Your messages have been imported successfully!": "Vos messages ont été importés avec succès !", diff --git a/src/frontend/src/features/api/gen/models/index.ts b/src/frontend/src/features/api/gen/models/index.ts index 663f9ec9..6a693e72 100644 --- a/src/frontend/src/features/api/gen/models/index.ts +++ b/src/frontend/src/features/api/gen/models/index.ts @@ -137,6 +137,7 @@ export * from "./task_status_response"; export * from "./task_status_response_result"; export * from "./third_party_drive_retrieve_params"; export * from "./thread"; +export * from "./thread_abilities"; export * from "./thread_access"; export * from "./thread_access_detail"; export * from "./thread_access_request"; diff --git a/src/frontend/src/features/api/gen/models/thread.ts b/src/frontend/src/features/api/gen/models/thread.ts index fc1895d7..68c0b855 100644 --- a/src/frontend/src/features/api/gen/models/thread.ts +++ b/src/frontend/src/features/api/gen/models/thread.ts @@ -8,6 +8,7 @@ import type { ThreadAccessRoleChoices } from "./thread_access_role_choices"; import type { ThreadAccessDetail } from "./thread_access_detail"; import type { ThreadLabel } from "./thread_label"; +import type { ThreadAbilities } from "./thread_abilities"; /** * Serialize threads. @@ -71,4 +72,5 @@ export interface Thread { readonly labels: readonly ThreadLabel[]; readonly summary: string; readonly events_count: number; + readonly abilities: ThreadAbilities; } diff --git a/src/frontend/src/features/api/gen/models/thread_abilities.ts b/src/frontend/src/features/api/gen/models/thread_abilities.ts new file mode 100644 index 00000000..da54a200 --- /dev/null +++ b/src/frontend/src/features/api/gen/models/thread_abilities.ts @@ -0,0 +1,9 @@ +/** + * Generated by orval 🍺 + * Do not edit manually. + * messages API + * This is the messages API schema. + * OpenAPI spec version: 1.0.0 (v1.0) + */ + +export type ThreadAbilities = { [key: string]: boolean }; diff --git a/src/frontend/src/features/layouts/components/thread-panel/components/thread-panel-header.tsx b/src/frontend/src/features/layouts/components/thread-panel/components/thread-panel-header.tsx index ffcbd380..86657e9f 100644 --- a/src/frontend/src/features/layouts/components/thread-panel/components/thread-panel-header.tsx +++ b/src/frontend/src/features/layouts/components/thread-panel/components/thread-panel-header.tsx @@ -49,6 +49,19 @@ const ThreadPanelTitle = ({ selectedThreadIds, isAllSelected, isSomeSelected, is const { activeFilters } = useThreadPanelFilters(); + // Whether at least one selected thread has full edit rights — gates + // shared-state mutations (archive, spam, trash). The backend enforces + // per-thread permissions; the toast reports the actual updated count. + // Star and read/unread are personal state on the user's ThreadAccess + // and remain available regardless. + const canEditSelection = useMemo(() => { + if (selectedThreadIds.size === 0) return false; + return Array.from(selectedThreadIds).some((id) => { + const thread = threads?.results.find((t) => t.id === id); + return thread?.abilities?.edit === true; + }); + }, [selectedThreadIds, threads?.results]); + const title = useMemo(() => { if (searchParams.has('search')) return t('folder.search', { defaultValue: 'Search' }); if (searchParams.has('label_slug')) return (labelsQuery.data?.data || []).find((label) => label.slug === searchParams.get('label_slug'))?.name; @@ -182,7 +195,7 @@ const ThreadPanelTitle = ({ selectedThreadIds, isAllSelected, isSomeSelected, is aria-label={mainReadTooltip} /> - {isSelectionMode && ( + {isSelectionMode && canEditSelection && ( <> {!isSpamView && !isTrashedView && !isDraftsView && ( @@ -225,28 +238,26 @@ const ThreadPanelTitle = ({ selectedThreadIds, isAllSelected, isSomeSelected, is /> )} - { - !isDraftsView && ( - -