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

Allow Configurable Rate Limiting Per AS #1175

Merged
merged 6 commits into from
Oct 20, 2016
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions synapse/api/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -603,10 +603,12 @@ def get_user_by_req(self, request, allow_guest=False, rights="access"):
"""
# Can optionally look elsewhere in the request (e.g. headers)
try:
user_id = 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))
defer.returnValue(
synapse.types.create_requester(user_id, app_service=app_service)
)

access_token = get_access_token_from_request(
request, self.TOKEN_NOT_FOUND_HTTP_STATUS
Expand Down Expand Up @@ -644,7 +646,8 @@ def get_user_by_req(self, request, allow_guest=False, rights="access"):
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, app_service=app_service)
)
except KeyError:
raise AuthError(
self.TOKEN_NOT_FOUND_HTTP_STATUS, "Missing access token.",
Expand All @@ -659,14 +662,14 @@ def _get_appservice_user_id(self, request):
)
)
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(
Expand All @@ -678,7 +681,7 @@ def _get_appservice_user_id(self, request):
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"):
Expand Down
7 changes: 6 additions & 1 deletion synapse/appservice/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -95,6 +95,8 @@ def __init__(self, token, url=None, namespaces=None, hs_token=None,
else:
self.protocols = set()

self.rate_limited = rate_limited

def _check_namespaces(self, namespaces):
# Sanity check that it is of the form:
# {
Expand Down Expand Up @@ -234,5 +236,8 @@ def is_exclusive_alias(self, alias):
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__,)
6 changes: 6 additions & 0 deletions synapse/config/appservice.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
Expand Down Expand Up @@ -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
)
6 changes: 6 additions & 0 deletions synapse/handlers/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,16 @@ def ratelimit(self, requester):
time_now = self.clock.time()
user_id = requester.user.to_string()

# 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

# 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(
user_id, time_now,
msg_rate_hz=self.hs.config.rc_messages_per_second,
Expand Down
8 changes: 5 additions & 3 deletions synapse/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@


Requester = namedtuple("Requester",
["user", "access_token_id", "is_guest", "device_id"])
["user", "access_token_id", "is_guest", "device_id", "app_service"])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is slightly too long.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this be best?

Requester = namedtuple("Requester",
    ["user", "access_token_id", "is_guest", "device_id", "app_service"]
)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd probably do:

Requester = namedtuple("Requester", [
    "user", "access_token_id", "is_guest", "device_id", "app_service",
])

or

Requester = namedtuple("Requester", [
     "user",
    "access_token_id",
    "is_guest",
    "device_id",
    "app_service",
])

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 👍

"""
Represents the user making a request

Expand All @@ -29,11 +29,12 @@
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
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):
device_id=None, app_service=None):
"""
Create a new ``Requester`` object

Expand All @@ -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
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)
return Requester(user_id, access_token_id, is_guest, device_id, app_service)


def get_domain_from_id(string):
Expand Down