From b7d7d20a38b3ce21485c022c3162857a53d0beb8 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 21 Mar 2019 12:11:40 +0000 Subject: [PATCH] Correctly handle 3PID invites in create room spam check We also add an option to outright deny third party invites --- synapse/events/spamcheck.py | 9 +++++++-- synapse/handlers/room.py | 5 +++-- synapse/rulecheck/domain_rule_checker.py | 16 ++++++++++++++-- tests/rulecheck/test_domainrulecheck.py | 15 +++++++++++++++ 4 files changed, 39 insertions(+), 6 deletions(-) diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py index e4fc988cfc..87d66a9703 100644 --- a/synapse/events/spamcheck.py +++ b/synapse/events/spamcheck.py @@ -69,7 +69,8 @@ class SpamChecker(object): inviter_userid, invitee_userid, room_id, new_room, ) - def user_may_create_room(self, userid, invite_list, cloning): + def user_may_create_room(self, userid, invite_list, third_party_invite_list, + cloning): """Checks if a given user may create a room If this method returns false, the creation request will be rejected. @@ -78,6 +79,8 @@ class SpamChecker(object): userid (string): The sender's user ID invite_list (list[str]): List of user IDs that would be invited to the new room. + third_party_invite_list (list[dict]): List of third party invites + for the new room. cloning (bool): Whether the user is cloning an existing room, e.g. upgrading a room. @@ -87,7 +90,9 @@ class SpamChecker(object): if self.spam_checker is None: return True - return self.spam_checker.user_may_create_room(userid, invite_list, cloning) + return self.spam_checker.user_may_create_room( + userid, invite_list, third_party_invite_list, cloning, + ) def user_may_create_room_alias(self, userid, room_alias): """Checks if a given user may create a room alias diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 581cff9526..a710b51c3d 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -269,6 +269,7 @@ class RoomCreationHandler(BaseHandler): if not is_requester_admin and not self.spam_checker.user_may_create_room( user_id, invite_list=[], + third_party_invite_list=[], cloning=True, ): raise SynapseError(403, "You are not permitted to create rooms") @@ -492,6 +493,7 @@ class RoomCreationHandler(BaseHandler): yield self.auth.check_auth_blocking(user_id) invite_list = config.get("invite", []) + invite_3pid_list = config.get("invite_3pid", []) if (self._server_notices_mxid is not None and requester.user.to_string() == self._server_notices_mxid): @@ -505,6 +507,7 @@ class RoomCreationHandler(BaseHandler): if not is_requester_admin and not self.spam_checker.user_may_create_room( user_id, invite_list=invite_list, + third_party_invite_list=invite_3pid_list, cloning=False, ): raise SynapseError(403, "You are not permitted to create rooms") @@ -559,8 +562,6 @@ class RoomCreationHandler(BaseHandler): requester, ) - invite_3pid_list = config.get("invite_3pid", []) - visibility = config.get("visibility", None) is_public = visibility == "public" diff --git a/synapse/rulecheck/domain_rule_checker.py b/synapse/rulecheck/domain_rule_checker.py index 410757041b..48ab62faac 100644 --- a/synapse/rulecheck/domain_rule_checker.py +++ b/synapse/rulecheck/domain_rule_checker.py @@ -46,6 +46,9 @@ class DomainRuleChecker(object): # domain mapping rules above. can_only_invite_during_room_creation: false + # Allow third party invites + can_invite_by_third_party_id: true + Don't forget to consider if you can invite users from your own domain. """ @@ -62,6 +65,9 @@ class DomainRuleChecker(object): self.can_only_invite_during_room_creation = config.get( "can_only_invite_during_room_creation", False, ) + self.can_invite_by_third_party_id = config.get( + "can_invite_by_third_party_id", True, + ) def check_event_for_spam(self, event): """Implements synapse.events.SpamChecker.check_event_for_spam @@ -83,14 +89,20 @@ class DomainRuleChecker(object): return invitee_domain in self.domain_mapping[inviter_domain] - def user_may_create_room(self, userid, invite_list, cloning): + def user_may_create_room(self, userid, invite_list, third_party_invite_list, + cloning): """Implements synapse.events.SpamChecker.user_may_create_room """ if cloning: return True - if self.can_only_create_one_to_one_rooms and len(invite_list) != 1: + if not self.can_invite_by_third_party_id and third_party_invite_list: + return False + + number_of_invites = len(invite_list) + len(third_party_invite_list) + + if self.can_only_create_one_to_one_rooms and number_of_invites != 1: return False return True diff --git a/tests/rulecheck/test_domainrulecheck.py b/tests/rulecheck/test_domainrulecheck.py index de89f95e3c..a887249b59 100644 --- a/tests/rulecheck/test_domainrulecheck.py +++ b/tests/rulecheck/test_domainrulecheck.py @@ -131,6 +131,7 @@ class DomainRuleCheckerRoomTestCase(unittest.HomeserverTestCase): "can_only_join_rooms_with_invite": True, "can_only_create_one_to_one_rooms": True, "can_only_invite_during_room_creation": True, + "can_invite_by_third_party_id": False, }) hs = self.setup_test_homeserver(config=config) @@ -159,6 +160,20 @@ class DomainRuleCheckerRoomTestCase(unittest.HomeserverTestCase): }) assert channel.result["code"] == b"403", channel.result + # Test that it correctly counts both normal and third party invites + channel = self._create_room(self.normal_access_token, content={ + "invite": [self.other_user_id], + "invite_3pid": [{"medium": "email", "address": "foo@example.com"}], + }) + assert channel.result["code"] == b"403", channel.result + + # Test that it correctly rejects third party invites + channel = self._create_room(self.normal_access_token, content={ + "invite": [], + "invite_3pid": [{"medium": "email", "address": "foo@example.com"}], + }) + assert channel.result["code"] == b"403", channel.result + def test_normal_user_can_room_with_single_invites(self): channel = self._create_room(self.normal_access_token, content={ "invite": [self.other_user_id],