From 2477f1f7aa8db44a8a74d622dcea31ba9ae85a37 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 5 Sep 2019 16:25:37 +0100 Subject: Add tests --- tests/rest/client/test_room_access_rules.py | 105 ++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) (limited to 'tests/rest/client/test_room_access_rules.py') diff --git a/tests/rest/client/test_room_access_rules.py b/tests/rest/client/test_room_access_rules.py index 7e23add6b7..28e07b3928 100644 --- a/tests/rest/client/test_room_access_rules.py +++ b/tests/rest/client/test_room_access_rules.py @@ -483,6 +483,111 @@ class RoomAccessTestCase(unittest.HomeserverTestCase): expected_code=403, ) + def test_change_room_avatar(self): + """Tests that changing the room avatar is always allowed unless the room is a + direct chat, in which case it's forbidden. + """ + + avatar_content = { + "info": { + "h": 398, + "mimetype": "image/jpeg", + "size": 31037, + "w": 394 + }, + "url": "mxc://example.org/JWEIFJgwEIhweiWJE", + } + + self.helper.send_state( + room_id=self.restricted_room, + event_type=EventTypes.RoomAvatar, + body=avatar_content, + tok=self.tok, + expect_code=200, + ) + + self.helper.send_state( + room_id=self.unrestricted_room, + event_type=EventTypes.RoomAvatar, + body=avatar_content, + tok=self.tok, + expect_code=200, + ) + + self.helper.send_state( + room_id=self.direct_rooms[0], + event_type=EventTypes.RoomAvatar, + body=avatar_content, + tok=self.tok, + expect_code=403, + ) + + def test_change_room_name(self): + """Tests that changing the room name is always allowed unless the room is a direct + chat, in which case it's forbidden. + """ + + name_content = { + "name": "My super room", + } + + self.helper.send_state( + room_id=self.restricted_room, + event_type=EventTypes.Name, + body=name_content, + tok=self.tok, + expect_code=200, + ) + + self.helper.send_state( + room_id=self.unrestricted_room, + event_type=EventTypes.Name, + body=name_content, + tok=self.tok, + expect_code=200, + ) + + self.helper.send_state( + room_id=self.direct_rooms[0], + event_type=EventTypes.Name, + body=name_content, + tok=self.tok, + expect_code=403, + ) + + def test_change_room_topic(self): + """Tests that changing the room topic is always allowed unless the room is a + direct chat, in which case it's forbidden. + """ + + topic_content = { + "topic": "Welcome to this room", + } + + self.helper.send_state( + room_id=self.restricted_room, + event_type=EventTypes.Topic, + body=topic_content, + tok=self.tok, + expect_code=200, + ) + + self.helper.send_state( + room_id=self.unrestricted_room, + event_type=EventTypes.Topic, + body=topic_content, + tok=self.tok, + expect_code=200, + ) + + self.helper.send_state( + room_id=self.direct_rooms[0], + event_type=EventTypes.Topic, + body=topic_content, + tok=self.tok, + expect_code=403, + ) + def create_room( self, direct=False, rule=None, preset=RoomCreationPreset.TRUSTED_PRIVATE_CHAT, initial_state=None, expected_code=200, -- cgit 1.5.1 From b15557cd466c30281c92436c2d5a05b9680c1786 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 6 Sep 2019 15:32:11 +0100 Subject: Don't treat 3PID revokation as a new 3PID invite --- synapse/third_party_rules/access_rules.py | 26 ++++++++++---- tests/rest/client/test_room_access_rules.py | 55 +++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 7 deletions(-) (limited to 'tests/rest/client/test_room_access_rules.py') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 1a295ea7ce..5698e3e062 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -265,7 +265,7 @@ class RoomAccessRules(object): # Make sure we don't apply "direct" if the room has more than two members. if new_rule == ACCESS_RULE_DIRECT: existing_members, threepid_tokens = self._get_members_and_tokens_from_state( - state_events, + state_events, event ) if len(existing_members) > 2 or len(threepid_tokens) > 1: @@ -356,7 +356,7 @@ class RoomAccessRules(object): """ # Get the room memberships and 3PID invite tokens from the room's state. existing_members, threepid_tokens = self._get_members_and_tokens_from_state( - state_events, + state_events, event ) # There should never be more than one 3PID invite in the room state: if the second @@ -494,13 +494,14 @@ class RoomAccessRules(object): return join_rule_event.content.get("join_rule") @staticmethod - def _get_members_and_tokens_from_state(state_events): + def _get_members_and_tokens_from_state(state_events, event): """Retrieves from a list of state events the list of users that have a m.room.member event in the room, and the tokens of 3PID invites in the room. Args: state_events (dict[tuple[event type, state key], EventBase]): The set of state events. + event (EventBase): The event being checked. Returns: existing_members (list[str]): List of targets of the m.room.member events in the state. @@ -509,13 +510,24 @@ class RoomAccessRules(object): """ existing_members = [] threepid_invite_tokens = [] - for key, event in state_events.items(): + for key, state_event in state_events.items(): if key[0] == EventTypes.Member: - existing_members.append(event.state_key) + existing_members.append(state_event.state_key) if key[0] == EventTypes.ThirdPartyInvite: - threepid_invite_tokens.append(event.state_key) + threepid_invite_tokens.append(state_event.state_key) - return existing_members, threepid_invite_tokens + # If the event is a state event, there already is an event with the same state key + # in the room's state, then the event is updating an existing event from the + # room's state, in which case we need to remove the entry from the list in order + # to avoid conflicts. + if event.is_state(): + def filter_out_event(state_key): + return event.state_key != state_key + + existing_members = filter(filter_out_event, existing_members) + threepid_invite_tokens = filter(filter_out_event, threepid_invite_tokens) + + return list(existing_members), list(threepid_invite_tokens) @staticmethod def _is_invite_from_threepid(invite, threepid_invite_token): diff --git a/tests/rest/client/test_room_access_rules.py b/tests/rest/client/test_room_access_rules.py index 7e23add6b7..bb164c1e5e 100644 --- a/tests/rest/client/test_room_access_rules.py +++ b/tests/rest/client/test_room_access_rules.py @@ -483,6 +483,44 @@ class RoomAccessTestCase(unittest.HomeserverTestCase): expected_code=403, ) + def test_revoke_3pid_invite_direct(self): + """Tests that revoking a 3PID invite doesn't cause the room access rules module to + confuse the revokation as a new 3PID invite. + """ + invite_token = "sometoken" + + invite_body = { + "display_name": "ker...@exa...", + "public_keys": [ + { + "key_validity_url": "https://validity_url", + "public_key": "ta8IQ0u1sp44HVpxYi7dFOdS/bfwDjcy4xLFlfY5KOA" + }, + { + "key_validity_url": "https://validity_url", + "public_key": "4_9nzEeDwR5N9s51jPodBiLnqH43A2_g2InVT137t9I" + } + ], + "key_validity_url": "https://validity_url", + "public_key": "ta8IQ0u1sp44HVpxYi7dFOdS/bfwDjcy4xLFlfY5KOA" + } + + self.send_state_with_state_key( + room_id=self.direct_rooms[1], + event_type=EventTypes.ThirdPartyInvite, + state_key=invite_token, + body=invite_body, + tok=self.tok, + ) + + self.send_state_with_state_key( + room_id=self.direct_rooms[1], + event_type=EventTypes.ThirdPartyInvite, + state_key=invite_token, + body={}, + tok=self.tok, + ) + def create_room( self, direct=False, rule=None, preset=RoomCreationPreset.TRUSTED_PRIVATE_CHAT, initial_state=None, expected_code=200, @@ -574,3 +612,20 @@ class RoomAccessTestCase(unittest.HomeserverTestCase): ) self.render(request) self.assertEqual(channel.code, expected_code, channel.result) + + def send_state_with_state_key( + self, room_id, event_type, state_key, body, tok, expect_code=200 + ): + path = "/_matrix/client/r0/rooms/%s/state/%s/%s" % ( + room_id, event_type, state_key + ) + + request, channel = self.make_request( + "PUT", path, json.dumps(body), access_token=tok + ) + self.render(request) + + self.assertEqual(channel.code, expect_code, channel.result) + + return channel.json_body + -- cgit 1.5.1 From b2ec4467c942975dea8d69a0bec76d88e344f54d Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 6 Sep 2019 15:36:43 +0100 Subject: Don't process revoked/redacted events as part of the room's membership info --- synapse/third_party_rules/access_rules.py | 4 ++-- tests/rest/client/test_room_access_rules.py | 10 ++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) (limited to 'tests/rest/client/test_room_access_rules.py') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 5698e3e062..ac09807738 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -511,9 +511,9 @@ class RoomAccessRules(object): existing_members = [] threepid_invite_tokens = [] for key, state_event in state_events.items(): - if key[0] == EventTypes.Member: + if key[0] == EventTypes.Member and state_event.content: existing_members.append(state_event.state_key) - if key[0] == EventTypes.ThirdPartyInvite: + if key[0] == EventTypes.ThirdPartyInvite and state_event.content: threepid_invite_tokens.append(state_event.state_key) # If the event is a state event, there already is an event with the same state key diff --git a/tests/rest/client/test_room_access_rules.py b/tests/rest/client/test_room_access_rules.py index bb164c1e5e..8fa828fa15 100644 --- a/tests/rest/client/test_room_access_rules.py +++ b/tests/rest/client/test_room_access_rules.py @@ -521,6 +521,16 @@ class RoomAccessTestCase(unittest.HomeserverTestCase): tok=self.tok, ) + invite_token = "someothertoken" + + self.send_state_with_state_key( + room_id=self.direct_rooms[1], + event_type=EventTypes.ThirdPartyInvite, + state_key=invite_token, + body=invite_body, + tok=self.tok, + ) + def create_room( self, direct=False, rule=None, preset=RoomCreationPreset.TRUSTED_PRIVATE_CHAT, initial_state=None, expected_code=200, -- cgit 1.5.1 From e35c30ed4bb132cb20132c31cdabaebfce1b69bb Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 6 Sep 2019 15:44:48 +0100 Subject: Fix bogus conflict resolution --- tests/rest/client/test_room_access_rules.py | 1 + 1 file changed, 1 insertion(+) (limited to 'tests/rest/client/test_room_access_rules.py') diff --git a/tests/rest/client/test_room_access_rules.py b/tests/rest/client/test_room_access_rules.py index 8da56c29dd..ab76e0f25a 100644 --- a/tests/rest/client/test_room_access_rules.py +++ b/tests/rest/client/test_room_access_rules.py @@ -586,6 +586,7 @@ class RoomAccessTestCase(unittest.HomeserverTestCase): body=topic_content, tok=self.tok, expect_code=403, + ) def test_revoke_3pid_invite_direct(self): """Tests that revoking a 3PID invite doesn't cause the room access rules module to -- cgit 1.5.1 From 6a78a0ce9b843ba779042a3a262283bd6d7938a0 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 6 Sep 2019 15:51:52 +0100 Subject: Lint --- tests/rest/client/test_room_access_rules.py | 1 - 1 file changed, 1 deletion(-) (limited to 'tests/rest/client/test_room_access_rules.py') diff --git a/tests/rest/client/test_room_access_rules.py b/tests/rest/client/test_room_access_rules.py index ab76e0f25a..7d3ba0ee2a 100644 --- a/tests/rest/client/test_room_access_rules.py +++ b/tests/rest/client/test_room_access_rules.py @@ -743,4 +743,3 @@ class RoomAccessTestCase(unittest.HomeserverTestCase): self.assertEqual(channel.code, expect_code, channel.result) return channel.json_body - -- cgit 1.5.1 From d6b7606e07a83c9bcea4e1bff49fae4b1a25de35 Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Tue, 12 Nov 2019 11:38:35 +0000 Subject: Create configurable ratelimiter for 3pid invites (#11) --- changelog.d/11.feature | 1 + docs/sample_config.yaml | 6 ++++++ synapse/config/ratelimiting.py | 9 +++++++++ synapse/handlers/room_member.py | 16 ++++++++-------- tests/rest/client/test_room_access_rules.py | 9 +++++++++ 5 files changed, 33 insertions(+), 8 deletions(-) create mode 100644 changelog.d/11.feature (limited to 'tests/rest/client/test_room_access_rules.py') diff --git a/changelog.d/11.feature b/changelog.d/11.feature new file mode 100644 index 0000000000..362e4b1efd --- /dev/null +++ b/changelog.d/11.feature @@ -0,0 +1 @@ +Allow server admins to configure a custom global rate-limiting for third party invites. \ No newline at end of file diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 63051dd56f..b4713b687e 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -568,6 +568,8 @@ log_config: "CONFDIR/SERVERNAME.log.config" # - one for login that ratelimits login requests based on the account the # client is attempting to log into, based on the amount of failed login # attempts for this account. +# - one that ratelimits third-party invites requests based on the account +# that's making the requests. # # The defaults are as shown below. # @@ -589,6 +591,10 @@ log_config: "CONFDIR/SERVERNAME.log.config" # failed_attempts: # per_second: 0.17 # burst_count: 3 +# +#rc_third_party_invite: +# per_second: 0.2 +# burst_count: 10 # Ratelimiting settings for incoming federation diff --git a/synapse/config/ratelimiting.py b/synapse/config/ratelimiting.py index 5a9adac480..2a4fe43406 100644 --- a/synapse/config/ratelimiting.py +++ b/synapse/config/ratelimiting.py @@ -68,6 +68,9 @@ class RatelimitConfig(Config): ) self.rc_registration = RateLimitConfig(config.get("rc_registration", {})) + self.rc_third_party_invite = RateLimitConfig( + config.get("rc_third_party_invite", {}) + ) rc_login_config = config.get("rc_login", {}) self.rc_login_address = RateLimitConfig(rc_login_config.get("address", {})) @@ -102,6 +105,8 @@ class RatelimitConfig(Config): # - one for login that ratelimits login requests based on the account the # client is attempting to log into, based on the amount of failed login # attempts for this account. + # - one that ratelimits third-party invites requests based on the account + # that's making the requests. # # The defaults are as shown below. # @@ -123,6 +128,10 @@ class RatelimitConfig(Config): # failed_attempts: # per_second: 0.17 # burst_count: 3 + # + #rc_third_party_invite: + # per_second: 0.2 + # burst_count: 10 # Ratelimiting settings for incoming federation diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index e940e4183b..790aeba9f5 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -26,12 +26,11 @@ import synapse.server import synapse.types from synapse.api.constants import EventTypes, Membership from synapse.api.errors import AuthError, Codes, ProxiedRequestError, SynapseError +from synapse.api.ratelimiting import Ratelimiter from synapse.types import RoomID, UserID from synapse.util.async_helpers import Linearizer from synapse.util.distributor import user_joined_room, user_left_room -from ._base import BaseHandler - logger = logging.getLogger(__name__) id_server_scheme = "https://" @@ -74,11 +73,7 @@ class RoomMemberHandler(object): self.rewrite_identity_server_urls = self.config.rewrite_identity_server_urls self._enable_lookup = hs.config.enable_3pid_lookup self.allow_per_room_profiles = self.config.allow_per_room_profiles - - # This is only used to get at ratelimit function, and - # maybe_kick_guest_users. It's fine there are multiple of these as - # it doesn't store state. - self.base_handler = BaseHandler(hs) + self.ratelimiter = Ratelimiter() @abc.abstractmethod def _remote_join(self, requester, remote_room_hosts, room_id, user, content): @@ -773,7 +768,12 @@ class RoomMemberHandler(object): # We need to rate limit *before* we send out any 3PID invites, so we # can't just rely on the standard ratelimiting of events. - yield self.base_handler.ratelimit(requester) + self.ratelimiter.ratelimit( + requester.user.to_string(), time_now_s=self.hs.clock.time(), + rate_hz=self.hs.config.rc_third_party_invite.per_second, + burst_count=self.hs.config.rc_third_party_invite.burst_count, + update=True, + ) can_invite = yield self.third_party_event_rules.check_threepid_can_be_invited( medium, address, room_id, diff --git a/tests/rest/client/test_room_access_rules.py b/tests/rest/client/test_room_access_rules.py index 7d3ba0ee2a..13caea3b01 100644 --- a/tests/rest/client/test_room_access_rules.py +++ b/tests/rest/client/test_room_access_rules.py @@ -326,6 +326,12 @@ class RoomAccessTestCase(unittest.HomeserverTestCase): expect_code=200, ) + # Disable the 3pid invite ratelimiter + burst = self.hs.config.rc_third_party_invite.burst_count + per_second = self.hs.config.rc_third_party_invite.per_second + self.hs.config.rc_third_party_invite.burst_count = 10 + self.hs.config.rc_third_party_invite.per_second = 0.1 + # We can't send a 3PID invite to a room that already has two members. self.send_threepid_invite( address="test@allowed_domain", @@ -354,6 +360,9 @@ class RoomAccessTestCase(unittest.HomeserverTestCase): expected_code=403, ) + self.hs.config.rc_third_party_invite.burst_count = burst + self.hs.config.rc_third_party_invite.per_second = per_second + def test_unrestricted(self): """Tests that, in unrestricted mode, we can invite whoever we want, but we can only change the power level of users that wouldn't be forbidden in restricted -- cgit 1.5.1