Skip to content

Commit f2260a1

Browse files
committed
🐛(permissions) resolve MultipleObjectsReturned when user has access to multiple mailboxes on same thread
The has_object_permission method was failing with MultipleObjectsReturned exception when a user had access to multiple mailboxes that both have access to the same thread. Changes: - Filter thread accesses by EDITOR role and check user mailbox permissions - Use filter().exists() instead of get() to avoid MultipleObjectsReturned - Add test case to verify users can send messages when they have access to multiple mailboxes on the same thread
1 parent 7898f7b commit f2260a1

File tree

2 files changed

+106
-24
lines changed

2 files changed

+106
-24
lines changed

src/backend/core/api/permissions.py

Lines changed: 13 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -148,31 +148,20 @@ def has_object_permission(self, request, view, obj):
148148

149149
# Only EDITOR, SENDER or ADMIN role can destroy. SENDER or ADMIN can send.
150150
if view.action in ["destroy", "send"]:
151-
mailbox = thread.accesses.get(mailbox__accesses__user=user).mailbox
152-
if (
153-
models.ThreadAccess.objects.filter(
154-
thread=thread,
155-
mailbox=mailbox,
156-
role=enums.ThreadAccessRoleChoices.EDITOR,
157-
).exists()
158-
and models.MailboxAccess.objects.filter(
159-
mailbox=mailbox,
160-
user=user,
161-
role__in=[
162-
enums.MailboxRoleChoices.ADMIN,
163-
enums.MailboxRoleChoices.SENDER,
164-
]
165-
+ (
166-
[enums.MailboxRoleChoices.EDITOR]
167-
if view.action == "destroy"
168-
else []
169-
),
170-
).exists()
171-
):
172-
return True
151+
# filter only mailboxes with editor role on thread
152+
# and check if user has enough role on those mailboxes to destroy or send the message
153+
minimal_user_role = (
154+
enums.MailboxRoleChoices.EDITOR
155+
if view.action == "destroy"
156+
else enums.MailboxRoleChoices.SENDER
157+
)
158+
return thread.accesses.filter(
159+
role=enums.ThreadAccessRoleChoices.EDITOR,
160+
mailbox__accesses__user=user,
161+
mailbox__accesses__role__gte=minimal_user_role,
162+
).exists()
173163
# for retrieve action has_access is already checked above
174-
else:
175-
return True
164+
return True
176165

177166
# Deny access for other object types or if type is unknown
178167
return False

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

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1458,3 +1458,96 @@ def test_api_email_exchange_single_thread(self, send_url):
14581458
# Final check: Still only one thread per mailbox
14591459
assert models.Thread.objects.filter(accesses__mailbox=mailbox1).count() == 1
14601460
assert models.Thread.objects.filter(accesses__mailbox=mailbox2).count() == 1
1461+
1462+
def test_send_message_with_user_having_role_on_two_mailboxes_on_same_thread(
1463+
self, mailbox, mailbox2, authenticated_user, send_url
1464+
):
1465+
"""
1466+
Test that sending a message succeeds when a user has access to two mailboxes
1467+
that both have access to the same thread.
1468+
"""
1469+
# Create a mailbox access on this mailbox for the authenticated user
1470+
factories.MailboxAccessFactory(
1471+
mailbox=mailbox,
1472+
user=authenticated_user,
1473+
role=enums.MailboxRoleChoices.SENDER,
1474+
)
1475+
# Give the user access to mailbox2 with ADMIN role
1476+
factories.MailboxAccessFactory(
1477+
mailbox=mailbox2,
1478+
user=authenticated_user,
1479+
role=enums.MailboxRoleChoices.ADMIN,
1480+
)
1481+
# Create a thread with a message
1482+
thread_access = factories.ThreadAccessFactory(
1483+
mailbox=mailbox,
1484+
role=enums.ThreadAccessRoleChoices.EDITOR,
1485+
)
1486+
_thread_access_2 = factories.ThreadAccessFactory(
1487+
mailbox=mailbox2,
1488+
thread=thread_access.thread,
1489+
role=enums.ThreadAccessRoleChoices.EDITOR,
1490+
)
1491+
message = factories.MessageFactory(thread=thread_access.thread, read_at=None)
1492+
factories.MessageRecipientFactory(
1493+
message=message,
1494+
type=enums.MessageRecipientTypeChoices.TO,
1495+
)
1496+
1497+
# Create a client and authenticate the user
1498+
client = APIClient()
1499+
client.force_authenticate(user=authenticated_user)
1500+
1501+
# Step 1: Create a draft reply
1502+
draft_response = client.post(
1503+
reverse("draft-message"),
1504+
{
1505+
"parentId": message.id, # ID of the message we're replying to
1506+
"senderId": mailbox.id,
1507+
"subject": "test reply",
1508+
"draftBody": "<p>test reply</p> or test reply",
1509+
"to": ["[email protected]"],
1510+
},
1511+
format="json",
1512+
)
1513+
1514+
# Assert the draft response is successful
1515+
assert draft_response.status_code == status.HTTP_201_CREATED
1516+
1517+
# Assert the draft message is created
1518+
draft_message = models.Message.objects.get(id=draft_response.data["id"])
1519+
assert draft_message.is_draft is True
1520+
assert draft_message.parent == message
1521+
1522+
draft_api_message = client.get(
1523+
reverse("messages-detail", kwargs={"id": draft_message.id})
1524+
).data
1525+
assert draft_api_message["is_draft"] is True
1526+
assert draft_api_message["parent_id"] == str(message.id)
1527+
1528+
# Step 2: Send the draft reply
1529+
send_response = client.post(
1530+
send_url,
1531+
{
1532+
"messageId": draft_message.id,
1533+
"senderId": mailbox.id,
1534+
"textBody": "test",
1535+
},
1536+
format="json",
1537+
)
1538+
1539+
# Assert the send response is successful
1540+
assert send_response.status_code == status.HTTP_200_OK
1541+
1542+
# Assert the message is now sent
1543+
sent_message = models.Message.objects.get(id=draft_message.id)
1544+
assert sent_message.is_draft is False
1545+
assert sent_message.sent_at is not None
1546+
1547+
# Assert the message and thread are created correctly
1548+
assert models.Message.objects.count() == 2
1549+
assert models.Thread.objects.count() == 1
1550+
1551+
# Assert the message is correct
1552+
assert sent_message.subject == "test reply"
1553+
assert sent_message.thread == thread_access.thread

0 commit comments

Comments
 (0)