Merge pull request #621 from matrix-org/daniel/ratelimiting

Pass whole requester to ratelimiting

synapse/handlers/_base.py

@@ -160,10 +160,10 @@ class BaseHandler(object):
         )
         defer.returnValue(res.get(user_id, []))
 
-    def ratelimit(self, user_id):
+    def ratelimit(self, requester):
         time_now = self.clock.time()
         allowed, time_allowed = self.ratelimiter.send_message(
-            user_id, time_now,
+            requester.user.to_string(), time_now,
             msg_rate_hz=self.hs.config.rc_messages_per_second,
             burst_count=self.hs.config.rc_message_burst_count,
         )
@@ -263,11 +263,18 @@ class BaseHandler(object):
         return False
 
     @defer.inlineCallbacks
-    def handle_new_client_event(self, event, context, ratelimit=True, extra_users=[]):
+    def handle_new_client_event(
+        self,
+        requester,
+        event,
+        context,
+        ratelimit=True,
+        extra_users=[]
+    ):
         # We now need to go and hit out to wherever we need to hit out to.
 
         if ratelimit:
-            self.ratelimit(event.sender)
+            self.ratelimit(requester)
 
         self.auth.check(event, auth_events=context.current_state)
 

synapse/handlers/directory.py

@@ -212,17 +212,21 @@ class DirectoryHandler(BaseHandler):
             )
 
     @defer.inlineCallbacks
-    def send_room_alias_update_event(self, user_id, room_id):
+    def send_room_alias_update_event(self, requester, user_id, room_id):
         aliases = yield self.store.get_aliases_for_room(room_id)
 
         msg_handler = self.hs.get_handlers().message_handler
-        yield msg_handler.create_and_send_nonmember_event({
+        yield msg_handler.create_and_send_nonmember_event(
+            requester,
+            {
                 "type": EventTypes.Aliases,
                 "state_key": self.hs.hostname,
                 "room_id": room_id,
                 "sender": user_id,
                 "content": {"aliases": aliases},
-        }, ratelimit=False)
+            },
+            ratelimit=False
+        )
 
     @defer.inlineCallbacks
     def get_association_from_room_alias(self, room_alias):

synapse/handlers/federation.py

@@ -1657,7 +1657,7 @@ class FederationHandler(BaseHandler):
             self.auth.check(event, context.current_state)
             yield self._check_signature(event, auth_events=context.current_state)
             member_handler = self.hs.get_handlers().room_member_handler
