From 586e0df62d859d6e811b745d48c2153a8c25ec09 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 8 Sep 2014 11:07:52 -0700 Subject: [PATCH 01/18] Updated spec and api docs to desired new format. --- .../swagger_matrix/api-docs-registration | 91 ++++++++++++++----- docs/specification.rst | 15 ++- 2 files changed, 73 insertions(+), 33 deletions(-) diff --git a/docs/client-server/swagger_matrix/api-docs-registration b/docs/client-server/swagger_matrix/api-docs-registration index f4669ea2f..11c170c3e 100644 --- a/docs/client-server/swagger_matrix/api-docs-registration +++ b/docs/client-server/swagger_matrix/api-docs-registration @@ -3,35 +3,38 @@ "apis": [ { "operations": [ + { + "method": "GET", + "nickname": "get_registration_info", + "notes": "All login stages MUST be mentioned if there is >1 login type.", + "summary": "Get the login mechanism to use when registering.", + "type": "RegistrationFlows" + }, { "method": "POST", - "nickname": "register", - "notes": "Volatile: This API is likely to change.", + "nickname": "submit_registration", + "notes": "If this is part of a multi-stage registration, there MUST be a 'session' key.", "parameters": [ { - "description": "A registration request", + "description": "A registration submission", "name": "body", "paramType": "body", "required": true, - "type": "RegistrationRequest" + "type": "RegistrationSubmission" } ], "responseMessages": [ { "code": 400, - "message": "No JSON object." + "message": "Bad login type" }, { "code": 400, - "message": "User ID must only contain characters which do not require url encoding." - }, - { - "code": 400, - "message": "User ID already taken." + "message": "Missing JSON keys" } ], - "summary": "Register with the home server.", - "type": "RegistrationResponse" + "summary": "Submit a registration action.", + "type": "RegistrationResult" } ], "path": "/register" @@ -42,30 +45,68 @@ "application/json" ], "models": { - "RegistrationResponse": { - "id": "RegistrationResponse", + "RegistrationFlows": { + "id": "RegistrationFlows", + "properties": { + "flows": { + "description": "A list of valid registration flows.", + "type": "array", + "items": { + "$ref": "RegistrationInfo" + } + } + } + }, + "RegistrationInfo": { + "id": "RegistrationInfo", + "properties": { + "stages": { + "description": "Multi-stage registration only: An array of all the login types required to registration.", + "items": { + "$ref": "string" + }, + "type": "array" + }, + "type": { + "description": "The first login type that must be used when logging in.", + "type": "string" + } + } + }, + "RegistrationResult": { + "id": "RegistrationResult", "properties": { "access_token": { - "description": "The access token for this user.", + "description": "The access token for this user's registration if this is the final stage of the registration process.", "type": "string" - }, + }, "user_id": { - "description": "The fully-qualified user ID.", + "description": "The user's fully-qualified user ID.", + "type": "string" + }, + "next": { + "description": "Multi-stage registration only: The next registration type to submit.", "type": "string" }, - "home_server": { - "description": "The name of the home server.", + "session": { + "description": "Multi-stage registration only: The session token to send when submitting the next registration type.", "type": "string" } } }, - "RegistrationRequest": { - "id": "RegistrationRequest", + "RegistrationSubmission": { + "id": "RegistrationSubmission", "properties": { - "user_id": { - "description": "The desired user ID. If not specified, a random user ID will be allocated.", - "type": "string", - "required": false + "type": { + "description": "The type of registration being submitted.", + "type": "string" + }, + "session": { + "description": "Multi-stage registration only: The session token from an earlier registration stage.", + "type": "string" + }, + "_registration_type_defined_keys_": { + "description": "Keys as defined by the specified registration type, e.g. \"user\", \"password\"" } } } diff --git a/docs/specification.rst b/docs/specification.rst index b15792c00..acfe47605 100644 --- a/docs/specification.rst +++ b/docs/specification.rst @@ -1279,12 +1279,6 @@ display name other than it being a valid unicode string. Registration and login ====================== -.. WARNING:: - The registration API is likely to change. - -.. TODO - - TODO Kegan : Make registration like login (just omit the "user" key on the - initial request?) Clients must register with a home server in order to use Matrix. After registering, the client will be given an access token which must be used in ALL @@ -1297,9 +1291,11 @@ a token sent to their email address, etc. This specification does not define how home servers should authorise their users who want to login to their existing accounts, but instead defines the standard interface which implementations should follow so that ANY client can login to ANY home server. Clients login -using the |login|_ API. +using the |login|_ API. Clients register using the |register|_ API. Registration +follows the same procedure as login, but the path requests are sent to are +different. -The login process breaks down into the following: +The registration/login process breaks down into the following: 1. Determine the requirements for logging in. 2. Submit the login stage credentials. 3. Get credentials or be told the next stage in the login process and repeat @@ -2216,6 +2212,9 @@ Transaction: .. |login| replace:: ``/login`` .. _login: /docs/api/client-server/#!/-login +.. |register| replace:: ``/register`` +.. _register: /docs/api/client-server/#!/-registration + .. |/rooms//messages| replace:: ``/rooms//messages`` .. _/rooms//messages: /docs/api/client-server/#!/-rooms/get_messages From 34878bc26a2ed4b796412830a4e1bf9edddc0089 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 15 Sep 2014 10:23:20 +0100 Subject: [PATCH 02/18] Added LoginType constants. Created general structure for processing registrations. --- synapse/api/constants.py | 9 ++++ synapse/rest/register.py | 95 +++++++++++++++++++++++++++++++--------- 2 files changed, 84 insertions(+), 20 deletions(-) diff --git a/synapse/api/constants.py b/synapse/api/constants.py index fcef062fc..618d3d757 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -50,3 +50,12 @@ class JoinRules(object): KNOCK = u"knock" INVITE = u"invite" PRIVATE = u"private" + + +class LoginType(object): + PASSWORD = u"m.login.password" + OAUTH = u"m.login.oauth2" + EMAIL_CODE = u"m.login.email.code" + EMAIL_URL = u"m.login.email.url" + EMAIL_IDENTITY = u"m.login.email.identity" + RECAPTCHA = u"m.login.recaptcha" \ No newline at end of file diff --git a/synapse/rest/register.py b/synapse/rest/register.py index 48d3c6eca..8faa26572 100644 --- a/synapse/rest/register.py +++ b/synapse/rest/register.py @@ -17,6 +17,7 @@ from twisted.internet import defer from synapse.api.errors import SynapseError, Codes +from synapse.api.constants import LoginType from base import RestServlet, client_path_pattern import json @@ -26,31 +27,64 @@ import urllib class RegisterRestServlet(RestServlet): PATTERN = client_path_pattern("/register$") + def on_GET(self, request): + return (200, { + "flows": [ + { + "type": LoginType.RECAPTCHA, + "stages": ([LoginType.RECAPTCHA, LoginType.EMAIL_IDENTITY, + LoginType.PASSWORD]) + }, + { + "type": LoginType.RECAPTCHA, + "stages": [LoginType.RECAPTCHA, LoginType.PASSWORD] + }, + ] + }) + @defer.inlineCallbacks def on_POST(self, request): - desired_user_id = None - password = None + register_json = _parse_json(request) + + session = (register_json["session"] if "session" in register_json + else None) try: - register_json = json.loads(request.content.read()) - if "password" in register_json: - password = register_json["password"].encode("utf-8") - - if type(register_json["user_id"]) == unicode: - desired_user_id = register_json["user_id"].encode("utf-8") - if urllib.quote(desired_user_id) != desired_user_id: - raise SynapseError( - 400, - "User ID must only contain characters which do not " + - "require URL encoding.") - except ValueError: - defer.returnValue((400, "No JSON object.")) + login_type = register_json["type"] + stages = { + LoginType.RECAPTCHA: self._do_recaptcha, + LoginType.PASSWORD: self._do_password, + LoginType.EMAIL_IDENTITY: self._do_email_identity + } + + session_info = None + if session: + session_info = self._get_session_info(session) + + response = yield stages[login_type](register_json, session_info) + defer.returnValue((200, response)) except KeyError: - pass # user_id is optional + raise SynapseError(400, "Bad login type.") + + + desired_user_id = None + password = None + + if "password" in register_json: + password = register_json["password"].encode("utf-8") + + if ("user_id" in register_json and + type(register_json["user_id"]) == unicode): + desired_user_id = register_json["user_id"].encode("utf-8") + if urllib.quote(desired_user_id) != desired_user_id: + raise SynapseError( + 400, + "User ID must only contain characters which do not " + + "require URL encoding.") threepidCreds = None if 'threepidCreds' in register_json: threepidCreds = register_json['threepidCreds'] - + captcha = {} if self.hs.config.enable_registration_captcha: challenge = None @@ -65,7 +99,7 @@ class RegisterRestServlet(RestServlet): except KeyError: raise SynapseError(400, "Captcha response is required", errcode=Codes.CAPTCHA_NEEDED) - + # TODO determine the source IP : May be an X-Forwarding-For header depending on config ip_addr = request.getClientIP() if self.hs.config.captcha_ip_origin_is_x_forwarded: @@ -73,14 +107,14 @@ class RegisterRestServlet(RestServlet): if request.requestHeaders.hasHeader("X-Forwarded-For"): ip_addr = request.requestHeaders.getRawHeaders( "X-Forwarded-For")[0] - + captcha = { "ip": ip_addr, "private_key": self.hs.config.recaptcha_private_key, "challenge": challenge, "response": user_response } - + handler = self.handlers.registration_handler (user_id, token) = yield handler.register( @@ -101,6 +135,27 @@ class RegisterRestServlet(RestServlet): def on_OPTIONS(self, request): return (200, {}) + def _get_session_info(self, session_id): + pass + + def _do_recaptcha(self, register_json, session): + pass + + def _do_email_identity(self, register_json, session): + pass + + def _do_password(self, register_json, session): + pass + + +def _parse_json(request): + try: + content = json.loads(request.content.read()) + if type(content) != dict: + raise SynapseError(400, "Content must be a JSON object.") + return content + except ValueError: + raise SynapseError(400, "Content not JSON.") def register_servlets(hs, http_server): RegisterRestServlet(hs).register(http_server) From 285ecaacd0308f8088e38c64d49cd2e56b514d3d Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 15 Sep 2014 12:42:36 +0100 Subject: [PATCH 03/18] Split out password/captcha/email logic. --- synapse/handlers/register.py | 120 +++++++++--------- synapse/rest/register.py | 237 ++++++++++++++++++++++------------- 2 files changed, 217 insertions(+), 140 deletions(-) diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py index 0b841d6d3..a019d770d 100644 --- a/synapse/handlers/register.py +++ b/synapse/handlers/register.py @@ -40,8 +40,7 @@ class RegistrationHandler(BaseHandler): self.distributor.declare("registered_user") @defer.inlineCallbacks - def register(self, localpart=None, password=None, threepidCreds=None, - captcha_info={}): + def register(self, localpart=None, password=None): """Registers a new client on the server. Args: @@ -54,37 +53,6 @@ class RegistrationHandler(BaseHandler): Raises: RegistrationError if there was a problem registering. """ - if captcha_info: - captcha_response = yield self._validate_captcha( - captcha_info["ip"], - captcha_info["private_key"], - captcha_info["challenge"], - captcha_info["response"] - ) - if not captcha_response["valid"]: - logger.info("Invalid captcha entered from %s. Error: %s", - captcha_info["ip"], captcha_response["error_url"]) - raise InvalidCaptchaError( - error_url=captcha_response["error_url"] - ) - else: - logger.info("Valid captcha entered from %s", captcha_info["ip"]) - - if threepidCreds: - for c in threepidCreds: - logger.info("validating theeepidcred sid %s on id server %s", - c['sid'], c['idServer']) - try: - threepid = yield self._threepid_from_creds(c) - except: - logger.err() - raise RegistrationError(400, "Couldn't validate 3pid") - - if not threepid: - raise RegistrationError(400, "Couldn't validate 3pid") - logger.info("got threepid medium %s address %s", - threepid['medium'], threepid['address']) - password_hash = None if password: password_hash = bcrypt.hashpw(password, bcrypt.gensalt()) @@ -126,15 +94,54 @@ class RegistrationHandler(BaseHandler): raise RegistrationError( 500, "Cannot generate user ID.") - # Now we have a matrix ID, bind it to the threepids we were given - if threepidCreds: - for c in threepidCreds: - # XXX: This should be a deferred list, shouldn't it? - yield self._bind_threepid(c, user_id) - - defer.returnValue((user_id, token)) + @defer.inlineCallbacks + def check_recaptcha(self, ip, private_key, challenge, response): + """Checks a recaptcha is correct.""" + + captcha_response = yield self._validate_captcha( + ip, + private_key, + challenge, + response + ) + if not captcha_response["valid"]: + logger.info("Invalid captcha entered from %s. Error: %s", + ip, captcha_response["error_url"]) + raise InvalidCaptchaError( + error_url=captcha_response["error_url"] + ) + else: + logger.info("Valid captcha entered from %s", ip) + + @defer.inlineCallbacks + def register_email(self, threepidCreds): + """Registers emails with an identity server.""" + + for c in threepidCreds: + logger.info("validating theeepidcred sid %s on id server %s", + c['sid'], c['idServer']) + try: + threepid = yield self._threepid_from_creds(c) + except: + logger.err() + raise RegistrationError(400, "Couldn't validate 3pid") + + if not threepid: + raise RegistrationError(400, "Couldn't validate 3pid") + logger.info("got threepid medium %s address %s", + threepid['medium'], threepid['address']) + + @defer.inlineCallbacks + def bind_emails(self, user_id, threepidCreds): + """Links emails with a user ID and informs an identity server.""" + + # Now we have a matrix ID, bind it to the threepids we were given + for c in threepidCreds: + # XXX: This should be a deferred list, shouldn't it? + yield self._bind_threepid(c, user_id) + def _generate_token(self, user_id): # urlsafe variant uses _ and - so use . as the separator and replace # all =s with .s so http clients don't quote =s when it is used as @@ -149,17 +156,17 @@ class RegistrationHandler(BaseHandler): def _threepid_from_creds(self, creds): httpCli = PlainHttpClient(self.hs) # XXX: make this configurable! - trustedIdServers = [ 'matrix.org:8090' ] + trustedIdServers = ['matrix.org:8090'] if not creds['idServer'] in trustedIdServers: - logger.warn('%s is not a trusted ID server: rejecting 3pid '+ + logger.warn('%s is not a trusted ID server: rejecting 3pid ' + 'credentials', creds['idServer']) defer.returnValue(None) data = yield httpCli.get_json( creds['idServer'], "/_matrix/identity/api/v1/3pid/getValidated3pid", - { 'sid': creds['sid'], 'clientSecret': creds['clientSecret'] } + {'sid': creds['sid'], 'clientSecret': creds['clientSecret']} ) - + if 'medium' in data: defer.returnValue(data) defer.returnValue(None) @@ -170,44 +177,45 @@ class RegistrationHandler(BaseHandler): data = yield httpCli.post_urlencoded_get_json( creds['idServer'], "/_matrix/identity/api/v1/3pid/bind", - { 'sid': creds['sid'], 'clientSecret': creds['clientSecret'], - 'mxid':mxid } + {'sid': creds['sid'], 'clientSecret': creds['clientSecret'], + 'mxid': mxid} ) defer.returnValue(data) - + @defer.inlineCallbacks def _validate_captcha(self, ip_addr, private_key, challenge, response): """Validates the captcha provided. - + Returns: dict: Containing 'valid'(bool) and 'error_url'(str) if invalid. - + """ - response = yield self._submit_captcha(ip_addr, private_key, challenge, + response = yield self._submit_captcha(ip_addr, private_key, challenge, response) # parse Google's response. Lovely format.. lines = response.split('\n') json = { "valid": lines[0] == 'true', - "error_url": "http://www.google.com/recaptcha/api/challenge?"+ + "error_url": "http://www.google.com/recaptcha/api/challenge?" + "error=%s" % lines[1] } defer.returnValue(json) - + @defer.inlineCallbacks def _submit_captcha(self, ip_addr, private_key, challenge, response): client = PlainHttpClient(self.hs) data = yield client.post_urlencoded_get_raw( "www.google.com:80", "/recaptcha/api/verify", - accept_partial=True, # twisted dislikes google's response, no content length. - args={ - 'privatekey': private_key, + # twisted dislikes google's response, no content length. + accept_partial=True, + args={ + 'privatekey': private_key, 'remoteip': ip_addr, 'challenge': challenge, 'response': response } ) defer.returnValue(data) - + diff --git a/synapse/rest/register.py b/synapse/rest/register.py index 8faa26572..8036c3c40 100644 --- a/synapse/rest/register.py +++ b/synapse/rest/register.py @@ -19,28 +19,62 @@ from twisted.internet import defer from synapse.api.errors import SynapseError, Codes from synapse.api.constants import LoginType from base import RestServlet, client_path_pattern +import synapse.util.stringutils as stringutils import json +import logging import urllib +logger = logging.getLogger(__name__) + class RegisterRestServlet(RestServlet): + """Handles registration with the home server. + + This servlet is in control of the registration flow; the registration + handler doesn't have a concept of multi-stages or sessions. + """ + PATTERN = client_path_pattern("/register$") + def __init__(self, hs): + super(RegisterRestServlet, self).__init__(hs) + # sessions are stored as: + # self.sessions = { + # "session_id" : { __session_dict__ } + # } + # TODO: persistent storage + self.sessions = {} + def on_GET(self, request): - return (200, { - "flows": [ - { - "type": LoginType.RECAPTCHA, - "stages": ([LoginType.RECAPTCHA, LoginType.EMAIL_IDENTITY, - LoginType.PASSWORD]) - }, - { - "type": LoginType.RECAPTCHA, - "stages": [LoginType.RECAPTCHA, LoginType.PASSWORD] - }, - ] - }) + if self.hs.config.enable_registration_captcha: + return (200, { + "flows": [ + { + "type": LoginType.RECAPTCHA, + "stages": ([LoginType.RECAPTCHA, + LoginType.EMAIL_IDENTITY, + LoginType.PASSWORD]) + }, + { + "type": LoginType.RECAPTCHA, + "stages": [LoginType.RECAPTCHA, LoginType.PASSWORD] + } + ] + }) + else: + return (200, { + "flows": [ + { + "type": LoginType.EMAIL_IDENTITY, + "stages": ([LoginType.EMAIL_IDENTITY, + LoginType.PASSWORD]) + }, + { + "type": LoginType.PASSWORD + } + ] + }) @defer.inlineCallbacks def on_POST(self, request): @@ -56,96 +90,130 @@ class RegisterRestServlet(RestServlet): LoginType.EMAIL_IDENTITY: self._do_email_identity } - session_info = None - if session: - session_info = self._get_session_info(session) + session_info = self._get_session_info(request, session) + logger.debug("%s : session info %s request info %s", + login_type, session_info, register_json) + response = yield stages[login_type]( + request, + register_json, + session_info + ) + + if "access_token" not in response: + # isn't a final response + response["session"] = session_info["id"] - response = yield stages[login_type](register_json, session_info) defer.returnValue((200, response)) - except KeyError: - raise SynapseError(400, "Bad login type.") + except KeyError as e: + logger.exception(e) + raise SynapseError(400, "Missing JSON keys or bad login type.") + def on_OPTIONS(self, request): + return (200, {}) - desired_user_id = None - password = None + def _get_session_info(self, request, session_id): + if not session_id: + # create a new session + while session_id is None or session_id in self.sessions: + session_id = stringutils.random_string(24) + self.sessions[session_id] = { + "id": session_id, + LoginType.EMAIL_IDENTITY: False, + LoginType.RECAPTCHA: False + } - if "password" in register_json: - password = register_json["password"].encode("utf-8") + return self.sessions[session_id] - if ("user_id" in register_json and - type(register_json["user_id"]) == unicode): - desired_user_id = register_json["user_id"].encode("utf-8") - if urllib.quote(desired_user_id) != desired_user_id: - raise SynapseError( - 400, - "User ID must only contain characters which do not " + - "require URL encoding.") + def _save_session(self, session): + # TODO: Persistent storage + logger.debug("Saving session %s", session) + self.sessions[session["id"]] = session - threepidCreds = None - if 'threepidCreds' in register_json: - threepidCreds = register_json['threepidCreds'] + def _remove_session(self, session): + logger.debug("Removing session %s", session) + self.sessions.pop(session["id"]) - captcha = {} - if self.hs.config.enable_registration_captcha: - challenge = None - user_response = None - try: - captcha_type = register_json["captcha"]["type"] - if captcha_type != "m.login.recaptcha": - raise SynapseError(400, "Sorry, only m.login.recaptcha " + - "requests are supported.") - challenge = register_json["captcha"]["challenge"] - user_response = register_json["captcha"]["response"] - except KeyError: - raise SynapseError(400, "Captcha response is required", - errcode=Codes.CAPTCHA_NEEDED) - - # TODO determine the source IP : May be an X-Forwarding-For header depending on config - ip_addr = request.getClientIP() - if self.hs.config.captcha_ip_origin_is_x_forwarded: - # use the header - if request.requestHeaders.hasHeader("X-Forwarded-For"): - ip_addr = request.requestHeaders.getRawHeaders( - "X-Forwarded-For")[0] - - captcha = { - "ip": ip_addr, - "private_key": self.hs.config.recaptcha_private_key, - "challenge": challenge, - "response": user_response - } + def _do_recaptcha(self, request, register_json, session): + if not self.hs.config.enable_registration_captcha: + raise SynapseError(400, "Captcha not required.") + challenge = None + user_response = None + try: + challenge = register_json["challenge"] + user_response = register_json["response"] + except KeyError: + raise SynapseError(400, "Captcha response is required", + errcode=Codes.CAPTCHA_NEEDED) + + # May be an X-Forwarding-For header depending on config + ip_addr = request.getClientIP() + if self.hs.config.captcha_ip_origin_is_x_forwarded: + # use the header + if request.requestHeaders.hasHeader("X-Forwarded-For"): + ip_addr = request.requestHeaders.getRawHeaders( + "X-Forwarded-For")[0] handler = self.handlers.registration_handler + yield handler.check_recaptcha( + ip_addr, + self.hs.config.recaptcha_private_key, + challenge, + user_response + ) + session[LoginType.RECAPTCHA] = True # mark captcha as done + self._save_session(session) + defer.returnValue({ + "next": [LoginType.PASSWORD, LoginType.EMAIL_IDENTITY] + }) + + @defer.inlineCallbacks + def _do_email_identity(self, request, register_json, session): + if (self.hs.config.enable_registration_captcha and + not session[LoginType.RECAPTCHA]): + raise SynapseError(400, "Captcha is required.") + + threepidCreds = register_json['threepidCreds'] + handler = self.handlers.registration_handler + yield handler.register_email(threepidCreds) + session["threepidCreds"] = threepidCreds # store creds for next stage + session[LoginType.EMAIL_IDENTITY] = True # mark email as done + self._save_session(session) + defer.returnValue({ + "next": LoginType.PASSWORD + }) + + @defer.inlineCallbacks + def _do_password(self, request, register_json, session): + if (self.hs.config.enable_registration_captcha and + not session[LoginType.RECAPTCHA]): + # captcha should've been done by this stage! + raise SynapseError(400, "Captcha is required.") + + password = register_json["password"].encode("utf-8") + desired_user_id = (register_json["user_id"].encode("utf-8") if "user_id" + in register_json else None) + if desired_user_id and urllib.quote(desired_user_id) != desired_user_id: + raise SynapseError( + 400, + "User ID must only contain characters which do not " + + "require URL encoding.") + handler = self.handlers.registration_handler (user_id, token) = yield handler.register( localpart=desired_user_id, - password=password, - threepidCreds=threepidCreds, - captcha_info=captcha) + password=password + ) + + if session[LoginType.EMAIL_IDENTITY]: + yield handler.bind_emails(user_id, session["threepidCreds"]) result = { "user_id": user_id, "access_token": token, "home_server": self.hs.hostname, } - defer.returnValue( - (200, result) - ) - - def on_OPTIONS(self, request): - return (200, {}) - - def _get_session_info(self, session_id): - pass - - def _do_recaptcha(self, register_json, session): - pass - - def _do_email_identity(self, register_json, session): - pass - - def _do_password(self, register_json, session): - pass + self._remove_session(session) + defer.returnValue(result) def _parse_json(request): @@ -157,5 +225,6 @@ def _parse_json(request): except ValueError: raise SynapseError(400, "Content not JSON.") + def register_servlets(hs, http_server): RegisterRestServlet(hs).register(http_server) From 5bd9369a62c6d6cf677e9eef7d58096449542cdf Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 15 Sep 2014 13:26:05 +0100 Subject: [PATCH 04/18] Correctly handle the 'age' key in events and pdus --- synapse/api/events/__init__.py | 13 +++++++++++++ synapse/api/events/factory.py | 8 ++++++++ synapse/federation/replication.py | 15 ++++++++++++--- synapse/handlers/events.py | 10 ++++------ synapse/handlers/message.py | 6 +++--- synapse/handlers/room.py | 2 +- synapse/rest/events.py | 2 +- synapse/rest/room.py | 2 +- synapse/server.py | 4 ++++ synapse/storage/_base.py | 4 ++++ 10 files changed, 51 insertions(+), 15 deletions(-) diff --git a/synapse/api/events/__init__.py b/synapse/api/events/__init__.py index 5f300de10..72c493db5 100644 --- a/synapse/api/events/__init__.py +++ b/synapse/api/events/__init__.py @@ -17,6 +17,18 @@ from synapse.api.errors import SynapseError, Codes from synapse.util.jsonobject import JsonEncodedObject +def serialize_event(hs, e): + # FIXME(erikj): To handle the case of presence events and the like + if not isinstance(e, SynapseEvent): + return e + + d = e.get_dict() + if "age_ts" in d: + d["age"] = int(hs.get_clock().time_msec()) - d["age_ts"] + + return d + + class SynapseEvent(JsonEncodedObject): """Base class for Synapse events. These are JSON objects which must abide @@ -43,6 +55,7 @@ class SynapseEvent(JsonEncodedObject): "content", # HTTP body, JSON "state_key", "required_power_level", + "age_ts", ] internal_keys = [ diff --git a/synapse/api/events/factory.py b/synapse/api/events/factory.py index 5e38cdbc4..d3d96d73e 100644 --- a/synapse/api/events/factory.py +++ b/synapse/api/events/factory.py @@ -59,6 +59,14 @@ class EventFactory(object): if "ts" not in kwargs: kwargs["ts"] = int(self.clock.time_msec()) + # The "age" key is a delta timestamp that should be converted into an + # absolute timestamp the minute we see it. + if "age" in kwargs: + kwargs["age_ts"] = int(self.clock.time_msec()) - int(kwargs["age"]) + del kwargs["age"] + elif "age_ts" not in kwargs: + kwargs["age_ts"] = int(self.clock.time_msec()) + if etype in self._event_list: handler = self._event_list[etype] else: diff --git a/synapse/federation/replication.py b/synapse/federation/replication.py index e12510017..c79ce4468 100644 --- a/synapse/federation/replication.py +++ b/synapse/federation/replication.py @@ -291,6 +291,12 @@ class ReplicationLayer(object): def on_incoming_transaction(self, transaction_data): transaction = Transaction(**transaction_data) + for p in transaction.pdus: + if "age" in p: + p["age_ts"] = int(self.clock.time_msec()) - int(p["age"]) + + pdu_list = [Pdu(**p) for p in transaction.pdus] + logger.debug("[%s] Got transaction", transaction.transaction_id) response = yield self.transaction_actions.have_responded(transaction) @@ -303,8 +309,6 @@ class ReplicationLayer(object): logger.debug("[%s] Transacition is new", transaction.transaction_id) - pdu_list = [Pdu(**p) for p in transaction.pdus] - dl = [] for pdu in pdu_list: dl.append(self._handle_new_pdu(pdu)) @@ -405,9 +409,14 @@ class ReplicationLayer(object): """Returns a new Transaction containing the given PDUs suitable for transmission. """ + pdus = [p.get_dict() for p in pdu_list] + for p in pdus: + if "age_ts" in pdus: + p["age"] = int(self.clock.time_msec()) - p["age_ts"] + return Transaction( - pdus=[p.get_dict() for p in pdu_list], origin=self.server_name, + pdus=pdus, ts=int(self._clock.time_msec()), destination=None, ) diff --git a/synapse/handlers/events.py b/synapse/handlers/events.py index fd24a11fb..93dcd4032 100644 --- a/synapse/handlers/events.py +++ b/synapse/handlers/events.py @@ -15,7 +15,6 @@ from twisted.internet import defer -from synapse.api.events import SynapseEvent from synapse.util.logutils import log_function from ._base import BaseHandler @@ -71,10 +70,7 @@ class EventStreamHandler(BaseHandler): auth_user, room_ids, pagin_config, timeout ) - chunks = [ - e.get_dict() if isinstance(e, SynapseEvent) else e - for e in events - ] + chunks = [self.hs.serialize_event(e) for e in events] chunk = { "chunk": chunks, @@ -92,7 +88,9 @@ class EventStreamHandler(BaseHandler): # 10 seconds of grace to allow the client to reconnect again # before we think they're gone def _later(): - logger.debug("_later stopped_user_eventstream %s", auth_user) + logger.debug( + "_later stopped_user_eventstream %s", auth_user + ) self.distributor.fire( "stopped_user_eventstream", auth_user ) diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 87fc04478..b63863e5b 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -124,7 +124,7 @@ class MessageHandler(BaseHandler): ) chunk = { - "chunk": [e.get_dict() for e in events], + "chunk": [self.hs.serialize_event(e) for e in events], "start": pagin_config.from_token.to_string(), "end": next_token.to_string(), } @@ -296,7 +296,7 @@ class MessageHandler(BaseHandler): end_token = now_token.copy_and_replace("room_key", token[1]) d["messages"] = { - "chunk": [m.get_dict() for m in messages], + "chunk": [self.hs.serialize_event(m) for m in messages], "start": start_token.to_string(), "end": end_token.to_string(), } @@ -304,7 +304,7 @@ class MessageHandler(BaseHandler): current_state = yield self.store.get_current_state( event.room_id ) - d["state"] = [c.get_dict() for c in current_state] + d["state"] = [self.hs.serialize_event(c) for c in current_state] except: logger.exception("Failed to get snapshot") diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 310cb46fe..5bc128043 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -335,7 +335,7 @@ class RoomMemberHandler(BaseHandler): member_list = yield self.store.get_room_members(room_id=room_id) event_list = [ - entry.get_dict() + self.hs.serialize_event(entry) for entry in member_list ] chunk_data = { diff --git a/synapse/rest/events.py b/synapse/rest/events.py index 7fde14320..097195d7c 100644 --- a/synapse/rest/events.py +++ b/synapse/rest/events.py @@ -59,7 +59,7 @@ class EventRestServlet(RestServlet): event = yield handler.get_event(auth_user, event_id) if event: - defer.returnValue((200, event.get_dict())) + defer.returnValue((200, self.hs.serialize_event(event))) else: defer.returnValue((404, "Event not found.")) diff --git a/synapse/rest/room.py b/synapse/rest/room.py index cef700c81..ecb1e346d 100644 --- a/synapse/rest/room.py +++ b/synapse/rest/room.py @@ -378,7 +378,7 @@ class RoomTriggerBackfill(RestServlet): handler = self.handlers.federation_handler events = yield handler.backfill(remote_server, room_id, limit) - res = [event.get_dict() for event in events] + res = [self.hs.serialize_event(event) for event in events] defer.returnValue((200, res)) diff --git a/synapse/server.py b/synapse/server.py index 83368ea5a..7c185537a 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -20,6 +20,7 @@ # Imports required for the default HomeServer() implementation from synapse.federation import initialize_http_replication +from synapse.api.events import serialize_event from synapse.api.events.factory import EventFactory from synapse.notifier import Notifier from synapse.api.auth import Auth @@ -138,6 +139,9 @@ class BaseHomeServer(object): object.""" return RoomID.from_string(s, hs=self) + def serialize_event(self, e): + return serialize_event(self, e) + # Build magic accessors for every dependency for depname in BaseHomeServer.DEPENDENCIES: BaseHomeServer._make_dependency_method(depname) diff --git a/synapse/storage/_base.py b/synapse/storage/_base.py index 8deaaf93b..cf88bfc22 100644 --- a/synapse/storage/_base.py +++ b/synapse/storage/_base.py @@ -315,6 +315,10 @@ class SQLBaseStore(object): d["content"] = json.loads(d["content"]) del d["unrecognized_keys"] + if "age_ts" not in d: + # For compatibility + d["age_ts"] = d["ts"] if "ts" in d else 0 + return self.event_factory.create_event( etype=d["type"], **d From d821755b49e5387879a63a01e0f8916e360bfc5b Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 15 Sep 2014 14:31:53 +0100 Subject: [PATCH 05/18] Updated webclient to support the new registration logic. --- webclient/components/matrix/matrix-service.js | 143 +++++++++++++++--- 1 file changed, 120 insertions(+), 23 deletions(-) diff --git a/webclient/components/matrix/matrix-service.js b/webclient/components/matrix/matrix-service.js index 68ef16800..d7d278a7f 100644 --- a/webclient/components/matrix/matrix-service.js +++ b/webclient/components/matrix/matrix-service.js @@ -81,38 +81,135 @@ angular.module('matrixService', []) return $http(request); }; + + var doRegisterLogin = function(path, loginType, sessionId, userName, password, threepidCreds) { + var data = {}; + if (loginType === "m.login.recaptcha") { + var challengeToken = Recaptcha.get_challenge(); + var captchaEntry = Recaptcha.get_response(); + data = { + type: "m.login.recaptcha", + challenge: challengeToken, + response: captchaEntry + }; + } + else if (loginType === "m.login.email.identity") { + data = { + threepidCreds: threepidCreds + }; + } + else if (loginType === "m.login.password") { + data = { + user_id: userName, + password: password + }; + } + + if (sessionId) { + data.session = sessionId; + } + data.type = loginType; + console.log("doRegisterLogin >>> " + loginType); + return doRequest("POST", path, undefined, data); + }; return { /****** Home server API ******/ prefix: prefixPath, // Register an user - register: function(user_name, password, threepidCreds, useCaptcha) { - // The REST path spec + register: function(user_name, password, threepidCreds, useCaptcha) { + // registration is composed of multiple requests, to check you can + // register, then to actually register. This deferred will fire when + // all the requests are done, along with the final response. + var deferred = $q.defer(); var path = "/register"; - var data = { - user_id: user_name, - password: password, - threepidCreds: threepidCreds - }; + // check we can actually register with this HS. + doRequest("GET", path, undefined, undefined).then( + function(response) { + console.log("/register [1] : "+JSON.stringify(response)); + var flows = response.data.flows; + var knownTypes = [ + "m.login.password", + "m.login.recaptcha", + "m.login.email.identity" + ]; + // if they entered 3pid creds, we want to use a flow which uses it. + var useThreePidFlow = threepidCreds != undefined; + var flowIndex = 0; + var firstRegType = undefined; + + for (var i=0; i Date: Mon, 15 Sep 2014 14:52:39 +0100 Subject: [PATCH 06/18] Make captcha work again with the new registration logic. --- synapse/rest/register.py | 1 + webclient/components/matrix/matrix-service.js | 22 ++++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/synapse/rest/register.py b/synapse/rest/register.py index 8036c3c40..fe8f0ed23 100644 --- a/synapse/rest/register.py +++ b/synapse/rest/register.py @@ -133,6 +133,7 @@ class RegisterRestServlet(RestServlet): logger.debug("Removing session %s", session) self.sessions.pop(session["id"]) + @defer.inlineCallbacks def _do_recaptcha(self, request, register_json, session): if not self.hs.config.enable_registration_captcha: raise SynapseError(400, "Captcha not required.") diff --git a/webclient/components/matrix/matrix-service.js b/webclient/components/matrix/matrix-service.js index d7d278a7f..35ebca961 100644 --- a/webclient/components/matrix/matrix-service.js +++ b/webclient/components/matrix/matrix-service.js @@ -154,6 +154,13 @@ angular.module('matrixService', []) } if (!useCaptcha && regType == "m.login.recaptcha") { console.error("Web client setup to not use captcha, but HS demands a captcha."); + deferred.reject({ + data: { + errcode: "M_CAPTCHA_NEEDED", + error: "Home server requires a captcha." + } + }); + return; } } } @@ -183,7 +190,20 @@ angular.module('matrixService', []) deferred.resolve(response); } else if (response.data.next) { - return doRegisterLogin(path, response.data.next, sessionId, user_name, password, threepidCreds).then( + var nextType = response.data.next; + if (response.data.next instanceof Array) { + for (var i=0; i Date: Mon, 15 Sep 2014 15:09:21 +0100 Subject: [PATCH 07/18] Updated cmdclient to use new registration logic. --- cmdclient/console.py | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/cmdclient/console.py b/cmdclient/console.py index 2e6b02676..5a9d4c3c4 100755 --- a/cmdclient/console.py +++ b/cmdclient/console.py @@ -145,35 +145,50 @@ class SynapseCmd(cmd.Cmd): : Do not automatically clobber config values. """ args = self._parse(line, ["userid", "noupdate"]) - path = "/register" password = None pwd = None pwd2 = "_" while pwd != pwd2: - pwd = getpass.getpass("(Optional) Type a password for this user: ") - if len(pwd) == 0: - print "Not using a password for this user." - break + pwd = getpass.getpass("Type a password for this user: ") pwd2 = getpass.getpass("Retype the password: ") - if pwd != pwd2: + if pwd != pwd2 or len(pwd) == 0: print "Password mismatch." + pwd = None else: password = pwd - body = {} + body = { + "type": "m.login.password" + } if "userid" in args: body["user_id"] = args["userid"] if password: body["password"] = password - reactor.callFromThread(self._do_register, "POST", path, body, + reactor.callFromThread(self._do_register, body, "noupdate" not in args) @defer.inlineCallbacks - def _do_register(self, method, path, data, update_config): - url = self._url() + path - json_res = yield self.http_client.do_request(method, url, data=data) + def _do_register(self, data, update_config): + # check the registration flows + url = self._url() + "/register" + json_res = yield self.http_client.do_request("GET", url) + print json.dumps(json_res, indent=4) + + passwordFlow = None + for flow in json_res["flows"]: + if flow["type"] == "m.login.recaptcha" or ("stages" in flow and "m.login.recaptcha" in flow["stages"]): + print "Unable to register: Home server requires captcha." + return + if flow["type"] == "m.login.password" and "stages" not in flow: + passwordFlow = flow + break + + if not passwordFlow: + return + + json_res = yield self.http_client.do_request("POST", url, data=data) print json.dumps(json_res, indent=4) if update_config and "user_id" in json_res: self.config["user"] = json_res["user_id"] From c04caff55c47364634b4148e9a4b328a250a31ab Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 15 Sep 2014 15:14:19 +0100 Subject: [PATCH 08/18] Fix unit tests. --- tests/rest/utils.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/rest/utils.py b/tests/rest/utils.py index ce2e8fd98..25ed1388c 100644 --- a/tests/rest/utils.py +++ b/tests/rest/utils.py @@ -95,8 +95,14 @@ class RestTestCase(unittest.TestCase): @defer.inlineCallbacks def register(self, user_id): - (code, response) = yield self.mock_resource.trigger("POST", "/register", - '{"user_id":"%s"}' % user_id) + (code, response) = yield self.mock_resource.trigger( + "POST", + "/register", + json.dumps({ + "user_id": user_id, + "password": "test", + "type": "m.login.password" + })) self.assertEquals(200, code) defer.returnValue(response) From 0b8a3bc3b91db723d431ca0d2d8978524b0a411f Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 15 Sep 2014 15:27:17 +0100 Subject: [PATCH 09/18] Update spec to include m.login.email.identity --- docs/specification.rst | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/docs/specification.rst b/docs/specification.rst index b06f14f8c..a2e348fa2 100644 --- a/docs/specification.rst +++ b/docs/specification.rst @@ -1379,7 +1379,7 @@ This specification defines the following login types: - ``m.login.oauth2`` - ``m.login.email.code`` - ``m.login.email.url`` - + - ``m.login.email.identity`` Password-based -------------- @@ -1527,6 +1527,31 @@ If the link has not been visited yet, a standard error response with an errcode ``M_LOGIN_EMAIL_URL_NOT_YET`` should be returned. +Email-based (identity server) +----------------------------- +:Type: + ``m.login.email.identity`` +:Description: + Login is supported by authorising an email address with an identity server. + +Prior to submitting this, the client should authenticate with an identity server. +After authenticating, the session information should be submitted to the home server. + +To respond to this type, reply with:: + + { + "type": "m.login.email.identity", + "threepidCreds": [ + { + "sid": "", + "clientSecret": "", + "idServer": "" + } + ] + } + + + N-Factor Authentication ----------------------- Multiple login stages can be combined to create N-factor authentication during login. From 2c00e1ecd9774463d687559d5df38b2a76340b32 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 15 Sep 2014 15:38:29 +0100 Subject: [PATCH 10/18] Be consistent when associating keys with login types for registration/login. --- cmdclient/console.py | 2 +- synapse/rest/register.py | 2 +- tests/rest/utils.py | 2 +- webclient/components/matrix/matrix-service.js | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmdclient/console.py b/cmdclient/console.py index 5a9d4c3c4..d9c6ec6a7 100755 --- a/cmdclient/console.py +++ b/cmdclient/console.py @@ -162,7 +162,7 @@ class SynapseCmd(cmd.Cmd): "type": "m.login.password" } if "userid" in args: - body["user_id"] = args["userid"] + body["user"] = args["userid"] if password: body["password"] = password diff --git a/synapse/rest/register.py b/synapse/rest/register.py index fe8f0ed23..c2c80e70c 100644 --- a/synapse/rest/register.py +++ b/synapse/rest/register.py @@ -192,7 +192,7 @@ class RegisterRestServlet(RestServlet): raise SynapseError(400, "Captcha is required.") password = register_json["password"].encode("utf-8") - desired_user_id = (register_json["user_id"].encode("utf-8") if "user_id" + desired_user_id = (register_json["user"].encode("utf-8") if "user" in register_json else None) if desired_user_id and urllib.quote(desired_user_id) != desired_user_id: raise SynapseError( diff --git a/tests/rest/utils.py b/tests/rest/utils.py index 25ed1388c..579441fb4 100644 --- a/tests/rest/utils.py +++ b/tests/rest/utils.py @@ -99,7 +99,7 @@ class RestTestCase(unittest.TestCase): "POST", "/register", json.dumps({ - "user_id": user_id, + "user": user_id, "password": "test", "type": "m.login.password" })) diff --git a/webclient/components/matrix/matrix-service.js b/webclient/components/matrix/matrix-service.js index 35ebca961..069e02e93 100644 --- a/webclient/components/matrix/matrix-service.js +++ b/webclient/components/matrix/matrix-service.js @@ -100,7 +100,7 @@ angular.module('matrixService', []) } else if (loginType === "m.login.password") { data = { - user_id: userName, + user: userName, password: password }; } From 688c37ebf4357064f9d9bac01797800cade991a5 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 15 Sep 2014 15:53:05 +0100 Subject: [PATCH 11/18] Updated CHANGES and UPGRADE to reflect registration API changes. --- CHANGES.rst | 11 +++++++++++ UPGRADE.rst | 23 +++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index d3beea3ed..0853c0312 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,3 +1,14 @@ +Latest +====== + +Registration API: + * The registration API has been overhauled to function like the login API. In + practice, this means registration requests must now include the following: + 'type':'m.login.password'. See UPGRADE for more information on this. + * The 'user_id' key has been renamed to 'user' to better match the login API. + * There is an additional login type: 'm.login.email.identity'. + * The command client and web client have been updated to reflect these changes. + Changes in synapse 0.2.3 (2014-09-12) ===================================== diff --git a/UPGRADE.rst b/UPGRADE.rst index da2a7a0a2..44c0af728 100644 --- a/UPGRADE.rst +++ b/UPGRADE.rst @@ -1,3 +1,26 @@ +Upgrading to Latest +=================== + +This registration API now closely matches the login API. This introduces a bit +more backwards and forwards between the HS and the client, but this improves +the overall flexibility of the API. You can now GET on /register to retrieve a list +of valid registration flows. Upon choosing one, they are submitted in the same +way as login, e.g:: + + { + type: m.login.password, + user: foo, + password: bar + } + +The default HS supports 2 flows, with and without Identity Server email +authentication. Enabling captcha on the HS will add in an extra step to all +flows: ``m.login.recaptcha`` which must be completed before you can transition +to the next stage. There is a new login type: ``m.login.email.identity`` which +contains the ``threepidCreds`` key which were previously sent in the original +register request. For more information on this, see the specification. + + Upgrading to v0.2.0 =================== From 34d7896b06ba72c4a7ea28d5c42124a35df121bd Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 15 Sep 2014 16:05:51 +0100 Subject: [PATCH 12/18] More helpful 400 error messages. --- synapse/rest/register.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/synapse/rest/register.py b/synapse/rest/register.py index c2c80e70c..af528a44f 100644 --- a/synapse/rest/register.py +++ b/synapse/rest/register.py @@ -82,6 +82,10 @@ class RegisterRestServlet(RestServlet): session = (register_json["session"] if "session" in register_json else None) + login_type = None + if "type" not in register_json: + raise SynapseError(400, "Missing 'type' key.") + try: login_type = register_json["type"] stages = { @@ -106,7 +110,7 @@ class RegisterRestServlet(RestServlet): defer.returnValue((200, response)) except KeyError as e: logger.exception(e) - raise SynapseError(400, "Missing JSON keys or bad login type.") + raise SynapseError(400, "Missing JSON keys for login type %s." % login_type) def on_OPTIONS(self, request): return (200, {}) From 6ac0b4ade86d1bdb59c01ff8edff6b149cf1981e Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 15 Sep 2014 14:54:25 +0100 Subject: [PATCH 13/18] Fix 'age' key to update on retries --- synapse/federation/replication.py | 19 ++++++++++++++++--- synapse/federation/transport.py | 17 +++++++++++++++-- synapse/http/client.py | 13 ++++++++++--- 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/synapse/federation/replication.py b/synapse/federation/replication.py index c79ce4468..a48a7ac15 100644 --- a/synapse/federation/replication.py +++ b/synapse/federation/replication.py @@ -292,8 +292,8 @@ class ReplicationLayer(object): transaction = Transaction(**transaction_data) for p in transaction.pdus: - if "age" in p: - p["age_ts"] = int(self.clock.time_msec()) - int(p["age"]) + if "age_ts" in p: + p["age"] = int(self._clock.time_msec()) - int(p["age_ts"]) pdu_list = [Pdu(**p) for p in transaction.pdus] @@ -602,8 +602,21 @@ class _TransactionQueue(object): logger.debug("TX [%s] Sending transaction...", destination) # Actually send the transaction + + # FIXME (erikj): This is a bit of a hack to make the Pdu age + # keys work + def cb(transaction): + now = int(self._clock.time_msec()) + if "pdus" in transaction: + for p in transaction["pdus"]: + if "age_ts" in p: + p["age"] = now - int(p["age_ts"]) + + return transaction + code, response = yield self.transport_layer.send_transaction( - transaction + transaction, + on_send_callback=cb, ) logger.debug("TX [%s] Sent transaction", destination) diff --git a/synapse/federation/transport.py b/synapse/federation/transport.py index 6e62ae7c7..afc777ec9 100644 --- a/synapse/federation/transport.py +++ b/synapse/federation/transport.py @@ -144,7 +144,7 @@ class TransportLayer(object): @defer.inlineCallbacks @log_function - def send_transaction(self, transaction): + def send_transaction(self, transaction, on_send_callback=None): """ Sends the given Transaction to it's destination Args: @@ -165,10 +165,23 @@ class TransportLayer(object): data = transaction.get_dict() + # FIXME (erikj): This is a bit of a hack to make the Pdu age + # keys work + def cb(destination, method, path_bytes, producer): + if not on_send_callback: + return + + transaction = json.loads(producer.body) + + new_transaction = on_send_callback(transaction) + + producer.reset(new_transaction) + code, response = yield self.client.put_json( transaction.destination, path=PREFIX + "/send/%s/" % transaction.transaction_id, - data=data + data=data, + on_send_callback=cb, ) logger.debug( diff --git a/synapse/http/client.py b/synapse/http/client.py index ece6318e0..eb11bfd4d 100644 --- a/synapse/http/client.py +++ b/synapse/http/client.py @@ -122,7 +122,7 @@ class TwistedHttpClient(HttpClient): self.hs = hs @defer.inlineCallbacks - def put_json(self, destination, path, data): + def put_json(self, destination, path, data, on_send_callback=None): if destination in _destination_mappings: destination = _destination_mappings[destination] @@ -131,7 +131,8 @@ class TwistedHttpClient(HttpClient): "PUT", path.encode("ascii"), producer=_JsonProducer(data), - headers_dict={"Content-Type": ["application/json"]} + headers_dict={"Content-Type": ["application/json"]}, + on_send_callback=on_send_callback, ) logger.debug("Getting resp body") @@ -218,7 +219,7 @@ class TwistedHttpClient(HttpClient): @defer.inlineCallbacks def _create_request(self, destination, method, path_bytes, param_bytes=b"", query_bytes=b"", producer=None, headers_dict={}, - retry_on_dns_fail=True): + retry_on_dns_fail=True, on_send_callback=None): """ Creates and sends a request to the given url """ headers_dict[b"User-Agent"] = [b"Synapse"] @@ -242,6 +243,9 @@ class TwistedHttpClient(HttpClient): endpoint = self._getEndpoint(reactor, destination); while True: + if on_send_callback: + on_send_callback(destination, method, path_bytes, producer) + try: response = yield self.agent.request( destination, @@ -310,6 +314,9 @@ class _JsonProducer(object): """ Used by the twisted http client to create the HTTP body from json """ def __init__(self, jsn): + self.reset(jsn) + + def reset(self, jsn): self.body = encode_canonical_json(jsn) self.length = len(self.body) From 0897a09f49ea3e259acebe5ec630a06a6acfb08d Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 15 Sep 2014 15:05:12 +0100 Subject: [PATCH 14/18] Fix unit tests after adding extra argument on put_json --- tests/federation/test_federation.py | 9 ++++++--- tests/handlers/test_presence.py | 23 ++++++++++++++++++----- tests/handlers/test_typing.py | 6 ++++-- 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/tests/federation/test_federation.py b/tests/federation/test_federation.py index 954ccac2a..bb17e9aaf 100644 --- a/tests/federation/test_federation.py +++ b/tests/federation/test_federation.py @@ -17,7 +17,7 @@ from twisted.internet import defer from tests import unittest # python imports -from mock import Mock +from mock import Mock, ANY from ..utils import MockHttpResource, MockClock @@ -181,7 +181,8 @@ class FederationTestCase(unittest.TestCase): "depth": 1, }, ] - } + }, + on_send_callback=ANY, ) @defer.inlineCallbacks @@ -212,7 +213,9 @@ class FederationTestCase(unittest.TestCase): "content": {"testing": "content here"}, } ], - }) + }, + on_send_callback=ANY, + ) @defer.inlineCallbacks def test_recv_edu(self): diff --git a/tests/handlers/test_presence.py b/tests/handlers/test_presence.py index 06f5f9c2b..0cb4dfba3 100644 --- a/tests/handlers/test_presence.py +++ b/tests/handlers/test_presence.py @@ -319,7 +319,8 @@ class PresenceInvitesTestCase(unittest.TestCase): "observer_user": "@apple:test", "observed_user": "@cabbage:elsewhere", } - ) + ), + on_send_callback=ANY, ), defer.succeed((200, "OK")) ) @@ -345,7 +346,8 @@ class PresenceInvitesTestCase(unittest.TestCase): "observer_user": "@cabbage:elsewhere", "observed_user": "@apple:test", } - ) + ), + on_send_callback=ANY, ), defer.succeed((200, "OK")) ) @@ -376,7 +378,8 @@ class PresenceInvitesTestCase(unittest.TestCase): "observer_user": "@cabbage:elsewhere", "observed_user": "@durian:test", } - ) + ), + on_send_callback=ANY, ), defer.succeed((200, "OK")) ) @@ -765,7 +768,8 @@ class PresencePushTestCase(unittest.TestCase): "last_active_ago": 0}, ], } - ) + ), + on_send_callback=ANY, ), defer.succeed((200, "OK")) ) @@ -780,7 +784,8 @@ class PresencePushTestCase(unittest.TestCase): "last_active_ago": 0}, ], } - ) + ), + on_send_callback=ANY, ), defer.succeed((200, "OK")) ) @@ -906,6 +911,7 @@ class PresencePushTestCase(unittest.TestCase): ], } ), + on_send_callback=ANY, ), defer.succeed((200, "OK")) ) @@ -920,6 +926,7 @@ class PresencePushTestCase(unittest.TestCase): ], } ), + on_send_callback=ANY, ), defer.succeed((200, "OK")) ) @@ -949,6 +956,7 @@ class PresencePushTestCase(unittest.TestCase): ], } ), + on_send_callback=ANY, ), defer.succeed((200, "OK")) ) @@ -1145,6 +1153,7 @@ class PresencePollingTestCase(unittest.TestCase): "poll": [ "@potato:remote" ], }, ), + on_send_callback=ANY, ), defer.succeed((200, "OK")) ) @@ -1157,6 +1166,7 @@ class PresencePollingTestCase(unittest.TestCase): "push": [ {"user_id": "@clementine:test" }], }, ), + on_send_callback=ANY, ), defer.succeed((200, "OK")) ) @@ -1185,6 +1195,7 @@ class PresencePollingTestCase(unittest.TestCase): "push": [ {"user_id": "@fig:test" }], }, ), + on_send_callback=ANY, ), defer.succeed((200, "OK")) ) @@ -1217,6 +1228,7 @@ class PresencePollingTestCase(unittest.TestCase): "unpoll": [ "@potato:remote" ], }, ), + on_send_callback=ANY, ), defer.succeed((200, "OK")) ) @@ -1248,6 +1260,7 @@ class PresencePollingTestCase(unittest.TestCase): ], }, ), + on_send_callback=ANY, ), defer.succeed((200, "OK")) ) diff --git a/tests/handlers/test_typing.py b/tests/handlers/test_typing.py index ab908cdfc..a66f208ab 100644 --- a/tests/handlers/test_typing.py +++ b/tests/handlers/test_typing.py @@ -169,7 +169,8 @@ class TypingNotificationsTestCase(unittest.TestCase): "user_id": self.u_apple.to_string(), "typing": True, } - ) + ), + on_send_callback=ANY, ), defer.succeed((200, "OK")) ) @@ -219,7 +220,8 @@ class TypingNotificationsTestCase(unittest.TestCase): "user_id": self.u_apple.to_string(), "typing": False, } - ) + ), + on_send_callback=ANY, ), defer.succeed((200, "OK")) ) From e639a3516d271c395862bcd0c6facfd8c5c9ff58 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 15 Sep 2014 15:18:51 +0100 Subject: [PATCH 15/18] Improve logging in federation handler. --- synapse/handlers/federation.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 59cbf71d7..5187bcb5b 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -93,6 +93,8 @@ class FederationHandler(BaseHandler): """ event = self.pdu_codec.event_from_pdu(pdu) + logger.debug("Got event: %s", event.event_id) + with (yield self.lock_manager.lock(pdu.context)): if event.is_state and not backfilled: is_new_state = yield self.state_handler.handle_new_state( @@ -106,7 +108,7 @@ class FederationHandler(BaseHandler): # respond to PDU. if hasattr(event, "state_key") and not is_new_state: - logger.debug("Ignoring old state.") + logger.debug("Ignoring old state: %s", event.event_id) return target_is_mine = False From 59516a8bb1cd8040bd07420f84b856bd8904d6c8 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 15 Sep 2014 16:40:44 +0100 Subject: [PATCH 16/18] Correctly handle receiving 'missing' Pdus from federation, rather than just discarding them. --- synapse/handlers/federation.py | 12 +++++------- synapse/storage/__init__.py | 15 ++++++++++----- tests/handlers/test_federation.py | 4 +++- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 5187bcb5b..001c6c110 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -100,17 +100,11 @@ class FederationHandler(BaseHandler): is_new_state = yield self.state_handler.handle_new_state( pdu ) - if not is_new_state: - return else: is_new_state = False # TODO: Implement something in federation that allows us to # respond to PDU. - if hasattr(event, "state_key") and not is_new_state: - logger.debug("Ignoring old state: %s", event.event_id) - return - target_is_mine = False if hasattr(event, "target_host"): target_is_mine = event.target_host == self.hs.hostname @@ -141,7 +135,11 @@ class FederationHandler(BaseHandler): else: with (yield self.room_lock.lock(event.room_id)): - yield self.store.persist_event(event, backfilled) + yield self.store.persist_event( + event, + backfilled, + is_new_state=is_new_state + ) room = yield self.store.get_room(event.room_id) diff --git a/synapse/storage/__init__.py b/synapse/storage/__init__.py index 9201a377b..1cede2809 100644 --- a/synapse/storage/__init__.py +++ b/synapse/storage/__init__.py @@ -68,7 +68,8 @@ class DataStore(RoomMemberStore, RoomStore, @defer.inlineCallbacks @log_function - def persist_event(self, event=None, backfilled=False, pdu=None): + def persist_event(self, event=None, backfilled=False, pdu=None, + is_new_state=True): stream_ordering = None if backfilled: if not self.min_token_deferred.called: @@ -83,6 +84,7 @@ class DataStore(RoomMemberStore, RoomStore, event=event, backfilled=backfilled, stream_ordering=stream_ordering, + is_new_state=is_new_state, ) except _RollbackButIsFineException as e: pass @@ -109,12 +111,14 @@ class DataStore(RoomMemberStore, RoomStore, defer.returnValue(event) def _persist_pdu_event_txn(self, txn, pdu=None, event=None, - backfilled=False, stream_ordering=None): + backfilled=False, stream_ordering=None, + is_new_state=True): if pdu is not None: self._persist_event_pdu_txn(txn, pdu) if event is not None: return self._persist_event_txn( - txn, event, backfilled, stream_ordering + txn, event, backfilled, stream_ordering, + is_new_state=is_new_state, ) def _persist_event_pdu_txn(self, txn, pdu): @@ -141,7 +145,8 @@ class DataStore(RoomMemberStore, RoomStore, self._update_min_depth_for_context_txn(txn, pdu.context, pdu.depth) @log_function - def _persist_event_txn(self, txn, event, backfilled, stream_ordering=None): + def _persist_event_txn(self, txn, event, backfilled, stream_ordering=None, + is_new_state=True): if event.type == RoomMemberEvent.TYPE: self._store_room_member_txn(txn, event) elif event.type == FeedbackEvent.TYPE: @@ -195,7 +200,7 @@ class DataStore(RoomMemberStore, RoomStore, ) raise _RollbackButIsFineException("_persist_event") - if not backfilled and hasattr(event, "state_key"): + if is_new_state and hasattr(event, "state_key"): vals = { "event_id": event.event_id, "room_id": event.room_id, diff --git a/tests/handlers/test_federation.py b/tests/handlers/test_federation.py index f0308a29d..eb6b7c22e 100644 --- a/tests/handlers/test_federation.py +++ b/tests/handlers/test_federation.py @@ -74,7 +74,9 @@ class FederationTestCase(unittest.TestCase): yield self.handlers.federation_handler.on_receive_pdu(pdu, False) - self.datastore.persist_event.assert_called_once_with(ANY, False) + self.datastore.persist_event.assert_called_once_with( + ANY, False, is_new_state=False + ) self.notifier.on_new_room_event.assert_called_once_with(ANY) @defer.inlineCallbacks From 40d2f38abe604525fb03622995c377904f1ea3dd Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 15 Sep 2014 16:55:39 +0100 Subject: [PATCH 17/18] Fix bug where we incorrectly calculated 'age_ts' from 'age' key rather than the reverse. Don't transmit age_ts to clients for now. --- synapse/api/events/__init__.py | 1 + synapse/federation/replication.py | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/synapse/api/events/__init__.py b/synapse/api/events/__init__.py index 72c493db5..add81ec3e 100644 --- a/synapse/api/events/__init__.py +++ b/synapse/api/events/__init__.py @@ -25,6 +25,7 @@ def serialize_event(hs, e): d = e.get_dict() if "age_ts" in d: d["age"] = int(hs.get_clock().time_msec()) - d["age_ts"] + del d["age_ts"] return d diff --git a/synapse/federation/replication.py b/synapse/federation/replication.py index a48a7ac15..96b82f00c 100644 --- a/synapse/federation/replication.py +++ b/synapse/federation/replication.py @@ -292,8 +292,9 @@ class ReplicationLayer(object): transaction = Transaction(**transaction_data) for p in transaction.pdus: - if "age_ts" in p: - p["age"] = int(self._clock.time_msec()) - int(p["age_ts"]) + if "age" in p: + p["age_ts"] = int(self._clock.time_msec()) - int(p["age"]) + del p["age"] pdu_list = [Pdu(**p) for p in transaction.pdus] From 1e4b971f95ac953e9fbd4a8e4cc0d0d2edc5e5ea Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 15 Sep 2014 17:43:46 +0100 Subject: [PATCH 18/18] Fix bug where we didn't always get 'prev_content' key --- synapse/api/events/__init__.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/synapse/api/events/__init__.py b/synapse/api/events/__init__.py index add81ec3e..a9991e9c9 100644 --- a/synapse/api/events/__init__.py +++ b/synapse/api/events/__init__.py @@ -57,6 +57,7 @@ class SynapseEvent(JsonEncodedObject): "state_key", "required_power_level", "age_ts", + "prev_content", ] internal_keys = [ @@ -172,10 +173,6 @@ class SynapseEvent(JsonEncodedObject): class SynapseStateEvent(SynapseEvent): - valid_keys = SynapseEvent.valid_keys + [ - "prev_content", - ] - def __init__(self, **kwargs): if "state_key" not in kwargs: kwargs["state_key"] = ""