From 5b54d51d1e98450451b8ffe3a57ad98373e8f5e6 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Tue, 18 Oct 2016 17:04:09 +0100 Subject: [PATCH 1/6] Allow Configurable Rate Limiting Per AS This adds a flag loaded from the registration file of an AS that will determine whether or not its users are rate limited (by ratelimit in _base.py). Needed for IRC bridge reasons - see https://github.com/matrix-org/matrix-appservice-irc/issues/240. --- synapse/appservice/__init__.py | 7 ++++++- synapse/config/appservice.py | 6 ++++++ synapse/handlers/_base.py | 14 ++++++++++++++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/synapse/appservice/__init__.py b/synapse/appservice/__init__.py index 126a10efb7..91471f7e89 100644 --- a/synapse/appservice/__init__.py +++ b/synapse/appservice/__init__.py @@ -81,7 +81,7 @@ class ApplicationService(object): NS_LIST = [NS_USERS, NS_ALIASES, NS_ROOMS] def __init__(self, token, url=None, namespaces=None, hs_token=None, - sender=None, id=None, protocols=None): + sender=None, id=None, protocols=None, rate_limited=True): self.token = token self.url = url self.hs_token = hs_token @@ -95,6 +95,8 @@ class ApplicationService(object): else: self.protocols = set() + self.rate_limited = rate_limited + def _check_namespaces(self, namespaces): # Sanity check that it is of the form: # { @@ -234,5 +236,8 @@ class ApplicationService(object): def is_exclusive_room(self, room_id): return self._is_exclusive(ApplicationService.NS_ROOMS, room_id) + def is_rate_limited(self): + return self.rate_limited + def __str__(self): return "ApplicationService: %s" % (self.__dict__,) diff --git a/synapse/config/appservice.py b/synapse/config/appservice.py index d7537e8d44..82c50b8240 100644 --- a/synapse/config/appservice.py +++ b/synapse/config/appservice.py @@ -110,6 +110,11 @@ def _load_appservice(hostname, as_info, config_filename): user = UserID(localpart, hostname) user_id = user.to_string() + # Rate limiting for users of this AS is on by default (excludes sender) + rate_limited = True + if isinstance(as_info.get("rate_limited"), bool): + rate_limited = as_info.get("rate_limited") + # namespace checks if not isinstance(as_info.get("namespaces"), dict): raise KeyError("Requires 'namespaces' object.") @@ -155,4 +160,5 @@ def _load_appservice(hostname, as_info, config_filename): sender=user_id, id=as_info["id"], protocols=protocols, + rate_limited=rate_limited ) diff --git a/synapse/handlers/_base.py b/synapse/handlers/_base.py index 4981643166..a377b1225b 100644 --- a/synapse/handlers/_base.py +++ b/synapse/handlers/_base.py @@ -57,10 +57,24 @@ class BaseHandler(object): time_now = self.clock.time() user_id = requester.user.to_string() + # Disable rate limiting of users belonging to any AS that is configured + # not to be rate limited in its registration file (rate_limited: true|false). + # The AS user itself is never rate limited. + app_service = self.store.get_app_service_by_user_id(user_id) if app_service is not None: return # do not ratelimit app service senders + should_rate_limit = True + + for service in self.store.get_app_services(): + if service.is_interested_in_user(user_id): + should_rate_limit = service.is_rate_limited() + break + + if not should_rate_limit: + return + allowed, time_allowed = self.ratelimiter.send_message( user_id, time_now, msg_rate_hz=self.hs.config.rc_messages_per_second, From 1b17d1a106604ddf1d8b97d499db8de1dc0651b5 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 20 Oct 2016 11:43:05 +0100 Subject: [PATCH 2/6] Use real AS object by passing it through the requester This means synapse does not have to check if the AS is interested, but instead it effectively re-uses what it already knew about the requesting user --- synapse/api/auth.py | 14 +++++++------- synapse/handlers/_base.py | 11 +++-------- synapse/types.py | 8 +++++--- 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 1b3b55d517..c0bd0890fa 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -603,10 +603,10 @@ class Auth(object): """ # Can optionally look elsewhere in the request (e.g. headers) try: - user_id = yield self._get_appservice_user_id(request) + user_id, as_user = yield self._get_appservice_user_id(request) if user_id: request.authenticated_entity = user_id - defer.returnValue(synapse.types.create_requester(user_id)) + defer.returnValue(synapse.types.create_requester(user_id, as_user=as_user)) access_token = get_access_token_from_request( request, self.TOKEN_NOT_FOUND_HTTP_STATUS @@ -644,7 +644,7 @@ class Auth(object): request.authenticated_entity = user.to_string() defer.returnValue(synapse.types.create_requester( - user, token_id, is_guest, device_id)) + user, token_id, is_guest, device_id, as_user=as_user)) except KeyError: raise AuthError( self.TOKEN_NOT_FOUND_HTTP_STATUS, "Missing access token.", @@ -659,14 +659,14 @@ class Auth(object): ) ) if app_service is None: - defer.returnValue(None) + defer.returnValue((None, None)) if "user_id" not in request.args: - defer.returnValue(app_service.sender) + defer.returnValue((app_service.sender, app_service)) user_id = request.args["user_id"][0] if app_service.sender == user_id: - defer.returnValue(app_service.sender) + defer.returnValue((app_service.sender, app_service)) if not app_service.is_interested_in_user(user_id): raise AuthError( @@ -678,7 +678,7 @@ class Auth(object): 403, "Application service has not registered this user" ) - defer.returnValue(user_id) + defer.returnValue((user_id, app_service)) @defer.inlineCallbacks def get_user_by_access_token(self, token, rights="access"): diff --git a/synapse/handlers/_base.py b/synapse/handlers/_base.py index a377b1225b..ba62746214 100644 --- a/synapse/handlers/_base.py +++ b/synapse/handlers/_base.py @@ -65,14 +65,9 @@ class BaseHandler(object): if app_service is not None: return # do not ratelimit app service senders - should_rate_limit = True - - for service in self.store.get_app_services(): - if service.is_interested_in_user(user_id): - should_rate_limit = service.is_rate_limited() - break - - if not should_rate_limit: + if requester.as_user and not requester.as_user.is_rate_limited(): + # do not ratelimit users of which a non-rate-limited AS is + # acting on behalf return allowed, time_allowed = self.ratelimiter.send_message( diff --git a/synapse/types.py b/synapse/types.py index 1694af1250..35e05b9c41 100644 --- a/synapse/types.py +++ b/synapse/types.py @@ -19,7 +19,7 @@ from collections import namedtuple Requester = namedtuple("Requester", - ["user", "access_token_id", "is_guest", "device_id"]) + ["user", "access_token_id", "is_guest", "device_id", "as_user"]) """ Represents the user making a request @@ -29,11 +29,12 @@ Attributes: request, or None if it came via the appservice API or similar is_guest (bool): True if the user making this request is a guest user device_id (str|None): device_id which was set at authentication time + as_user (ApplicationService|None): the AS requesting on behalf of the user """ def create_requester(user_id, access_token_id=None, is_guest=False, - device_id=None): + device_id=None, as_user=None): """ Create a new ``Requester`` object @@ -43,13 +44,14 @@ def create_requester(user_id, access_token_id=None, is_guest=False, request, or None if it came via the appservice API or similar is_guest (bool): True if the user making this request is a guest user device_id (str|None): device_id which was set at authentication time + as_user (ApplicationService|None): the AS requesting on behalf of the user Returns: Requester """ if not isinstance(user_id, UserID): user_id = UserID.from_string(user_id) - return Requester(user_id, access_token_id, is_guest, device_id) + return Requester(user_id, access_token_id, is_guest, device_id, as_user) def get_domain_from_id(string): From 8bfd01f6191df2bdd1275b30cdd6b9e99f4d6473 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 20 Oct 2016 11:52:46 +0100 Subject: [PATCH 3/6] flake8 --- synapse/api/auth.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index c0bd0890fa..5fc0150bdc 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -606,7 +606,9 @@ class Auth(object): user_id, as_user = yield self._get_appservice_user_id(request) if user_id: request.authenticated_entity = user_id - defer.returnValue(synapse.types.create_requester(user_id, as_user=as_user)) + defer.returnValue( + synapse.types.create_requester(user_id, as_user=as_user) + ) access_token = get_access_token_from_request( request, self.TOKEN_NOT_FOUND_HTTP_STATUS From f09db236b1867a20ef1328af40a2422bca35944e Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 20 Oct 2016 12:04:54 +0100 Subject: [PATCH 4/6] as_user->app_service, less redundant comments, better positioned comments --- synapse/api/auth.py | 6 +++--- synapse/handlers/_base.py | 9 +++------ synapse/types.py | 10 +++++----- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 5fc0150bdc..154af6728a 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -603,11 +603,11 @@ class Auth(object): """ # Can optionally look elsewhere in the request (e.g. headers) try: - user_id, as_user = yield self._get_appservice_user_id(request) + user_id, app_service = yield self._get_appservice_user_id(request) if user_id: request.authenticated_entity = user_id defer.returnValue( - synapse.types.create_requester(user_id, as_user=as_user) + synapse.types.create_requester(user_id, app_service=app_service) ) access_token = get_access_token_from_request( @@ -646,7 +646,7 @@ class Auth(object): request.authenticated_entity = user.to_string() defer.returnValue(synapse.types.create_requester( - user, token_id, is_guest, device_id, as_user=as_user)) + user, token_id, is_guest, device_id, app_service=app_service)) except KeyError: raise AuthError( self.TOKEN_NOT_FOUND_HTTP_STATUS, "Missing access token.", diff --git a/synapse/handlers/_base.py b/synapse/handlers/_base.py index ba62746214..90f96209f8 100644 --- a/synapse/handlers/_base.py +++ b/synapse/handlers/_base.py @@ -57,17 +57,14 @@ class BaseHandler(object): time_now = self.clock.time() user_id = requester.user.to_string() - # Disable rate limiting of users belonging to any AS that is configured - # not to be rate limited in its registration file (rate_limited: true|false). # The AS user itself is never rate limited. - app_service = self.store.get_app_service_by_user_id(user_id) if app_service is not None: return # do not ratelimit app service senders - if requester.as_user and not requester.as_user.is_rate_limited(): - # do not ratelimit users of which a non-rate-limited AS is - # acting on behalf + # Disable rate limiting of users belonging to any AS that is configured + # not to be rate limited in its registration file (rate_limited: true|false). + if requester.app_service and not requester.app_service.is_rate_limited(): return allowed, time_allowed = self.ratelimiter.send_message( diff --git a/synapse/types.py b/synapse/types.py index 35e05b9c41..2b8afa9aab 100644 --- a/synapse/types.py +++ b/synapse/types.py @@ -19,7 +19,7 @@ from collections import namedtuple Requester = namedtuple("Requester", - ["user", "access_token_id", "is_guest", "device_id", "as_user"]) + ["user", "access_token_id", "is_guest", "device_id", "app_service"]) """ Represents the user making a request @@ -29,12 +29,12 @@ Attributes: request, or None if it came via the appservice API or similar is_guest (bool): True if the user making this request is a guest user device_id (str|None): device_id which was set at authentication time - as_user (ApplicationService|None): the AS requesting on behalf of the user + app_service (ApplicationService|None): the AS requesting on behalf of the user """ def create_requester(user_id, access_token_id=None, is_guest=False, - device_id=None, as_user=None): + device_id=None, app_service=None): """ Create a new ``Requester`` object @@ -44,14 +44,14 @@ def create_requester(user_id, access_token_id=None, is_guest=False, request, or None if it came via the appservice API or similar is_guest (bool): True if the user making this request is a guest user device_id (str|None): device_id which was set at authentication time - as_user (ApplicationService|None): the AS requesting on behalf of the user + app_service (ApplicationService|None): the AS requesting on behalf of the user Returns: Requester """ if not isinstance(user_id, UserID): user_id = UserID.from_string(user_id) - return Requester(user_id, access_token_id, is_guest, device_id, as_user) + return Requester(user_id, access_token_id, is_guest, device_id, app_service) def get_domain_from_id(string): From 07caa749bf2b2b2511d31ae73d149075bbea4319 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 20 Oct 2016 12:07:16 +0100 Subject: [PATCH 5/6] Closing brace on following line --- synapse/api/auth.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 154af6728a..b6a151a7ec 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -646,7 +646,8 @@ class Auth(object): request.authenticated_entity = user.to_string() defer.returnValue(synapse.types.create_requester( - user, token_id, is_guest, device_id, app_service=app_service)) + user, token_id, is_guest, device_id, app_service=app_service) + ) except KeyError: raise AuthError( self.TOKEN_NOT_FOUND_HTTP_STATUS, "Missing access token.", From 6fdd31915ba8fa28462e137cc755082baa3d8ba5 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 20 Oct 2016 13:53:15 +0100 Subject: [PATCH 6/6] Style --- synapse/types.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/synapse/types.py b/synapse/types.py index 2b8afa9aab..ffab12df09 100644 --- a/synapse/types.py +++ b/synapse/types.py @@ -18,8 +18,9 @@ from synapse.api.errors import SynapseError from collections import namedtuple -Requester = namedtuple("Requester", - ["user", "access_token_id", "is_guest", "device_id", "app_service"]) +Requester = namedtuple("Requester", [ + "user", "access_token_id", "is_guest", "device_id", "app_service", +]) """ Represents the user making a request