Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Add a spamchecker callback to allow or deny room joins #10910

Merged
merged 8 commits into from
Oct 6, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/10910.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add a spam checker callback to allow or deny room joins.
12 changes: 12 additions & 0 deletions docs/modules/spam_checker_callbacks.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,18 @@ either a `bool` to indicate whether the event must be rejected because of spam,
to indicate the event must be rejected because of spam and to give a rejection reason to
forward to clients.

### `user_may_join_room`

```python
async def user_may_join_room(user: str, room: str, is_invited: bool) -> bool
```

Called when a user is trying to join a room. The module must return a `bool` to indicate
whether the user can join the room. The user is represented by their Matrix user ID (e.g.
`@alice:example.com`) and the room is represented by its Matrix ID (e.g.
`!room:example.com`). The module is also given a boolean to indicate whether the user
currently has a pending invite in the room.
babolivier marked this conversation as resolved.
Show resolved Hide resolved

### `user_may_invite`

```python
Expand Down
24 changes: 24 additions & 0 deletions synapse/events/spamcheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
["synapse.events.EventBase"],
Awaitable[Union[bool, str]],
]
USER_MAY_JOIN_ROOM_CALLBACK = Callable[[str, str, bool], Awaitable[bool]]
USER_MAY_INVITE_CALLBACK = Callable[[str, str, str], Awaitable[bool]]
USER_MAY_CREATE_ROOM_CALLBACK = Callable[[str], Awaitable[bool]]
USER_MAY_CREATE_ROOM_WITH_INVITES_CALLBACK = Callable[
Expand Down Expand Up @@ -165,6 +166,7 @@ def run(*args, **kwargs):
class SpamChecker:
def __init__(self):
self._check_event_for_spam_callbacks: List[CHECK_EVENT_FOR_SPAM_CALLBACK] = []
self._user_may_join_room_callbacks: List[USER_MAY_JOIN_ROOM_CALLBACK] = []
self._user_may_invite_callbacks: List[USER_MAY_INVITE_CALLBACK] = []
self._user_may_create_room_callbacks: List[USER_MAY_CREATE_ROOM_CALLBACK] = []
self._user_may_create_room_with_invites_callbacks: List[
Expand All @@ -187,6 +189,7 @@ def __init__(self):
def register_callbacks(
self,
check_event_for_spam: Optional[CHECK_EVENT_FOR_SPAM_CALLBACK] = None,
user_may_join_room: Optional[USER_MAY_JOIN_ROOM_CALLBACK] = None,
user_may_invite: Optional[USER_MAY_INVITE_CALLBACK] = None,
user_may_create_room: Optional[USER_MAY_CREATE_ROOM_CALLBACK] = None,
user_may_create_room_with_invites: Optional[
Expand All @@ -206,6 +209,9 @@ def register_callbacks(
if check_event_for_spam is not None:
self._check_event_for_spam_callbacks.append(check_event_for_spam)

if user_may_join_room is not None:
self._user_may_join_room_callbacks.append(user_may_join_room)

if user_may_invite is not None:
self._user_may_invite_callbacks.append(user_may_invite)

Expand Down Expand Up @@ -259,6 +265,24 @@ async def check_event_for_spam(

return False

async def user_may_join_room(self, userid: str, room_id: str, is_invited: bool):
babolivier marked this conversation as resolved.
Show resolved Hide resolved
"""Checks if a given users is allowed to join a room.
Not called when a user creates a room.

Args:
userid: The ID of the user wanting to join the room
room_id: The ID of the room the user wants to join
is_invited: Whether the user is invited into the room

Returns:
bool: Whether the user may join the room
"""
for callback in self._user_may_join_room_callbacks:
if await callback(userid, room_id, is_invited) is False:
return False

return True

async def user_may_invite(
self, inviter_userid: str, invitee_userid: str, room_id: str
) -> bool:
Expand Down
2 changes: 2 additions & 0 deletions synapse/handlers/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -858,6 +858,7 @@ async def create_room(
"invite",
ratelimit=False,
content=content,
new_room=True,
)

for invite_3pid in invite_3pid_list:
Expand Down Expand Up @@ -960,6 +961,7 @@ async def send(etype: str, content: JsonDict, **kwargs: Any) -> int:
"join",
ratelimit=ratelimit,
content=creator_join_profile,
new_room=True,
)

# We treat the power levels override specially as this needs to be one
Expand Down
31 changes: 31 additions & 0 deletions synapse/handlers/room_member.py
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,7 @@ async def update_membership(
third_party_signed: Optional[dict] = None,
ratelimit: bool = True,
content: Optional[dict] = None,
new_room: bool = False,
require_consent: bool = True,
outlier: bool = False,
prev_event_ids: Optional[List[str]] = None,
Expand All @@ -451,6 +452,8 @@ async def update_membership(
third_party_signed: Information from a 3PID invite.
ratelimit: Whether to rate limit the request.
content: The content of the created event.
new_room: Whether the membership update is happening in the context of a room
creation.
require_consent: Whether consent is required.
outlier: Indicates whether the event is an `outlier`, i.e. if
it's from an arbitrary point and floating in the DAG as
Expand Down Expand Up @@ -485,6 +488,7 @@ async def update_membership(
third_party_signed=third_party_signed,
ratelimit=ratelimit,
content=content,
new_room=new_room,
require_consent=require_consent,
outlier=outlier,
prev_event_ids=prev_event_ids,
Expand All @@ -504,6 +508,7 @@ async def update_membership_locked(
third_party_signed: Optional[dict] = None,
ratelimit: bool = True,
content: Optional[dict] = None,
new_room: bool = False,
require_consent: bool = True,
outlier: bool = False,
prev_event_ids: Optional[List[str]] = None,
Expand All @@ -523,6 +528,8 @@ async def update_membership_locked(
third_party_signed:
ratelimit:
content:
new_room: Whether the membership update is happening in the context of a room
creation.
require_consent:
outlier: Indicates whether the event is an `outlier`, i.e. if
it's from an arbitrary point and floating in the DAG as
Expand Down Expand Up @@ -718,6 +725,30 @@ async def update_membership_locked(
# so don't really fit into the general auth process.
raise AuthError(403, "Guest access not allowed")

# Figure out whether the user is a server admin to determine whether they
# should be able to bypass the spam checker.
if (
self._server_notices_mxid is not None
and requester.user.to_string() == self._server_notices_mxid
):
# allow the server notices mxid to join rooms
is_requester_admin = True
babolivier marked this conversation as resolved.
Show resolved Hide resolved

else:
is_requester_admin = await self.auth.is_server_admin(requester.user)

inviter = await self._get_inviter(target.to_string(), room_id)
if (
not is_requester_admin
# We assume that if the spam checker allowed the user to create
# a room then they're allowed to join it.
and not new_room
and not await self.spam_checker.user_may_join_room(
target.to_string(), room_id, is_invited=inviter is not None
)
):
raise SynapseError(403, "Not allowed to join this room")

# Check if a remote join should be performed.
remote_join, remote_room_hosts = await self._should_perform_remote_join(
target.to_string(), room_id, remote_room_hosts, content, is_host_in_room
Expand Down
101 changes: 101 additions & 0 deletions tests/rest/client/test_rooms.py
Original file line number Diff line number Diff line change
Expand Up @@ -784,6 +784,30 @@ async def user_may_create_room_with_invites(
# Check that do_3pid_invite wasn't called this time.
self.assertEquals(do_3pid_invite_mock.call_count, len(invited_3pids))

def test_spam_checker_may_join_room(self):
"""Tests that the user_may_join_room spam checker callback is correctly bypassed
when creating a new room.
"""

async def user_may_join_room(
mxid: str,
room_id: str,
is_invite: bool,
) -> bool:
return False

join_mock = Mock(side_effect=user_may_join_room)
self.hs.get_spam_checker()._user_may_join_room_callbacks.append(join_mock)

channel = self.make_request(
"POST",
"/createRoom",
{},
)
self.assertEquals(channel.code, 200, channel.json_body)

self.assertEquals(join_mock.call_count, 0)


class RoomTopicTestCase(RoomBase):
"""Tests /rooms/$room_id/topic REST events."""
Expand Down Expand Up @@ -975,6 +999,83 @@ def test_invites_by_users_ratelimit(self):
self.helper.invite(room_id, self.user_id, "@other-users:red", expect_code=429)


class RoomJoinTestCase(RoomBase):

servlets = [
admin.register_servlets,
login.register_servlets,
room.register_servlets,
]

def prepare(self, reactor, clock, homeserver):
self.user1 = self.register_user("thomas", "hackme")
self.tok1 = self.login("thomas", "hackme")

self.user2 = self.register_user("teresa", "hackme")
self.tok2 = self.login("teresa", "hackme")

self.room1 = self.helper.create_room_as(room_creator=self.user1, tok=self.tok1)
self.room2 = self.helper.create_room_as(room_creator=self.user1, tok=self.tok1)
self.room3 = self.helper.create_room_as(room_creator=self.user1, tok=self.tok1)

def test_spam_checker_may_join_room(self):
"""Tests that the user_may_join_room spam checker callback is correctly called
and blocks room joins when needed.
"""

# Register a dummy callback. Make it allow all room joins for now.
return_value = True

async def user_may_join_room(
userid: str,
room_id: str,
is_invited: bool,
) -> bool:
return return_value

callback_mock = Mock(side_effect=user_may_join_room)
self.hs.get_spam_checker()._user_may_join_room_callbacks.append(callback_mock)

# Join a first room, without being invited to it.
self.helper.join(self.room1, self.user2, tok=self.tok2)

# Check that the callback was called with the right arguments.
expected_call_args = (
(
self.user2,
self.room1,
False,
),
)
self.assertEquals(
callback_mock.call_args,
expected_call_args,
callback_mock.call_args,
)

# Join a second room, this time with an invite for it.
self.helper.invite(self.room2, self.user1, self.user2, tok=self.tok1)
self.helper.join(self.room2, self.user2, tok=self.tok2)

# Check that the callback was called with the right arguments.
expected_call_args = (
(
self.user2,
self.room2,
True,
),
)
self.assertEquals(
callback_mock.call_args,
expected_call_args,
callback_mock.call_args,
)

# Now make the callback deny all room joins, and check that a join actually fails.
return_value = False
self.helper.join(self.room3, self.user2, expect_code=403, tok=self.tok2)


class RoomJoinRatelimitTestCase(RoomBase):
user_id = "@sid1:red"

Expand Down