-            yield member_handler.send_membership_event(event, context, from_client=False)
+            yield member_handler.send_membership_event(None, event, context)
         else:
             destinations = set(x.split(":", 1)[-1] for x in (sender_user_id, room_id))
             yield self.replication_layer.forward_third_party_invite(
@@ -1686,7 +1686,7 @@ class FederationHandler(BaseHandler):
         # TODO: Make sure the signatures actually are correct.
         event.signatures.update(returned_invite.signatures)
         member_handler = self.hs.get_handlers().room_member_handler
-        yield member_handler.send_membership_event(event, context, from_client=False)
+        yield member_handler.send_membership_event(None, event, context)
 
     @defer.inlineCallbacks
     def add_display_name_to_third_party_invite(self, event_dict, event, context):

synapse/handlers/message.py

@@ -215,7 +215,7 @@ class MessageHandler(BaseHandler):
         defer.returnValue((event, context))
 
     @defer.inlineCallbacks
-    def send_nonmember_event(self, event, context, ratelimit=True):
+    def send_nonmember_event(self, requester, event, context, ratelimit=True):
         """
         Persists and notifies local clients and federation of an event.
 
@@ -241,6 +241,7 @@ class MessageHandler(BaseHandler):
                 defer.returnValue(prev_state)
 
         yield self.handle_new_client_event(
+            requester=requester,
             event=event,
             context=context,
             ratelimit=ratelimit,
@@ -268,9 +269,9 @@ class MessageHandler(BaseHandler):
     @defer.inlineCallbacks
     def create_and_send_nonmember_event(
         self,
+        requester,
         event_dict,
         ratelimit=True,
-        token_id=None,
         txn_id=None
     ):
         """
@@ -280,10 +281,11 @@ class MessageHandler(BaseHandler):
         """
         event, context = yield self.create_event(
             event_dict,
-            token_id=token_id,
+            token_id=requester.access_token_id,
             txn_id=txn_id
         )
         yield self.send_nonmember_event(
+            requester,
             event,
             context,
             ratelimit=ratelimit,

synapse/handlers/profile.py

@@ -89,13 +89,13 @@ class ProfileHandler(BaseHandler):
                 defer.returnValue(result["displayname"])
 
     @defer.inlineCallbacks
-    def set_displayname(self, target_user, auth_user, new_displayname):
+    def set_displayname(self, target_user, requester, new_displayname):
         """target_user is the user whose displayname is to be changed;
         auth_user is the user attempting to make this change."""
         if not self.hs.is_mine(target_user):
             raise SynapseError(400, "User is not hosted on this Home Server")
 
-        if target_user != auth_user:
+        if target_user != requester.user:
             raise AuthError(400, "Cannot set another user's displayname")
 
         if new_displayname == '':
@@ -109,7 +109,7 @@ class ProfileHandler(BaseHandler):
             "displayname": new_displayname,
         })
 
-        yield self._update_join_states(target_user)
+        yield self._update_join_states(requester)
 
     @defer.inlineCallbacks
     def get_avatar_url(self, target_user):
@@ -139,13 +139,13 @@ class ProfileHandler(BaseHandler):
             defer.returnValue(result["avatar_url"])
 
     @defer.inlineCallbacks
-    def set_avatar_url(self, target_user, auth_user, new_avatar_url):
+    def set_avatar_url(self, target_user, requester, new_avatar_url):
         """target_user is the user whose avatar_url is to be changed;
         auth_user is the user attempting to make this change."""
         if not self.hs.is_mine(target_user):
             raise SynapseError(400, "User is not hosted on this Home Server")
 
-        if target_user != auth_user:
+        if target_user != requester.user:
             raise AuthError(400, "Cannot set another user's avatar_url")
 
         yield self.store.set_profile_avatar_url(
@@ -156,7 +156,7 @@ class ProfileHandler(BaseHandler):
             "avatar_url": new_avatar_url,
         })
 
-        yield self._update_join_states(target_user)
+        yield self._update_join_states(requester)
 
     @defer.inlineCallbacks
     def collect_presencelike_data(self, user, state):
@@ -199,11 +199,12 @@ class ProfileHandler(BaseHandler):
         defer.returnValue(response)
 
     @defer.inlineCallbacks
-    def _update_join_states(self, user):
+    def _update_join_states(self, requester):
+        user = requester.user
         if not self.hs.is_mine(user):
             return
 
-        self.ratelimit(user.to_string())
+        self.ratelimit(requester)
 
         joins = yield self.store.get_rooms_for_user(
             user.to_string(),

synapse/handlers/room.py

@@ -18,7 +18,7 @@ from twisted.internet import defer
 
 from ._base import BaseHandler
 
-from synapse.types import UserID, RoomAlias, RoomID, RoomStreamToken
+from synapse.types import UserID, RoomAlias, RoomID, RoomStreamToken, Requester
 from synapse.api.constants import (
     EventTypes, Membership, JoinRules, RoomCreationPreset,
 )
@@ -90,7 +90,7 @@ class RoomCreationHandler(BaseHandler):
         """
         user_id = requester.user.to_string()
 
-        self.ratelimit(user_id)
+        self.ratelimit(requester)
 
         if "room_alias_name" in config:
             for wchar in string.whitespace:
@@ -185,23 +185,29 @@ class RoomCreationHandler(BaseHandler):
 
         if "name" in config:
             name = config["name"]
-            yield msg_handler.create_and_send_nonmember_event({
+            yield msg_handler.create_and_send_nonmember_event(
+                requester,
+                {
                     "type": EventTypes.Name,
                     "room_id": room_id,
                     "sender": user_id,
                     "state_key": "",
                     "content": {"name": name},
-            }, ratelimit=False)
+                },
+                ratelimit=False)
 
         if "topic" in config:
             topic = config["topic"]
-            yield msg_handler.create_and_send_nonmember_event({
+            yield msg_handler.create_and_send_nonmember_event(
+                requester,
+                {
                     "type": EventTypes.Topic,
                     "room_id": room_id,
                     "sender": user_id,
                     "state_key": "",
                     "content": {"topic": topic},
-            }, ratelimit=False)
+                },
+                ratelimit=False)
 
         for invitee in invite_list:
             room_member_handler.update_membership(
@@ -231,7 +237,7 @@ class RoomCreationHandler(BaseHandler):
         if room_alias:
             result["room_alias"] = room_alias.to_string()
             yield directory_handler.send_room_alias_update_event(
-                user_id, room_id
+                requester, user_id, room_id
             )
 
         defer.returnValue(result)
@@ -263,7 +269,11 @@ class RoomCreationHandler(BaseHandler):
         @defer.inlineCallbacks
         def send(etype, content, **kwargs):
             event = create(etype, content, **kwargs)
-            yield msg_handler.create_and_send_nonmember_event(event, ratelimit=False)
+            yield msg_handler.create_and_send_nonmember_event(
+                creator,
+                event,
+                ratelimit=False
+            )
 
         config = RoomCreationHandler.PRESETS_DICT[preset_config]
 
@@ -454,12 +464,11 @@ class RoomMemberHandler(BaseHandler):
 
         member_handler = self.hs.get_handlers().room_member_handler
         yield member_handler.send_membership_event(
+            requester,
             event,
             context,
-            is_guest=requester.is_guest,
             ratelimit=ratelimit,
             remote_room_hosts=remote_room_hosts,
-            from_client=True,
         )
 
         if action == "forget":
@@ -468,17 +477,19 @@ class RoomMemberHandler(BaseHandler):
     @defer.inlineCallbacks
     def send_membership_event(
             self,
+            requester,
             event,
             context,
-            is_guest=False,
             remote_room_hosts=None,
             ratelimit=True,
-            from_client=True,
     ):
         """
         Change the membership status of a user in a room.
 
         Args:
+            requester (Requester): The local user who requested the membership
+                event. If None, certain checks, like whether this homeserver can
+                act as the sender, will be skipped.
             event (SynapseEvent): The membership event.
             context: The context of the event.
             is_guest (bool): Whether the sender is a guest.
@@ -486,19 +497,21 @@ class RoomMemberHandler(BaseHandler):
                 the room, and could be danced with in order to join this
                 homeserver for the first time.
             ratelimit (bool): Whether to rate limit this request.
-            from_client (bool): Whether this request is the result of a local
-                client request (rather than over federation). If so, we will
-                perform extra checks, like that this homeserver can act as this
-                client.
         Raises:
             SynapseError if there was a problem changing the membership.
         """
         target_user = UserID.from_string(event.state_key)
         room_id = event.room_id
 
-        if from_client:
+        if requester is not None:
             sender = UserID.from_string(event.sender)
+            assert sender == requester.user, (
+                "Sender (%s) must be same as requester (%s)" %
+                (sender, requester.user)
+            )
             assert self.hs.is_mine(sender), "Sender must be our own: %s" % (sender,)
+        else:
+            requester = Requester(target_user, None, False)
 
         message_handler = self.hs.get_handlers().message_handler
         prev_event = message_handler.deduplicate_state_event(event, context)
@@ -508,7 +521,7 @@ class RoomMemberHandler(BaseHandler):
         action = "send"
 
         if event.membership == Membership.JOIN:
-            if is_guest and not self._can_guest_join(context.current_state):
+            if requester.is_guest and not self._can_guest_join(context.current_state):
                 # This should be an auth check, but guests are a local concept,
                 # so don't really fit into the general auth process.
                 raise AuthError(403, "Guest access not allowed")
@@ -551,6 +564,7 @@ class RoomMemberHandler(BaseHandler):
             )
         else:
             yield self.handle_new_client_event(
+                requester,
                 event,
                 context,
                 extra_users=[target_user],
@@ -669,12 +683,12 @@ class RoomMemberHandler(BaseHandler):
             )
         else:
             yield self._make_and_store_3pid_invite(
+                requester,
                 id_server,
                 medium,
                 address,
                 room_id,
                 inviter,
-                requester.access_token_id,
                 txn_id=txn_id
             )
 
@@ -732,12 +746,12 @@ class RoomMemberHandler(BaseHandler):
     @defer.inlineCallbacks
     def _make_and_store_3pid_invite(
             self,
+            requester,
             id_server,
             medium,
             address,
             room_id,
             user,
-            token_id,
             txn_id
     ):
         room_state = yield self.hs.get_state_handler().get_current_state(room_id)
@@ -787,6 +801,7 @@ class RoomMemberHandler(BaseHandler):
 
         msg_handler = self.hs.get_handlers().message_handler
         yield msg_handler.create_and_send_nonmember_event(
+            requester,
             {
                 "type": EventTypes.ThirdPartyInvite,
                 "content": {
@@ -801,7 +816,6 @@ class RoomMemberHandler(BaseHandler):
                 "sender": user.to_string(),
                 "state_key": token,
             },
-            token_id=token_id,
             txn_id=txn_id,
         )
 

synapse/rest/client/v1/directory.py

@@ -75,7 +75,11 @@ class ClientDirectoryServer(ClientV1RestServlet):
                 yield dir_handler.create_association(
                     user_id, room_alias, room_id, servers
                 )
-                yield dir_handler.send_room_alias_update_event(user_id, room_id)
+                yield dir_handler.send_room_alias_update_event(
+                    requester,
+                    user_id,
+                    room_id
+                )
             except SynapseError as e:
                 raise e
             except:

synapse/rest/client/v1/profile.py

@@ -51,7 +51,7 @@ class ProfileDisplaynameRestServlet(ClientV1RestServlet):
             defer.returnValue((400, "Unable to parse name"))
 
         yield self.handlers.profile_handler.set_displayname(
-            user, requester.user, new_name)
+            user, requester, new_name)
 
         defer.returnValue((200, {}))
 
@@ -88,7 +88,7 @@ class ProfileAvatarURLRestServlet(ClientV1RestServlet):
             defer.returnValue((400, "Unable to parse name"))
 
         yield self.handlers.profile_handler.set_avatar_url(
-            user, requester.user, new_name)
+            user, requester, new_name)
 
         defer.returnValue((200, {}))
 

