From 4e5162106436f3fddd12561d316d19fd23148800 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 6 Oct 2021 17:18:13 +0200 Subject: [PATCH] Add a spamchecker method to allow or deny 3pid invites (#10894) This is in the context of creating new module callbacks that modules in https://github.com/matrix-org/synapse-dinsic can use, in an effort to reconcile the spam checker API in synapse-dinsic with the one in mainline. Note that a module callback already exists for 3pid invites (https://matrix-org.github.io/synapse/develop/modules/third_party_rules_callbacks.html#check_threepid_can_be_invited) but it doesn't check whether the sender of the invite is allowed to send it. --- changelog.d/10894.feature | 1 + docs/modules/spam_checker_callbacks.md | 35 +++++++++++++ synapse/events/spamcheck.py | 35 +++++++++++++ synapse/handlers/room_member.py | 12 +++++ tests/rest/client/test_rooms.py | 70 ++++++++++++++++++++++++++ 5 files changed, 153 insertions(+) create mode 100644 changelog.d/10894.feature diff --git a/changelog.d/10894.feature b/changelog.d/10894.feature new file mode 100644 index 000000000..a4f968bed --- /dev/null +++ b/changelog.d/10894.feature @@ -0,0 +1 @@ +Add a `user_may_send_3pid_invite` spam checker callback for modules to allow or deny 3PID invites. diff --git a/docs/modules/spam_checker_callbacks.md b/docs/modules/spam_checker_callbacks.md index 92376df99..787e99074 100644 --- a/docs/modules/spam_checker_callbacks.md +++ b/docs/modules/spam_checker_callbacks.md @@ -44,6 +44,41 @@ Called when processing an invitation. The module must return a `bool` indicating the inviter can invite the invitee to the given room. Both inviter and invitee are represented by their Matrix user ID (e.g. `@alice:example.com`). +### `user_may_send_3pid_invite` + +```python +async def user_may_send_3pid_invite( + inviter: str, + medium: str, + address: str, + room_id: str, +) -> bool +``` + +Called when processing an invitation using a third-party identifier (also called a 3PID, +e.g. an email address or a phone number). The module must return a `bool` indicating +whether the inviter can invite the invitee to the given room. + +The inviter is represented by their Matrix user ID (e.g. `@alice:example.com`), and the +invitee is represented by its medium (e.g. "email") and its address +(e.g. `alice@example.com`). See [the Matrix specification](https://matrix.org/docs/spec/appendices#pid-types) +for more information regarding third-party identifiers. + +For example, a call to this callback to send an invitation to the email address +`alice@example.com` would look like this: + +```python +await user_may_send_3pid_invite( + "@bob:example.com", # The inviter's user ID + "email", # The medium of the 3PID to invite + "alice@example.com", # The address of the 3PID to invite + "!some_room:example.com", # The ID of the room to send the invite into +) +``` + +**Note**: If the third-party identifier is already associated with a matrix user ID, +[`user_may_invite`](#user_may_invite) will be used instead. + ### `user_may_create_room` ```python diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py index ec8863e39..ae4c8ab25 100644 --- a/synapse/events/spamcheck.py +++ b/synapse/events/spamcheck.py @@ -46,6 +46,7 @@ CHECK_EVENT_FOR_SPAM_CALLBACK = Callable[ ] USER_MAY_JOIN_ROOM_CALLBACK = Callable[[str, str, bool], Awaitable[bool]] 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] @@ -168,6 +169,9 @@ class SpamChecker: 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_send_3pid_invite_callbacks: List[ + 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 @@ -191,6 +195,7 @@ class SpamChecker: 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_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 @@ -215,6 +220,11 @@ class SpamChecker: if user_may_invite is not None: self._user_may_invite_callbacks.append(user_may_invite) + if user_may_send_3pid_invite is not None: + self._user_may_send_3pid_invite_callbacks.append( + user_may_send_3pid_invite, + ) + if user_may_create_room is not None: self._user_may_create_room_callbacks.append(user_may_create_room) @@ -304,6 +314,31 @@ class SpamChecker: return True + async def user_may_send_3pid_invite( + self, inviter_userid: str, medium: str, address: str, room_id: str + ) -> bool: + """Checks if a given user may invite a given threepid into the room + + If this method returns false, the threepid invite will be rejected. + + Note that if the threepid is already associated with a Matrix user ID, Synapse + will call user_may_invite with said user ID instead. + + Args: + inviter_userid: The user ID of the sender of the invitation + medium: The 3PID's medium (e.g. "email") + address: The 3PID's address (e.g. "alice@example.com") + room_id: The room ID + + Returns: + True if the user may send the invite, otherwise False + """ + for callback in self._user_may_send_3pid_invite_callbacks: + if await callback(inviter_userid, medium, address, room_id) is False: + return False + + return True + async def user_may_create_room(self, userid: str) -> bool: """Checks if a given user may create a room diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index c05461bf2..eef337fee 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -1299,10 +1299,22 @@ class RoomMemberHandler(metaclass=abc.ABCMeta): if invitee: # Note that update_membership with an action of "invite" can raise # a ShadowBanError, but this was done above already. + # We don't check the invite against the spamchecker(s) here (through + # user_may_invite) because we'll do it further down the line anyway (in + # update_membership_locked). _, stream_id = await self.update_membership( requester, UserID.from_string(invitee), room_id, "invite", txn_id=txn_id ) else: + # Check if the spamchecker(s) allow this invite to go through. + if not await self.spam_checker.user_may_send_3pid_invite( + inviter_userid=requester.user.to_string(), + medium=medium, + address=address, + room_id=room_id, + ): + raise SynapseError(403, "Cannot send threepid invite") + stream_id = await self._make_and_store_3pid_invite( requester, id_server, diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index a41ec6a98..376853fd6 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -2531,3 +2531,73 @@ class RoomCanonicalAliasTestCase(unittest.HomeserverTestCase): """An alias which does not point to the room raises a SynapseError.""" self._set_canonical_alias({"alias": "@unknown:test"}, expected_code=400) self._set_canonical_alias({"alt_aliases": ["@unknown:test"]}, expected_code=400) + + +class ThreepidInviteTestCase(unittest.HomeserverTestCase): + + servlets = [ + admin.register_servlets, + login.register_servlets, + room.register_servlets, + ] + + def prepare(self, reactor, clock, homeserver): + self.user_id = self.register_user("thomas", "hackme") + self.tok = self.login("thomas", "hackme") + + self.room_id = self.helper.create_room_as(self.user_id, tok=self.tok) + + def test_threepid_invite_spamcheck(self): + # Mock a few functions to prevent the test from failing due to failing to talk to + # a remote IS. We keep the mock for _mock_make_and_store_3pid_invite around so we + # can check its call_count later on during the test. + make_invite_mock = Mock(return_value=make_awaitable(0)) + self.hs.get_room_member_handler()._make_and_store_3pid_invite = make_invite_mock + self.hs.get_identity_handler().lookup_3pid = Mock( + return_value=make_awaitable(None), + ) + + # Add a mock to the spamchecker callbacks for user_may_send_3pid_invite. Make it + # allow everything for now. + mock = Mock(return_value=make_awaitable(True)) + self.hs.get_spam_checker()._user_may_send_3pid_invite_callbacks.append(mock) + + # Send a 3PID invite into the room and check that it succeeded. + email_to_invite = "teresa@example.com" + channel = self.make_request( + method="POST", + path="/rooms/" + self.room_id + "/invite", + content={ + "id_server": "example.com", + "id_access_token": "sometoken", + "medium": "email", + "address": email_to_invite, + }, + access_token=self.tok, + ) + self.assertEquals(channel.code, 200) + + # Check that the callback was called with the right params. + mock.assert_called_with(self.user_id, "email", email_to_invite, self.room_id) + + # Check that the call to send the invite was made. + make_invite_mock.assert_called_once() + + # Now change the return value of the callback to deny any invite and test that + # we can't send the invite. + mock.return_value = make_awaitable(False) + channel = self.make_request( + method="POST", + path="/rooms/" + self.room_id + "/invite", + content={ + "id_server": "example.com", + "id_access_token": "sometoken", + "medium": "email", + "address": email_to_invite, + }, + access_token=self.tok, + ) + self.assertEquals(channel.code, 403) + + # Also check that it stopped before calling _make_and_store_3pid_invite. + make_invite_mock.assert_called_once()