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

Commit 6e1c64a

Browse files
committed
Merge tag 'v1.19.1rc1' into develop
Synapse 1.19.1rc1 (2020-08-25) ============================== Bugfixes -------- - Fix a bug introduced in v1.19.0 where appservices with ratelimiting disabled would still be ratelimited when joining rooms. ([\#8139](#8139)) - Fix a bug introduced in v1.19.0 that would cause e.g. profile updates to fail due to incorrect application of rate limits on join requests. ([\#8153](#8153))
2 parents 2231dff + 0a4e541 commit 6e1c64a

File tree

5 files changed

+129
-26
lines changed

5 files changed

+129
-26
lines changed

CHANGES.md

+10
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,16 @@ from Synapse as most users have updated their client. Further context can be
1212
found at [\#6766](https://github.com/matrix-org/synapse/issues/6766).
1313

1414

15+
Synapse 1.19.1rc1 (2020-08-25)
16+
==============================
17+
18+
Bugfixes
19+
--------
20+
21+
- Fix a bug introduced in v1.19.0 where appservices with ratelimiting disabled would still be ratelimited when joining rooms. ([\#8139](https://github.com/matrix-org/synapse/issues/8139))
22+
- Fix a bug introduced in v1.19.0 that would cause e.g. profile updates to fail due to incorrect application of rate limits on join requests. ([\#8153](https://github.com/matrix-org/synapse/issues/8153))
23+
24+
1525
Synapse 1.19.0 (2020-08-17)
1626
===========================
1727

synapse/__init__.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@
4848
except ImportError:
4949
pass
5050

51-
__version__ = "1.19.0"
51+
__version__ = "1.19.1rc1"
5252

5353
if bool(os.environ.get("SYNAPSE_TEST_PATCH_LOG_CONTEXTS", False)):
5454
# We import here so that we don't have to install a bunch of deps when

synapse/handlers/room_member.py

+25-21
Original file line numberDiff line numberDiff line change
@@ -225,24 +225,40 @@ async def _local_membership_update(
225225
_, stream_id = await self.store.get_event_ordering(duplicate.event_id)
226226
return duplicate.event_id, stream_id
227227

228-
stream_id = await self.event_creation_handler.handle_new_client_event(
229-
requester, event, context, extra_users=[target], ratelimit=ratelimit,
230-
)
231-
232228
prev_state_ids = await context.get_prev_state_ids()
233229

234230
prev_member_event_id = prev_state_ids.get((EventTypes.Member, user_id), None)
235231

232+
newly_joined = False
236233
if event.membership == Membership.JOIN:
237-
# Only fire user_joined_room if the user has actually joined the
238-
# room. Don't bother if the user is just changing their profile
239-
# info.
240234
newly_joined = True
241235
if prev_member_event_id:
242236
prev_member_event = await self.store.get_event(prev_member_event_id)
243237
newly_joined = prev_member_event.membership != Membership.JOIN
238+
239+
# Only rate-limit if the user actually joined the room, otherwise we'll end
240+
# up blocking profile updates.
244241
if newly_joined:
245-
await self._user_joined_room(target, room_id)
242+
time_now_s = self.clock.time()
243+
(
244+
allowed,
245+
time_allowed,
246+
) = self._join_rate_limiter_local.can_requester_do_action(requester)
247+
248+
if not allowed:
249+
raise LimitExceededError(
250+
retry_after_ms=int(1000 * (time_allowed - time_now_s))
251+
)
252+
253+
stream_id = await self.event_creation_handler.handle_new_client_event(
254+
requester, event, context, extra_users=[target], ratelimit=ratelimit,
255+
)
256+
257+
if event.membership == Membership.JOIN and newly_joined:
258+
# Only fire user_joined_room if the user has actually joined the
259+
# room. Don't bother if the user is just changing their profile
260+
# info.
261+
await self._user_joined_room(target, room_id)
246262
elif event.membership == Membership.LEAVE:
247263
if prev_member_event_id:
248264
prev_member_event = await self.store.get_event(prev_member_event_id)
@@ -497,19 +513,7 @@ async def _update_membership(
497513
# so don't really fit into the general auth process.
498514
raise AuthError(403, "Guest access not allowed")
499515

500-
if is_host_in_room:
501-
time_now_s = self.clock.time()
502-
(
503-
allowed,
504-
time_allowed,
505-
) = self._join_rate_limiter_local.can_requester_do_action(requester,)
506-
507-
if not allowed:
508-
raise LimitExceededError(
509-
retry_after_ms=int(1000 * (time_allowed - time_now_s))
510-
)
511-
512-
else:
516+
if not is_host_in_room:
513517
time_now_s = self.clock.time()
514518
(
515519
allowed,

tests/rest/client/v1/test_rooms.py

+86-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
from synapse.handlers.pagination import PurgeStatus
2929
from synapse.rest.client.v1 import directory, login, profile, room
3030
from synapse.rest.client.v2_alpha import account, room_upgrade_rest_servlet
31-
from synapse.types import JsonDict, RoomAlias
31+
from synapse.types import JsonDict, RoomAlias, UserID
3232
from synapse.util.stringutils import random_string
3333

3434
from tests import unittest
@@ -675,6 +675,91 @@ def test_rooms_members_other_custom_keys(self):
675675
self.assertEquals(json.loads(content), channel.json_body)
676676

677677

678+
class RoomJoinRatelimitTestCase(RoomBase):
679+
user_id = "@sid1:red"
680+
681+
servlets = [
682+
profile.register_servlets,
683+
room.register_servlets,
684+
]
685+
686+
@unittest.override_config(
687+
{"rc_joins": {"local": {"per_second": 3, "burst_count": 3}}}
688+
)
689+
def test_join_local_ratelimit(self):
690+
"""Tests that local joins are actually rate-limited."""
691+
for i in range(5):
692+
self.helper.create_room_as(self.user_id)
693+
694+
self.helper.create_room_as(self.user_id, expect_code=429)
695+
696+
@unittest.override_config(
697+
{"rc_joins": {"local": {"per_second": 3, "burst_count": 3}}}
698+
)
699+
def test_join_local_ratelimit_profile_change(self):
700+
"""Tests that sending a profile update into all of the user's joined rooms isn't
701+
rate-limited by the rate-limiter on joins."""
702+
703+
# Create and join more rooms than the rate-limiting config allows in a second.
704+
room_ids = [
705+
self.helper.create_room_as(self.user_id),
706+
self.helper.create_room_as(self.user_id),
707+
self.helper.create_room_as(self.user_id),
708+
]
709+
self.reactor.advance(1)
710+
room_ids = room_ids + [
711+
self.helper.create_room_as(self.user_id),
712+
self.helper.create_room_as(self.user_id),
713+
self.helper.create_room_as(self.user_id),
714+
]
715+
716+
# Create a profile for the user, since it hasn't been done on registration.
717+
store = self.hs.get_datastore()
718+
store.create_profile(UserID.from_string(self.user_id).localpart)
719+
720+
# Update the display name for the user.
721+
path = "/_matrix/client/r0/profile/%s/displayname" % self.user_id
722+
request, channel = self.make_request("PUT", path, {"displayname": "John Doe"})
723+
self.render(request)
724+
self.assertEquals(channel.code, 200, channel.json_body)
725+
726+
# Check that all the rooms have been sent a profile update into.
727+
for room_id in room_ids:
728+
path = "/_matrix/client/r0/rooms/%s/state/m.room.member/%s" % (
729+
room_id,
730+
self.user_id,
731+
)
732+
733+
request, channel = self.make_request("GET", path)
734+
self.render(request)
735+
self.assertEquals(channel.code, 200)
736+
737+
self.assertIn("displayname", channel.json_body)
738+
self.assertEquals(channel.json_body["displayname"], "John Doe")
739+
740+
@unittest.override_config(
741+
{"rc_joins": {"local": {"per_second": 3, "burst_count": 3}}}
742+
)
743+
def test_join_local_ratelimit_idempotent(self):
744+
"""Tests that the room join endpoints remain idempotent despite rate-limiting
745+
on room joins."""
746+
room_id = self.helper.create_room_as(self.user_id)
747+
748+
# Let's test both paths to be sure.
749+
paths_to_test = [
750+
"/_matrix/client/r0/rooms/%s/join",
751+
"/_matrix/client/r0/join/%s",
752+
]
753+
754+
for path in paths_to_test:
755+
# Make sure we send more requests than the rate-limiting config would allow
756+
# if all of these requests ended up joining the user to a room.
757+
for i in range(6):
758+
request, channel = self.make_request("POST", path % room_id, {})
759+
self.render(request)
760+
self.assertEquals(channel.code, 200)
761+
762+
678763
class RoomMessagesTestCase(RoomBase):
679764
""" Tests /rooms/$room_id/messages/$user_id/$msg_id REST events. """
680765

tests/rest/client/v1/utils.py

+7-3
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@ class RestHelper(object):
3939
resource = attr.ib()
4040
auth_user_id = attr.ib()
4141

42-
def create_room_as(self, room_creator=None, is_public=True, tok=None):
42+
def create_room_as(
43+
self, room_creator=None, is_public=True, tok=None, expect_code=200,
44+
):
4345
temp_id = self.auth_user_id
4446
self.auth_user_id = room_creator
4547
path = "/_matrix/client/r0/createRoom"
@@ -54,9 +56,11 @@ def create_room_as(self, room_creator=None, is_public=True, tok=None):
5456
)
5557
render(request, self.resource, self.hs.get_reactor())
5658

57-
assert channel.result["code"] == b"200", channel.result
59+
assert channel.result["code"] == b"%d" % expect_code, channel.result
5860
self.auth_user_id = temp_id
59-
return channel.json_body["room_id"]
61+
62+
if expect_code == 200:
63+
return channel.json_body["room_id"]
6064

6165
def invite(self, room=None, src=None, targ=None, expect_code=200, tok=None):
6266
self.change_membership(

0 commit comments

Comments
 (0)