synapse/rest/client/v1/room.py

@@ -158,12 +158,12 @@ class RoomStateEventRestServlet(ClientV1RestServlet):
 
         if event_type == EventTypes.Member:
             yield self.handlers.room_member_handler.send_membership_event(
+                requester,
                 event,
                 context,
-                is_guest=requester.is_guest,
             )
         else:
-            yield msg_handler.send_nonmember_event(event, context)
+            yield msg_handler.send_nonmember_event(requester, event, context)
 
         defer.returnValue((200, {"event_id": event.event_id}))
 
@@ -183,13 +183,13 @@ class RoomSendEventRestServlet(ClientV1RestServlet):
 
         msg_handler = self.handlers.message_handler
         event = yield msg_handler.create_and_send_nonmember_event(
+            requester,
             {
                 "type": event_type,
                 "content": content,
                 "room_id": room_id,
                 "sender": requester.user.to_string(),
             },
-            token_id=requester.access_token_id,
             txn_id=txn_id,
         )
 
@@ -504,6 +504,7 @@ class RoomRedactEventRestServlet(ClientV1RestServlet):
 
         msg_handler = self.handlers.message_handler
         event = yield msg_handler.create_and_send_nonmember_event(
+            requester,
             {
                 "type": EventTypes.Redaction,
                 "content": content,
@@ -511,7 +512,6 @@ class RoomRedactEventRestServlet(ClientV1RestServlet):
                 "sender": requester.user.to_string(),
                 "redacts": event_id,
             },
-            token_id=requester.access_token_id,
             txn_id=txn_id,
         )
 

