From 90d17a3d28d7a87fe1231db3726759339c914753 Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Thu, 5 Sep 2019 14:00:30 +0100 Subject: [PATCH 01/26] Add POST /_matrix/client/r0/account/3pid/unbind (MSC2140) (#5980) Implements `POST /_matrix/client/r0/account/3pid/unbind` from [MSC2140](https://github.com/matrix-org/matrix-doc/blob/dbkr/tos_2/proposals/2140-terms-of-service-2.md#post-_matrixclientr0account3pidunbind). --- changelog.d/5980.feature | 1 + synapse/handlers/identity.py | 3 ++- synapse/rest/client/v2_alpha/account.py | 33 +++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 changelog.d/5980.feature diff --git a/changelog.d/5980.feature b/changelog.d/5980.feature new file mode 100644 index 0000000000..f25d8d81d9 --- /dev/null +++ b/changelog.d/5980.feature @@ -0,0 +1 @@ +Add POST /_matrix/client/r0/account/3pid/unbind endpoint from MSC2140 for unbinding a 3PID from an identity server without removing it from the homeserver user account. \ No newline at end of file diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py index d199521b58..5540f9f4d5 100644 --- a/synapse/handlers/identity.py +++ b/synapse/handlers/identity.py @@ -137,7 +137,8 @@ class IdentityHandler(BaseHandler): @defer.inlineCallbacks def try_unbind_threepid(self, mxid, threepid): - """Removes a binding from an identity server + """Attempt to remove a 3PID from an identity server, or if one is not provided, all + identity servers we're aware the binding is present on Args: mxid (str): Matrix user ID of binding to be removed diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 0620a4d0cf..a4be518006 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -571,6 +571,38 @@ class ThreepidRestServlet(RestServlet): return 200, {} +class ThreepidUnbindRestServlet(RestServlet): + PATTERNS = client_patterns("/account/3pid/unbind$") + + def __init__(self, hs): + super(ThreepidUnbindRestServlet, self).__init__() + self.hs = hs + self.identity_handler = hs.get_handlers().identity_handler + self.auth = hs.get_auth() + self.datastore = self.hs.get_datastore() + + @defer.inlineCallbacks + def on_POST(self, request): + """Unbind the given 3pid from a specific identity server, or identity servers that are + known to have this 3pid bound + """ + requester = yield self.auth.get_user_by_req(request) + body = parse_json_object_from_request(request) + assert_params_in_dict(body, ["medium", "address"]) + + medium = body.get("medium") + address = body.get("address") + id_server = body.get("id_server") + + # Attempt to unbind the threepid from an identity server. If id_server is None, try to + # unbind from all identity servers this threepid has been added to in the past + result = yield self.identity_handler.try_unbind_threepid( + requester.user.to_string(), + {"address": address, "medium": medium, "id_server": id_server}, + ) + return 200, {"id_server_unbind_result": "success" if result else "no-support"} + + class ThreepidDeleteRestServlet(RestServlet): PATTERNS = client_patterns("/account/3pid/delete$") @@ -629,5 +661,6 @@ def register_servlets(hs, http_server): EmailThreepidRequestTokenRestServlet(hs).register(http_server) MsisdnThreepidRequestTokenRestServlet(hs).register(http_server) ThreepidRestServlet(hs).register(http_server) + ThreepidUnbindRestServlet(hs).register(http_server) ThreepidDeleteRestServlet(hs).register(http_server) WhoamiRestServlet(hs).register(http_server) From b9cfd3c375c551902093b0dac1df9e0b4d6759cc Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 5 Sep 2019 14:22:15 +0100 Subject: [PATCH 02/26] Fix opentracing contexts missing from outbound replication requests (#5982) --- changelog.d/5982.bugfix | 1 + synapse/logging/opentracing.py | 20 ++++++++++++++++---- 2 files changed, 17 insertions(+), 4 deletions(-) create mode 100644 changelog.d/5982.bugfix diff --git a/changelog.d/5982.bugfix b/changelog.d/5982.bugfix new file mode 100644 index 0000000000..3ea281a3a0 --- /dev/null +++ b/changelog.d/5982.bugfix @@ -0,0 +1 @@ +Include missing opentracing contexts in outbout replication requests. diff --git a/synapse/logging/opentracing.py b/synapse/logging/opentracing.py index 256b972aaa..dbf80e2024 100644 --- a/synapse/logging/opentracing.py +++ b/synapse/logging/opentracing.py @@ -493,6 +493,11 @@ def inject_active_span_twisted_headers(headers, destination, check_destination=T Args: headers (twisted.web.http_headers.Headers) + destination (str): address of entity receiving the span context. If check_destination + is true the context will only be injected if the destination matches the + opentracing whitelist + check_destination (bool): If false, destination will be ignored and the context + will always be injected. span (opentracing.Span) Returns: @@ -525,6 +530,11 @@ def inject_active_span_byte_dict(headers, destination, check_destination=True): Args: headers (dict) + destination (str): address of entity receiving the span context. If check_destination + is true the context will only be injected if the destination matches the + opentracing whitelist + check_destination (bool): If false, destination will be ignored and the context + will always be injected. span (opentracing.Span) Returns: @@ -537,7 +547,7 @@ def inject_active_span_byte_dict(headers, destination, check_destination=True): here: https://github.com/jaegertracing/jaeger-client-python/blob/master/jaeger_client/constants.py """ - if not whitelisted_homeserver(destination): + if check_destination and not whitelisted_homeserver(destination): return span = opentracing.tracer.active_span @@ -556,9 +566,11 @@ def inject_active_span_text_map(carrier, destination, check_destination=True): Args: carrier (dict) - destination (str): the name of the remote server. The span context - will only be injected if the destination matches the homeserver_whitelist - or destination is None. + destination (str): address of entity receiving the span context. If check_destination + is true the context will only be injected if the destination matches the + opentracing whitelist + check_destination (bool): If false, destination will be ignored and the context + will always be injected. Returns: In-place modification of carrier From a0d294c306d2e345bb53078791858c41f3101424 Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Thu, 5 Sep 2019 14:31:22 +0100 Subject: [PATCH 03/26] Switch to using v2 Identity Service APIs other than lookup (MSC 2140) (#5892) --- changelog.d/5892.misc | 1 + contrib/cmdclient/console.py | 5 + synapse/handlers/identity.py | 162 ++++++++++++++++++------ synapse/rest/client/v2_alpha/account.py | 13 +- 4 files changed, 133 insertions(+), 48 deletions(-) create mode 100644 changelog.d/5892.misc diff --git a/changelog.d/5892.misc b/changelog.d/5892.misc new file mode 100644 index 0000000000..939fe8c655 --- /dev/null +++ b/changelog.d/5892.misc @@ -0,0 +1 @@ +Compatibility with v2 Identity Service APIs other than /lookup. \ No newline at end of file diff --git a/contrib/cmdclient/console.py b/contrib/cmdclient/console.py index af8f39c8c2..899c650b0c 100755 --- a/contrib/cmdclient/console.py +++ b/contrib/cmdclient/console.py @@ -268,6 +268,7 @@ class SynapseCmd(cmd.Cmd): @defer.inlineCallbacks def _do_emailrequest(self, args): + # TODO: Update to use v2 Identity Service API endpoint url = ( self._identityServerUrl() + "/_matrix/identity/api/v1/validate/email/requestToken" @@ -302,6 +303,7 @@ class SynapseCmd(cmd.Cmd): @defer.inlineCallbacks def _do_emailvalidate(self, args): + # TODO: Update to use v2 Identity Service API endpoint url = ( self._identityServerUrl() + "/_matrix/identity/api/v1/validate/email/submitToken" @@ -330,6 +332,7 @@ class SynapseCmd(cmd.Cmd): @defer.inlineCallbacks def _do_3pidbind(self, args): + # TODO: Update to use v2 Identity Service API endpoint url = self._identityServerUrl() + "/_matrix/identity/api/v1/3pid/bind" json_res = yield self.http_client.do_request( @@ -398,6 +401,7 @@ class SynapseCmd(cmd.Cmd): @defer.inlineCallbacks def _do_invite(self, roomid, userstring): if not userstring.startswith("@") and self._is_on("complete_usernames"): + # TODO: Update to use v2 Identity Service API endpoint url = self._identityServerUrl() + "/_matrix/identity/api/v1/lookup" json_res = yield self.http_client.do_request( @@ -407,6 +411,7 @@ class SynapseCmd(cmd.Cmd): mxid = None if "mxid" in json_res and "signatures" in json_res: + # TODO: Update to use v2 Identity Service API endpoint url = ( self._identityServerUrl() + "/_matrix/identity/api/v1/pubkey/ed25519" diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py index 5540f9f4d5..583b612dd9 100644 --- a/synapse/handlers/identity.py +++ b/synapse/handlers/identity.py @@ -61,21 +61,76 @@ class IdentityHandler(BaseHandler): return False return True - @defer.inlineCallbacks - def threepid_from_creds(self, creds): - if "id_server" in creds: - id_server = creds["id_server"] - elif "idServer" in creds: - id_server = creds["idServer"] - else: - raise SynapseError(400, "No id_server in creds") + def _extract_items_from_creds_dict(self, creds): + """ + Retrieve entries from a "credentials" dictionary - if "client_secret" in creds: - client_secret = creds["client_secret"] - elif "clientSecret" in creds: - client_secret = creds["clientSecret"] + Args: + creds (dict[str, str]): Dictionary of credentials that contain the following keys: + * client_secret|clientSecret: A unique secret str provided by the client + * id_server|idServer: the domain of the identity server to query + * id_access_token: The access token to authenticate to the identity + server with. + + Returns: + tuple(str, str, str|None): A tuple containing the client_secret, the id_server, + and the id_access_token value if available. + """ + client_secret = creds.get("client_secret") or creds.get("clientSecret") + if not client_secret: + raise SynapseError( + 400, "No client_secret in creds", errcode=Codes.MISSING_PARAM + ) + + id_server = creds.get("id_server") or creds.get("idServer") + if not id_server: + raise SynapseError( + 400, "No id_server in creds", errcode=Codes.MISSING_PARAM + ) + + id_access_token = creds.get("id_access_token") + return client_secret, id_server, id_access_token + + @defer.inlineCallbacks + def threepid_from_creds(self, creds, use_v2=True): + """ + Retrieve and validate a threepid identitier from a "credentials" dictionary + + Args: + creds (dict[str, str]): Dictionary of credentials that contain the following keys: + * client_secret|clientSecret: A unique secret str provided by the client + * id_server|idServer: the domain of the identity server to query + * id_access_token: The access token to authenticate to the identity + server with. Required if use_v2 is true + use_v2 (bool): Whether to use v2 Identity Service API endpoints + + Returns: + Deferred[dict[str,str|int]|None]: A dictionary consisting of response params to + the /getValidated3pid endpoint of the Identity Service API, or None if the + threepid was not found + """ + client_secret, id_server, id_access_token = self._extract_items_from_creds_dict( + creds + ) + + # If an id_access_token is not supplied, force usage of v1 + if id_access_token is None: + use_v2 = False + + query_params = {"sid": creds["sid"], "client_secret": client_secret} + + # Decide which API endpoint URLs and query parameters to use + if use_v2: + url = "https://%s%s" % ( + id_server, + "/_matrix/identity/v2/3pid/getValidated3pid", + ) + query_params["id_access_token"] = id_access_token else: - raise SynapseError(400, "No client_secret in creds") + url = "https://%s%s" % ( + id_server, + "/_matrix/identity/api/v1/3pid/getValidated3pid", + ) if not self._should_trust_id_server(id_server): logger.warn( @@ -85,43 +140,55 @@ class IdentityHandler(BaseHandler): return None try: - data = yield self.http_client.get_json( - "https://%s%s" - % (id_server, "/_matrix/identity/api/v1/3pid/getValidated3pid"), - {"sid": creds["sid"], "client_secret": client_secret}, - ) + data = yield self.http_client.get_json(url, query_params) + return data if "medium" in data else None except HttpResponseException as e: - logger.info("getValidated3pid failed with Matrix error: %r", e) - raise e.to_synapse_error() + if e.code != 404 or not use_v2: + # Generic failure + logger.info("getValidated3pid failed with Matrix error: %r", e) + raise e.to_synapse_error() - if "medium" in data: - return data - return None + # This identity server is too old to understand Identity Service API v2 + # Attempt v1 endpoint + logger.info("Got 404 when POSTing JSON %s, falling back to v1 URL", url) + return (yield self.threepid_from_creds(creds, use_v2=False)) @defer.inlineCallbacks - def bind_threepid(self, creds, mxid): + def bind_threepid(self, creds, mxid, use_v2=True): + """Bind a 3PID to an identity server + + Args: + creds (dict[str, str]): Dictionary of credentials that contain the following keys: + * client_secret|clientSecret: A unique secret str provided by the client + * id_server|idServer: the domain of the identity server to query + * id_access_token: The access token to authenticate to the identity + server with. Required if use_v2 is true + mxid (str): The MXID to bind the 3PID to + use_v2 (bool): Whether to use v2 Identity Service API endpoints + + Returns: + Deferred[dict]: The response from the identity server + """ logger.debug("binding threepid %r to %s", creds, mxid) - data = None - if "id_server" in creds: - id_server = creds["id_server"] - elif "idServer" in creds: - id_server = creds["idServer"] - else: - raise SynapseError(400, "No id_server in creds") + client_secret, id_server, id_access_token = self._extract_items_from_creds_dict( + creds + ) - if "client_secret" in creds: - client_secret = creds["client_secret"] - elif "clientSecret" in creds: - client_secret = creds["clientSecret"] + # If an id_access_token is not supplied, force usage of v1 + if id_access_token is None: + use_v2 = False + + # Decide which API endpoint URLs to use + bind_data = {"sid": creds["sid"], "client_secret": client_secret, "mxid": mxid} + if use_v2: + bind_url = "https://%s/_matrix/identity/v2/3pid/bind" % (id_server,) + bind_data["id_access_token"] = id_access_token else: - raise SynapseError(400, "No client_secret in creds") + bind_url = "https://%s/_matrix/identity/api/v1/3pid/bind" % (id_server,) try: - data = yield self.http_client.post_json_get_json( - "https://%s%s" % (id_server, "/_matrix/identity/api/v1/3pid/bind"), - {"sid": creds["sid"], "client_secret": client_secret, "mxid": mxid}, - ) + data = yield self.http_client.post_json_get_json(bind_url, bind_data) logger.debug("bound threepid %r to %s", creds, mxid) # Remember where we bound the threepid @@ -131,9 +198,18 @@ class IdentityHandler(BaseHandler): address=data["address"], id_server=id_server, ) + + return data + except HttpResponseException as e: + if e.code != 404 or not use_v2: + logger.error("3PID bind failed with Matrix error: %r", e) + raise e.to_synapse_error() except CodeMessageException as e: data = json.loads(e.msg) # XXX WAT? - return data + return data + + logger.info("Got 404 when POSTing JSON %s, falling back to v1 URL", bind_url) + return (yield self.bind_threepid(creds, mxid, use_v2=False)) @defer.inlineCallbacks def try_unbind_threepid(self, mxid, threepid): @@ -189,6 +265,8 @@ class IdentityHandler(BaseHandler): server doesn't support unbinding """ url = "https://%s/_matrix/identity/api/v1/3pid/unbind" % (id_server,) + url_bytes = "/_matrix/identity/api/v1/3pid/unbind".encode("ascii") + content = { "mxid": mxid, "threepid": {"medium": threepid["medium"], "address": threepid["address"]}, @@ -200,7 +278,7 @@ class IdentityHandler(BaseHandler): auth_headers = self.federation_http_client.build_auth_headers( destination=None, method="POST", - url_bytes="/_matrix/identity/api/v1/3pid/unbind".encode("ascii"), + url_bytes=url_bytes, content=content, destination_is=id_server, ) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index a4be518006..e9cc953bdd 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -542,15 +542,16 @@ class ThreepidRestServlet(RestServlet): def on_POST(self, request): body = parse_json_object_from_request(request) - threePidCreds = body.get("threePidCreds") - threePidCreds = body.get("three_pid_creds", threePidCreds) - if threePidCreds is None: - raise SynapseError(400, "Missing param", Codes.MISSING_PARAM) + threepid_creds = body.get("threePidCreds") or body.get("three_pid_creds") + if threepid_creds is None: + raise SynapseError( + 400, "Missing param three_pid_creds", Codes.MISSING_PARAM + ) requester = yield self.auth.get_user_by_req(request) user_id = requester.user.to_string() - threepid = yield self.identity_handler.threepid_from_creds(threePidCreds) + threepid = yield self.identity_handler.threepid_from_creds(threepid_creds) if not threepid: raise SynapseError(400, "Failed to auth 3pid", Codes.THREEPID_AUTH_FAILED) @@ -566,7 +567,7 @@ class ThreepidRestServlet(RestServlet): if "bind" in body and body["bind"]: logger.debug("Binding threepid %s to %s", threepid, user_id) - yield self.identity_handler.bind_threepid(threePidCreds, user_id) + yield self.identity_handler.bind_threepid(threepid_creds, user_id) return 200, {} From 1d65292e94077390af0ad9c5ee8cd8b0db9b357c Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 5 Sep 2019 14:41:04 +0100 Subject: [PATCH 04/26] Link the send loop with the edus contexts The contexts were being filtered too early so the send loop wasn't being linked to them unless the destination was whitelisted. --- synapse/federation/sender/transaction_manager.py | 11 ++++++++--- synapse/federation/units.py | 3 +++ synapse/handlers/devicemessage.py | 5 +---- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/synapse/federation/sender/transaction_manager.py b/synapse/federation/sender/transaction_manager.py index 62ca6a3e87..42f46394bc 100644 --- a/synapse/federation/sender/transaction_manager.py +++ b/synapse/federation/sender/transaction_manager.py @@ -26,6 +26,7 @@ from synapse.logging.opentracing import ( set_tag, start_active_span_follows_from, tags, + whitelisted_homeserver, ) from synapse.util.metrics import measure_func @@ -59,9 +60,13 @@ class TransactionManager(object): # The span_contexts is a generator so that it won't be evaluated if # opentracing is disabled. (Yay speed!) - span_contexts = ( - extract_text_map(json.loads(edu.get_context())) for edu in pending_edus - ) + span_contexts = [] + keep_destination = whitelisted_homeserver(destination) + + for edu in pending_edus: + span_contexts.append(extract_text_map(json.loads(edu.get_context()))) + if keep_destination: + edu.strip_context() with start_active_span_follows_from("send_transaction", span_contexts): diff --git a/synapse/federation/units.py b/synapse/federation/units.py index aa84621206..b4d743cde7 100644 --- a/synapse/federation/units.py +++ b/synapse/federation/units.py @@ -41,6 +41,9 @@ class Edu(JsonEncodedObject): def get_context(self): return getattr(self, "content", {}).get("org.matrix.opentracing_context", "{}") + def strip_context(self): + getattr(self, "content", {})["org.matrix.opentracing_context"] = "{}" + class Transaction(JsonEncodedObject): """ A transaction is a list of Pdus and Edus to be sent to a remote home diff --git a/synapse/handlers/devicemessage.py b/synapse/handlers/devicemessage.py index 01731cb2d0..0043cbea17 100644 --- a/synapse/handlers/devicemessage.py +++ b/synapse/handlers/devicemessage.py @@ -25,7 +25,6 @@ from synapse.logging.opentracing import ( log_kv, set_tag, start_active_span, - whitelisted_homeserver, ) from synapse.types import UserID, get_domain_from_id from synapse.util.stringutils import random_string @@ -121,9 +120,7 @@ class DeviceMessageHandler(object): "sender": sender_user_id, "type": message_type, "message_id": message_id, - "org.matrix.opentracing_context": json.dumps(context) - if whitelisted_homeserver(destination) - else None, + "org.matrix.opentracing_context": json.dumps(context), } log_kv({"local_messages": local_messages}) From 93bc9d73bfc3fafa1862ba0cc65fd31bfbb1add9 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 5 Sep 2019 14:45:07 +0100 Subject: [PATCH 05/26] newsfile --- changelog.d/5984.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5984.bugfix diff --git a/changelog.d/5984.bugfix b/changelog.d/5984.bugfix new file mode 100644 index 0000000000..98dcfb188e --- /dev/null +++ b/changelog.d/5984.bugfix @@ -0,0 +1 @@ +Link send loop opentracing to edu contexts regardles of destination. From 909827b422eb3396f905a1fb7ad1732f9727d500 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 5 Sep 2019 14:46:04 +0100 Subject: [PATCH 06/26] Add opentracing to all client servlets (#5983) --- changelog.d/5983.feature | 1 + synapse/federation/transport/server.py | 6 +++++- synapse/http/server.py | 13 ++++++++++++- synapse/http/servlet.py | 6 +----- synapse/logging/opentracing.py | 2 +- synapse/replication/http/_base.py | 16 ++++++---------- 6 files changed, 26 insertions(+), 18 deletions(-) create mode 100644 changelog.d/5983.feature diff --git a/changelog.d/5983.feature b/changelog.d/5983.feature new file mode 100644 index 0000000000..aa23ee6dcd --- /dev/null +++ b/changelog.d/5983.feature @@ -0,0 +1 @@ +Add minimum opentracing for client servlets. diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py index f9930b6460..132a8fb5e6 100644 --- a/synapse/federation/transport/server.py +++ b/synapse/federation/transport/server.py @@ -342,7 +342,11 @@ class BaseFederationServlet(object): continue server.register_paths( - method, (pattern,), self._wrap(code), self.__class__.__name__ + method, + (pattern,), + self._wrap(code), + self.__class__.__name__, + trace=False, ) diff --git a/synapse/http/server.py b/synapse/http/server.py index e6f351ba3b..cb9158fe1b 100644 --- a/synapse/http/server.py +++ b/synapse/http/server.py @@ -40,6 +40,7 @@ from synapse.api.errors import ( UnrecognizedRequestError, ) from synapse.logging.context import preserve_fn +from synapse.logging.opentracing import trace_servlet from synapse.util.caches import intern_dict logger = logging.getLogger(__name__) @@ -257,7 +258,9 @@ class JsonResource(HttpServer, resource.Resource): self.path_regexs = {} self.hs = hs - def register_paths(self, method, path_patterns, callback, servlet_classname): + def register_paths( + self, method, path_patterns, callback, servlet_classname, trace=True + ): """ Registers a request handler against a regular expression. Later request URLs are checked against these regular expressions in order to identify an appropriate @@ -273,8 +276,16 @@ class JsonResource(HttpServer, resource.Resource): servlet_classname (str): The name of the handler to be used in prometheus and opentracing logs. + + trace (bool): Whether we should start a span to trace the servlet. """ method = method.encode("utf-8") # method is bytes on py3 + + if trace: + # We don't extract the context from the servlet because we can't + # trust the sender + callback = trace_servlet(servlet_classname)(callback) + for path_pattern in path_patterns: logger.debug("Registering for %s %s", method, path_pattern.pattern) self.path_regexs.setdefault(method, []).append( diff --git a/synapse/http/servlet.py b/synapse/http/servlet.py index c186b31f59..274c1a6a87 100644 --- a/synapse/http/servlet.py +++ b/synapse/http/servlet.py @@ -20,7 +20,6 @@ import logging from canonicaljson import json from synapse.api.errors import Codes, SynapseError -from synapse.logging.opentracing import trace_servlet logger = logging.getLogger(__name__) @@ -298,10 +297,7 @@ class RestServlet(object): servlet_classname = self.__class__.__name__ method_handler = getattr(self, "on_%s" % (method,)) http_server.register_paths( - method, - patterns, - trace_servlet(servlet_classname)(method_handler), - servlet_classname, + method, patterns, method_handler, servlet_classname ) else: diff --git a/synapse/logging/opentracing.py b/synapse/logging/opentracing.py index dbf80e2024..2c34b54702 100644 --- a/synapse/logging/opentracing.py +++ b/synapse/logging/opentracing.py @@ -319,7 +319,7 @@ def whitelisted_homeserver(destination): Args: destination (str) """ - _homeserver_whitelist + if _homeserver_whitelist: return _homeserver_whitelist.match(destination) return False diff --git a/synapse/replication/http/_base.py b/synapse/replication/http/_base.py index c4be9273f6..afc9a8ff29 100644 --- a/synapse/replication/http/_base.py +++ b/synapse/replication/http/_base.py @@ -22,13 +22,13 @@ from six.moves import urllib from twisted.internet import defer -import synapse.logging.opentracing as opentracing from synapse.api.errors import ( CodeMessageException, HttpResponseException, RequestSendFailed, SynapseError, ) +from synapse.logging.opentracing import inject_active_span_byte_dict, trace_servlet from synapse.util.caches.response_cache import ResponseCache from synapse.util.stringutils import random_string @@ -167,9 +167,7 @@ class ReplicationEndpoint(object): # the master, and so whether we should clean up or not. while True: headers = {} - opentracing.inject_active_span_byte_dict( - headers, None, check_destination=False - ) + inject_active_span_byte_dict(headers, None, check_destination=False) try: result = yield request_func(uri, data, headers=headers) break @@ -210,13 +208,11 @@ class ReplicationEndpoint(object): args = "/".join("(?P<%s>[^/]+)" % (arg,) for arg in url_args) pattern = re.compile("^/_synapse/replication/%s/%s$" % (self.NAME, args)) + handler = trace_servlet(self.__class__.__name__, extract_context=True)(handler) + # We don't let register paths trace this servlet using the default tracing + # options because we wish to extract the context explicitly. http_server.register_paths( - method, - [pattern], - opentracing.trace_servlet(self.__class__.__name__, extract_context=True)( - handler - ), - self.__class__.__name__, + method, [pattern], handler, self.__class__.__name__, trace=False ) def _cached_handler(self, request, txn_id, **kwargs): From 5ade977d0836a9d7615b4e8cc578c48a26198ee8 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 5 Sep 2019 15:06:13 +0100 Subject: [PATCH 07/26] Opentracing context cannot be none --- synapse/storage/devices.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/devices.py b/synapse/storage/devices.py index 41f62828bd..79a58df591 100644 --- a/synapse/storage/devices.py +++ b/synapse/storage/devices.py @@ -856,7 +856,7 @@ class DeviceStore(DeviceWorkerStore, BackgroundUpdateStore): "ts": now, "opentracing_context": json.dumps(context) if whitelisted_homeserver(destination) - else None, + else "{}", } for destination in hosts for device_id in device_ids From 7093790fbc805162a5ebb36973aa52313aa1153b Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 5 Sep 2019 15:07:00 +0100 Subject: [PATCH 08/26] Bugfix phrasing Co-Authored-By: Erik Johnston --- changelog.d/5984.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/5984.bugfix b/changelog.d/5984.bugfix index 98dcfb188e..3387bf82bb 100644 --- a/changelog.d/5984.bugfix +++ b/changelog.d/5984.bugfix @@ -1 +1 @@ -Link send loop opentracing to edu contexts regardles of destination. +Fix sending of EDUs when opentracing is enabled with an empty whitelist. From ef20aa52ebb03b322e72a2fc4fefd21a373593a6 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 5 Sep 2019 15:07:17 +0100 Subject: [PATCH 09/26] use access methods (duh..) Co-Authored-By: Erik Johnston --- synapse/federation/sender/transaction_manager.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/synapse/federation/sender/transaction_manager.py b/synapse/federation/sender/transaction_manager.py index 42f46394bc..5b6c79c51a 100644 --- a/synapse/federation/sender/transaction_manager.py +++ b/synapse/federation/sender/transaction_manager.py @@ -64,7 +64,9 @@ class TransactionManager(object): keep_destination = whitelisted_homeserver(destination) for edu in pending_edus: - span_contexts.append(extract_text_map(json.loads(edu.get_context()))) + context = edu.get_context() + if context: + span_contexts.append(extract_text_map(json.loads(context))) if keep_destination: edu.strip_context() From bc604e7f9428a288816c284f6562ed08d2c4c540 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 5 Sep 2019 17:33:29 +0100 Subject: [PATCH 10/26] Gracefully handle log context slips and missing opentracing import errors. (#5988) --- changelog.d/5988.bugfix | 1 + synapse/logging/opentracing.py | 82 ++++++++++++++++++++++++---------- 2 files changed, 59 insertions(+), 24 deletions(-) create mode 100644 changelog.d/5988.bugfix diff --git a/changelog.d/5988.bugfix b/changelog.d/5988.bugfix new file mode 100644 index 0000000000..5c3597cb53 --- /dev/null +++ b/changelog.d/5988.bugfix @@ -0,0 +1 @@ +Fix invalid references to None while opentracing if the log context slips. diff --git a/synapse/logging/opentracing.py b/synapse/logging/opentracing.py index 2c34b54702..8c574ddd28 100644 --- a/synapse/logging/opentracing.py +++ b/synapse/logging/opentracing.py @@ -239,8 +239,7 @@ _homeserver_whitelist = None def only_if_tracing(func): - """Executes the function only if we're tracing. Otherwise return. - Assumes the function wrapped may return None""" + """Executes the function only if we're tracing. Otherwise returns None.""" @wraps(func) def _only_if_tracing_inner(*args, **kwargs): @@ -252,6 +251,41 @@ def only_if_tracing(func): return _only_if_tracing_inner +def ensure_active_span(message, ret=None): + """Executes the operation only if opentracing is enabled and there is an active span. + If there is no active span it logs message at the error level. + + Args: + message (str): Message which fills in "There was no active span when trying to %s" + in the error log if there is no active span and opentracing is enabled. + ret (object): return value if opentracing is None or there is no active span. + + Returns (object): The result of the func or ret if opentracing is disabled or there + was no active span. + """ + + def ensure_active_span_inner_1(func): + @wraps(func) + def ensure_active_span_inner_2(*args, **kwargs): + if not opentracing: + return ret + + if not opentracing.tracer.active_span: + logger.error( + "There was no active span when trying to %s." + " Did you forget to start one or did a context slip?", + message, + ) + + return ret + + return func(*args, **kwargs) + + return ensure_active_span_inner_2 + + return ensure_active_span_inner_1 + + @contextlib.contextmanager def _noop_context_manager(*args, **kwargs): """Does exactly what it says on the tin""" @@ -349,26 +383,24 @@ def start_active_span( if opentracing is None: return _noop_context_manager() - else: - # We need to enter the scope here for the logcontext to become active - return opentracing.tracer.start_active_span( - operation_name, - child_of=child_of, - references=references, - tags=tags, - start_time=start_time, - ignore_active_span=ignore_active_span, - finish_on_close=finish_on_close, - ) + return opentracing.tracer.start_active_span( + operation_name, + child_of=child_of, + references=references, + tags=tags, + start_time=start_time, + ignore_active_span=ignore_active_span, + finish_on_close=finish_on_close, + ) def start_active_span_follows_from(operation_name, contexts): if opentracing is None: return _noop_context_manager() - else: - references = [opentracing.follows_from(context) for context in contexts] - scope = start_active_span(operation_name, references=references) - return scope + + references = [opentracing.follows_from(context) for context in contexts] + scope = start_active_span(operation_name, references=references) + return scope def start_active_span_from_request( @@ -465,19 +497,19 @@ def start_active_span_from_edu( # Opentracing setters for tags, logs, etc -@only_if_tracing +@ensure_active_span("set a tag") def set_tag(key, value): """Sets a tag on the active span""" opentracing.tracer.active_span.set_tag(key, value) -@only_if_tracing +@ensure_active_span("log") def log_kv(key_values, timestamp=None): """Log to the active span""" opentracing.tracer.active_span.log_kv(key_values, timestamp) -@only_if_tracing +@ensure_active_span("set the traces operation name") def set_operation_name(operation_name): """Sets the operation name of the active span""" opentracing.tracer.active_span.set_operation_name(operation_name) @@ -486,7 +518,7 @@ def set_operation_name(operation_name): # Injection and extraction -@only_if_tracing +@ensure_active_span("inject the span into a header") def inject_active_span_twisted_headers(headers, destination, check_destination=True): """ Injects a span context into twisted headers in-place @@ -522,7 +554,7 @@ def inject_active_span_twisted_headers(headers, destination, check_destination=T headers.addRawHeaders(key, value) -@only_if_tracing +@ensure_active_span("inject the span into a byte dict") def inject_active_span_byte_dict(headers, destination, check_destination=True): """ Injects a span context into a dict where the headers are encoded as byte @@ -559,7 +591,7 @@ def inject_active_span_byte_dict(headers, destination, check_destination=True): headers[key.encode()] = [value.encode()] -@only_if_tracing +@ensure_active_span("inject the span into a text map") def inject_active_span_text_map(carrier, destination, check_destination=True): """ Injects a span context into a dict @@ -591,6 +623,7 @@ def inject_active_span_text_map(carrier, destination, check_destination=True): ) +@ensure_active_span("get the active span context as a dict", ret={}) def get_active_span_text_map(destination=None): """ Gets a span context as a dict. This can be used instead of manually @@ -603,7 +636,7 @@ def get_active_span_text_map(destination=None): dict: the active span's context if opentracing is enabled, otherwise empty. """ - if not opentracing or (destination and not whitelisted_homeserver(destination)): + if destination and not whitelisted_homeserver(destination): return {} carrier = {} @@ -614,6 +647,7 @@ def get_active_span_text_map(destination=None): return carrier +@ensure_active_span("get the span context as a string.", ret={}) def active_span_context_as_string(): """ Returns: From f7c873a64355506221648d55d64023c83d43f618 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 5 Sep 2019 17:44:55 +0100 Subject: [PATCH 11/26] Trace how long it takes for the send trasaction to complete, including retrys (#5986) --- changelog.d/5986.feature | 1 + synapse/http/client.py | 83 +++++++++++++++----------- synapse/http/matrixfederationclient.py | 1 - synapse/replication/http/_base.py | 7 ++- 4 files changed, 56 insertions(+), 36 deletions(-) create mode 100644 changelog.d/5986.feature diff --git a/changelog.d/5986.feature b/changelog.d/5986.feature new file mode 100644 index 0000000000..f56aec1b32 --- /dev/null +++ b/changelog.d/5986.feature @@ -0,0 +1 @@ +Trace replication send times. diff --git a/synapse/http/client.py b/synapse/http/client.py index 0ae6db8ea7..51765ae3c0 100644 --- a/synapse/http/client.py +++ b/synapse/http/client.py @@ -46,6 +46,7 @@ from synapse.http import ( redact_uri, ) from synapse.logging.context import make_deferred_yieldable +from synapse.logging.opentracing import set_tag, start_active_span, tags from synapse.util.async_helpers import timeout_deferred from synapse.util.caches import CACHE_SIZE_FACTOR @@ -269,42 +270,56 @@ class SimpleHttpClient(object): # log request but strip `access_token` (AS requests for example include this) logger.info("Sending request %s %s", method, redact_uri(uri)) - try: - body_producer = None - if data is not None: - body_producer = QuieterFileBodyProducer(BytesIO(data)) + with start_active_span( + "outgoing-client-request", + tags={ + tags.SPAN_KIND: tags.SPAN_KIND_RPC_CLIENT, + tags.HTTP_METHOD: method, + tags.HTTP_URL: uri, + }, + finish_on_close=True, + ): + try: + body_producer = None + if data is not None: + body_producer = QuieterFileBodyProducer(BytesIO(data)) - request_deferred = treq.request( - method, - uri, - agent=self.agent, - data=body_producer, - headers=headers, - **self._extra_treq_args - ) - request_deferred = timeout_deferred( - request_deferred, - 60, - self.hs.get_reactor(), - cancelled_to_request_timed_out_error, - ) - response = yield make_deferred_yieldable(request_deferred) + request_deferred = treq.request( + method, + uri, + agent=self.agent, + data=body_producer, + headers=headers, + **self._extra_treq_args + ) + request_deferred = timeout_deferred( + request_deferred, + 60, + self.hs.get_reactor(), + cancelled_to_request_timed_out_error, + ) + response = yield make_deferred_yieldable(request_deferred) - incoming_responses_counter.labels(method, response.code).inc() - logger.info( - "Received response to %s %s: %s", method, redact_uri(uri), response.code - ) - return response - except Exception as e: - incoming_responses_counter.labels(method, "ERR").inc() - logger.info( - "Error sending request to %s %s: %s %s", - method, - redact_uri(uri), - type(e).__name__, - e.args[0], - ) - raise + incoming_responses_counter.labels(method, response.code).inc() + logger.info( + "Received response to %s %s: %s", + method, + redact_uri(uri), + response.code, + ) + return response + except Exception as e: + incoming_responses_counter.labels(method, "ERR").inc() + logger.info( + "Error sending request to %s %s: %s %s", + method, + redact_uri(uri), + type(e).__name__, + e.args[0], + ) + set_tag(tags.ERROR, True) + set_tag("error_reason", e.args[0]) + raise @defer.inlineCallbacks def post_urlencoded_get_json(self, uri, args={}, headers=None): diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index 4326e98a28..3f7c93ffcb 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -345,7 +345,6 @@ class MatrixFederationHttpClient(object): else: query_bytes = b"" - # Retreive current span scope = start_active_span( "outgoing-federation-request", tags={ diff --git a/synapse/replication/http/_base.py b/synapse/replication/http/_base.py index afc9a8ff29..03560c1f0e 100644 --- a/synapse/replication/http/_base.py +++ b/synapse/replication/http/_base.py @@ -28,7 +28,11 @@ from synapse.api.errors import ( RequestSendFailed, SynapseError, ) -from synapse.logging.opentracing import inject_active_span_byte_dict, trace_servlet +from synapse.logging.opentracing import ( + inject_active_span_byte_dict, + trace, + trace_servlet, +) from synapse.util.caches.response_cache import ResponseCache from synapse.util.stringutils import random_string @@ -129,6 +133,7 @@ class ReplicationEndpoint(object): client = hs.get_simple_http_client() + @trace(opname="outgoing_replication_request") @defer.inlineCallbacks def send_request(**kwargs): data = yield cls._serialize_payload(**kwargs) From d8517da85b3057e186087de146f546de52d7e206 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 6 Sep 2019 10:07:12 +0100 Subject: [PATCH 12/26] Don't assume there is a 'self' arg in @trace decorator --- synapse/logging/opentracing.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/synapse/logging/opentracing.py b/synapse/logging/opentracing.py index 8c574ddd28..7246253018 100644 --- a/synapse/logging/opentracing.py +++ b/synapse/logging/opentracing.py @@ -702,15 +702,15 @@ def trace(func=None, opname=None): _opname = opname if opname else func.__name__ @wraps(func) - def _trace_inner(self, *args, **kwargs): + def _trace_inner(*args, **kwargs): if opentracing is None: - return func(self, *args, **kwargs) + return func(*args, **kwargs) scope = start_active_span(_opname) scope.__enter__() try: - result = func(self, *args, **kwargs) + result = func(*args, **kwargs) if isinstance(result, defer.Deferred): def call_back(result): @@ -750,13 +750,13 @@ def tag_args(func): return func @wraps(func) - def _tag_args_inner(self, *args, **kwargs): + def _tag_args_inner(*args, **kwargs): argspec = inspect.getargspec(func) for i, arg in enumerate(argspec.args[1:]): set_tag("ARG_" + arg, args[i]) set_tag("args", args[len(argspec.args) :]) set_tag("kwargs", kwargs) - return func(self, *args, **kwargs) + return func(*args, **kwargs) return _tag_args_inner From 4bc6b7130d60005d21ce9be16682c13e90447430 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 6 Sep 2019 10:13:10 +0100 Subject: [PATCH 13/26] Newsfile --- changelog.d/5991.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5991.bugfix diff --git a/changelog.d/5991.bugfix b/changelog.d/5991.bugfix new file mode 100644 index 0000000000..b5ba2f9e46 --- /dev/null +++ b/changelog.d/5991.bugfix @@ -0,0 +1 @@ +Fix broken replication requests when opentracing is enabled. From e5baf80237b1ac2dbaac43b623ab75d1b5c8185a Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 6 Sep 2019 10:53:05 +0100 Subject: [PATCH 14/26] Update changelog --- changelog.d/5991.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/5991.bugfix b/changelog.d/5991.bugfix index b5ba2f9e46..5c3597cb53 100644 --- a/changelog.d/5991.bugfix +++ b/changelog.d/5991.bugfix @@ -1 +1 @@ -Fix broken replication requests when opentracing is enabled. +Fix invalid references to None while opentracing if the log context slips. From 0c0b82b6d18102694f9ff1c40b94e5dd124c21d8 Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Fri, 6 Sep 2019 11:35:28 +0100 Subject: [PATCH 15/26] Allow Synapse to send registration emails + choose Synapse or an external server to handle 3pid validation (#5987) This is a combination of a few different PRs, finally all being merged into `develop`: * #5875 * #5876 * #5868 (This one added the `/versions` flag but the flag itself was actually [backed out](https://github.com/matrix-org/synapse/commit/891afb57cbdf9867f2848341b29c75d6f35eef5a#diff-e591d42d30690ffb79f63bb726200891) in #5969. What's left is just giving /versions access to the config file, which could be useful in the future) * #5835 * #5969 * #5940 Clients should not actually use the new registration functionality until https://github.com/matrix-org/synapse/pull/5972 is merged. UPGRADE.rst, changelog entries and config file changes should all be reviewed closely before this PR is merged. --- UPGRADE.rst | 63 +++++ changelog.d/5835.feature | 1 + changelog.d/5868.feature | 1 + changelog.d/5875.misc | 1 + changelog.d/5876.feature | 1 + changelog.d/5940.feature | 1 + changelog.d/5969.feature | 1 + contrib/cmdclient/console.py | 2 + docs/sample_config.yaml | 56 +++- synapse/app/client_reader.py | 2 +- synapse/config/emailconfig.py | 122 ++++++-- synapse/config/registration.py | 36 +++ synapse/handlers/account_validity.py | 12 +- synapse/handlers/auth.py | 34 +-- synapse/handlers/identity.py | 178 +++++++++--- synapse/push/mailer.py | 83 ++++-- synapse/push/pusher.py | 17 +- synapse/res/templates/password_reset.html | 2 +- synapse/res/templates/password_reset.txt | 4 +- .../res/templates/password_reset_failure.html | 4 +- synapse/res/templates/registration.html | 11 + synapse/res/templates/registration.txt | 10 + .../res/templates/registration_failure.html | 6 + .../res/templates/registration_success.html | 6 + synapse/rest/__init__.py | 2 +- synapse/rest/client/v2_alpha/_base.py | 2 + synapse/rest/client/v2_alpha/account.py | 264 ++++++++---------- synapse/rest/client/v2_alpha/register.py | 196 ++++++++++++- synapse/rest/client/versions.py | 8 +- 29 files changed, 822 insertions(+), 304 deletions(-) create mode 100644 changelog.d/5835.feature create mode 100644 changelog.d/5868.feature create mode 100644 changelog.d/5875.misc create mode 100644 changelog.d/5876.feature create mode 100644 changelog.d/5940.feature create mode 100644 changelog.d/5969.feature create mode 100644 synapse/res/templates/registration.html create mode 100644 synapse/res/templates/registration.txt create mode 100644 synapse/res/templates/registration_failure.html create mode 100644 synapse/res/templates/registration_success.html diff --git a/UPGRADE.rst b/UPGRADE.rst index cf228c7c52..dddcd75fda 100644 --- a/UPGRADE.rst +++ b/UPGRADE.rst @@ -49,6 +49,56 @@ returned by the Client-Server API: # configured on port 443. curl -kv https:///_matrix/client/versions 2>&1 | grep "Server:" +Upgrading to v1.4.0 +=================== + +Config options +-------------- + +**Note: Registration by email address or phone number will not work in this release unless +some config options are changed from their defaults.** + +This is due to Synapse v1.4.0 now defaulting to sending registration and password reset tokens +itself. This is for security reasons as well as putting less reliance on identity servers. +However, currently Synapse only supports sending emails, and does not have support for +phone-based password reset or account registration. If Synapse is configured to handle these on +its own, phone-based password resets and registration will be disabled. For Synapse to send +emails, the ``email`` block of the config must be filled out. If not, then password resets and +registration via email will be disabled entirely. + +This release also deprecates the ``email.trust_identity_server_for_password_resets`` option and +replaces it with the ``account_threepid_delegates`` dictionary. This option defines whether the +homeserver should delegate an external server (typically an `identity server +`_) to handle sending password reset or +registration messages via email and SMS. + +If ``email.trust_identity_server_for_password_resets`` is set to ``true``, and +``account_threepid_delegates.email`` is not set, then the first entry in +``trusted_third_party_id_servers`` will be used as the account threepid delegate for email. +This is to ensure compatibility with existing Synapse installs that set up external server +handling for these tasks before v1.4.0. If ``email.trust_identity_server_for_password_resets`` +is ``true`` and no trusted identity server domains are configured, Synapse will throw an error. + +If ``email.trust_identity_server_for_password_resets`` is ``false`` or absent and a threepid +type in ``account_threepid_delegates`` is not set to a domain, then Synapse will attempt to +send password reset and registration messages for that type. + +Email templates +--------------- + +If you have configured a custom template directory with the ``email.template_dir`` option, be +aware that there are new templates regarding registration. ``registration.html`` and +``registration.txt`` have been added and contain the content that is sent to a client upon +registering via an email address. + +``registration_success.html`` and ``registration_failure.html`` are also new HTML templates +that will be shown to the user when they click the link in their registration emai , either +showing them a success or failure page (assuming a redirect URL is not configured). + +Synapse will expect these files to exist inside the configured template directory. To view the +default templates, see `synapse/res/templates +`_. + Upgrading to v1.2.0 =================== @@ -132,6 +182,19 @@ server for password resets, set ``trust_identity_server_for_password_resets`` to See the `sample configuration file `_ for more details on these settings. +New email templates +--------------- +Some new templates have been added to the default template directory for the purpose of the +homeserver sending its own password reset emails. If you have configured a custom +``template_dir`` in your Synapse config, these files will need to be added. + +``password_reset.html`` and ``password_reset.txt`` are HTML and plain text templates +respectively that contain the contents of what will be emailed to the user upon attempting to +reset their password via email. ``password_reset_success.html`` and +``password_reset_failure.html`` are HTML files that the content of which (assuming no redirect +URL is set) will be shown to the user after they attempt to click the link in the email sent +to them. + Upgrading to v0.99.0 ==================== diff --git a/changelog.d/5835.feature b/changelog.d/5835.feature new file mode 100644 index 0000000000..3e8bf5068d --- /dev/null +++ b/changelog.d/5835.feature @@ -0,0 +1 @@ +Add the ability to send registration emails from the homeserver rather than delegating to an identity server. diff --git a/changelog.d/5868.feature b/changelog.d/5868.feature new file mode 100644 index 0000000000..69605c1ae1 --- /dev/null +++ b/changelog.d/5868.feature @@ -0,0 +1 @@ +Add `m.require_identity_server` key to `/versions`'s `unstable_features` section. \ No newline at end of file diff --git a/changelog.d/5875.misc b/changelog.d/5875.misc new file mode 100644 index 0000000000..e188c28d2f --- /dev/null +++ b/changelog.d/5875.misc @@ -0,0 +1 @@ +Deprecate the `trusted_third_party_id_servers` option. \ No newline at end of file diff --git a/changelog.d/5876.feature b/changelog.d/5876.feature new file mode 100644 index 0000000000..df88193fbd --- /dev/null +++ b/changelog.d/5876.feature @@ -0,0 +1 @@ +Replace `trust_identity_server_for_password_resets` config option with `account_threepid_delegates`. \ No newline at end of file diff --git a/changelog.d/5940.feature b/changelog.d/5940.feature new file mode 100644 index 0000000000..5b69b97fe7 --- /dev/null +++ b/changelog.d/5940.feature @@ -0,0 +1 @@ +Add the ability to send registration emails from the homeserver rather than delegating to an identity server. \ No newline at end of file diff --git a/changelog.d/5969.feature b/changelog.d/5969.feature new file mode 100644 index 0000000000..cf603fa0c6 --- /dev/null +++ b/changelog.d/5969.feature @@ -0,0 +1 @@ +Replace `trust_identity_server_for_password_resets` config option with `account_threepid_delegates`. diff --git a/contrib/cmdclient/console.py b/contrib/cmdclient/console.py index 899c650b0c..48da410d94 100755 --- a/contrib/cmdclient/console.py +++ b/contrib/cmdclient/console.py @@ -37,6 +37,8 @@ from signedjson.sign import verify_signed_json, SignatureVerifyException CONFIG_JSON = "cmdclient_config.json" +# TODO: The concept of trusted identity servers has been deprecated. This option and checks +# should be removed TRUSTED_ID_SERVERS = ["localhost:8001"] diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 43969bbb70..186cdbedd2 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -891,10 +891,42 @@ uploads_path: "DATADIR/uploads" # Also defines the ID server which will be called when an account is # deactivated (one will be picked arbitrarily). # +# Note: This option is deprecated. Since v0.99.4, Synapse has tracked which identity +# server a 3PID has been bound to. For 3PIDs bound before then, Synapse runs a +# background migration script, informing itself that the identity server all of its +# 3PIDs have been bound to is likely one of the below. +# +# As of Synapse v1.4.0, all other functionality of this option has been deprecated, and +# it is now solely used for the purposes of the background migration script, and can be +# removed once it has run. #trusted_third_party_id_servers: # - matrix.org # - vector.im +# Handle threepid (email/phone etc) registration and password resets through a set of +# *trusted* identity servers. Note that this allows the configured identity server to +# reset passwords for accounts! +# +# Be aware that if `email` is not set, and SMTP options have not been +# configured in the email config block, registration and user password resets via +# email will be globally disabled. +# +# Additionally, if `msisdn` is not set, registration and password resets via msisdn +# will be disabled regardless. This is due to Synapse currently not supporting any +# method of sending SMS messages on its own. +# +# To enable using an identity server for operations regarding a particular third-party +# identifier type, set the value to the URL of that identity server as shown in the +# examples below. +# +# Servers handling the these requests must answer the `/requestToken` endpoints defined +# by the Matrix Identity Service API specification: +# https://matrix.org/docs/spec/identity_service/latest +# +account_threepid_delegates: + #email: https://example.com # Delegate email sending to matrix.org + #msisdn: http://localhost:8090 # Delegate SMS sending to this local process + # Users who register on this homeserver will automatically be joined # to these rooms # @@ -1164,19 +1196,6 @@ password_config: # # # riot_base_url: "http://localhost/riot" # -# # Enable sending password reset emails via the configured, trusted -# # identity servers -# # -# # IMPORTANT! This will give a malicious or overtaken identity server -# # the ability to reset passwords for your users! Make absolutely sure -# # that you want to do this! It is strongly recommended that password -# # reset emails be sent by the homeserver instead -# # -# # If this option is set to false and SMTP options have not been -# # configured, resetting user passwords via email will be disabled -# # -# #trust_identity_server_for_password_resets: false -# # # Configure the time that a validation email or text message code # # will expire after sending # # @@ -1208,11 +1227,22 @@ password_config: # #password_reset_template_html: password_reset.html # #password_reset_template_text: password_reset.txt # +# # Templates for registration emails sent by the homeserver +# # +# #registration_template_html: registration.html +# #registration_template_text: registration.txt +# # # Templates for password reset success and failure pages that a user # # will see after attempting to reset their password # # # #password_reset_template_success_html: password_reset_success.html # #password_reset_template_failure_html: password_reset_failure.html +# +# # Templates for registration success and failure pages that a user +# # will see after attempting to register using an email or phone +# # +# #registration_template_success_html: registration_success.html +# #registration_template_failure_html: registration_failure.html #password_providers: diff --git a/synapse/app/client_reader.py b/synapse/app/client_reader.py index 86193d35a8..dbcc414c42 100644 --- a/synapse/app/client_reader.py +++ b/synapse/app/client_reader.py @@ -119,7 +119,7 @@ class ClientReaderServer(HomeServer): KeyChangesServlet(self).register(resource) VoipRestServlet(self).register(resource) PushRuleRestServlet(self).register(resource) - VersionsRestServlet().register(resource) + VersionsRestServlet(self).register(resource) resources.update({"/_matrix/client": resource}) diff --git a/synapse/config/emailconfig.py b/synapse/config/emailconfig.py index f83c05df44..e5de768b0c 100644 --- a/synapse/config/emailconfig.py +++ b/synapse/config/emailconfig.py @@ -20,6 +20,7 @@ from __future__ import print_function # This file can't be called email.py because if it is, we cannot: import email.utils import os +from enum import Enum import pkg_resources @@ -74,19 +75,48 @@ class EmailConfig(Config): "renew_at" ) - email_trust_identity_server_for_password_resets = email_config.get( - "trust_identity_server_for_password_resets", False + self.threepid_behaviour_email = ( + # Have Synapse handle the email sending if account_threepid_delegates.email + # is not defined + # msisdn is currently always remote while Synapse does not support any method of + # sending SMS messages + ThreepidBehaviour.REMOTE + if self.account_threepid_delegate_email + else ThreepidBehaviour.LOCAL ) - self.email_password_reset_behaviour = ( - "remote" if email_trust_identity_server_for_password_resets else "local" - ) - self.password_resets_were_disabled_due_to_email_config = False - if self.email_password_reset_behaviour == "local" and email_config == {}: + # Prior to Synapse v1.4.0, there was another option that defined whether Synapse would + # use an identity server to password reset tokens on its behalf. We now warn the user + # if they have this set and tell them to use the updated option, while using a default + # identity server in the process. + self.using_identity_server_from_trusted_list = False + if ( + not self.account_threepid_delegate_email + and config.get("trust_identity_server_for_password_resets", False) is True + ): + # Use the first entry in self.trusted_third_party_id_servers instead + if self.trusted_third_party_id_servers: + # XXX: It's a little confusing that account_threepid_delegate_email is modified + # both in RegistrationConfig and here. We should factor this bit out + self.account_threepid_delegate_email = self.trusted_third_party_id_servers[ + 0 + ] + self.using_identity_server_from_trusted_list = True + else: + raise ConfigError( + "Attempted to use an identity server from" + '"trusted_third_party_id_servers" but it is empty.' + ) + + self.local_threepid_handling_disabled_due_to_email_config = False + if ( + self.threepid_behaviour_email == ThreepidBehaviour.LOCAL + and email_config == {} + ): # We cannot warn the user this has happened here # Instead do so when a user attempts to reset their password - self.password_resets_were_disabled_due_to_email_config = True + self.local_threepid_handling_disabled_due_to_email_config = True - self.email_password_reset_behaviour = "off" + self.threepid_behaviour_email = ThreepidBehaviour.OFF # Get lifetime of a validation token in milliseconds self.email_validation_token_lifetime = self.parse_duration( @@ -96,7 +126,7 @@ class EmailConfig(Config): if ( self.email_enable_notifs or account_validity_renewal_enabled - or self.email_password_reset_behaviour == "local" + or self.threepid_behaviour_email == ThreepidBehaviour.LOCAL ): # make sure we can import the required deps import jinja2 @@ -106,7 +136,7 @@ class EmailConfig(Config): jinja2 bleach - if self.email_password_reset_behaviour == "local": + if self.threepid_behaviour_email == ThreepidBehaviour.LOCAL: required = ["smtp_host", "smtp_port", "notif_from"] missing = [] @@ -125,28 +155,45 @@ class EmailConfig(Config): % (", ".join(missing),) ) - # Templates for password reset emails + # These email templates have placeholders in them, and thus must be + # parsed using a templating engine during a request self.email_password_reset_template_html = email_config.get( "password_reset_template_html", "password_reset.html" ) self.email_password_reset_template_text = email_config.get( "password_reset_template_text", "password_reset.txt" ) + self.email_registration_template_html = email_config.get( + "registration_template_html", "registration.html" + ) + self.email_registration_template_text = email_config.get( + "registration_template_text", "registration.txt" + ) self.email_password_reset_template_failure_html = email_config.get( "password_reset_template_failure_html", "password_reset_failure.html" ) - # This template does not support any replaceable variables, so we will - # read it from the disk once during setup + self.email_registration_template_failure_html = email_config.get( + "registration_template_failure_html", "registration_failure.html" + ) + + # These templates do not support any placeholder variables, so we + # will read them from disk once during setup email_password_reset_template_success_html = email_config.get( "password_reset_template_success_html", "password_reset_success.html" ) + email_registration_template_success_html = email_config.get( + "registration_template_success_html", "registration_success.html" + ) # Check templates exist for f in [ self.email_password_reset_template_html, self.email_password_reset_template_text, + self.email_registration_template_html, + self.email_registration_template_text, self.email_password_reset_template_failure_html, email_password_reset_template_success_html, + email_registration_template_success_html, ]: p = os.path.join(self.email_template_dir, f) if not os.path.isfile(p): @@ -156,9 +203,15 @@ class EmailConfig(Config): filepath = os.path.join( self.email_template_dir, email_password_reset_template_success_html ) - self.email_password_reset_template_success_html_content = self.read_file( + self.email_password_reset_template_success_html = self.read_file( filepath, "email.password_reset_template_success_html" ) + filepath = os.path.join( + self.email_template_dir, email_registration_template_success_html + ) + self.email_registration_template_success_html_content = self.read_file( + filepath, "email.registration_template_success_html" + ) if self.email_enable_notifs: required = [ @@ -239,19 +292,6 @@ class EmailConfig(Config): # # # riot_base_url: "http://localhost/riot" # - # # Enable sending password reset emails via the configured, trusted - # # identity servers - # # - # # IMPORTANT! This will give a malicious or overtaken identity server - # # the ability to reset passwords for your users! Make absolutely sure - # # that you want to do this! It is strongly recommended that password - # # reset emails be sent by the homeserver instead - # # - # # If this option is set to false and SMTP options have not been - # # configured, resetting user passwords via email will be disabled - # # - # #trust_identity_server_for_password_resets: false - # # # Configure the time that a validation email or text message code # # will expire after sending # # @@ -283,9 +323,35 @@ class EmailConfig(Config): # #password_reset_template_html: password_reset.html # #password_reset_template_text: password_reset.txt # + # # Templates for registration emails sent by the homeserver + # # + # #registration_template_html: registration.html + # #registration_template_text: registration.txt + # # # Templates for password reset success and failure pages that a user # # will see after attempting to reset their password # # # #password_reset_template_success_html: password_reset_success.html # #password_reset_template_failure_html: password_reset_failure.html + # + # # Templates for registration success and failure pages that a user + # # will see after attempting to register using an email or phone + # # + # #registration_template_success_html: registration_success.html + # #registration_template_failure_html: registration_failure.html """ + + +class ThreepidBehaviour(Enum): + """ + Enum to define the behaviour of Synapse with regards to when it contacts an identity + server for 3pid registration and password resets + + REMOTE = use an external server to send tokens + LOCAL = send tokens ourselves + OFF = disable registration via 3pid and password resets + """ + + REMOTE = "remote" + LOCAL = "local" + OFF = "off" diff --git a/synapse/config/registration.py b/synapse/config/registration.py index e2bee3c116..9548560edb 100644 --- a/synapse/config/registration.py +++ b/synapse/config/registration.py @@ -99,6 +99,10 @@ class RegistrationConfig(Config): self.trusted_third_party_id_servers = config.get( "trusted_third_party_id_servers", ["matrix.org", "vector.im"] ) + account_threepid_delegates = config.get("account_threepid_delegates") or {} + self.account_threepid_delegate_email = account_threepid_delegates.get("email") + self.account_threepid_delegate_msisdn = account_threepid_delegates.get("msisdn") + self.default_identity_server = config.get("default_identity_server") self.allow_guest_access = config.get("allow_guest_access", False) @@ -257,10 +261,42 @@ class RegistrationConfig(Config): # Also defines the ID server which will be called when an account is # deactivated (one will be picked arbitrarily). # + # Note: This option is deprecated. Since v0.99.4, Synapse has tracked which identity + # server a 3PID has been bound to. For 3PIDs bound before then, Synapse runs a + # background migration script, informing itself that the identity server all of its + # 3PIDs have been bound to is likely one of the below. + # + # As of Synapse v1.4.0, all other functionality of this option has been deprecated, and + # it is now solely used for the purposes of the background migration script, and can be + # removed once it has run. #trusted_third_party_id_servers: # - matrix.org # - vector.im + # Handle threepid (email/phone etc) registration and password resets through a set of + # *trusted* identity servers. Note that this allows the configured identity server to + # reset passwords for accounts! + # + # Be aware that if `email` is not set, and SMTP options have not been + # configured in the email config block, registration and user password resets via + # email will be globally disabled. + # + # Additionally, if `msisdn` is not set, registration and password resets via msisdn + # will be disabled regardless. This is due to Synapse currently not supporting any + # method of sending SMS messages on its own. + # + # To enable using an identity server for operations regarding a particular third-party + # identifier type, set the value to the URL of that identity server as shown in the + # examples below. + # + # Servers handling the these requests must answer the `/requestToken` endpoints defined + # by the Matrix Identity Service API specification: + # https://matrix.org/docs/spec/identity_service/latest + # + account_threepid_delegates: + #email: https://example.com # Delegate email sending to matrix.org + #msisdn: http://localhost:8090 # Delegate SMS sending to this local process + # Users who register on this homeserver will automatically be joined # to these rooms # diff --git a/synapse/handlers/account_validity.py b/synapse/handlers/account_validity.py index 34574f1a12..d04e0fe576 100644 --- a/synapse/handlers/account_validity.py +++ b/synapse/handlers/account_validity.py @@ -38,6 +38,7 @@ logger = logging.getLogger(__name__) class AccountValidityHandler(object): def __init__(self, hs): self.hs = hs + self.config = hs.config self.store = self.hs.get_datastore() self.sendmail = self.hs.get_sendmail() self.clock = self.hs.get_clock() @@ -62,9 +63,14 @@ class AccountValidityHandler(object): self._raw_from = email.utils.parseaddr(self._from_string)[1] self._template_html, self._template_text = load_jinja2_templates( - config=self.hs.config, - template_html_name=self.hs.config.email_expiry_template_html, - template_text_name=self.hs.config.email_expiry_template_text, + self.config.email_template_dir, + [ + self.config.email_expiry_template_html, + self.config.email_expiry_template_text, + ], + apply_format_ts_filter=True, + apply_mxc_to_http_filter=True, + public_baseurl=self.config.public_baseurl, ) # Check the renewal emails to send and send them every 30min. diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index f844409d21..d0c0142740 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -38,6 +38,7 @@ from synapse.api.errors import ( UserDeactivatedError, ) from synapse.api.ratelimiting import Ratelimiter +from synapse.config.emailconfig import ThreepidBehaviour from synapse.logging.context import defer_to_thread from synapse.module_api import ModuleApi from synapse.types import UserID @@ -158,7 +159,7 @@ class AuthHandler(BaseHandler): return params @defer.inlineCallbacks - def check_auth(self, flows, clientdict, clientip, password_servlet=False): + def check_auth(self, flows, clientdict, clientip): """ Takes a dictionary sent by the client in the login / registration protocol and handles the User-Interactive Auth flow. @@ -182,16 +183,6 @@ class AuthHandler(BaseHandler): clientip (str): The IP address of the client. - password_servlet (bool): Whether the request originated from - PasswordRestServlet. - XXX: This is a temporary hack to distinguish between checking - for threepid validations locally (in the case of password - resets) and using the identity server (in the case of binding - a 3PID during registration). Once we start using the - homeserver for both tasks, this distinction will no longer be - necessary. - - Returns: defer.Deferred[dict, dict, str]: a deferred tuple of (creds, params, session_id). @@ -247,9 +238,7 @@ class AuthHandler(BaseHandler): if "type" in authdict: login_type = authdict["type"] try: - result = yield self._check_auth_dict( - authdict, clientip, password_servlet=password_servlet - ) + result = yield self._check_auth_dict(authdict, clientip) if result: creds[login_type] = result self._save_session(session) @@ -356,7 +345,7 @@ class AuthHandler(BaseHandler): return sess.setdefault("serverdict", {}).get(key, default) @defer.inlineCallbacks - def _check_auth_dict(self, authdict, clientip, password_servlet=False): + def _check_auth_dict(self, authdict, clientip): """Attempt to validate the auth dict provided by a client Args: @@ -374,11 +363,7 @@ class AuthHandler(BaseHandler): login_type = authdict["type"] checker = self.checkers.get(login_type) if checker is not None: - # XXX: Temporary workaround for having Synapse handle password resets - # See AuthHandler.check_auth for further details - res = yield checker( - authdict, clientip=clientip, password_servlet=password_servlet - ) + res = yield checker(authdict, clientip=clientip) return res # build a v1-login-style dict out of the authdict and fall back to the @@ -449,7 +434,7 @@ class AuthHandler(BaseHandler): return defer.succeed(True) @defer.inlineCallbacks - def _check_threepid(self, medium, authdict, password_servlet=False, **kwargs): + def _check_threepid(self, medium, authdict, **kwargs): if "threepid_creds" not in authdict: raise LoginError(400, "Missing threepid_creds", Codes.MISSING_PARAM) @@ -458,12 +443,9 @@ class AuthHandler(BaseHandler): identity_handler = self.hs.get_handlers().identity_handler logger.info("Getting validated threepid. threepidcreds: %r", (threepid_creds,)) - if ( - not password_servlet - or self.hs.config.email_password_reset_behaviour == "remote" - ): + if self.hs.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE: threepid = yield identity_handler.threepid_from_creds(threepid_creds) - elif self.hs.config.email_password_reset_behaviour == "local": + elif self.hs.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL: row = yield self.store.get_threepid_validation_session( medium, threepid_creds["client_secret"], diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py index 583b612dd9..71b5a87392 100644 --- a/synapse/handlers/identity.py +++ b/synapse/handlers/identity.py @@ -29,6 +29,7 @@ from synapse.api.errors import ( HttpResponseException, SynapseError, ) +from synapse.util.stringutils import random_string from ._base import BaseHandler @@ -41,25 +42,7 @@ class IdentityHandler(BaseHandler): self.http_client = hs.get_simple_http_client() self.federation_http_client = hs.get_http_client() - - self.trusted_id_servers = set(hs.config.trusted_third_party_id_servers) - self.trust_any_id_server_just_for_testing_do_not_use = ( - hs.config.use_insecure_ssl_client_just_for_testing_do_not_use - ) - - def _should_trust_id_server(self, id_server): - if id_server not in self.trusted_id_servers: - if self.trust_any_id_server_just_for_testing_do_not_use: - logger.warn( - "Trusting untrustworthy ID server %r even though it isn't" - " in the trusted id list for testing because" - " 'use_insecure_ssl_client_just_for_testing_do_not_use'" - " is set in the config", - id_server, - ) - else: - return False - return True + self.hs = hs def _extract_items_from_creds_dict(self, creds): """ @@ -132,13 +115,6 @@ class IdentityHandler(BaseHandler): "/_matrix/identity/api/v1/3pid/getValidated3pid", ) - if not self._should_trust_id_server(id_server): - logger.warn( - "%s is not a trusted ID server: rejecting 3pid " + "credentials", - id_server, - ) - return None - try: data = yield self.http_client.get_json(url, query_params) return data if "medium" in data else None @@ -305,28 +281,122 @@ class IdentityHandler(BaseHandler): return changed + @defer.inlineCallbacks + def send_threepid_validation( + self, + email_address, + client_secret, + send_attempt, + send_email_func, + next_link=None, + ): + """Send a threepid validation email for password reset or + registration purposes + + Args: + email_address (str): The user's email address + client_secret (str): The provided client secret + send_attempt (int): Which send attempt this is + send_email_func (func): A function that takes an email address, token, + client_secret and session_id, sends an email + and returns a Deferred. + next_link (str|None): The URL to redirect the user to after validation + + Returns: + The new session_id upon success + + Raises: + SynapseError is an error occurred when sending the email + """ + # Check that this email/client_secret/send_attempt combo is new or + # greater than what we've seen previously + session = yield self.store.get_threepid_validation_session( + "email", client_secret, address=email_address, validated=False + ) + + # Check to see if a session already exists and that it is not yet + # marked as validated + if session and session.get("validated_at") is None: + session_id = session["session_id"] + last_send_attempt = session["last_send_attempt"] + + # Check that the send_attempt is higher than previous attempts + if send_attempt <= last_send_attempt: + # If not, just return a success without sending an email + return session_id + else: + # An non-validated session does not exist yet. + # Generate a session id + session_id = random_string(16) + + # Generate a new validation token + token = random_string(32) + + # Send the mail with the link containing the token, client_secret + # and session_id + try: + yield send_email_func(email_address, token, client_secret, session_id) + except Exception: + logger.exception( + "Error sending threepid validation email to %s", email_address + ) + raise SynapseError(500, "An error was encountered when sending the email") + + token_expires = ( + self.hs.clock.time_msec() + self.hs.config.email_validation_token_lifetime + ) + + yield self.store.start_or_continue_validation_session( + "email", + email_address, + session_id, + client_secret, + send_attempt, + next_link, + token, + token_expires, + ) + + return session_id + @defer.inlineCallbacks def requestEmailToken( self, id_server, email, client_secret, send_attempt, next_link=None ): - if not self._should_trust_id_server(id_server): - raise SynapseError( - 400, "Untrusted ID server '%s'" % id_server, Codes.SERVER_NOT_TRUSTED - ) + """ + Request an external server send an email on our behalf for the purposes of threepid + validation. + Args: + id_server (str): The identity server to proxy to + email (str): The email to send the message to + client_secret (str): The unique client_secret sends by the user + send_attempt (int): Which attempt this is + next_link: A link to redirect the user to once they submit the token + + Returns: + The json response body from the server + """ params = { "email": email, "client_secret": client_secret, "send_attempt": send_attempt, } - if next_link: - params.update({"next_link": next_link}) + params["next_link"] = next_link + + if self.hs.config.using_identity_server_from_trusted_list: + # Warn that a deprecated config option is in use + logger.warn( + 'The config option "trust_identity_server_for_password_resets" ' + 'has been replaced by "account_threepid_delegate". ' + "Please consult the sample config at docs/sample_config.yaml for " + "details and update your config file." + ) try: data = yield self.http_client.post_json_get_json( - "https://%s%s" - % (id_server, "/_matrix/identity/api/v1/validate/email/requestToken"), + id_server + "/_matrix/identity/api/v1/validate/email/requestToken", params, ) return data @@ -336,25 +406,49 @@ class IdentityHandler(BaseHandler): @defer.inlineCallbacks def requestMsisdnToken( - self, id_server, country, phone_number, client_secret, send_attempt, **kwargs + self, + id_server, + country, + phone_number, + client_secret, + send_attempt, + next_link=None, ): - if not self._should_trust_id_server(id_server): - raise SynapseError( - 400, "Untrusted ID server '%s'" % id_server, Codes.SERVER_NOT_TRUSTED - ) + """ + Request an external server send an SMS message on our behalf for the purposes of + threepid validation. + Args: + id_server (str): The identity server to proxy to + country (str): The country code of the phone number + phone_number (str): The number to send the message to + client_secret (str): The unique client_secret sends by the user + send_attempt (int): Which attempt this is + next_link: A link to redirect the user to once they submit the token + Returns: + The json response body from the server + """ params = { "country": country, "phone_number": phone_number, "client_secret": client_secret, "send_attempt": send_attempt, } - params.update(kwargs) + if next_link: + params["next_link"] = next_link + + if self.hs.config.using_identity_server_from_trusted_list: + # Warn that a deprecated config option is in use + logger.warn( + 'The config option "trust_identity_server_for_password_resets" ' + 'has been replaced by "account_threepid_delegate". ' + "Please consult the sample config at docs/sample_config.yaml for " + "details and update your config file." + ) try: data = yield self.http_client.post_json_get_json( - "https://%s%s" - % (id_server, "/_matrix/identity/api/v1/validate/msisdn/requestToken"), + id_server + "/_matrix/identity/api/v1/validate/msisdn/requestToken", params, ) return data diff --git a/synapse/push/mailer.py b/synapse/push/mailer.py index 4245ce26f3..3dfd527849 100644 --- a/synapse/push/mailer.py +++ b/synapse/push/mailer.py @@ -131,14 +131,11 @@ class Mailer(object): email_address (str): Email address we're sending the password reset to token (str): Unique token generated by the server to verify - password reset email was received + the email was received client_secret (str): Unique token generated by the client to group together multiple email sending attempts sid (str): The generated session ID """ - if email.utils.parseaddr(email_address)[1] == "": - raise RuntimeError("Invalid 'to' email address") - link = ( self.hs.config.public_baseurl + "_matrix/client/unstable/password_reset/email/submit_token" @@ -149,7 +146,34 @@ class Mailer(object): yield self.send_email( email_address, - "[%s] Password Reset Email" % self.hs.config.server_name, + "[%s] Password Reset" % self.hs.config.server_name, + template_vars, + ) + + @defer.inlineCallbacks + def send_registration_mail(self, email_address, token, client_secret, sid): + """Send an email with a registration confirmation link to a user + + Args: + email_address (str): Email address we're sending the registration + link to + token (str): Unique token generated by the server to verify + the email was received + client_secret (str): Unique token generated by the client to + group together multiple email sending attempts + sid (str): The generated session ID + """ + link = ( + self.hs.config.public_baseurl + + "_matrix/client/unstable/registration/email/submit_token" + "?token=%s&client_secret=%s&sid=%s" % (token, client_secret, sid) + ) + + template_vars = {"link": link} + + yield self.send_email( + email_address, + "[%s] Register your Email Address" % self.hs.config.server_name, template_vars, ) @@ -605,25 +629,50 @@ def format_ts_filter(value, format): return time.strftime(format, time.localtime(value / 1000)) -def load_jinja2_templates(config, template_html_name, template_text_name): - """Load the jinja2 email templates from disk +def load_jinja2_templates( + template_dir, + template_filenames, + apply_format_ts_filter=False, + apply_mxc_to_http_filter=False, + public_baseurl=None, +): + """Loads and returns one or more jinja2 templates and applies optional filters + + Args: + template_dir (str): The directory where templates are stored + template_filenames (list[str]): A list of template filenames + apply_format_ts_filter (bool): Whether to apply a template filter that formats + timestamps + apply_mxc_to_http_filter (bool): Whether to apply a template filter that converts + mxc urls to http urls + public_baseurl (str|None): The public baseurl of the server. Required for + apply_mxc_to_http_filter to be enabled Returns: - (template_html, template_text) + A list of jinja2 templates corresponding to the given list of filenames, + with order preserved """ - logger.info("loading email templates from '%s'", config.email_template_dir) - loader = jinja2.FileSystemLoader(config.email_template_dir) + logger.info( + "loading email templates %s from '%s'", template_filenames, template_dir + ) + loader = jinja2.FileSystemLoader(template_dir) env = jinja2.Environment(loader=loader) - env.filters["format_ts"] = format_ts_filter - env.filters["mxc_to_http"] = _create_mxc_to_http_filter(config) - template_html = env.get_template(template_html_name) - template_text = env.get_template(template_text_name) + if apply_format_ts_filter: + env.filters["format_ts"] = format_ts_filter - return template_html, template_text + if apply_mxc_to_http_filter and public_baseurl: + env.filters["mxc_to_http"] = _create_mxc_to_http_filter(public_baseurl) + + templates = [] + for template_filename in template_filenames: + template = env.get_template(template_filename) + templates.append(template) + + return templates -def _create_mxc_to_http_filter(config): +def _create_mxc_to_http_filter(public_baseurl): def mxc_to_http_filter(value, width, height, resize_method="crop"): if value[0:6] != "mxc://": return "" @@ -636,7 +685,7 @@ def _create_mxc_to_http_filter(config): params = {"width": width, "height": height, "method": resize_method} return "%s_matrix/media/v1/thumbnail/%s?%s%s" % ( - config.public_baseurl, + public_baseurl, serverAndMediaId, urllib.parse.urlencode(params), fragment or "", diff --git a/synapse/push/pusher.py b/synapse/push/pusher.py index a9c64a9c54..f277aeb131 100644 --- a/synapse/push/pusher.py +++ b/synapse/push/pusher.py @@ -35,6 +35,7 @@ except Exception: class PusherFactory(object): def __init__(self, hs): self.hs = hs + self.config = hs.config self.pusher_types = {"http": HttpPusher} @@ -42,12 +43,16 @@ class PusherFactory(object): if hs.config.email_enable_notifs: self.mailers = {} # app_name -> Mailer - templates = load_jinja2_templates( - config=hs.config, - template_html_name=hs.config.email_notif_template_html, - template_text_name=hs.config.email_notif_template_text, + self.notif_template_html, self.notif_template_text = load_jinja2_templates( + self.config.email_template_dir, + [ + self.config.email_notif_template_html, + self.config.email_notif_template_text, + ], + apply_format_ts_filter=True, + apply_mxc_to_http_filter=True, + public_baseurl=self.config.public_baseurl, ) - self.notif_template_html, self.notif_template_text = templates self.pusher_types["email"] = self._create_email_pusher @@ -78,6 +83,6 @@ class PusherFactory(object): if "data" in pusherdict and "brand" in pusherdict["data"]: app_name = pusherdict["data"]["brand"] else: - app_name = self.hs.config.email_app_name + app_name = self.config.email_app_name return app_name diff --git a/synapse/res/templates/password_reset.html b/synapse/res/templates/password_reset.html index 4fa7b36734..a197bf872c 100644 --- a/synapse/res/templates/password_reset.html +++ b/synapse/res/templates/password_reset.html @@ -4,6 +4,6 @@ {{ link }} -

If this was not you, please disregard this email and contact your server administrator. Thank you.

+

If this was not you, do not click the link above and instead contact your server administrator. Thank you.

diff --git a/synapse/res/templates/password_reset.txt b/synapse/res/templates/password_reset.txt index f0deff59a7..6aa6527560 100644 --- a/synapse/res/templates/password_reset.txt +++ b/synapse/res/templates/password_reset.txt @@ -3,5 +3,5 @@ was you, please click the link below to confirm resetting your password: {{ link }} -If this was not you, please disregard this email and contact your server -administrator. Thank you. +If this was not you, DO NOT click the link above and instead contact your +server administrator. Thank you. diff --git a/synapse/res/templates/password_reset_failure.html b/synapse/res/templates/password_reset_failure.html index 0b132cf8db..9e3c4446e3 100644 --- a/synapse/res/templates/password_reset_failure.html +++ b/synapse/res/templates/password_reset_failure.html @@ -1,6 +1,8 @@ -

{{ failure_reason }}. Your password has not been reset.

+

The request failed for the following reason: {{ failure_reason }}.

+ +

Your password has not been reset.

diff --git a/synapse/res/templates/registration.html b/synapse/res/templates/registration.html new file mode 100644 index 0000000000..16730a527f --- /dev/null +++ b/synapse/res/templates/registration.html @@ -0,0 +1,11 @@ + + +

You have asked us to register this email with a new Matrix account. If this was you, please click the link below to confirm your email address:

+ + Verify Your Email Address + +

If this was not you, you can safely disregard this email.

+ +

Thank you.

+ + diff --git a/synapse/res/templates/registration.txt b/synapse/res/templates/registration.txt new file mode 100644 index 0000000000..cb4f16a90c --- /dev/null +++ b/synapse/res/templates/registration.txt @@ -0,0 +1,10 @@ +Hello there, + +You have asked us to register this email with a new Matrix account. If this +was you, please click the link below to confirm your email address: + +{{ link }} + +If this was not you, you can safely disregard this email. + +Thank you. diff --git a/synapse/res/templates/registration_failure.html b/synapse/res/templates/registration_failure.html new file mode 100644 index 0000000000..2833d79c37 --- /dev/null +++ b/synapse/res/templates/registration_failure.html @@ -0,0 +1,6 @@ + + + +

Validation failed for the following reason: {{ failure_reason }}.

+ + diff --git a/synapse/res/templates/registration_success.html b/synapse/res/templates/registration_success.html new file mode 100644 index 0000000000..fbd6e4018f --- /dev/null +++ b/synapse/res/templates/registration_success.html @@ -0,0 +1,6 @@ + + + +

Your email has now been validated, please return to your client. You may now close this window.

+ + diff --git a/synapse/rest/__init__.py b/synapse/rest/__init__.py index 1d20b96d03..4a1fc2ec2b 100644 --- a/synapse/rest/__init__.py +++ b/synapse/rest/__init__.py @@ -73,7 +73,7 @@ class ClientRestResource(JsonResource): @staticmethod def register_servlets(client_resource, hs): - versions.register_servlets(client_resource) + versions.register_servlets(hs, client_resource) # Deprecated in r0 initial_sync.register_servlets(hs, client_resource) diff --git a/synapse/rest/client/v2_alpha/_base.py b/synapse/rest/client/v2_alpha/_base.py index e3d59ac3ac..8250ae0ae1 100644 --- a/synapse/rest/client/v2_alpha/_base.py +++ b/synapse/rest/client/v2_alpha/_base.py @@ -37,6 +37,7 @@ def client_patterns(path_regex, releases=(0,), unstable=True, v1=False): SRE_Pattern """ patterns = [] + if unstable: unstable_prefix = CLIENT_API_PREFIX + "/unstable" patterns.append(re.compile("^" + unstable_prefix + path_regex)) @@ -46,6 +47,7 @@ def client_patterns(path_regex, releases=(0,), unstable=True, v1=False): for release in releases: new_prefix = CLIENT_API_PREFIX + "/r%d" % (release,) patterns.append(re.compile("^" + new_prefix + path_regex)) + return patterns diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index e9cc953bdd..997557dfb0 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -18,12 +18,11 @@ import logging from six.moves import http_client -import jinja2 - from twisted.internet import defer from synapse.api.constants import LoginType from synapse.api.errors import Codes, SynapseError, ThreepidValidationError +from synapse.config.emailconfig import ThreepidBehaviour from synapse.http.server import finish_request from synapse.http.servlet import ( RestServlet, @@ -31,8 +30,8 @@ from synapse.http.servlet import ( parse_json_object_from_request, parse_string, ) +from synapse.push.mailer import Mailer, load_jinja2_templates from synapse.util.msisdn import phone_number_to_msisdn -from synapse.util.stringutils import random_string from synapse.util.threepids import check_3pid_allowed from ._base import client_patterns, interactive_auth_handler @@ -50,25 +49,28 @@ class EmailPasswordRequestTokenRestServlet(RestServlet): self.config = hs.config self.identity_handler = hs.get_handlers().identity_handler - if self.config.email_password_reset_behaviour == "local": - from synapse.push.mailer import Mailer, load_jinja2_templates - - templates = load_jinja2_templates( - config=hs.config, - template_html_name=hs.config.email_password_reset_template_html, - template_text_name=hs.config.email_password_reset_template_text, + if self.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL: + template_html, template_text = load_jinja2_templates( + self.config.email_template_dir, + [ + self.config.email_password_reset_template_html, + self.config.email_password_reset_template_text, + ], + apply_format_ts_filter=True, + apply_mxc_to_http_filter=True, + public_baseurl=self.config.public_baseurl, ) self.mailer = Mailer( hs=self.hs, app_name=self.config.email_app_name, - template_html=templates[0], - template_text=templates[1], + template_html=template_html, + template_text=template_text, ) @defer.inlineCallbacks def on_POST(self, request): - if self.config.email_password_reset_behaviour == "off": - if self.config.password_resets_were_disabled_due_to_email_config: + if self.config.threepid_behaviour_email == ThreepidBehaviour.OFF: + if self.config.local_threepid_handling_disabled_due_to_email_config: logger.warn( "User password resets have been disabled due to lack of email config" ) @@ -93,25 +95,39 @@ class EmailPasswordRequestTokenRestServlet(RestServlet): Codes.THREEPID_DENIED, ) - existingUid = yield self.hs.get_datastore().get_user_id_by_threepid( + existing_user_id = yield self.hs.get_datastore().get_user_id_by_threepid( "email", email ) - if existingUid is None: + if existing_user_id is None: raise SynapseError(400, "Email not found", Codes.THREEPID_NOT_FOUND) - if self.config.email_password_reset_behaviour == "remote": - if "id_server" not in body: - raise SynapseError(400, "Missing 'id_server' param in body") + if self.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE: + # Have the configured identity server handle the request + if not self.hs.config.account_threepid_delegate_email: + logger.warn( + "No upstream email account_threepid_delegate configured on the server to " + "handle this request" + ) + raise SynapseError( + 400, "Password reset by email is not supported on this homeserver" + ) - # Have the identity server handle the password reset flow ret = yield self.identity_handler.requestEmailToken( - body["id_server"], email, client_secret, send_attempt, next_link + self.hs.config.account_threepid_delegate_email, + email, + client_secret, + send_attempt, + next_link, ) else: # Send password reset emails from Synapse - sid = yield self.send_password_reset( - email, client_secret, send_attempt, next_link + sid = yield self.identity_handler.send_threepid_validation( + email, + client_secret, + send_attempt, + self.mailer.send_password_reset_mail, + next_link, ) # Wrap the session id in a JSON object @@ -119,74 +135,6 @@ class EmailPasswordRequestTokenRestServlet(RestServlet): return 200, ret - @defer.inlineCallbacks - def send_password_reset(self, email, client_secret, send_attempt, next_link=None): - """Send a password reset email - - Args: - email (str): The user's email address - client_secret (str): The provided client secret - send_attempt (int): Which send attempt this is - - Returns: - The new session_id upon success - - Raises: - SynapseError is an error occurred when sending the email - """ - # Check that this email/client_secret/send_attempt combo is new or - # greater than what we've seen previously - session = yield self.datastore.get_threepid_validation_session( - "email", client_secret, address=email, validated=False - ) - - # Check to see if a session already exists and that it is not yet - # marked as validated - if session and session.get("validated_at") is None: - session_id = session["session_id"] - last_send_attempt = session["last_send_attempt"] - - # Check that the send_attempt is higher than previous attempts - if send_attempt <= last_send_attempt: - # If not, just return a success without sending an email - return session_id - else: - # An non-validated session does not exist yet. - # Generate a session id - session_id = random_string(16) - - # Generate a new validation token - token = random_string(32) - - # Send the mail with the link containing the token, client_secret - # and session_id - try: - yield self.mailer.send_password_reset_mail( - email, token, client_secret, session_id - ) - except Exception: - logger.exception("Error sending a password reset email to %s", email) - raise SynapseError( - 500, "An error was encountered when sending the password reset email" - ) - - token_expires = ( - self.hs.clock.time_msec() + self.config.email_validation_token_lifetime - ) - - yield self.datastore.start_or_continue_validation_session( - "email", - email, - session_id, - client_secret, - send_attempt, - next_link, - token, - token_expires, - ) - - return session_id - class MsisdnPasswordRequestTokenRestServlet(RestServlet): PATTERNS = client_patterns("/account/password/msisdn/requestToken$") @@ -202,11 +150,15 @@ class MsisdnPasswordRequestTokenRestServlet(RestServlet): body = parse_json_object_from_request(request) assert_params_in_dict( - body, - ["id_server", "client_secret", "country", "phone_number", "send_attempt"], + body, ["client_secret", "country", "phone_number", "send_attempt"] ) + client_secret = body["client_secret"] + country = body["country"] + phone_number = body["phone_number"] + send_attempt = body["send_attempt"] + next_link = body.get("next_link") # Optional param - msisdn = phone_number_to_msisdn(body["country"], body["phone_number"]) + msisdn = phone_number_to_msisdn(country, phone_number) if not check_3pid_allowed(self.hs, "msisdn", msisdn): raise SynapseError( @@ -215,12 +167,32 @@ class MsisdnPasswordRequestTokenRestServlet(RestServlet): Codes.THREEPID_DENIED, ) - existingUid = yield self.datastore.get_user_id_by_threepid("msisdn", msisdn) + existing_user_id = yield self.datastore.get_user_id_by_threepid( + "msisdn", msisdn + ) - if existingUid is None: + if existing_user_id is None: raise SynapseError(400, "MSISDN not found", Codes.THREEPID_NOT_FOUND) - ret = yield self.identity_handler.requestMsisdnToken(**body) + if not self.hs.config.account_threepid_delegate_msisdn: + logger.warn( + "No upstream msisdn account_threepid_delegate configured on the server to " + "handle this request" + ) + raise SynapseError( + 400, + "Password reset by phone number is not supported on this homeserver", + ) + + ret = yield self.identity_handler.requestMsisdnToken( + self.hs.config.account_threepid_delegate_msisdn, + country, + phone_number, + client_secret, + send_attempt, + next_link, + ) + return 200, ret @@ -241,31 +213,32 @@ class PasswordResetSubmitTokenServlet(RestServlet): self.auth = hs.get_auth() self.config = hs.config self.clock = hs.get_clock() - self.datastore = hs.get_datastore() + self.store = hs.get_datastore() @defer.inlineCallbacks def on_GET(self, request, medium): + # We currently only handle threepid token submissions for email if medium != "email": raise SynapseError( 400, "This medium is currently not supported for password resets" ) - if self.config.email_password_reset_behaviour == "off": - if self.config.password_resets_were_disabled_due_to_email_config: + if self.config.threepid_behaviour_email == ThreepidBehaviour.OFF: + if self.config.local_threepid_handling_disabled_due_to_email_config: logger.warn( - "User password resets have been disabled due to lack of email config" + "Password reset emails have been disabled due to lack of an email config" ) raise SynapseError( - 400, "Email-based password resets have been disabled on this server" + 400, "Email-based password resets are disabled on this server" ) - sid = parse_string(request, "sid") - client_secret = parse_string(request, "client_secret") - token = parse_string(request, "token") + sid = parse_string(request, "sid", required=True) + client_secret = parse_string(request, "client_secret", required=True) + token = parse_string(request, "token", required=True) - # Attempt to validate a 3PID sesssion + # Attempt to validate a 3PID session try: # Mark the session as valid - next_link = yield self.datastore.validate_threepid_session( + next_link = yield self.store.validate_threepid_session( sid, client_secret, token, self.clock.time_msec() ) @@ -282,38 +255,22 @@ class PasswordResetSubmitTokenServlet(RestServlet): return None # Otherwise show the success template - html = self.config.email_password_reset_template_success_html_content + html = self.config.email_password_reset_template_success_html request.setResponseCode(200) except ThreepidValidationError as e: - # Show a failure page with a reason - html = self.load_jinja2_template( - self.config.email_template_dir, - self.config.email_password_reset_template_failure_html, - template_vars={"failure_reason": e.msg}, - ) request.setResponseCode(e.code) + # Show a failure page with a reason + html_template = load_jinja2_templates( + self.config.email_template_dir, + [self.config.email_password_reset_template_failure_html], + ) + + template_vars = {"failure_reason": e.msg} + html = html_template.render(**template_vars) + request.write(html.encode("utf-8")) finish_request(request) - return None - - def load_jinja2_template(self, template_dir, template_filename, template_vars): - """Loads a jinja2 template with variables to insert - - Args: - template_dir (str): The directory where templates are stored - template_filename (str): The name of the template in the template_dir - template_vars (Dict): Dictionary of keys in the template - alongside their values to insert - - Returns: - str containing the contents of the rendered template - """ - loader = jinja2.FileSystemLoader(template_dir) - env = jinja2.Environment(loader=loader) - - template = env.get_template(template_filename) - return template.render(**template_vars) @defer.inlineCallbacks def on_POST(self, request, medium): @@ -325,7 +282,7 @@ class PasswordResetSubmitTokenServlet(RestServlet): body = parse_json_object_from_request(request) assert_params_in_dict(body, ["sid", "client_secret", "token"]) - valid, _ = yield self.datastore.validate_threepid_validation_token( + valid, _ = yield self.store.validate_threepid_session( body["sid"], body["client_secret"], body["token"], self.clock.time_msec() ) response_code = 200 if valid else 400 @@ -371,7 +328,6 @@ class PasswordRestServlet(RestServlet): [[LoginType.EMAIL_IDENTITY], [LoginType.MSISDN]], body, self.hs.get_ip_from_request(request), - password_servlet=True, ) if LoginType.EMAIL_IDENTITY in result: @@ -454,10 +410,11 @@ class EmailThreepidRequestTokenRestServlet(RestServlet): PATTERNS = client_patterns("/account/3pid/email/requestToken$") def __init__(self, hs): - self.hs = hs super(EmailThreepidRequestTokenRestServlet, self).__init__() + self.hs = hs + self.config = hs.config self.identity_handler = hs.get_handlers().identity_handler - self.datastore = self.hs.get_datastore() + self.store = self.hs.get_datastore() @defer.inlineCallbacks def on_POST(self, request): @@ -465,22 +422,29 @@ class EmailThreepidRequestTokenRestServlet(RestServlet): assert_params_in_dict( body, ["id_server", "client_secret", "email", "send_attempt"] ) + id_server = "https://" + body["id_server"] # Assume https + client_secret = body["client_secret"] + email = body["email"] + send_attempt = body["send_attempt"] + next_link = body.get("next_link") # Optional param - if not check_3pid_allowed(self.hs, "email", body["email"]): + if not check_3pid_allowed(self.hs, "email", email): raise SynapseError( 403, "Your email domain is not authorized on this server", Codes.THREEPID_DENIED, ) - existingUid = yield self.datastore.get_user_id_by_threepid( + existing_user_id = yield self.store.get_user_id_by_threepid( "email", body["email"] ) - if existingUid is not None: + if existing_user_id is not None: raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE) - ret = yield self.identity_handler.requestEmailToken(**body) + ret = yield self.identity_handler.requestEmailToken( + id_server, email, client_secret, send_attempt, next_link + ) return 200, ret @@ -490,8 +454,8 @@ class MsisdnThreepidRequestTokenRestServlet(RestServlet): def __init__(self, hs): self.hs = hs super(MsisdnThreepidRequestTokenRestServlet, self).__init__() + self.store = self.hs.get_datastore() self.identity_handler = hs.get_handlers().identity_handler - self.datastore = self.hs.get_datastore() @defer.inlineCallbacks def on_POST(self, request): @@ -500,8 +464,14 @@ class MsisdnThreepidRequestTokenRestServlet(RestServlet): body, ["id_server", "client_secret", "country", "phone_number", "send_attempt"], ) + id_server = "https://" + body["id_server"] # Assume https + client_secret = body["client_secret"] + country = body["country"] + phone_number = body["phone_number"] + send_attempt = body["send_attempt"] + next_link = body.get("next_link") # Optional param - msisdn = phone_number_to_msisdn(body["country"], body["phone_number"]) + msisdn = phone_number_to_msisdn(country, phone_number) if not check_3pid_allowed(self.hs, "msisdn", msisdn): raise SynapseError( @@ -510,12 +480,14 @@ class MsisdnThreepidRequestTokenRestServlet(RestServlet): Codes.THREEPID_DENIED, ) - existingUid = yield self.datastore.get_user_id_by_threepid("msisdn", msisdn) + existing_user_id = yield self.store.get_user_id_by_threepid("msisdn", msisdn) - if existingUid is not None: + if existing_user_id is not None: raise SynapseError(400, "MSISDN is already in use", Codes.THREEPID_IN_USE) - ret = yield self.identity_handler.requestMsisdnToken(**body) + ret = yield self.identity_handler.requestMsisdnToken( + id_server, country, phone_number, client_secret, send_attempt, next_link + ) return 200, ret diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index 1ccd2bed2f..7ab534581e 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -28,16 +28,20 @@ from synapse.api.errors import ( Codes, LimitExceededError, SynapseError, + ThreepidValidationError, UnrecognizedRequestError, ) +from synapse.config.emailconfig import ThreepidBehaviour from synapse.config.ratelimiting import FederationRateLimitConfig from synapse.config.server import is_threepid_reserved +from synapse.http.server import finish_request from synapse.http.servlet import ( RestServlet, assert_params_in_dict, parse_json_object_from_request, parse_string, ) +from synapse.push.mailer import load_jinja2_templates from synapse.util.msisdn import phone_number_to_msisdn from synapse.util.ratelimitutils import FederationRateLimiter from synapse.util.threepids import check_3pid_allowed @@ -70,30 +74,92 @@ class EmailRegisterRequestTokenRestServlet(RestServlet): super(EmailRegisterRequestTokenRestServlet, self).__init__() self.hs = hs self.identity_handler = hs.get_handlers().identity_handler + self.config = hs.config + + if self.hs.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL: + from synapse.push.mailer import Mailer, load_jinja2_templates + + template_html, template_text = load_jinja2_templates( + self.config.email_template_dir, + [ + self.config.email_registration_template_html, + self.config.email_registration_template_text, + ], + apply_format_ts_filter=True, + apply_mxc_to_http_filter=True, + public_baseurl=self.config.public_baseurl, + ) + self.mailer = Mailer( + hs=self.hs, + app_name=self.config.email_app_name, + template_html=template_html, + template_text=template_text, + ) @defer.inlineCallbacks def on_POST(self, request): + if self.hs.config.threepid_behaviour_email == ThreepidBehaviour.OFF: + if self.hs.config.local_threepid_handling_disabled_due_to_email_config: + logger.warn( + "Email registration has been disabled due to lack of email config" + ) + raise SynapseError( + 400, "Email-based registration has been disabled on this server" + ) body = parse_json_object_from_request(request) - assert_params_in_dict( - body, ["id_server", "client_secret", "email", "send_attempt"] - ) + assert_params_in_dict(body, ["client_secret", "email", "send_attempt"]) - if not check_3pid_allowed(self.hs, "email", body["email"]): + # Extract params from body + client_secret = body["client_secret"] + email = body["email"] + send_attempt = body["send_attempt"] + next_link = body.get("next_link") # Optional param + + if not check_3pid_allowed(self.hs, "email", email): raise SynapseError( 403, "Your email domain is not authorized to register on this server", Codes.THREEPID_DENIED, ) - existingUid = yield self.hs.get_datastore().get_user_id_by_threepid( + existing_user_id = yield self.hs.get_datastore().get_user_id_by_threepid( "email", body["email"] ) - if existingUid is not None: + if existing_user_id is not None: raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE) - ret = yield self.identity_handler.requestEmailToken(**body) + if self.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE: + if not self.hs.config.account_threepid_delegate_email: + logger.warn( + "No upstream email account_threepid_delegate configured on the server to " + "handle this request" + ) + raise SynapseError( + 400, "Registration by email is not supported on this homeserver" + ) + + ret = yield self.identity_handler.requestEmailToken( + self.hs.config.account_threepid_delegate_email, + email, + client_secret, + send_attempt, + next_link, + ) + else: + # Send registration emails from Synapse + sid = yield self.identity_handler.send_threepid_validation( + email, + client_secret, + send_attempt, + self.mailer.send_registration_mail, + next_link, + ) + + # Wrap the session id in a JSON object + ret = {"sid": sid} + return 200, ret @@ -114,11 +180,15 @@ class MsisdnRegisterRequestTokenRestServlet(RestServlet): body = parse_json_object_from_request(request) assert_params_in_dict( - body, - ["id_server", "client_secret", "country", "phone_number", "send_attempt"], + body, ["client_secret", "country", "phone_number", "send_attempt"] ) + client_secret = body["client_secret"] + country = body["country"] + phone_number = body["phone_number"] + send_attempt = body["send_attempt"] + next_link = body.get("next_link") # Optional param - msisdn = phone_number_to_msisdn(body["country"], body["phone_number"]) + msisdn = phone_number_to_msisdn(country, phone_number) if not check_3pid_allowed(self.hs, "msisdn", msisdn): raise SynapseError( @@ -127,19 +197,114 @@ class MsisdnRegisterRequestTokenRestServlet(RestServlet): Codes.THREEPID_DENIED, ) - existingUid = yield self.hs.get_datastore().get_user_id_by_threepid( + existing_user_id = yield self.hs.get_datastore().get_user_id_by_threepid( "msisdn", msisdn ) - if existingUid is not None: + if existing_user_id is not None: raise SynapseError( 400, "Phone number is already in use", Codes.THREEPID_IN_USE ) - ret = yield self.identity_handler.requestMsisdnToken(**body) + if not self.hs.config.account_threepid_delegate_msisdn: + logger.warn( + "No upstream msisdn account_threepid_delegate configured on the server to " + "handle this request" + ) + raise SynapseError( + 400, "Registration by phone number is not supported on this homeserver" + ) + + ret = yield self.identity_handler.requestMsisdnToken( + self.hs.config.account_threepid_delegate_msisdn, + country, + phone_number, + client_secret, + send_attempt, + next_link, + ) + return 200, ret +class RegistrationSubmitTokenServlet(RestServlet): + """Handles registration 3PID validation token submission""" + + PATTERNS = client_patterns( + "/registration/(?P[^/]*)/submit_token$", releases=(), unstable=True + ) + + def __init__(self, hs): + """ + Args: + hs (synapse.server.HomeServer): server + """ + super(RegistrationSubmitTokenServlet, self).__init__() + self.hs = hs + self.auth = hs.get_auth() + self.config = hs.config + self.clock = hs.get_clock() + self.store = hs.get_datastore() + + @defer.inlineCallbacks + def on_GET(self, request, medium): + if medium != "email": + raise SynapseError( + 400, "This medium is currently not supported for registration" + ) + if self.config.threepid_behaviour_email == ThreepidBehaviour.OFF: + if self.config.local_threepid_handling_disabled_due_to_email_config: + logger.warn( + "User registration via email has been disabled due to lack of email config" + ) + raise SynapseError( + 400, "Email-based registration is disabled on this server" + ) + + sid = parse_string(request, "sid", required=True) + client_secret = parse_string(request, "client_secret", required=True) + token = parse_string(request, "token", required=True) + + # Attempt to validate a 3PID session + try: + # Mark the session as valid + next_link = yield self.store.validate_threepid_session( + sid, client_secret, token, self.clock.time_msec() + ) + + # Perform a 302 redirect if next_link is set + if next_link: + if next_link.startswith("file:///"): + logger.warn( + "Not redirecting to next_link as it is a local file: address" + ) + else: + request.setResponseCode(302) + request.setHeader("Location", next_link) + finish_request(request) + return None + + # Otherwise show the success template + html = self.config.email_registration_template_success_html_content + + request.setResponseCode(200) + except ThreepidValidationError as e: + # Show a failure page with a reason + request.setResponseCode(e.code) + + # Show a failure page with a reason + html_template = load_jinja2_templates( + self.config.email_template_dir, + [self.config.email_registration_template_failure_html], + ) + + template_vars = {"failure_reason": e.msg} + html = html_template.render(**template_vars) + + request.write(html.encode("utf-8")) + finish_request(request) + + class UsernameAvailabilityRestServlet(RestServlet): PATTERNS = client_patterns("/register/available") @@ -438,11 +603,11 @@ class RegisterRestServlet(RestServlet): medium = auth_result[login_type]["medium"] address = auth_result[login_type]["address"] - existingUid = yield self.store.get_user_id_by_threepid( + existing_user_id = yield self.store.get_user_id_by_threepid( medium, address ) - if existingUid is not None: + if existing_user_id is not None: raise SynapseError( 400, "%s is already in use" % medium, @@ -550,4 +715,5 @@ def register_servlets(hs, http_server): EmailRegisterRequestTokenRestServlet(hs).register(http_server) MsisdnRegisterRequestTokenRestServlet(hs).register(http_server) UsernameAvailabilityRestServlet(hs).register(http_server) + RegistrationSubmitTokenServlet(hs).register(http_server) RegisterRestServlet(hs).register(http_server) diff --git a/synapse/rest/client/versions.py b/synapse/rest/client/versions.py index 0e09191632..0058b6b459 100644 --- a/synapse/rest/client/versions.py +++ b/synapse/rest/client/versions.py @@ -24,6 +24,10 @@ logger = logging.getLogger(__name__) class VersionsRestServlet(RestServlet): PATTERNS = [re.compile("^/_matrix/client/versions$")] + def __init__(self, hs): + super(VersionsRestServlet, self).__init__() + self.config = hs.config + def on_GET(self, request): return ( 200, @@ -49,5 +53,5 @@ class VersionsRestServlet(RestServlet): ) -def register_servlets(http_server): - VersionsRestServlet().register(http_server) +def register_servlets(hs, http_server): + VersionsRestServlet(hs).register(http_server) From 1ab1479a92d940fc17afd4a5d991ce5a7d210a85 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 6 Sep 2019 13:02:21 +0100 Subject: [PATCH 16/26] Add changelog --- changelog.d/5993.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5993.bugfix diff --git a/changelog.d/5993.bugfix b/changelog.d/5993.bugfix new file mode 100644 index 0000000000..d7fb687c6e --- /dev/null +++ b/changelog.d/5993.bugfix @@ -0,0 +1 @@ +Fix a bug where registration via threepid was broken due to the store method being in the wrong place. \ No newline at end of file From e059c5e6487f0e86e395f1e79d400ae0b0d24a82 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 6 Sep 2019 13:10:11 +0100 Subject: [PATCH 17/26] Move get_threepid_validation_session into RegistrationWorkerStore --- synapse/storage/registration.py | 108 ++++++++++++++++---------------- 1 file changed, 54 insertions(+), 54 deletions(-) diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py index 2d3c7e2dc9..24509bd455 100644 --- a/synapse/storage/registration.py +++ b/synapse/storage/registration.py @@ -614,6 +614,60 @@ class RegistrationWorkerStore(SQLBaseStore): # Convert the integer into a boolean. return res == 1 + def get_threepid_validation_session( + self, medium, client_secret, address=None, sid=None, validated=True + ): + """Gets a session_id and last_send_attempt (if available) for a + client_secret/medium/(address|session_id) combo + + Args: + medium (str|None): The medium of the 3PID + address (str|None): The address of the 3PID + sid (str|None): The ID of the validation session + client_secret (str|None): A unique string provided by the client to + help identify this validation attempt + validated (bool|None): Whether sessions should be filtered by + whether they have been validated already or not. None to + perform no filtering + + Returns: + deferred {str, int}|None: A dict containing the + latest session_id and send_attempt count for this 3PID. + Otherwise None if there hasn't been a previous attempt + """ + keyvalues = {"medium": medium, "client_secret": client_secret} + if address: + keyvalues["address"] = address + if sid: + keyvalues["session_id"] = sid + + assert address or sid + + def get_threepid_validation_session_txn(txn): + sql = """ + SELECT address, session_id, medium, client_secret, + last_send_attempt, validated_at + FROM threepid_validation_session WHERE %s + """ % ( + " AND ".join("%s = ?" % k for k in iterkeys(keyvalues)), + ) + + if validated is not None: + sql += " AND validated_at IS " + ("NOT NULL" if validated else "NULL") + + sql += " LIMIT 1" + + txn.execute(sql, list(keyvalues.values())) + rows = self.cursor_to_dict(txn) + if not rows: + return None + + return rows[0] + + return self.runInteraction( + "get_threepid_validation_session", get_threepid_validation_session_txn + ) + class RegistrationStore( RegistrationWorkerStore, background_updates.BackgroundUpdateStore @@ -1082,60 +1136,6 @@ class RegistrationStore( return 1 - def get_threepid_validation_session( - self, medium, client_secret, address=None, sid=None, validated=True - ): - """Gets a session_id and last_send_attempt (if available) for a - client_secret/medium/(address|session_id) combo - - Args: - medium (str|None): The medium of the 3PID - address (str|None): The address of the 3PID - sid (str|None): The ID of the validation session - client_secret (str|None): A unique string provided by the client to - help identify this validation attempt - validated (bool|None): Whether sessions should be filtered by - whether they have been validated already or not. None to - perform no filtering - - Returns: - deferred {str, int}|None: A dict containing the - latest session_id and send_attempt count for this 3PID. - Otherwise None if there hasn't been a previous attempt - """ - keyvalues = {"medium": medium, "client_secret": client_secret} - if address: - keyvalues["address"] = address - if sid: - keyvalues["session_id"] = sid - - assert address or sid - - def get_threepid_validation_session_txn(txn): - sql = """ - SELECT address, session_id, medium, client_secret, - last_send_attempt, validated_at - FROM threepid_validation_session WHERE %s - """ % ( - " AND ".join("%s = ?" % k for k in iterkeys(keyvalues)), - ) - - if validated is not None: - sql += " AND validated_at IS " + ("NOT NULL" if validated else "NULL") - - sql += " LIMIT 1" - - txn.execute(sql, list(keyvalues.values())) - rows = self.cursor_to_dict(txn) - if not rows: - return None - - return rows[0] - - return self.runInteraction( - "get_threepid_validation_session", get_threepid_validation_session_txn - ) - def validate_threepid_session(self, session_id, client_secret, token, current_ts): """Attempt to validate a threepid session using a token From 5a7e9fdd846a5c231ab1b1ed9bd12fef7ff5d408 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 6 Sep 2019 13:18:03 +0100 Subject: [PATCH 18/26] Change changelog --- changelog.d/5993.bugfix | 1 - changelog.d/5993.feature | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) delete mode 100644 changelog.d/5993.bugfix create mode 100644 changelog.d/5993.feature diff --git a/changelog.d/5993.bugfix b/changelog.d/5993.bugfix deleted file mode 100644 index d7fb687c6e..0000000000 --- a/changelog.d/5993.bugfix +++ /dev/null @@ -1 +0,0 @@ -Fix a bug where registration via threepid was broken due to the store method being in the wrong place. \ No newline at end of file diff --git a/changelog.d/5993.feature b/changelog.d/5993.feature new file mode 100644 index 0000000000..3e8bf5068d --- /dev/null +++ b/changelog.d/5993.feature @@ -0,0 +1 @@ +Add the ability to send registration emails from the homeserver rather than delegating to an identity server. From 6ddda8152ed4f8111d8108f6f2f92f365b9069ba Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 6 Sep 2019 13:23:10 +0100 Subject: [PATCH 19/26] Move delete_threepid_session into RegistrationWorkerStore --- synapse/storage/registration.py | 50 ++++++++++++++++----------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py index 24509bd455..5138792a5f 100644 --- a/synapse/storage/registration.py +++ b/synapse/storage/registration.py @@ -668,6 +668,31 @@ class RegistrationWorkerStore(SQLBaseStore): "get_threepid_validation_session", get_threepid_validation_session_txn ) + def delete_threepid_session(self, session_id): + """Removes a threepid validation session from the database. This can + be done after validation has been performed and whatever action was + waiting on it has been carried out + + Args: + session_id (str): The ID of the session to delete + """ + + def delete_threepid_session_txn(txn): + self._simple_delete_txn( + txn, + table="threepid_validation_token", + keyvalues={"session_id": session_id}, + ) + self._simple_delete_txn( + txn, + table="threepid_validation_session", + keyvalues={"session_id": session_id}, + ) + + return self.runInteraction( + "delete_threepid_session", delete_threepid_session_txn + ) + class RegistrationStore( RegistrationWorkerStore, background_updates.BackgroundUpdateStore @@ -1323,31 +1348,6 @@ class RegistrationStore( self.clock.time_msec(), ) - def delete_threepid_session(self, session_id): - """Removes a threepid validation session from the database. This can - be done after validation has been performed and whatever action was - waiting on it has been carried out - - Args: - session_id (str): The ID of the session to delete - """ - - def delete_threepid_session_txn(txn): - self._simple_delete_txn( - txn, - table="threepid_validation_token", - keyvalues={"session_id": session_id}, - ) - self._simple_delete_txn( - txn, - table="threepid_validation_session", - keyvalues={"session_id": session_id}, - ) - - return self.runInteraction( - "delete_threepid_session", delete_threepid_session_txn - ) - def set_user_deactivated_status_txn(self, txn, user_id, deactivated): self._simple_update_one_txn( txn=txn, From ca74b140f264e8b5429e352ac054288e1f78b980 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 6 Sep 2019 13:25:06 +0100 Subject: [PATCH 20/26] Fix destructuring assumption bug --- synapse/rest/client/v2_alpha/account.py | 2 +- synapse/rest/client/v2_alpha/register.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 997557dfb0..5babed7da8 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -261,7 +261,7 @@ class PasswordResetSubmitTokenServlet(RestServlet): request.setResponseCode(e.code) # Show a failure page with a reason - html_template = load_jinja2_templates( + [html_template] = load_jinja2_templates( self.config.email_template_dir, [self.config.email_password_reset_template_failure_html], ) diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index 7ab534581e..27f4addc41 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -293,7 +293,7 @@ class RegistrationSubmitTokenServlet(RestServlet): request.setResponseCode(e.code) # Show a failure page with a reason - html_template = load_jinja2_templates( + [html_template] = load_jinja2_templates( self.config.email_template_dir, [self.config.email_registration_template_failure_html], ) From 5d833f09236b0202197ee70824d25c7bfd31c161 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 6 Sep 2019 13:27:55 +0100 Subject: [PATCH 21/26] Add changelog --- changelog.d/5994.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5994.feature diff --git a/changelog.d/5994.feature b/changelog.d/5994.feature new file mode 100644 index 0000000000..5b69b97fe7 --- /dev/null +++ b/changelog.d/5994.feature @@ -0,0 +1 @@ +Add the ability to send registration emails from the homeserver rather than delegating to an identity server. \ No newline at end of file From cf5a420c8a5cbcb6ffbbdbc562e07cfde8d09ab4 Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Fri, 6 Sep 2019 13:34:42 +0100 Subject: [PATCH 22/26] Apply suggestions from code review Co-Authored-By: Erik Johnston --- synapse/rest/client/v2_alpha/account.py | 2 +- synapse/rest/client/v2_alpha/register.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 5babed7da8..785d01ea52 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -261,7 +261,7 @@ class PasswordResetSubmitTokenServlet(RestServlet): request.setResponseCode(e.code) # Show a failure page with a reason - [html_template] = load_jinja2_templates( + html_template, = load_jinja2_templates( self.config.email_template_dir, [self.config.email_password_reset_template_failure_html], ) diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index 27f4addc41..5c7a5f3579 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -293,7 +293,7 @@ class RegistrationSubmitTokenServlet(RestServlet): request.setResponseCode(e.code) # Show a failure page with a reason - [html_template] = load_jinja2_templates( + html_template, = load_jinja2_templates( self.config.email_template_dir, [self.config.email_registration_template_failure_html], ) From 142c9325c27432f9e40c345166def1dd20355433 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 6 Sep 2019 14:09:16 +0100 Subject: [PATCH 23/26] Correctly handle non-bool m.federate flag --- synapse/handlers/stats.py | 4 +++- synapse/storage/stats.py | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/synapse/handlers/stats.py b/synapse/handlers/stats.py index 921735edb3..3c265f3718 100644 --- a/synapse/handlers/stats.py +++ b/synapse/handlers/stats.py @@ -260,7 +260,9 @@ class StatsHandler(StateDeltasHandler): room_stats_delta["local_users_in_room"] += delta elif typ == EventTypes.Create: - room_state["is_federatable"] = event_content.get("m.federate", True) + room_state["is_federatable"] = ( + event_content.get("m.federate", True) is True + ) if sender and self.is_mine_id(sender): user_to_stats_deltas.setdefault(sender, Counter())[ "rooms_created" diff --git a/synapse/storage/stats.py b/synapse/storage/stats.py index 6560173c08..09190d684e 100644 --- a/synapse/storage/stats.py +++ b/synapse/storage/stats.py @@ -823,7 +823,9 @@ class StatsStore(StateDeltasStore): elif event.type == EventTypes.CanonicalAlias: room_state["canonical_alias"] = event.content.get("alias") elif event.type == EventTypes.Create: - room_state["is_federatable"] = event.content.get("m.federate", True) + room_state["is_federatable"] = ( + event.content.get("m.federate", True) is True + ) yield self.update_room_state(room_id, room_state) From 85275c89d79c7780f9a49ff206e906701308d55c Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 6 Sep 2019 14:21:14 +0100 Subject: [PATCH 24/26] Newsfile --- changelog.d/5998.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5998.bugfix diff --git a/changelog.d/5998.bugfix b/changelog.d/5998.bugfix new file mode 100644 index 0000000000..9ea095103b --- /dev/null +++ b/changelog.d/5998.bugfix @@ -0,0 +1 @@ +Fix room and user stats tracking. From 78801e7f9e355660d2c80f0923ff7c4c19f0f004 Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Fri, 6 Sep 2019 15:36:50 +0100 Subject: [PATCH 25/26] Ensure a sid parameter is passed to bind_threepid (#5995) `sid` is required to be part of `three_pid_creds`. We were 500'ing if it wasn't provided instead of returning `M_MISSING_PARAM`. --- changelog.d/5995.bugfix | 1 + synapse/handlers/identity.py | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 changelog.d/5995.bugfix diff --git a/changelog.d/5995.bugfix b/changelog.d/5995.bugfix new file mode 100644 index 0000000000..e03ab98bc6 --- /dev/null +++ b/changelog.d/5995.bugfix @@ -0,0 +1 @@ +Return a M_MISSING_PARAM if `sid` is not provided to `/account/3pid`. \ No newline at end of file diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py index 71b5a87392..45db1c1c06 100644 --- a/synapse/handlers/identity.py +++ b/synapse/handlers/identity.py @@ -151,12 +151,18 @@ class IdentityHandler(BaseHandler): creds ) + sid = creds.get("sid") + if not sid: + raise SynapseError( + 400, "No sid in three_pid_creds", errcode=Codes.MISSING_PARAM + ) + # If an id_access_token is not supplied, force usage of v1 if id_access_token is None: use_v2 = False # Decide which API endpoint URLs to use - bind_data = {"sid": creds["sid"], "client_secret": client_secret, "mxid": mxid} + bind_data = {"sid": sid, "client_secret": client_secret, "mxid": mxid} if use_v2: bind_url = "https://%s/_matrix/identity/v2/3pid/bind" % (id_server,) bind_data["id_access_token"] = id_access_token From 55d5b3af8863167432017f23cd8a04a0c14c9d23 Mon Sep 17 00:00:00 2001 From: Amber Brown Date: Sat, 7 Sep 2019 01:45:51 +1000 Subject: [PATCH 26/26] Servers-known-about statistic (#5981) --- changelog.d/5981.feature | 1 + docs/sample_config.yaml | 10 +++ synapse/config/metrics.py | 31 ++++++++ synapse/storage/roommember.py | 59 +++++++++++++++ tests/config/test_generate.py | 25 ++++--- tests/config/test_load.py | 34 ++++++--- tests/storage/test_roommember.py | 124 ++++++++++++++++++++++--------- 7 files changed, 225 insertions(+), 59 deletions(-) create mode 100644 changelog.d/5981.feature diff --git a/changelog.d/5981.feature b/changelog.d/5981.feature new file mode 100644 index 0000000000..e39514273d --- /dev/null +++ b/changelog.d/5981.feature @@ -0,0 +1 @@ +Setting metrics_flags.known_servers to True in the configuration will publish the synapse_federation_known_servers metric over Prometheus. This represents the total number of servers your server knows about (i.e. is in rooms with), including itself. diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 186cdbedd2..93c0edd8ce 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -958,6 +958,16 @@ account_threepid_delegates: #sentry: # dsn: "..." +# Flags to enable Prometheus metrics which are not suitable to be +# enabled by default, either for performance reasons or limited use. +# +metrics_flags: + # Publish synapse_federation_known_servers, a g auge of the number of + # servers this homeserver knows about, including itself. May cause + # performance problems on large homeservers. + # + #known_servers: true + # Whether or not to report anonymized homeserver usage statistics. # report_stats: true|false diff --git a/synapse/config/metrics.py b/synapse/config/metrics.py index 3698441963..653b990e67 100644 --- a/synapse/config/metrics.py +++ b/synapse/config/metrics.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- # Copyright 2015, 2016 OpenMarket Ltd +# Copyright 2019 The Matrix.org Foundation C.I.C. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -13,6 +14,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +import attr + from ._base import Config, ConfigError MISSING_SENTRY = """Missing sentry-sdk library. This is required to enable sentry @@ -20,6 +23,18 @@ MISSING_SENTRY = """Missing sentry-sdk library. This is required to enable sentr """ +@attr.s +class MetricsFlags(object): + known_servers = attr.ib(default=False, validator=attr.validators.instance_of(bool)) + + @classmethod + def all_off(cls): + """ + Instantiate the flags with all options set to off. + """ + return cls(**{x.name: False for x in attr.fields(cls)}) + + class MetricsConfig(Config): def read_config(self, config, **kwargs): self.enable_metrics = config.get("enable_metrics", False) @@ -27,6 +42,12 @@ class MetricsConfig(Config): self.metrics_port = config.get("metrics_port") self.metrics_bind_host = config.get("metrics_bind_host", "127.0.0.1") + if self.enable_metrics: + _metrics_config = config.get("metrics_flags") or {} + self.metrics_flags = MetricsFlags(**_metrics_config) + else: + self.metrics_flags = MetricsFlags.all_off() + self.sentry_enabled = "sentry" in config if self.sentry_enabled: try: @@ -58,6 +79,16 @@ class MetricsConfig(Config): #sentry: # dsn: "..." + # Flags to enable Prometheus metrics which are not suitable to be + # enabled by default, either for performance reasons or limited use. + # + metrics_flags: + # Publish synapse_federation_known_servers, a g auge of the number of + # servers this homeserver knows about, including itself. May cause + # performance problems on large homeservers. + # + #known_servers: true + # Whether or not to report anonymized homeserver usage statistics. """ diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index f8b682ebd9..4df8ebdacd 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -24,8 +24,10 @@ from canonicaljson import json from twisted.internet import defer from synapse.api.constants import EventTypes, Membership +from synapse.metrics import LaterGauge from synapse.metrics.background_process_metrics import run_as_background_process from synapse.storage._base import LoggingTransaction +from synapse.storage.engines import Sqlite3Engine from synapse.storage.events_worker import EventsWorkerStore from synapse.types import get_domain_from_id from synapse.util.async_helpers import Linearizer @@ -74,6 +76,63 @@ class RoomMemberWorkerStore(EventsWorkerStore): self._check_safe_current_state_events_membership_updated_txn(txn) txn.close() + if self.hs.config.metrics_flags.known_servers: + self._known_servers_count = 1 + self.hs.get_clock().looping_call( + run_as_background_process, + 60 * 1000, + "_count_known_servers", + self._count_known_servers, + ) + self.hs.get_clock().call_later( + 1000, + run_as_background_process, + "_count_known_servers", + self._count_known_servers, + ) + LaterGauge( + "synapse_federation_known_servers", + "", + [], + lambda: self._known_servers_count, + ) + + @defer.inlineCallbacks + def _count_known_servers(self): + """ + Count the servers that this server knows about. + + The statistic is stored on the class for the + `synapse_federation_known_servers` LaterGauge to collect. + """ + + def _transact(txn): + if isinstance(self.database_engine, Sqlite3Engine): + query = """ + SELECT COUNT(DISTINCT substr(out.user_id, pos+1)) + FROM ( + SELECT rm.user_id as user_id, instr(rm.user_id, ':') + AS pos FROM room_memberships as rm + INNER JOIN current_state_events as c ON rm.event_id = c.event_id + WHERE c.type = 'm.room.member' + ) as out + """ + else: + query = """ + SELECT COUNT(DISTINCT split_part(state_key, ':', 2)) + FROM current_state_events + WHERE type = 'm.room.member' AND membership = 'join'; + """ + txn.execute(query) + return list(txn)[0][0] + + count = yield self.runInteraction("get_known_servers", _transact) + + # We always know about ourselves, even if we have nothing in + # room_memberships (for example, the server is new). + self._known_servers_count = max([count, 1]) + return self._known_servers_count + def _check_safe_current_state_events_membership_updated_txn(self, txn): """Checks if it is safe to assume the new current_state_events membership column is up to date diff --git a/tests/config/test_generate.py b/tests/config/test_generate.py index 5017cbce85..2684e662de 100644 --- a/tests/config/test_generate.py +++ b/tests/config/test_generate.py @@ -17,6 +17,8 @@ import os.path import re import shutil import tempfile +from contextlib import redirect_stdout +from io import StringIO from synapse.config.homeserver import HomeServerConfig @@ -32,17 +34,18 @@ class ConfigGenerationTestCase(unittest.TestCase): shutil.rmtree(self.dir) def test_generate_config_generates_files(self): - HomeServerConfig.load_or_generate_config( - "", - [ - "--generate-config", - "-c", - self.file, - "--report-stats=yes", - "-H", - "lemurs.win", - ], - ) + with redirect_stdout(StringIO()): + HomeServerConfig.load_or_generate_config( + "", + [ + "--generate-config", + "-c", + self.file, + "--report-stats=yes", + "-H", + "lemurs.win", + ], + ) self.assertSetEqual( set(["homeserver.yaml", "lemurs.win.log.config", "lemurs.win.signing.key"]), diff --git a/tests/config/test_load.py b/tests/config/test_load.py index 6bfc1970ad..b3e557bd6a 100644 --- a/tests/config/test_load.py +++ b/tests/config/test_load.py @@ -15,6 +15,8 @@ import os.path import shutil import tempfile +from contextlib import redirect_stdout +from io import StringIO import yaml @@ -26,7 +28,6 @@ from tests import unittest class ConfigLoadingTestCase(unittest.TestCase): def setUp(self): self.dir = tempfile.mkdtemp() - print(self.dir) self.file = os.path.join(self.dir, "homeserver.yaml") def tearDown(self): @@ -94,18 +95,27 @@ class ConfigLoadingTestCase(unittest.TestCase): ) self.assertTrue(config.enable_registration) + def test_stats_enabled(self): + self.generate_config_and_remove_lines_containing("enable_metrics") + self.add_lines_to_config(["enable_metrics: true"]) + + # The default Metrics Flags are off by default. + config = HomeServerConfig.load_config("", ["-c", self.file]) + self.assertFalse(config.metrics_flags.known_servers) + def generate_config(self): - HomeServerConfig.load_or_generate_config( - "", - [ - "--generate-config", - "-c", - self.file, - "--report-stats=yes", - "-H", - "lemurs.win", - ], - ) + with redirect_stdout(StringIO()): + HomeServerConfig.load_or_generate_config( + "", + [ + "--generate-config", + "-c", + self.file, + "--report-stats=yes", + "-H", + "lemurs.win", + ], + ) def generate_config_and_remove_lines_containing(self, needle): self.generate_config() diff --git a/tests/storage/test_roommember.py b/tests/storage/test_roommember.py index 64cb294c37..447a3c6ffb 100644 --- a/tests/storage/test_roommember.py +++ b/tests/storage/test_roommember.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- # Copyright 2014-2016 OpenMarket Ltd +# Copyright 2019 The Matrix.org Foundation C.I.C. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -13,78 +14,129 @@ # See the License for the specific language governing permissions and # limitations under the License. - -from mock import Mock - -from twisted.internet import defer +from unittest.mock import Mock from synapse.api.constants import EventTypes, Membership from synapse.api.room_versions import RoomVersions -from synapse.types import Requester, RoomID, UserID +from synapse.rest.admin import register_servlets_for_client_rest_resource +from synapse.rest.client.v1 import login, room +from synapse.types import Requester, UserID from tests import unittest -from tests.utils import create_room, setup_test_homeserver -class RoomMemberStoreTestCase(unittest.TestCase): - @defer.inlineCallbacks - def setUp(self): - hs = yield setup_test_homeserver( - self.addCleanup, resource_for_federation=Mock(), http_client=None +class RoomMemberStoreTestCase(unittest.HomeserverTestCase): + + servlets = [ + login.register_servlets, + register_servlets_for_client_rest_resource, + room.register_servlets, + ] + + def make_homeserver(self, reactor, clock): + hs = self.setup_test_homeserver( + resource_for_federation=Mock(), http_client=None ) + return hs + + def prepare(self, reactor, clock, hs): + # We can't test the RoomMemberStore on its own without the other event # storage logic self.store = hs.get_datastore() self.event_builder_factory = hs.get_event_builder_factory() self.event_creation_handler = hs.get_event_creation_handler() - self.u_alice = UserID.from_string("@alice:test") - self.u_bob = UserID.from_string("@bob:test") + self.u_alice = self.register_user("alice", "pass") + self.t_alice = self.login("alice", "pass") + self.u_bob = self.register_user("bob", "pass") # User elsewhere on another host self.u_charlie = UserID.from_string("@charlie:elsewhere") - self.room = RoomID.from_string("!abc123:test") - - yield create_room(hs, self.room.to_string(), self.u_alice.to_string()) - - @defer.inlineCallbacks def inject_room_member(self, room, user, membership, replaces_state=None): builder = self.event_builder_factory.for_room_version( RoomVersions.V1, { "type": EventTypes.Member, - "sender": user.to_string(), - "state_key": user.to_string(), - "room_id": room.to_string(), + "sender": user, + "state_key": user, + "room_id": room, "content": {"membership": membership}, }, ) - event, context = yield self.event_creation_handler.create_new_client_event( - builder + event, context = self.get_success( + self.event_creation_handler.create_new_client_event(builder) ) - yield self.store.persist_event(event, context) + self.get_success(self.store.persist_event(event, context)) return event - @defer.inlineCallbacks def test_one_member(self): - yield self.inject_room_member(self.room, self.u_alice, Membership.JOIN) - self.assertEquals( - [self.room.to_string()], - [ - m.room_id - for m in ( - yield self.store.get_rooms_for_user_where_membership_is( - self.u_alice.to_string(), [Membership.JOIN] - ) - ) - ], + # Alice creates the room, and is automatically joined + self.room = self.helper.create_room_as(self.u_alice, tok=self.t_alice) + + rooms_for_user = self.get_success( + self.store.get_rooms_for_user_where_membership_is( + self.u_alice, [Membership.JOIN] + ) ) + self.assertEquals([self.room], [m.room_id for m in rooms_for_user]) + + def test_count_known_servers(self): + """ + _count_known_servers will calculate how many servers are in a room. + """ + self.room = self.helper.create_room_as(self.u_alice, tok=self.t_alice) + self.inject_room_member(self.room, self.u_bob, Membership.JOIN) + self.inject_room_member(self.room, self.u_charlie.to_string(), Membership.JOIN) + + servers = self.get_success(self.store._count_known_servers()) + self.assertEqual(servers, 2) + + def test_count_known_servers_stat_counter_disabled(self): + """ + If enabled, the metrics for how many servers are known will be counted. + """ + self.assertTrue("_known_servers_count" not in self.store.__dict__.keys()) + + self.room = self.helper.create_room_as(self.u_alice, tok=self.t_alice) + self.inject_room_member(self.room, self.u_bob, Membership.JOIN) + self.inject_room_member(self.room, self.u_charlie.to_string(), Membership.JOIN) + + self.pump(20) + + self.assertTrue("_known_servers_count" not in self.store.__dict__.keys()) + + @unittest.override_config( + {"enable_metrics": True, "metrics_flags": {"known_servers": True}} + ) + def test_count_known_servers_stat_counter_enabled(self): + """ + If enabled, the metrics for how many servers are known will be counted. + """ + # Initialises to 1 -- itself + self.assertEqual(self.store._known_servers_count, 1) + + self.pump(20) + + # No rooms have been joined, so technically the SQL returns 0, but it + # will still say it knows about itself. + self.assertEqual(self.store._known_servers_count, 1) + + self.room = self.helper.create_room_as(self.u_alice, tok=self.t_alice) + self.inject_room_member(self.room, self.u_bob, Membership.JOIN) + self.inject_room_member(self.room, self.u_charlie.to_string(), Membership.JOIN) + + self.pump(20) + + # It now knows about Charlie's server. + self.assertEqual(self.store._known_servers_count, 2) + class CurrentStateMembershipUpdateTestCase(unittest.HomeserverTestCase): def prepare(self, reactor, clock, homeserver):