Skip to content

Commit

Permalink
Fix Internal Server Error for Non-Local Users in Room Actions (#17607)
Browse files Browse the repository at this point in the history
  • Loading branch information
TrevisGordan authored Aug 29, 2024
1 parent b21134d commit 594cd5f
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 5 deletions.
1 change: 1 addition & 0 deletions changelog.d/17607.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Return `400 M_BAD_JSON` upon attempting to complete various room actions with a non-local user ID and unknown room ID, rather than an internal server error.
8 changes: 4 additions & 4 deletions synapse/storage/databases/main/roommember.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#
#
import logging
from http import HTTPStatus
from typing import (
TYPE_CHECKING,
AbstractSet,
Expand All @@ -39,6 +40,7 @@
import attr

from synapse.api.constants import EventTypes, Membership
from synapse.api.errors import Codes, SynapseError
from synapse.logging.opentracing import trace
from synapse.metrics import LaterGauge
from synapse.metrics.background_process_metrics import wrap_as_background_process
Expand Down Expand Up @@ -631,10 +633,8 @@ async def get_local_current_membership_for_user_in_room(
"""
# Paranoia check.
if not self.hs.is_mine_id(user_id):
raise Exception(
"Cannot call 'get_local_current_membership_for_user_in_room' on "
"non-local user %s" % (user_id,),
)
message = f"Provided user_id {user_id} is a non-local user"
raise SynapseError(HTTPStatus.BAD_REQUEST, message, errcode=Codes.BAD_JSON)

results = cast(
Optional[Tuple[str, str]],
Expand Down
22 changes: 21 additions & 1 deletion tests/handlers/test_room_member.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import synapse.rest.client.login
import synapse.rest.client.room
from synapse.api.constants import EventTypes, Membership
from synapse.api.errors import LimitExceededError, SynapseError
from synapse.api.errors import Codes, LimitExceededError, SynapseError
from synapse.crypto.event_signing import add_hashes_and_signatures
from synapse.events import FrozenEventV3
from synapse.federation.federation_client import SendJoinResult
Expand Down Expand Up @@ -383,6 +383,26 @@ def test_forget_when_not_left(self) -> None:
"""Tests that a user cannot not forgets a room that has not left."""
self.get_failure(self.handler.forget(self.alice_ID, self.room_id), SynapseError)

def test_nonlocal_room_user_action(self) -> None:
"""
Test that non-local user ids cannot perform room actions through
this homeserver.
"""
alien_user_id = UserID.from_string("@cheeky_monkey:matrix.org")
bad_room_id = f"{self.room_id}+BAD_ID"

exc = self.get_failure(
self.handler.update_membership(
create_requester(self.alice),
alien_user_id,
bad_room_id,
"unban",
),
SynapseError,
).value

self.assertEqual(exc.errcode, Codes.BAD_JSON)

def test_rejoin_forgotten_by_user(self) -> None:
"""Test that a user that has forgotten a room can do a re-join.
The room was not forgotten from the local server.
Expand Down

0 comments on commit 594cd5f

Please sign in to comment.