tests/handlers/test_profile.py

@@ -23,7 +23,7 @@ from synapse.api.errors import AuthError
 from synapse.handlers.profile import ProfileHandler
 from synapse.types import UserID
 
-from tests.utils import setup_test_homeserver
+from tests.utils import setup_test_homeserver, requester_for_user
 
 
 class ProfileHandlers(object):
@@ -84,7 +84,11 @@ class ProfileTestCase(unittest.TestCase):
 
     @defer.inlineCallbacks
     def test_set_my_name(self):
-        yield self.handler.set_displayname(self.frank, self.frank, "Frank Jr.")
+        yield self.handler.set_displayname(
+            self.frank,
+            requester_for_user(self.frank),
+            "Frank Jr."
+        )
 
         self.assertEquals(
             (yield self.store.get_profile_displayname(self.frank.localpart)),
@@ -93,7 +97,11 @@ class ProfileTestCase(unittest.TestCase):
 
     @defer.inlineCallbacks
     def test_set_my_name_noauth(self):
-        d = self.handler.set_displayname(self.frank, self.bob, "Frank Jr.")
+        d = self.handler.set_displayname(
+            self.frank,
+            requester_for_user(self.bob),
+            "Frank Jr."
+        )
 
         yield self.assertFailure(d, AuthError)
 
@@ -136,7 +144,7 @@ class ProfileTestCase(unittest.TestCase):
     @defer.inlineCallbacks
     def test_set_my_avatar(self):
         yield self.handler.set_avatar_url(
-            self.frank, self.frank, "http://my.server/pic.gif"
+            self.frank, requester_for_user(self.frank), "http://my.server/pic.gif"
         )
 
         self.assertEquals(

tests/replication/test_resource.py

@@ -18,7 +18,7 @@ from synapse.types import Requester, UserID
 
 from twisted.internet import defer
 from tests import unittest
-from tests.utils import setup_test_homeserver
+from tests.utils import setup_test_homeserver, requester_for_user
 from mock import Mock, NonCallableMock
 import json
 import contextlib
@@ -133,12 +133,15 @@ class ReplicationResourceCase(unittest.TestCase):
     @defer.inlineCallbacks
     def send_text_message(self, room_id, message):
         handler = self.hs.get_handlers().message_handler
-        event = yield handler.create_and_send_nonmember_event({
+        event = yield handler.create_and_send_nonmember_event(
+            requester_for_user(self.user),
+            {
                 "type": "m.room.message",
                 "content": {"body": "message", "msgtype": "m.text"},
                 "room_id": room_id,
                 "sender": self.user.to_string(),
-        })
+            }
+        )
         defer.returnValue(event.event_id)
 
     @defer.inlineCallbacks

tests/rest/client/v1/test_profile.py

@@ -86,7 +86,7 @@ class ProfileTestCase(unittest.TestCase):
 
         self.assertEquals(200, code)
         self.assertEquals(mocked_set.call_args[0][0].localpart, "1234ABCD")
-        self.assertEquals(mocked_set.call_args[0][1].localpart, "1234ABCD")
+        self.assertEquals(mocked_set.call_args[0][1].user.localpart, "1234ABCD")
         self.assertEquals(mocked_set.call_args[0][2], "Frank Jr.")
 
     @defer.inlineCallbacks
@@ -155,5 +155,5 @@ class ProfileTestCase(unittest.TestCase):
 
         self.assertEquals(200, code)
         self.assertEquals(mocked_set.call_args[0][0].localpart, "1234ABCD")
-        self.assertEquals(mocked_set.call_args[0][1].localpart, "1234ABCD")
+        self.assertEquals(mocked_set.call_args[0][1].user.localpart, "1234ABCD")
         self.assertEquals(mocked_set.call_args[0][2], "http://my.server/pic.gif")

tests/utils.py

@@ -20,6 +20,7 @@ from synapse.storage.prepare_database import prepare_database
 from synapse.storage.engines import create_engine
 from synapse.server import HomeServer
 from synapse.federation.transport import server
+from synapse.types import Requester
 from synapse.util.ratelimitutils import FederationRateLimiter
 
 from synapse.util.logcontext import LoggingContext
@@ -510,3 +511,7 @@ class DeferredMockCallable(object):
                     "call(%s)" % _format_call(c[0], c[1]) for c in calls
                 ])
             )
+
+
+def requester_for_user(user):
+    return Requester(user, None, False)