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

Remove deprecated user_may_create_room_with_invites callback #11950

Merged
merged 6 commits into from
Feb 11, 2022
Merged
Show file tree
Hide file tree
Changes from 3 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/11950.removal
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Remove deprecated `user_may_create_room_with_invites` spam checker callback.
babolivier marked this conversation as resolved.
Show resolved Hide resolved
babolivier marked this conversation as resolved.
Show resolved Hide resolved
42 changes: 0 additions & 42 deletions synapse/events/spamcheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,6 @@
USER_MAY_INVITE_CALLBACK = Callable[[str, str, str], Awaitable[bool]]
USER_MAY_SEND_3PID_INVITE_CALLBACK = Callable[[str, str, str, str], Awaitable[bool]]
USER_MAY_CREATE_ROOM_CALLBACK = Callable[[str], Awaitable[bool]]
USER_MAY_CREATE_ROOM_WITH_INVITES_CALLBACK = Callable[
[str, List[str], List[Dict[str, str]]], Awaitable[bool]
]
USER_MAY_CREATE_ROOM_ALIAS_CALLBACK = Callable[[str, RoomAlias], Awaitable[bool]]
USER_MAY_PUBLISH_ROOM_CALLBACK = Callable[[str, str], Awaitable[bool]]
CHECK_USERNAME_FOR_SPAM_CALLBACK = Callable[[Dict[str, str]], Awaitable[bool]]
Expand Down Expand Up @@ -174,9 +171,6 @@ def __init__(self) -> None:
USER_MAY_SEND_3PID_INVITE_CALLBACK
] = []
self._user_may_create_room_callbacks: List[USER_MAY_CREATE_ROOM_CALLBACK] = []
self._user_may_create_room_with_invites_callbacks: List[
USER_MAY_CREATE_ROOM_WITH_INVITES_CALLBACK
] = []
self._user_may_create_room_alias_callbacks: List[
USER_MAY_CREATE_ROOM_ALIAS_CALLBACK
] = []
Expand All @@ -198,9 +192,6 @@ def register_callbacks(
user_may_invite: Optional[USER_MAY_INVITE_CALLBACK] = None,
user_may_send_3pid_invite: Optional[USER_MAY_SEND_3PID_INVITE_CALLBACK] = None,
user_may_create_room: Optional[USER_MAY_CREATE_ROOM_CALLBACK] = None,
user_may_create_room_with_invites: Optional[
USER_MAY_CREATE_ROOM_WITH_INVITES_CALLBACK
] = None,
user_may_create_room_alias: Optional[
USER_MAY_CREATE_ROOM_ALIAS_CALLBACK
] = None,
Expand Down Expand Up @@ -229,11 +220,6 @@ def register_callbacks(
if user_may_create_room is not None:
self._user_may_create_room_callbacks.append(user_may_create_room)

if user_may_create_room_with_invites is not None:
self._user_may_create_room_with_invites_callbacks.append(
user_may_create_room_with_invites,
)

if user_may_create_room_alias is not None:
self._user_may_create_room_alias_callbacks.append(
user_may_create_room_alias,
Expand Down Expand Up @@ -359,34 +345,6 @@ async def user_may_create_room(self, userid: str) -> bool:

return True

async def user_may_create_room_with_invites(
self,
userid: str,
invites: List[str],
threepid_invites: List[Dict[str, str]],
) -> bool:
"""Checks if a given user may create a room with invites

If this method returns false, the creation request will be rejected.

Args:
userid: The ID of the user attempting to create a room
invites: The IDs of the Matrix users to be invited if the room creation is
allowed.
threepid_invites: The threepids to be invited if the room creation is allowed,
as a dict including a "medium" key indicating the threepid's medium (e.g.
"email") and an "address" key indicating the threepid's address (e.g.
"alice@example.com")

Returns:
True if the user may create the room, otherwise False
"""
for callback in self._user_may_create_room_with_invites_callbacks:
if await callback(userid, invites, threepid_invites) is False:
return False

return True

async def user_may_create_room_alias(
self, userid: str, room_alias: RoomAlias
) -> bool:
Expand Down
5 changes: 0 additions & 5 deletions synapse/handlers/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -694,11 +694,6 @@ async def create_room(

if not is_requester_admin and not (
await self.spam_checker.user_may_create_room(user_id)
and await self.spam_checker.user_may_create_room_with_invites(
user_id,
invite_list,
invite_3pid_list,
)
):
raise SynapseError(
403, "You are not permitted to create rooms", Codes.FORBIDDEN
Expand Down
5 changes: 0 additions & 5 deletions synapse/module_api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
CHECK_USERNAME_FOR_SPAM_CALLBACK,
USER_MAY_CREATE_ROOM_ALIAS_CALLBACK,
USER_MAY_CREATE_ROOM_CALLBACK,
USER_MAY_CREATE_ROOM_WITH_INVITES_CALLBACK,
USER_MAY_INVITE_CALLBACK,
USER_MAY_JOIN_ROOM_CALLBACK,
USER_MAY_PUBLISH_ROOM_CALLBACK,
Expand Down Expand Up @@ -217,9 +216,6 @@ def register_spam_checker_callbacks(
user_may_invite: Optional[USER_MAY_INVITE_CALLBACK] = None,
user_may_send_3pid_invite: Optional[USER_MAY_SEND_3PID_INVITE_CALLBACK] = None,
user_may_create_room: Optional[USER_MAY_CREATE_ROOM_CALLBACK] = None,
user_may_create_room_with_invites: Optional[
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we make the parameters to register_spam_checker_callbacks keyword only via the * flag?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could, I'd suggest opening a new issue for this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put up a PR at #11975

USER_MAY_CREATE_ROOM_WITH_INVITES_CALLBACK
] = None,
user_may_create_room_alias: Optional[
USER_MAY_CREATE_ROOM_ALIAS_CALLBACK
] = None,
Expand All @@ -240,7 +236,6 @@ def register_spam_checker_callbacks(
user_may_invite=user_may_invite,
user_may_send_3pid_invite=user_may_send_3pid_invite,
user_may_create_room=user_may_create_room,
user_may_create_room_with_invites=user_may_create_room_with_invites,
user_may_create_room_alias=user_may_create_room_alias,
user_may_publish_room=user_may_publish_room,
check_username_for_spam=check_username_for_spam,
Expand Down
119 changes: 2 additions & 117 deletions tests/rest/client/test_rooms.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"""Tests REST events for /rooms paths."""

import json
from typing import Dict, Iterable, List, Optional
from typing import Iterable, List
from unittest.mock import Mock, call
from urllib import parse as urlparse

Expand All @@ -35,7 +35,7 @@
from synapse.handlers.pagination import PurgeStatus
from synapse.rest import admin
from synapse.rest.client import account, directory, login, profile, room, sync
from synapse.types import JsonDict, Requester, RoomAlias, UserID, create_requester
from synapse.types import JsonDict, RoomAlias, UserID, create_requester
from synapse.util.stringutils import random_string

from tests import unittest
Expand Down Expand Up @@ -674,121 +674,6 @@ def test_post_room_invitees_ratelimit(self):
channel = self.make_request("POST", "/createRoom", content)
self.assertEqual(200, channel.code)

def test_spamchecker_invites(self):
"""Tests the user_may_create_room_with_invites spam checker callback."""

# Mock do_3pid_invite, so we don't fail from failing to send a 3PID invite to an
# IS.
async def do_3pid_invite(
room_id: str,
inviter: UserID,
medium: str,
address: str,
id_server: str,
requester: Requester,
txn_id: Optional[str],
id_access_token: Optional[str] = None,
) -> int:
return 0

do_3pid_invite_mock = Mock(side_effect=do_3pid_invite)
self.hs.get_room_member_handler().do_3pid_invite = do_3pid_invite_mock

# Add a mock callback for user_may_create_room_with_invites. Make it allow any
# room creation request for now.
return_value = True

async def user_may_create_room_with_invites(
user: str,
invites: List[str],
threepid_invites: List[Dict[str, str]],
) -> bool:
return return_value

callback_mock = Mock(side_effect=user_may_create_room_with_invites)
self.hs.get_spam_checker()._user_may_create_room_with_invites_callbacks.append(
callback_mock,
)

# The MXIDs we'll try to invite.
invited_mxids = [
"@alice1:red",
"@alice2:red",
"@alice3:red",
"@alice4:red",
]

# The 3PIDs we'll try to invite.
invited_3pids = [
{
"id_server": "example.com",
"id_access_token": "sometoken",
"medium": "email",
"address": "alice1@example.com",
},
{
"id_server": "example.com",
"id_access_token": "sometoken",
"medium": "email",
"address": "alice2@example.com",
},
{
"id_server": "example.com",
"id_access_token": "sometoken",
"medium": "email",
"address": "alice3@example.com",
},
]

# Create a room and invite the Matrix users, and check that it succeeded.
channel = self.make_request(
"POST",
"/createRoom",
json.dumps({"invite": invited_mxids}).encode("utf8"),
)
self.assertEqual(200, channel.code)

# Check that the callback was called with the right arguments.
expected_call_args = ((self.user_id, invited_mxids, []),)
self.assertEquals(
callback_mock.call_args,
expected_call_args,
callback_mock.call_args,
)

# Create a room and invite the 3PIDs, and check that it succeeded.
channel = self.make_request(
"POST",
"/createRoom",
json.dumps({"invite_3pid": invited_3pids}).encode("utf8"),
)
self.assertEqual(200, channel.code)

# Check that do_3pid_invite was called the right amount of time
self.assertEquals(do_3pid_invite_mock.call_count, len(invited_3pids))

# Check that the callback was called with the right arguments.
expected_call_args = ((self.user_id, [], invited_3pids),)
self.assertEquals(
callback_mock.call_args,
expected_call_args,
callback_mock.call_args,
)

# Now deny any room creation.
return_value = False

# Create a room and invite the 3PIDs, and check that it failed.
channel = self.make_request(
"POST",
"/createRoom",
json.dumps({"invite_3pid": invited_3pids}).encode("utf8"),
)
self.assertEqual(403, channel.code)

# 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.
Expand Down