From 8e01263587d47c95b234ae10c30ef0d74585ba53 Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Fri, 9 Sep 2016 18:17:42 +0100 Subject: Allow clients to supply access_tokens as headers Clients can continue to supply access tokens as query parameters or can supply the token as a header: Authorization: Bearer This matches the ouath2 format of https://tools.ietf.org/html/rfc6750#section-2.1 --- synapse/api/auth.py | 46 +++++++++++++++++++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 9 deletions(-) (limited to 'synapse/api') diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 98a50f0948..d8856b8193 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -1158,7 +1158,8 @@ def has_access_token(request): bool: False if no access_token was given, True otherwise. """ query_params = request.args.get("access_token") - return bool(query_params) + auth_headers = request.requestHeaders.getRawHeaders("Authorization") + return bool(query_params) or bool(auth_headers) def get_access_token_from_request(request, token_not_found_http_status=401): @@ -1176,13 +1177,40 @@ def get_access_token_from_request(request, token_not_found_http_status=401): Raises: AuthError: If there isn't an access_token in the request. """ + + auth_headers = request.requestHeaders.getRawHeaders("Authorization") query_params = request.args.get("access_token") - # Try to get the access_token from the query params. - if not query_params: - raise AuthError( - token_not_found_http_status, - "Missing access token.", - errcode=Codes.MISSING_TOKEN - ) + if auth_headers is not None: + # Try the get the access_token from a "Authorization: Bearer" + # header + if query_params is not None: + raise AuthError( + token_not_found_http_status, + "Mixing Authorization headers and access_token query parameters.", + errcode=Codes.MISSING_TOKEN, + ) + if len(auth_headers) > 1: + raise AuthError( + token_not_found_http_status, + "Too many Authorization headers.", + errcode=Codes.MISSING_TOKEN, + ) + parts = auth_headers[0].split(" ") + if parts[0] == "Bearer" and len(parts) == 2: + return parts[1] + else: + raise AuthError( + token_not_found_http_status, + "Invalid Authorization header.", + errcode=Codes.MISSING_TOKEN, + ) + else: + # Try to get the access_token from the query params. + if not query_params: + raise AuthError( + token_not_found_http_status, + "Missing access token.", + errcode=Codes.MISSING_TOKEN + ) - return query_params[0] + return query_params[0] -- cgit 1.4.1 From ec609f80949972327274142e4eb5b506f55aba1a Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Mon, 12 Sep 2016 10:46:02 +0100 Subject: Fix unit tests --- synapse/api/auth.py | 2 +- tests/api/test_auth.py | 18 +++++++++--------- tests/handlers/test_typing.py | 3 ++- tests/rest/client/v1/test_register.py | 2 ++ tests/rest/client/v2_alpha/test_register.py | 2 ++ tests/utils.py | 18 ++++++++++++++---- 6 files changed, 30 insertions(+), 15 deletions(-) (limited to 'synapse/api') diff --git a/synapse/api/auth.py b/synapse/api/auth.py index d8856b8193..5029f7c534 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -1180,7 +1180,7 @@ def get_access_token_from_request(request, token_not_found_http_status=401): auth_headers = request.requestHeaders.getRawHeaders("Authorization") query_params = request.args.get("access_token") - if auth_headers is not None: + if auth_headers: # Try the get the access_token from a "Authorization: Bearer" # header if query_params is not None: diff --git a/tests/api/test_auth.py b/tests/api/test_auth.py index e91723ca3d..2cf262bb46 100644 --- a/tests/api/test_auth.py +++ b/tests/api/test_auth.py @@ -20,7 +20,7 @@ from mock import Mock from synapse.api.auth import Auth from synapse.api.errors import AuthError from synapse.types import UserID -from tests.utils import setup_test_homeserver +from tests.utils import setup_test_homeserver, mock_getRawHeaders import pymacaroons @@ -51,7 +51,7 @@ class AuthTestCase(unittest.TestCase): request = Mock(args={}) request.args["access_token"] = [self.test_token] - request.requestHeaders.getRawHeaders = Mock(return_value=[""]) + request.requestHeaders.getRawHeaders = mock_getRawHeaders() requester = yield self.auth.get_user_by_req(request) self.assertEquals(requester.user.to_string(), self.test_user) @@ -61,7 +61,7 @@ class AuthTestCase(unittest.TestCase): request = Mock(args={}) request.args["access_token"] = [self.test_token] - request.requestHeaders.getRawHeaders = Mock(return_value=[""]) + request.requestHeaders.getRawHeaders = mock_getRawHeaders() d = self.auth.get_user_by_req(request) self.failureResultOf(d, AuthError) @@ -74,7 +74,7 @@ class AuthTestCase(unittest.TestCase): self.store.get_user_by_access_token = Mock(return_value=user_info) request = Mock(args={}) - request.requestHeaders.getRawHeaders = Mock(return_value=[""]) + request.requestHeaders.getRawHeaders = mock_getRawHeaders() d = self.auth.get_user_by_req(request) self.failureResultOf(d, AuthError) @@ -86,7 +86,7 @@ class AuthTestCase(unittest.TestCase): request = Mock(args={}) request.args["access_token"] = [self.test_token] - request.requestHeaders.getRawHeaders = Mock(return_value=[""]) + request.requestHeaders.getRawHeaders = mock_getRawHeaders() requester = yield self.auth.get_user_by_req(request) self.assertEquals(requester.user.to_string(), self.test_user) @@ -96,7 +96,7 @@ class AuthTestCase(unittest.TestCase): request = Mock(args={}) request.args["access_token"] = [self.test_token] - request.requestHeaders.getRawHeaders = Mock(return_value=[""]) + request.requestHeaders.getRawHeaders = mock_getRawHeaders() d = self.auth.get_user_by_req(request) self.failureResultOf(d, AuthError) @@ -106,7 +106,7 @@ class AuthTestCase(unittest.TestCase): self.store.get_user_by_access_token = Mock(return_value=None) request = Mock(args={}) - request.requestHeaders.getRawHeaders = Mock(return_value=[""]) + request.requestHeaders.getRawHeaders = mock_getRawHeaders() d = self.auth.get_user_by_req(request) self.failureResultOf(d, AuthError) @@ -121,7 +121,7 @@ class AuthTestCase(unittest.TestCase): request = Mock(args={}) request.args["access_token"] = [self.test_token] request.args["user_id"] = [masquerading_user_id] - request.requestHeaders.getRawHeaders = Mock(return_value=[""]) + request.requestHeaders.getRawHeaders = mock_getRawHeaders() requester = yield self.auth.get_user_by_req(request) self.assertEquals(requester.user.to_string(), masquerading_user_id) @@ -135,7 +135,7 @@ class AuthTestCase(unittest.TestCase): request = Mock(args={}) request.args["access_token"] = [self.test_token] request.args["user_id"] = [masquerading_user_id] - request.requestHeaders.getRawHeaders = Mock(return_value=[""]) + request.requestHeaders.getRawHeaders = mock_getRawHeaders() d = self.auth.get_user_by_req(request) self.failureResultOf(d, AuthError) diff --git a/tests/handlers/test_typing.py b/tests/handlers/test_typing.py index ea1f0f7c33..d71ac4838e 100644 --- a/tests/handlers/test_typing.py +++ b/tests/handlers/test_typing.py @@ -219,7 +219,8 @@ class TypingNotificationsTestCase(unittest.TestCase): "user_id": self.u_onion.to_string(), "typing": True, } - ) + ), + federation_auth=True, ) self.on_new_event.assert_has_calls([ diff --git a/tests/rest/client/v1/test_register.py b/tests/rest/client/v1/test_register.py index 4a898a034f..73822f4d8c 100644 --- a/tests/rest/client/v1/test_register.py +++ b/tests/rest/client/v1/test_register.py @@ -17,6 +17,7 @@ from synapse.rest.client.v1.register import CreateUserRestServlet from twisted.internet import defer from mock import Mock from tests import unittest +from tests.utils import mock_getRawHeaders import json @@ -30,6 +31,7 @@ class CreateUserServletTestCase(unittest.TestCase): path='/_matrix/client/api/v1/createUser' ) self.request.args = {} + self.request.requestHeaders.getRawHeaders = mock_getRawHeaders() self.appservice = None self.auth = Mock(get_appservice_by_req=Mock( diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py index 8ac56a1fb2..d7ffd1d427 100644 --- a/tests/rest/client/v2_alpha/test_register.py +++ b/tests/rest/client/v2_alpha/test_register.py @@ -3,6 +3,7 @@ from synapse.api.errors import SynapseError from twisted.internet import defer from mock import Mock from tests import unittest +from tests.utils import mock_getRawHeaders import json @@ -16,6 +17,7 @@ class RegisterRestServletTestCase(unittest.TestCase): path='/_matrix/api/v2_alpha/register' ) self.request.args = {} + self.request.requestHeaders.getRawHeaders = mock_getRawHeaders() self.appservice = None self.auth = Mock(get_appservice_by_req=Mock( diff --git a/tests/utils.py b/tests/utils.py index 915b934e94..1b89470c60 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -115,6 +115,15 @@ def get_mock_call_args(pattern_func, mock_func): return getcallargs(pattern_func, *invoked_args, **invoked_kargs) +def mock_getRawHeaders(headers=None): + headers = headers if headers is not None else {} + + def getRawHeaders(name, default=None): + return headers.get(name, default) + + return getRawHeaders + + # This is a mock /resource/ not an entire server class MockHttpResource(HttpServer): @@ -127,7 +136,7 @@ class MockHttpResource(HttpServer): @patch('twisted.web.http.Request') @defer.inlineCallbacks - def trigger(self, http_method, path, content, mock_request): + def trigger(self, http_method, path, content, mock_request, federation_auth=False): """ Fire an HTTP event. Args: @@ -155,9 +164,10 @@ class MockHttpResource(HttpServer): mock_request.getClientIP.return_value = "-" - mock_request.requestHeaders.getRawHeaders.return_value = [ - "X-Matrix origin=test,key=,sig=" - ] + headers = {} + if federation_auth: + headers["Authorization"] = ["X-Matrix origin=test,key=,sig="] + mock_request.requestHeaders.getRawHeaders = mock_getRawHeaders(headers) # return the right path if the event requires it mock_request.path = path -- cgit 1.4.1 From 550308c7a1e8e6f7bf6ee27baf2d67796591657a Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 19 Oct 2016 10:45:24 +0100 Subject: Check whether to ratelimit sooner to avoid work --- synapse/api/ratelimiting.py | 14 +++++++++----- synapse/handlers/message.py | 14 +++++++++++++- 2 files changed, 22 insertions(+), 6 deletions(-) (limited to 'synapse/api') diff --git a/synapse/api/ratelimiting.py b/synapse/api/ratelimiting.py index 660dfb56e5..06cc8d90b8 100644 --- a/synapse/api/ratelimiting.py +++ b/synapse/api/ratelimiting.py @@ -23,7 +23,7 @@ class Ratelimiter(object): def __init__(self): self.message_counts = collections.OrderedDict() - def send_message(self, user_id, time_now_s, msg_rate_hz, burst_count): + def send_message(self, user_id, time_now_s, msg_rate_hz, burst_count, update=True): """Can the user send a message? Args: user_id: The user sending a message. @@ -32,12 +32,15 @@ class Ratelimiter(object): second. burst_count: How many messages the user can send before being limited. + update (bool): Whether to update the message rates or not. This is + useful to check if a message would be allowed to be sent before + its ready to be actually sent. Returns: A pair of a bool indicating if they can send a message now and a time in seconds of when they can next send a message. """ self.prune_message_counts(time_now_s) - message_count, time_start, _ignored = self.message_counts.pop( + message_count, time_start, _ignored = self.message_counts.get( user_id, (0., time_now_s, None), ) time_delta = time_now_s - time_start @@ -52,9 +55,10 @@ class Ratelimiter(object): allowed = True message_count += 1 - self.message_counts[user_id] = ( - message_count, time_start, msg_rate_hz - ) + if update: + self.message_counts[user_id] = ( + message_count, time_start, msg_rate_hz + ) if msg_rate_hz > 0: time_allowed = ( diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 30ea9630f7..a94a514338 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -16,7 +16,7 @@ from twisted.internet import defer from synapse.api.constants import EventTypes, Membership -from synapse.api.errors import AuthError, Codes, SynapseError +from synapse.api.errors import AuthError, Codes, SynapseError, LimitExceededError from synapse.crypto.event_signing import add_hashes_and_signatures from synapse.events.utils import serialize_event from synapse.events.validator import EventValidator @@ -239,6 +239,18 @@ class MessageHandler(BaseHandler): "Tried to send member event through non-member codepath" ) + time_now = self.clock.time() + allowed, time_allowed = self.ratelimiter.send_message( + event.sender, time_now, + msg_rate_hz=self.hs.config.rc_messages_per_second, + burst_count=self.hs.config.rc_message_burst_count, + update=False, + ) + if not allowed: + raise LimitExceededError( + retry_after_ms=int(1000 * (time_allowed - time_now)), + ) + user = UserID.from_string(event.sender) assert self.hs.is_mine(user), "User must be our own: %s" % (user,) -- cgit 1.4.1 From 1b17d1a106604ddf1d8b97d499db8de1dc0651b5 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 20 Oct 2016 11:43:05 +0100 Subject: 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(-) (limited to 'synapse/api') 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): -- cgit 1.4.1 From 8bfd01f6191df2bdd1275b30cdd6b9e99f4d6473 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 20 Oct 2016 11:52:46 +0100 Subject: flake8 --- synapse/api/auth.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'synapse/api') 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 -- cgit 1.4.1 From f09db236b1867a20ef1328af40a2422bca35944e Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 20 Oct 2016 12:04:54 +0100 Subject: 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(-) (limited to 'synapse/api') 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): -- cgit 1.4.1 From 07caa749bf2b2b2511d31ae73d149075bbea4319 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 20 Oct 2016 12:07:16 +0100 Subject: Closing brace on following line --- synapse/api/auth.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'synapse/api') 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.", -- cgit 1.4.1