Honour ratelimit flag for application services for invite ratelimiting (#9302)

4 files changed, 13 insertions, 51 deletions

diff --git a/changelog.d/9302.bugfix b/changelog.d/9302.bugfix
new file mode 100644
index 0000000000..c1cdea52a3
--- /dev/null
+++ b/changelog.d/9302.bugfix
@@ -0,0 +1 @@
+Fix new ratelimiting for invites to respect the `ratelimit` flag on application services. Introduced in v1.27.0rc1.
diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py
index dbdfd56ff5..eddc7582d0 100644
--- a/synapse/handlers/federation.py
+++ b/synapse/handlers/federation.py
@@ -1619,7 +1619,9 @@ class FederationHandler(BaseHandler):
 
         # We retrieve the room member handler here as to not cause a cyclic dependency
         member_handler = self.hs.get_room_member_handler()
-        member_handler.ratelimit_invite(event.room_id, event.state_key)
+        # We don't rate limit based on room ID, as that should be done by
+        # sending server.
+        member_handler.ratelimit_invite(None, event.state_key)
 
         # keep a record of the room version, if we don't yet know it.
         # (this may get overwritten if we later get a different room version in a
diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py
index d335da6f19..a5da97cfe0 100644
--- a/synapse/handlers/room_member.py
+++ b/synapse/handlers/room_member.py
@@ -155,10 +155,14 @@ class RoomMemberHandler(metaclass=abc.ABCMeta):
         """
         raise NotImplementedError()
 
-    def ratelimit_invite(self, room_id: str, invitee_user_id: str):
+    def ratelimit_invite(self, room_id: Optional[str], invitee_user_id: str):
         """Ratelimit invites by room and by target user.
+
+        If room ID is missing then we just rate limit by target user.
         """
-        self._invites_per_room_limiter.ratelimit(room_id)
+        if room_id:
+            self._invites_per_room_limiter.ratelimit(room_id)
+
         self._invites_per_user_limiter.ratelimit(invitee_user_id)
 
     async def _local_membership_update(
@@ -406,7 +410,9 @@ class RoomMemberHandler(metaclass=abc.ABCMeta):
         if effective_membership_state == Membership.INVITE:
             target_id = target.to_string()
             if ratelimit:
-                self.ratelimit_invite(room_id, target_id)
+                # Don't ratelimit application services.
+                if not requester.app_service or requester.app_service.is_rate_limited():
+                    self.ratelimit_invite(room_id, target_id)
 
             # block any attempts to invite the server notices mxid
             if target_id == self._server_notices_mxid:
diff --git a/tests/handlers/test_federation.py b/tests/handlers/test_federation.py
index 74503112f5..983e368592 100644
--- a/tests/handlers/test_federation.py
+++ b/tests/handlers/test_federation.py
@@ -192,53 +192,6 @@ class FederationTestCase(unittest.HomeserverTestCase):
         self.assertEqual(sg, sg2)
 
     @unittest.override_config(
-        {"rc_invites": {"per_room": {"per_second": 0.5, "burst_count": 3}}}
-    )
-    def test_invite_by_room_ratelimit(self):
-        """Tests that invites from federation in a room are actually rate-limited.
-        """
-        other_server = "otherserver"
-        other_user = "@otheruser:" + other_server
-
-        # create the room
-        user_id = self.register_user("kermit", "test")
-        tok = self.login("kermit", "test")
-        room_id = self.helper.create_room_as(room_creator=user_id, tok=tok)
-        room_version = self.get_success(self.store.get_room_version(room_id))
-
-        def create_invite_for(local_user):
-            return event_from_pdu_json(
-                {
-                    "type": EventTypes.Member,
-                    "content": {"membership": "invite"},
-                    "room_id": room_id,
-                    "sender": other_user,
-                    "state_key": local_user,
-                    "depth": 32,
-                    "prev_events": [],
-                    "auth_events": [],
-                    "origin_server_ts": self.clock.time_msec(),
-                },
-                room_version,
-            )
-
-        for i in range(3):
-            self.get_success(
-                self.handler.on_invite_request(
-                    other_server,
-                    create_invite_for("@user-%d:test" % (i,)),
-                    room_version,
-                )
-            )
-
-        self.get_failure(
-            self.handler.on_invite_request(
-                other_server, create_invite_for("@user-4:test"), room_version,
-            ),
-            exc=LimitExceededError,
-        )
-
-    @unittest.override_config(
         {"rc_invites": {"per_user": {"per_second": 0.5, "burst_count": 3}}}
     )
     def test_invite_by_user_ratelimit(self):