From 0e5d92dd83813eb102cde80ee9a082eeed88b38c Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 16 Sep 2019 18:24:54 +0100 Subject: [PATCH 01/43] Allow HS to send emails when adding an email to the HS --- synapse/config/emailconfig.py | 30 +++ synapse/push/mailer.py | 28 +++ synapse/res/templates/add_threepid.html | 9 + synapse/res/templates/add_threepid.txt | 6 + .../res/templates/add_threepid_failure.html | 8 + .../res/templates/add_threepid_success.html | 6 + synapse/rest/client/v2_alpha/account.py | 204 +++++++++++++++--- 7 files changed, 266 insertions(+), 25 deletions(-) create mode 100644 synapse/res/templates/add_threepid.html create mode 100644 synapse/res/templates/add_threepid.txt create mode 100644 synapse/res/templates/add_threepid_failure.html create mode 100644 synapse/res/templates/add_threepid_success.html diff --git a/synapse/config/emailconfig.py b/synapse/config/emailconfig.py index e5de768b0ce1..48d7a9548717 100644 --- a/synapse/config/emailconfig.py +++ b/synapse/config/emailconfig.py @@ -169,12 +169,22 @@ def read_config(self, config, **kwargs): self.email_registration_template_text = email_config.get( "registration_template_text", "registration.txt" ) + self.email_add_threepid_template_html = email_config.get( + "add_threepid_template_html", "add_threepid.html" + ) + self.email_add_threepid_template_text = email_config.get( + "add_threepid_template_text", "add_threepid.txt" + ) + self.email_password_reset_template_failure_html = email_config.get( "password_reset_template_failure_html", "password_reset_failure.html" ) self.email_registration_template_failure_html = email_config.get( "registration_template_failure_html", "registration_failure.html" ) + self.email_add_threepid_template_failure_html = email_config.get( + "add_threepid_template_failure_html", "add_threepid_failure.html" + ) # These templates do not support any placeholder variables, so we # will read them from disk once during setup @@ -184,6 +194,9 @@ def read_config(self, config, **kwargs): email_registration_template_success_html = email_config.get( "registration_template_success_html", "registration_success.html" ) + email_add_threepid_template_success_html = email_config.get( + "add_threepid_template_success_html", "add_threepid_success.html" + ) # Check templates exist for f in [ @@ -191,9 +204,14 @@ def read_config(self, config, **kwargs): self.email_password_reset_template_text, self.email_registration_template_html, self.email_registration_template_text, + self.email_add_threepid_template_html, + self.email_add_threepid_template_text, self.email_password_reset_template_failure_html, + self.email_registration_template_failure_html, + self.email_add_threepid_template_failure_html, email_password_reset_template_success_html, email_registration_template_success_html, + email_add_threepid_template_success_html, ]: p = os.path.join(self.email_template_dir, f) if not os.path.isfile(p): @@ -212,6 +230,12 @@ def read_config(self, config, **kwargs): self.email_registration_template_success_html_content = self.read_file( filepath, "email.registration_template_success_html" ) + filepath = os.path.join( + self.email_template_dir, email_add_threepid_template_success_html + ) + self.email_add_threepid_template_success_html_content = self.read_file( + filepath, "email.add_threepid_template_success_html" + ) if self.email_enable_notifs: required = [ @@ -339,6 +363,12 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs): # # # #registration_template_success_html: registration_success.html # #registration_template_failure_html: registration_failure.html + # + # # Templates for success and failure pages that a user will see after attempting + # # to add an email or phone to their account + # # + # #add_threepid_success_html: add_threepid.html + # #add_threepid_failure_html: add_threepid.html """ diff --git a/synapse/push/mailer.py b/synapse/push/mailer.py index 3dfd52784914..70795d2b88cf 100644 --- a/synapse/push/mailer.py +++ b/synapse/push/mailer.py @@ -177,6 +177,34 @@ def send_registration_mail(self, email_address, token, client_secret, sid): template_vars, ) + @defer.inlineCallbacks + def send_add_threepid_mail(self, email_address, token, client_secret, sid): + """Send an email with a validation link to a user for adding a 3pid to their account + + Args: + email_address (str): Email address we're sending the validation 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/add_threepid/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] Validate Your Email" % self.hs.config.server_name, + template_vars, + ) + @defer.inlineCallbacks def send_notification_mail( self, app_id, user_id, email_address, push_actions, reason diff --git a/synapse/res/templates/add_threepid.html b/synapse/res/templates/add_threepid.html new file mode 100644 index 000000000000..cc4ab07e0987 --- /dev/null +++ b/synapse/res/templates/add_threepid.html @@ -0,0 +1,9 @@ + + +

A request to add an email address to your Matrix account has been received. If this was you, please click the link below to confirm adding this email:

+ + {{ link }} + +

If this was not you, you can safely ignore this email. Thank you.

+ + diff --git a/synapse/res/templates/add_threepid.txt b/synapse/res/templates/add_threepid.txt new file mode 100644 index 000000000000..a60c1ff659b5 --- /dev/null +++ b/synapse/res/templates/add_threepid.txt @@ -0,0 +1,6 @@ +A request to add an email address to your Matrix account has been received. If this was you, +please click the link below to confirm adding this email: + +{{ link }} + +If this was not you, you can safely ignore this email. Thank you. diff --git a/synapse/res/templates/add_threepid_failure.html b/synapse/res/templates/add_threepid_failure.html new file mode 100644 index 000000000000..441d11c846c2 --- /dev/null +++ b/synapse/res/templates/add_threepid_failure.html @@ -0,0 +1,8 @@ + + + +

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

+ +

No changes have been made to your account.

+ + diff --git a/synapse/res/templates/add_threepid_success.html b/synapse/res/templates/add_threepid_success.html new file mode 100644 index 000000000000..fbd6e4018f7d --- /dev/null +++ b/synapse/res/templates/add_threepid_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/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 2ea515d2f6c6..44eaccc0176b 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -416,13 +416,26 @@ def __init__(self, hs): self.identity_handler = hs.get_handlers().identity_handler self.store = self.hs.get_datastore() + if self.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL: + template_html, template_text = load_jinja2_templates( + self.config.email_template_dir, + [ + self.config.email_add_threepid_template_html, + self.config.email_add_threepid_template_text, + ], + 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): body = parse_json_object_from_request(request) - assert_params_in_dict( - body, ["id_server", "client_secret", "email", "send_attempt"] - ) - id_server = "https://" + body["id_server"] # Assume https + assert_params_in_dict(body, ["client_secret", "email", "send_attempt"]) client_secret = body["client_secret"] email = body["email"] send_attempt = body["send_attempt"] @@ -442,9 +455,37 @@ def on_POST(self, request): 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( - id_server, email, client_secret, send_attempt, next_link - ) + 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" + ) + + ret = yield self.identity_handler.requestEmailToken( + self.hs.config.account_threepid_delegate_email, + email, + client_secret, + send_attempt, + next_link, + ) + else: + # Send password reset emails from Synapse + sid = yield self.identity_handler.send_threepid_validation( + email, + client_secret, + send_attempt, + self.mailer.send_add_threepid_mail, + next_link, + ) + + # Wrap the session id in a JSON object + ret = {"sid": sid} + return 200, ret @@ -461,10 +502,8 @@ def __init__(self, hs): def on_POST(self, request): 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"] ) - id_server = "https://" + body["id_server"] # Assume https client_secret = body["client_secret"] country = body["country"] phone_number = body["phone_number"] @@ -485,12 +524,118 @@ def on_POST(self, request): if existing_user_id is not None: raise SynapseError(400, "MSISDN is already in use", Codes.THREEPID_IN_USE) + 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( - id_server, country, phone_number, client_secret, send_attempt, next_link + self.hs.config.email_account_threepid_delegate_msisdn, + country, + phone_number, + client_secret, + send_attempt, + next_link, ) + return 200, ret +class AddThreepidSubmitTokenServlet(RestServlet): + """Handles 3PID validation token submission for adding a 3PID to a user's account""" + + PATTERNS = client_patterns( + "/add_threepid/(?P[^/]*)/submit_token/*$", releases=(), unstable=True + ) + + def __init__(self, hs): + """ + Args: + hs (synapse.server.HomeServer): server + """ + super(AddThreepidSubmitTokenServlet, 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): + # We currently only handle threepid token submissions for email + if medium != "email": + raise SynapseError(400, "This medium is currently not supported") + if self.config.threepid_behaviour_email == ThreepidBehaviour.OFF: + if self.config.local_threepid_handling_disabled_due_to_email_config: + logger.warn( + "Adding emails have been disabled due to lack of an email config" + ) + raise SynapseError( + 400, "Adding an email to your account 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_add_threepid_template_success_html_content + request.setResponseCode(200) + except ThreepidValidationError as e: + request.setResponseCode(e.code) + + # Show a failure page with a reason + html_template, = load_jinja2_templates( + self.config.email_template_dir, + [self.config.email_add_threepid_template_failure_html], + ) + + template_vars = {"failure_reason": e.msg} + html = html_template.render(**template_vars) + + request.write(html.encode("utf-8")) + finish_request(request) + + @defer.inlineCallbacks + def on_POST(self, request, medium): + if medium != "email": + raise SynapseError(400, "This medium is currently not supported") + + body = parse_json_object_from_request(request) + assert_params_in_dict(body, ["sid", "client_secret", "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 + + return response_code, {"success": valid} + + class ThreepidRestServlet(RestServlet): PATTERNS = client_patterns("/account/3pid$") @@ -523,24 +668,32 @@ def on_POST(self, request): requester = yield self.auth.get_user_by_req(request) user_id = requester.user.to_string() - # Specify None as the identity server to retrieve it from the request body instead - threepid = yield self.identity_handler.threepid_from_creds(None, threepid_creds) + if self.hs.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE: + # Specify None as the identity server to retrieve it from the request body instead + threepid = yield self.identity_handler.threepid_from_creds( + None, threepid_creds + ) - if not threepid: - raise SynapseError(400, "Failed to auth 3pid", Codes.THREEPID_AUTH_FAILED) + if not threepid: + raise SynapseError( + 400, "Failed to auth 3pid", Codes.THREEPID_AUTH_FAILED + ) - for reqd in ["medium", "address", "validated_at"]: - if reqd not in threepid: - logger.warn("Couldn't add 3pid: invalid response from ID server") - raise SynapseError(500, "Invalid response from ID Server") + for reqd in ["medium", "address", "validated_at"]: + if reqd not in threepid: + logger.warn("Couldn't add 3pid: invalid response from ID server") + raise SynapseError(500, "Invalid response from ID Server") - yield self.auth_handler.add_threepid( - user_id, threepid["medium"], threepid["address"], threepid["validated_at"] - ) + yield self.auth_handler.add_threepid( + user_id, + threepid["medium"], + threepid["address"], + threepid["validated_at"], + ) - if "bind" in body and body["bind"]: - logger.debug("Binding threepid %s to %s", threepid, user_id) - yield self.identity_handler.bind_threepid(threepid_creds, user_id) + if "bind" in body and body["bind"]: + logger.debug("Binding threepid %s to %s", threepid, user_id) + yield self.identity_handler.bind_threepid(threepid_creds, user_id) return 200, {} @@ -634,6 +787,7 @@ def register_servlets(hs, http_server): DeactivateAccountRestServlet(hs).register(http_server) EmailThreepidRequestTokenRestServlet(hs).register(http_server) MsisdnThreepidRequestTokenRestServlet(hs).register(http_server) + AddThreepidSubmitTokenServlet(hs).register(http_server) ThreepidRestServlet(hs).register(http_server) ThreepidUnbindRestServlet(hs).register(http_server) ThreepidDeleteRestServlet(hs).register(http_server) From 92b3246bb019cae3715c724279a337d6021914b9 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 16 Sep 2019 18:31:08 +0100 Subject: [PATCH 02/43] Add changelog --- changelog.d/6042.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/6042.feature diff --git a/changelog.d/6042.feature b/changelog.d/6042.feature new file mode 100644 index 000000000000..e65b74090b22 --- /dev/null +++ b/changelog.d/6042.feature @@ -0,0 +1 @@ +Allow homeservers to send emails when clients request adding a new email address to their account. \ No newline at end of file From 626ebbdde38f68eee8cb03b22eb95e94ffdf442c Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 17 Sep 2019 11:30:58 +0100 Subject: [PATCH 03/43] Add changelog --- changelog.d/6042.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/6042.feature diff --git a/changelog.d/6042.feature b/changelog.d/6042.feature new file mode 100644 index 000000000000..c16526606767 --- /dev/null +++ b/changelog.d/6042.feature @@ -0,0 +1 @@ +Allow homeserver to handle threepid validation itself when adding a threepid to your account. \ No newline at end of file From 8132107970b9159223d244a880072299cb704548 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 17 Sep 2019 11:35:01 +0100 Subject: [PATCH 04/43] Add 'add threepid' email templates to config. Gen sample config --- docs/sample_config.yaml | 12 ++++++++++++ synapse/config/emailconfig.py | 6 ++++++ 2 files changed, 18 insertions(+) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index dd4e2d5ebd90..4dc076ae76d7 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1261,6 +1261,12 @@ password_config: # #registration_template_html: registration.html # #registration_template_text: registration.txt # +# # Templates for validation emails sent by the homeserver when adding an email to +# # your user account +# # +# #add_threepid_template_html: add_threepid.html +# #add_threepid_template_text: add_threepid.txt +# # # Templates for password reset success and failure pages that a user # # will see after attempting to reset their password # # @@ -1272,6 +1278,12 @@ password_config: # # # #registration_template_success_html: registration_success.html # #registration_template_failure_html: registration_failure.html +# +# # Templates for success and failure pages that a user will see after attempting +# # to add an email or phone to their account +# # +# #add_threepid_success_html: add_threepid.html +# #add_threepid_failure_html: add_threepid.html #password_providers: diff --git a/synapse/config/emailconfig.py b/synapse/config/emailconfig.py index 48d7a9548717..5bd72e70a530 100644 --- a/synapse/config/emailconfig.py +++ b/synapse/config/emailconfig.py @@ -352,6 +352,12 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs): # #registration_template_html: registration.html # #registration_template_text: registration.txt # + # # Templates for validation emails sent by the homeserver when adding an email to + # # your user account + # # + # #add_threepid_template_html: add_threepid.html + # #add_threepid_template_text: add_threepid.txt + # # # Templates for password reset success and failure pages that a user # # will see after attempting to reset their password # # From 096808b7d1a1d655e9e7478add9c7ea6e5f35b38 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 17 Sep 2019 13:52:37 +0100 Subject: [PATCH 05/43] Add 3PID bind sytests to blacklist temporarily --- sytest-blacklist | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/sytest-blacklist b/sytest-blacklist index 11785fd43f90..e1874f260e6b 100644 --- a/sytest-blacklist +++ b/sytest-blacklist @@ -29,3 +29,13 @@ Enabling an unknown default rule fails with 404 # Blacklisted due to https://github.com/matrix-org/synapse/issues/1663 New federated private chats get full presence information (SYN-115) + +# Blacklisted temporarily due to https://github.com/matrix-org/matrix-doc/pull/2290 +# These sytests need to be updated with new endpoints, which will come in a later PR +# That PR will also remove this blacklist +Can login with 3pid and password using m.login.password +Can bind 3PID via home server +Can bind and unbind 3PID via homeserver +3PIDs are unbound after account deactivation +Can bind and unbind 3PID via /unbind by specifying the identity server +Can bind and unbind 3PID via /unbind without specifying the identity server \ No newline at end of file From 268bbbb69533f7c25096a74b8dcdae4d7e65d815 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 17 Sep 2019 14:11:07 +0100 Subject: [PATCH 06/43] Prevent /account/3pid from binding to an identity server --- synapse/rest/client/v2_alpha/account.py | 38 +++++++++++-------------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 44eaccc0176b..9bccfd9de2a6 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -665,35 +665,31 @@ def on_POST(self, request): 400, "Missing param three_pid_creds", Codes.MISSING_PARAM ) + # We ignore the bind parameter as this endpoint no longer is able to bind threepids + # to an identity server + requester = yield self.auth.get_user_by_req(request) user_id = requester.user.to_string() - if self.hs.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE: - # Specify None as the identity server to retrieve it from the request body instead - threepid = yield self.identity_handler.threepid_from_creds( - None, threepid_creds - ) + threepid_creds = body["three_pid_creds"] + assert_params_in_dict(threepid_creds, ["client_secret", "sid"]) - if not threepid: - raise SynapseError( - 400, "Failed to auth 3pid", Codes.THREEPID_AUTH_FAILED - ) + client_secret = body["client_secret"] + sid = body["sid"] - for reqd in ["medium", "address", "validated_at"]: - if reqd not in threepid: - logger.warn("Couldn't add 3pid: invalid response from ID server") - raise SynapseError(500, "Invalid response from ID Server") + # Get a validated session matching these details + validation_session = self.datastore.get_threepid_validation_session( + None, client_secret, sid=sid + ) - yield self.auth_handler.add_threepid( - user_id, - threepid["medium"], - threepid["address"], - threepid["validated_at"], + if not validation_session: + raise SynapseError( + 400, "No validated 3pid session found", Codes.THREEPID_AUTH_FAILED ) - if "bind" in body and body["bind"]: - logger.debug("Binding threepid %s to %s", threepid, user_id) - yield self.identity_handler.bind_threepid(threepid_creds, user_id) + address, _, medium, _, _, validated_at = validation_session + + yield self.auth_handler.add_threepid(user_id, medium, address, validated_at) return 200, {} From dda51373f9cfa2035f8a819e7fec39f7a5bccd0f Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 17 Sep 2019 14:24:21 +0100 Subject: [PATCH 07/43] Correct some small issues and fix bug There was a bug in how validate_threepid_session was called when POST'ing to the password reset endpoint. It thought the method returned a boolean to check if a validation was incorrect, but instead it raises an exception. This has been corrected --- synapse/rest/client/v2_alpha/account.py | 51 ++++++++++++++++--------- synapse/storage/registration.py | 4 ++ 2 files changed, 38 insertions(+), 17 deletions(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 9bccfd9de2a6..1976ad2c429e 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -282,12 +282,20 @@ def on_POST(self, request, medium): body = parse_json_object_from_request(request) assert_params_in_dict(body, ["sid", "client_secret", "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 - - return response_code, {"success": valid} + try: + yield self.store.validate_threepid_session( + body["sid"], + body["client_secret"], + body["token"], + self.clock.time_msec(), + ) + return 200, {"success": True} + except ThreepidValidationError: + # Validation failure + return 400, {"success": False} + except Exception as e: + logger.error("Error validating threepid session: %s", e) + raise e class PasswordRestServlet(RestServlet): @@ -463,7 +471,8 @@ def on_POST(self, request): "handle this request" ) raise SynapseError( - 400, "Password reset by email is not supported on this homeserver" + 400, + "Adding emails to user accounts is not supported by this homeserver", ) ret = yield self.identity_handler.requestEmailToken( @@ -474,7 +483,7 @@ def on_POST(self, request): next_link, ) else: - # Send password reset emails from Synapse + # Send threepid validation emails from Synapse sid = yield self.identity_handler.send_threepid_validation( email, client_secret, @@ -531,7 +540,7 @@ def on_POST(self, request): ) raise SynapseError( 400, - "Password reset by phone number is not supported on this homeserver", + "Adding phone numbers to user account is not supported by this homeserver", ) ret = yield self.identity_handler.requestMsisdnToken( @@ -569,7 +578,7 @@ def __init__(self, hs): 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") + raise SynapseError(400, "This medium is not supported") if self.config.threepid_behaviour_email == ThreepidBehaviour.OFF: if self.config.local_threepid_handling_disabled_due_to_email_config: logger.warn( @@ -623,17 +632,25 @@ def on_GET(self, request, medium): @defer.inlineCallbacks def on_POST(self, request, medium): if medium != "email": - raise SynapseError(400, "This medium is currently not supported") + raise SynapseError(400, "This medium is not supported") body = parse_json_object_from_request(request) assert_params_in_dict(body, ["sid", "client_secret", "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 - - return response_code, {"success": valid} + try: + yield self.store.validate_threepid_session( + body["sid"], + body["client_secret"], + body["token"], + self.clock.time_msec(), + ) + return 200, {"success": True} + except ThreepidValidationError: + # Validation failure + return 400, {"success": False} + except Exception as e: + logger.error("Error validating threepid session: %s", e) + raise e class ThreepidRestServlet(RestServlet): diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py index 109052fa41dd..4c3d59635652 100644 --- a/synapse/storage/registration.py +++ b/synapse/storage/registration.py @@ -1209,6 +1209,10 @@ def validate_threepid_session(self, session_id, client_secret, token, current_ts current_ts (int): The current unix time in milliseconds. Used for checking token expiry status + Raises: + ThreepidValidationError: if a matching validation token was not found or has + expired + Returns: deferred str|None: A str representing a link to redirect the user to if there is one. From 8e2481b7800bbddac34d59f79526fad9da127797 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 18 Sep 2019 12:10:50 +0100 Subject: [PATCH 08/43] Address review comments --- synapse/rest/client/v2_alpha/account.py | 22 +++++++++------------- synapse/storage/registration.py | 12 +++++++++--- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 1976ad2c429e..78deda7eb199 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -674,6 +674,8 @@ def on_GET(self, request): @defer.inlineCallbacks def on_POST(self, request): + requester = yield self.auth.get_user_by_req(request) + user_id = requester.user.to_string() body = parse_json_object_from_request(request) threepid_creds = body.get("threePidCreds") or body.get("three_pid_creds") @@ -681,22 +683,14 @@ def on_POST(self, request): raise SynapseError( 400, "Missing param three_pid_creds", Codes.MISSING_PARAM ) - - # We ignore the bind parameter as this endpoint no longer is able to bind threepids - # to an identity server - - requester = yield self.auth.get_user_by_req(request) - user_id = requester.user.to_string() - - threepid_creds = body["three_pid_creds"] assert_params_in_dict(threepid_creds, ["client_secret", "sid"]) - client_secret = body["client_secret"] - sid = body["sid"] + client_secret = threepid_creds["client_secret"] + sid = threepid_creds["sid"] # Get a validated session matching these details - validation_session = self.datastore.get_threepid_validation_session( - None, client_secret, sid=sid + validation_session = yield self.datastore.get_threepid_validation_session( + None, client_secret, sid=sid, validated=True ) if not validation_session: @@ -704,7 +698,9 @@ def on_POST(self, request): 400, "No validated 3pid session found", Codes.THREEPID_AUTH_FAILED ) - address, _, medium, _, _, validated_at = validation_session + address = validation_session["address"] + medium = validation_session["medium"] + validated_at = validation_session["validated_at"] yield self.auth_handler.add_threepid(user_id, medium, address, validated_at) diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py index 4c3d59635652..c051cba0be2f 100644 --- a/synapse/storage/registration.py +++ b/synapse/storage/registration.py @@ -668,9 +668,15 @@ def get_threepid_validation_session( 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 + deferred {str, str|int|None}|None: A dict containing the following: + * address - address of the 3pid + * medium - medium of the 3pid + * client_secret - a secret provided by the client for this validation session + * session_id - ID of the validation session + * send_attempt - a number serving to dedupe send attempts for this session + * validated_at - timestamp of when this session was validated if so + + Otherwise None if a validation session is not found """ keyvalues = {"medium": medium, "client_secret": client_secret} if address: From f6c0d5c4bede36fb17ead70d527a41941b7a1a38 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 18 Sep 2019 14:07:44 +0100 Subject: [PATCH 09/43] Remove the requirement for a medium argument --- synapse/storage/registration.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py index c051cba0be2f..c3a82d8af12d 100644 --- a/synapse/storage/registration.py +++ b/synapse/storage/registration.py @@ -678,7 +678,11 @@ def get_threepid_validation_session( Otherwise None if a validation session is not found """ - keyvalues = {"medium": medium, "client_secret": client_secret} + keyvalues = {} + if client_secret: + keyvalues["client_secret"] = client_secret + if medium: + keyvalues["medium"] = medium if address: keyvalues["address"] = address if sid: From 923736f6a669196a076cbb0cc2b1f631142ce4ee Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 18 Sep 2019 14:20:36 +0100 Subject: [PATCH 10/43] Handle the homeserver disabling email validation --- synapse/rest/client/v2_alpha/account.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 78deda7eb199..ad1bb025d96b 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -442,6 +442,15 @@ def __init__(self, hs): @defer.inlineCallbacks def on_POST(self, request): + if self.config.threepid_behaviour_email == ThreepidBehaviour.OFF: + if self.config.local_threepid_handling_disabled_due_to_email_config: + logger.warn( + "Adding emails have been disabled due to lack of an email config" + ) + raise SynapseError( + 400, "Adding an email to your account is disabled on this server" + ) + body = parse_json_object_from_request(request) assert_params_in_dict(body, ["client_secret", "email", "send_attempt"]) client_secret = body["client_secret"] From 4fc7796770bc4a51b8eec9cd3a8dd476aa824d60 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 18 Sep 2019 15:37:47 +0100 Subject: [PATCH 11/43] Remove blacklist --- sytest-blacklist | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/sytest-blacklist b/sytest-blacklist index e1874f260e6b..11785fd43f90 100644 --- a/sytest-blacklist +++ b/sytest-blacklist @@ -29,13 +29,3 @@ Enabling an unknown default rule fails with 404 # Blacklisted due to https://github.com/matrix-org/synapse/issues/1663 New federated private chats get full presence information (SYN-115) - -# Blacklisted temporarily due to https://github.com/matrix-org/matrix-doc/pull/2290 -# These sytests need to be updated with new endpoints, which will come in a later PR -# That PR will also remove this blacklist -Can login with 3pid and password using m.login.password -Can bind 3PID via home server -Can bind and unbind 3PID via homeserver -3PIDs are unbound after account deactivation -Can bind and unbind 3PID via /unbind by specifying the identity server -Can bind and unbind 3PID via /unbind without specifying the identity server \ No newline at end of file From 15bf1083eaafbd38f2eaa307d7e3564dc0ca384e Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 18 Sep 2019 15:48:12 +0100 Subject: [PATCH 12/43] Fix add_threepid template default values in emailconfig --- docs/sample_config.yaml | 4 ++-- synapse/config/emailconfig.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 4dc076ae76d7..c1ef73ea5f88 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1282,8 +1282,8 @@ password_config: # # Templates for success and failure pages that a user will see after attempting # # to add an email or phone to their account # # -# #add_threepid_success_html: add_threepid.html -# #add_threepid_failure_html: add_threepid.html +# #add_threepid_success_html: add_threepid_success.html +# #add_threepid_failure_html: add_threepid_failure.html #password_providers: diff --git a/synapse/config/emailconfig.py b/synapse/config/emailconfig.py index 5bd72e70a530..d9b43de66059 100644 --- a/synapse/config/emailconfig.py +++ b/synapse/config/emailconfig.py @@ -373,8 +373,8 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs): # # Templates for success and failure pages that a user will see after attempting # # to add an email or phone to their account # # - # #add_threepid_success_html: add_threepid.html - # #add_threepid_failure_html: add_threepid.html + # #add_threepid_success_html: add_threepid_success.html + # #add_threepid_failure_html: add_threepid_failure.html """ From d20feecf80689cd412f9016897ae85fa90b826b7 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 18 Sep 2019 15:56:35 +0100 Subject: [PATCH 13/43] Remove redundant logging --- synapse/rest/client/v2_alpha/account.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index ad1bb025d96b..481f5fe592c0 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -293,9 +293,6 @@ def on_POST(self, request, medium): except ThreepidValidationError: # Validation failure return 400, {"success": False} - except Exception as e: - logger.error("Error validating threepid session: %s", e) - raise e class PasswordRestServlet(RestServlet): From 0e5c7bf508782bd411b4d8e0cb936f22132024de Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 18 Sep 2019 15:58:29 +0100 Subject: [PATCH 14/43] Undo fix on password reset POST submit_token endpoint --- synapse/rest/client/v2_alpha/account.py | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 481f5fe592c0..3119d319e70f 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -282,17 +282,12 @@ def on_POST(self, request, medium): body = parse_json_object_from_request(request) assert_params_in_dict(body, ["sid", "client_secret", "token"]) - try: - yield self.store.validate_threepid_session( - body["sid"], - body["client_secret"], - body["token"], - self.clock.time_msec(), - ) - return 200, {"success": True} - except ThreepidValidationError: - # Validation failure - return 400, {"success": False} + 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 + + return response_code, {"success": valid} class PasswordRestServlet(RestServlet): From 46da943ccef7f1ee85261e67d5c57cb1c1f77518 Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Wed, 18 Sep 2019 16:00:32 +0100 Subject: [PATCH 15/43] Update synapse/storage/registration.py Co-Authored-By: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- synapse/storage/registration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py index c3a82d8af12d..cb8eb0c78358 100644 --- a/synapse/storage/registration.py +++ b/synapse/storage/registration.py @@ -668,7 +668,7 @@ def get_threepid_validation_session( perform no filtering Returns: - deferred {str, str|int|None}|None: A dict containing the following: + Deferred[dict|None]: A dict containing the following: * address - address of the 3pid * medium - medium of the 3pid * client_secret - a secret provided by the client for this validation session From 39414399eff3180f6e8e87e670f7b2932745f11e Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 18 Sep 2019 16:15:13 +0100 Subject: [PATCH 16/43] Use an assert instead of if-statement with SynapseError --- synapse/rest/client/v2_alpha/account.py | 23 ++++------------------- synapse/rest/client/v2_alpha/register.py | 10 ++-------- 2 files changed, 6 insertions(+), 27 deletions(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 3119d319e70f..d03870d2b2be 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -103,16 +103,9 @@ def on_POST(self, request): raise SynapseError(400, "Email not found", Codes.THREEPID_NOT_FOUND) 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" - ) + assert self.hs.config.account_threepid_delegate_email + # Have the configured identity server handle the request ret = yield self.identity_handler.requestEmailToken( self.hs.config.account_threepid_delegate_email, email, @@ -465,17 +458,9 @@ def on_POST(self, request): raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE) 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, - "Adding emails to user accounts is not supported by this homeserver", - ) + assert self.hs.config.account_threepid_delegate_email + # Have the configured identity server handle the request ret = yield self.identity_handler.requestEmailToken( self.hs.config.account_threepid_delegate_email, email, diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index 5c7a5f357910..522380fb3813 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -131,15 +131,9 @@ def on_POST(self, request): raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE) 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" - ) + assert self.hs.config.account_threepid_delegate_email + # Have the configured identity server handle the request ret = yield self.identity_handler.requestEmailToken( self.hs.config.account_threepid_delegate_email, email, From 0200e411e649ffc792786563f3eb9601122e1e49 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 18 Sep 2019 16:18:51 +0100 Subject: [PATCH 17/43] Remove trailing slashes from submit_token endpoint patterns --- synapse/rest/client/v2_alpha/account.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index d03870d2b2be..0cfec0a7b314 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -193,7 +193,7 @@ class PasswordResetSubmitTokenServlet(RestServlet): """Handles 3PID validation token submission""" PATTERNS = client_patterns( - "/password_reset/(?P[^/]*)/submit_token/*$", releases=(), unstable=True + "/password_reset/(?P[^/]*)/submit_token$", releases=(), unstable=True ) def __init__(self, hs): @@ -545,7 +545,7 @@ class AddThreepidSubmitTokenServlet(RestServlet): """Handles 3PID validation token submission for adding a 3PID to a user's account""" PATTERNS = client_patterns( - "/add_threepid/(?P[^/]*)/submit_token/*$", releases=(), unstable=True + "/add_threepid/(?P[^/]*)/submit_token$", releases=(), unstable=True ) def __init__(self, hs): From c8dbf53f020249076ead5c91b786f0aa87196668 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 18 Sep 2019 16:19:46 +0100 Subject: [PATCH 18/43] python3 super() --- synapse/rest/client/v2_alpha/account.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 0cfec0a7b314..ba8c4a9cefbf 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -553,7 +553,7 @@ def __init__(self, hs): Args: hs (synapse.server.HomeServer): server """ - super(AddThreepidSubmitTokenServlet, self).__init__() + super().__init__() self.hs = hs self.auth = hs.get_auth() self.config = hs.config From d32724bc475f4c2bfefa7b7b4160451f825cf0e4 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 18 Sep 2019 16:21:27 +0100 Subject: [PATCH 19/43] Remove self.hs set from AddThreepidSubmitTokenServlet --- synapse/rest/client/v2_alpha/account.py | 1 - 1 file changed, 1 deletion(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index ba8c4a9cefbf..e5a26a084da0 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -554,7 +554,6 @@ def __init__(self, hs): hs (synapse.server.HomeServer): server """ super().__init__() - self.hs = hs self.auth = hs.get_auth() self.config = hs.config self.clock = hs.get_clock() From 0812664b627fffdff97ef229fa6fd28fd3c5ca57 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 18 Sep 2019 16:23:29 +0100 Subject: [PATCH 20/43] Clarify changelog --- changelog.d/6042.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/6042.feature b/changelog.d/6042.feature index 2ca5874db6b8..48ec22249b31 100644 --- a/changelog.d/6042.feature +++ b/changelog.d/6042.feature @@ -1 +1 @@ -Allow homeserver to handle threepid validation itself when adding a threepid to a user's account. +Allow homeserver to handle or delegate threepid validation when adding a threepid to a user's account. From 46e6d921ffa7d520d269bf412114fdba05e799f2 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 18 Sep 2019 16:27:51 +0100 Subject: [PATCH 21/43] Sanity check ThreepidBehaviour.REMOTE on submit_token endpoint --- synapse/rest/client/v2_alpha/account.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index e5a26a084da0..186a29835b30 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -572,6 +572,12 @@ def on_GET(self, request, medium): raise SynapseError( 400, "Adding an email to your account is disabled on this server" ) + elif self.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE: + raise SynapseError( + 400, + "This homeserver is not validating threepids. Use an identity server " + "instead.", + ) sid = parse_string(request, "sid", required=True) client_secret = parse_string(request, "client_secret", required=True) From b37d71c7992b3bc669fd15c1cb30cc9fe5daf3ac Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 18 Sep 2019 18:20:24 +0100 Subject: [PATCH 22/43] Re-blacklist tests --- sytest-blacklist | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/sytest-blacklist b/sytest-blacklist index 11785fd43f90..7eab44137db7 100644 --- a/sytest-blacklist +++ b/sytest-blacklist @@ -29,3 +29,12 @@ Enabling an unknown default rule fails with 404 # Blacklisted due to https://github.com/matrix-org/synapse/issues/1663 New federated private chats get full presence information (SYN-115) + +# Blacklisted temporarily due to https://github.com/matrix-org/matrix-doc/pull/2290 +# These sytests need to be updated with new endpoints, which will come in a later PR +# That PR will also remove this blacklist +Can bind 3PID via home server +Can bind and unbind 3PID via homeserver +3PIDs are unbound after account deactivation +Can bind and unbind 3PID via /unbind by specifying the identity server +Can bind and unbind 3PID via /unbind without specifying the identity server \ No newline at end of file From 16eb35db4ff0ffacf2c5f02b70c4d084def81c6d Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 18 Sep 2019 18:36:35 +0100 Subject: [PATCH 23/43] Factor out removing id_server from msisdn --- synapse/rest/client/v2_alpha/account.py | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 186a29835b30..b315c8712ace 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -497,8 +497,10 @@ def __init__(self, hs): def on_POST(self, request): body = parse_json_object_from_request(request) assert_params_in_dict( - body, ["client_secret", "country", "phone_number", "send_attempt"] + 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"] @@ -519,23 +521,8 @@ def on_POST(self, request): if existing_user_id is not None: raise SynapseError(400, "MSISDN is already in use", Codes.THREEPID_IN_USE) - 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, - "Adding phone numbers to user account is not supported by this homeserver", - ) - ret = yield self.identity_handler.requestMsisdnToken( - self.hs.config.email_account_threepid_delegate_msisdn, - country, - phone_number, - client_secret, - send_attempt, - next_link, + id_server, country, phone_number, client_secret, send_attempt, next_link ) return 200, ret From 16d7ec2058e276786b0f6fe4cee99a1ed1956c1c Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 18 Sep 2019 18:55:51 +0100 Subject: [PATCH 24/43] Remove POST /add_threepid//submit_token --- synapse/rest/client/v2_alpha/account.py | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index b315c8712ace..29916b39dc27 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -607,29 +607,6 @@ def on_GET(self, request, medium): request.write(html.encode("utf-8")) finish_request(request) - @defer.inlineCallbacks - def on_POST(self, request, medium): - if medium != "email": - raise SynapseError(400, "This medium is not supported") - - body = parse_json_object_from_request(request) - assert_params_in_dict(body, ["sid", "client_secret", "token"]) - - try: - yield self.store.validate_threepid_session( - body["sid"], - body["client_secret"], - body["token"], - self.clock.time_msec(), - ) - return 200, {"success": True} - except ThreepidValidationError: - # Validation failure - return 400, {"success": False} - except Exception as e: - logger.error("Error validating threepid session: %s", e) - raise e - class ThreepidRestServlet(RestServlet): PATTERNS = client_patterns("/account/3pid$") From ab1ae2ff69b44f6e1d787e6b44e3e8a482e18edf Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 19 Sep 2019 10:50:08 +0100 Subject: [PATCH 25/43] Ensure REMOTE vs LOCAL ThreepidBehaviour is handled --- synapse/rest/client/v2_alpha/account.py | 67 ++++++++++++++++++++----- 1 file changed, 55 insertions(+), 12 deletions(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 29916b39dc27..44eacc722cc5 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -643,23 +643,66 @@ def on_POST(self, request): client_secret = threepid_creds["client_secret"] sid = threepid_creds["sid"] - # Get a validated session matching these details - validation_session = yield self.datastore.get_threepid_validation_session( - None, client_secret, sid=sid, validated=True - ) + # We don't actually know which medium this 3PID is. Thus we first assume it's email, + # and if validation fails we try msisdn + validation_session = None - if not validation_session: - raise SynapseError( - 400, "No validated 3pid session found", Codes.THREEPID_AUTH_FAILED + # Try to validate as email + if self.hs.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE: + # Ask our delegated email identity server + validation_session = yield self.identity_handler.threepid_from_creds( + self.hs.config.account_threepid_delegate_email, threepid_creds + ) + elif self.hs.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL: + # Get a validated session matching these details + validation_session = yield self.datastore.get_threepid_validation_session( + None, client_secret, sid=sid, validated=True + ) + + if self._add_threepid_to_account(user_id, validation_session): + return 200, {} + + # Try to validate as msisdn + if self.hs.config.account_threepid_delegate_msisdn: + # Ask our delegated msisdn identity server + validation_session = yield self.identity_handler.threepid_from_creds( + self.hs.config.account_threepid_delegate_msisdn, threepid_creds ) - address = validation_session["address"] - medium = validation_session["medium"] - validated_at = validation_session["validated_at"] + if self._add_threepid_to_account(user_id, validation_session): + return 200, {} - yield self.auth_handler.add_threepid(user_id, medium, address, validated_at) + raise SynapseError( + 400, "No validated 3pid session found", Codes.THREEPID_AUTH_FAILED + ) - return 200, {} + def _add_threepid_to_account(self, user_id, validation_session): + """Attempt to add a threepid wrapped in a validation_session dict to an account + + Args: + user_id (str): The mxid of the user to add this 3PID to + + validation_session (dict|None): A dict containing the following: + * medium - medium of the threepid + * address - address of the threepid + * validated_at - timestamp of when the validation occurred + + If validation_session is None, this method will return False + + Returns: + A boolean stating whether adding the threepid was successful + """ + if not validation_session: + return False + + yield self.auth_handler.add_threepid( + user_id, + validation_session["medium"], + validation_session["address"], + validation_session["validated_at"], + ) + + return True class ThreepidUnbindRestServlet(RestServlet): From 1be82f8c021632d06d9f2caf436bcc31c0bb8120 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 19 Sep 2019 14:57:03 +0100 Subject: [PATCH 26/43] URL encode validation link parameters --- synapse/push/mailer.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/synapse/push/mailer.py b/synapse/push/mailer.py index 70795d2b88cf..90f2520008db 100644 --- a/synapse/push/mailer.py +++ b/synapse/push/mailer.py @@ -191,10 +191,11 @@ def send_add_threepid_mail(self, email_address, token, client_secret, sid): sid (str): The generated session ID """ + params = {"token": token, "client_secret": client_secret, "sid": sid} link = ( self.hs.config.public_baseurl - + "_matrix/client/unstable/add_threepid/email/submit_token" - "?token=%s&client_secret=%s&sid=%s" % (token, client_secret, sid) + + "_matrix/client/unstable/add_threepid/email/submit_token?%s" + % urllib.parse.urlencode(params), ) template_vars = {"link": link} From 317332eb84a537e84870459159bbb6e2117322ef Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 19 Sep 2019 16:51:55 +0100 Subject: [PATCH 27/43] Move jinja failure template loading into servlet constructor --- synapse/rest/client/v2_alpha/account.py | 22 ++++++++++------------ synapse/rest/client/v2_alpha/register.py | 12 +++++------- 2 files changed, 15 insertions(+), 19 deletions(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 44eacc722cc5..cdb6b8deb3e4 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -207,6 +207,10 @@ def __init__(self, hs): self.config = hs.config self.clock = hs.get_clock() self.store = hs.get_datastore() + self.failure_email_template, = load_jinja2_templates( + self.config.email_template_dir, + [self.config.email_password_reset_template_failure_html], + ) @defer.inlineCallbacks def on_GET(self, request, medium): @@ -254,13 +258,8 @@ def on_GET(self, request, medium): 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) + html = self.failure_email_template.render(**template_vars) request.write(html.encode("utf-8")) finish_request(request) @@ -545,6 +544,10 @@ def __init__(self, hs): self.config = hs.config self.clock = hs.get_clock() self.store = hs.get_datastore() + self.failure_email_template, = load_jinja2_templates( + self.config.email_template_dir, + [self.config.email_add_threepid_template_failure_html], + ) @defer.inlineCallbacks def on_GET(self, request, medium): @@ -596,13 +599,8 @@ def on_GET(self, request, medium): request.setResponseCode(e.code) # Show a failure page with a reason - html_template, = load_jinja2_templates( - self.config.email_template_dir, - [self.config.email_add_threepid_template_failure_html], - ) - template_vars = {"failure_reason": e.msg} - html = html_template.render(**template_vars) + html = self.failure_email_template.render(**template_vars) request.write(html.encode("utf-8")) finish_request(request) diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index 522380fb3813..081b3b7872e6 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -239,6 +239,10 @@ def __init__(self, hs): self.config = hs.config self.clock = hs.get_clock() self.store = hs.get_datastore() + self.failure_email_template, = load_jinja2_templates( + self.config.email_template_dir, + [self.config.email_registration_template_failure_html], + ) @defer.inlineCallbacks def on_GET(self, request, medium): @@ -283,17 +287,11 @@ def on_GET(self, request, medium): 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) + html = self.failure_email_template.render(**template_vars) request.write(html.encode("utf-8")) finish_request(request) From 59e0aadde015dd838e16cc3927af3f4787e55172 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 19 Sep 2019 17:25:29 +0100 Subject: [PATCH 28/43] Make add_threepid submit_token email-only --- synapse/rest/client/v2_alpha/account.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index cdb6b8deb3e4..ce1dd8e18196 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -528,10 +528,10 @@ def on_POST(self, request): class AddThreepidSubmitTokenServlet(RestServlet): - """Handles 3PID validation token submission for adding a 3PID to a user's account""" + """Handles 3PID validation token submission for adding an email to a user's account""" PATTERNS = client_patterns( - "/add_threepid/(?P[^/]*)/submit_token$", releases=(), unstable=True + "/add_threepid/email/submit_token$", releases=(), unstable=True ) def __init__(self, hs): @@ -551,9 +551,6 @@ def __init__(self, hs): @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 not supported") if self.config.threepid_behaviour_email == ThreepidBehaviour.OFF: if self.config.local_threepid_handling_disabled_due_to_email_config: logger.warn( From 882642e3eeb2583ba86d464fb1778050d44409ba Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 19 Sep 2019 18:01:27 +0100 Subject: [PATCH 29/43] Make get_threepid_validation_session require client_secret --- synapse/storage/registration.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py index cb8eb0c78358..da27ad76b67a 100644 --- a/synapse/storage/registration.py +++ b/synapse/storage/registration.py @@ -24,7 +24,7 @@ from twisted.internet import defer from synapse.api.constants import UserTypes -from synapse.api.errors import Codes, StoreError, ThreepidValidationError +from synapse.api.errors import Codes, StoreError, SynapseError, ThreepidValidationError from synapse.metrics.background_process_metrics import run_as_background_process from synapse.storage import background_updates from synapse.storage._base import SQLBaseStore @@ -661,8 +661,8 @@ def get_threepid_validation_session( 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 + client_secret (str): 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 @@ -678,9 +678,12 @@ def get_threepid_validation_session( Otherwise None if a validation session is not found """ - keyvalues = {} - if client_secret: - keyvalues["client_secret"] = client_secret + if not client_secret: + raise SynapseError( + 400, "Missing parameter: client_secret", errcode=Codes.MISSING_PARAM + ) + + keyvalues = {"client_secret": client_secret} if medium: keyvalues["medium"] = medium if address: From 1464f1491b8f63be8b596dd96ac5dbf2af120ca4 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 19 Sep 2019 18:05:56 +0100 Subject: [PATCH 30/43] Update changelog entry --- changelog.d/6042.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/6042.feature b/changelog.d/6042.feature index 48ec22249b31..a737760363d5 100644 --- a/changelog.d/6042.feature +++ b/changelog.d/6042.feature @@ -1 +1 @@ -Allow homeserver to handle or delegate threepid validation when adding a threepid to a user's account. +Allow homeserver to handle or delegate email validation when adding an email to a user's account. From fb9c58237cfe46197fd65876523cd556273fb019 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 19 Sep 2019 18:08:07 +0100 Subject: [PATCH 31/43] Set email --- synapse/rest/client/v2_alpha/account.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index ce1dd8e18196..ce40bd0a783e 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -651,7 +651,7 @@ def on_POST(self, request): elif self.hs.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL: # Get a validated session matching these details validation_session = yield self.datastore.get_threepid_validation_session( - None, client_secret, sid=sid, validated=True + "email", client_secret, sid=sid, validated=True ) if self._add_threepid_to_account(user_id, validation_session): From f62ddf64e3f7962900f144a92f5f507ce4d5ca79 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 19 Sep 2019 18:27:02 +0100 Subject: [PATCH 32/43] Make sure templates only get loaded when necessary --- synapse/rest/client/v2_alpha/account.py | 18 ++++++++++-------- synapse/rest/client/v2_alpha/register.py | 10 ++++++---- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index ce40bd0a783e..9e3c3a2379e4 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -207,10 +207,11 @@ def __init__(self, hs): self.config = hs.config self.clock = hs.get_clock() self.store = hs.get_datastore() - self.failure_email_template, = load_jinja2_templates( - self.config.email_template_dir, - [self.config.email_password_reset_template_failure_html], - ) + if self.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL: + self.failure_email_template, = load_jinja2_templates( + self.config.email_template_dir, + [self.config.email_password_reset_template_failure_html], + ) @defer.inlineCallbacks def on_GET(self, request, medium): @@ -544,10 +545,11 @@ def __init__(self, hs): self.config = hs.config self.clock = hs.get_clock() self.store = hs.get_datastore() - self.failure_email_template, = load_jinja2_templates( - self.config.email_template_dir, - [self.config.email_add_threepid_template_failure_html], - ) + if self.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL: + self.failure_email_template, = load_jinja2_templates( + self.config.email_template_dir, + [self.config.email_add_threepid_template_failure_html], + ) @defer.inlineCallbacks def on_GET(self, request, medium): diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index 081b3b7872e6..34276ea3fad8 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -239,10 +239,12 @@ def __init__(self, hs): self.config = hs.config self.clock = hs.get_clock() self.store = hs.get_datastore() - self.failure_email_template, = load_jinja2_templates( - self.config.email_template_dir, - [self.config.email_registration_template_failure_html], - ) + + if self.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL: + self.failure_email_template, = load_jinja2_templates( + self.config.email_template_dir, + [self.config.email_registration_template_failure_html], + ) @defer.inlineCallbacks def on_GET(self, request, medium): From 0dcdfbbc33225bb1f42ad8f9370b9748a840dd10 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 20 Sep 2019 10:27:15 +0100 Subject: [PATCH 33/43] Remove unnecessary vars and params --- synapse/rest/client/v2_alpha/account.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 9e3c3a2379e4..7b9a3cf33831 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -541,7 +541,6 @@ def __init__(self, hs): hs (synapse.server.HomeServer): server """ super().__init__() - self.auth = hs.get_auth() self.config = hs.config self.clock = hs.get_clock() self.store = hs.get_datastore() @@ -552,7 +551,7 @@ def __init__(self, hs): ) @defer.inlineCallbacks - def on_GET(self, request, medium): + def on_GET(self, request): if self.config.threepid_behaviour_email == ThreepidBehaviour.OFF: if self.config.local_threepid_handling_disabled_due_to_email_config: logger.warn( From d683ae5a64a7449d37c423572d97a753bdce717d Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 20 Sep 2019 10:41:53 +0100 Subject: [PATCH 34/43] Ensure link is not a tuple --- synapse/push/mailer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/push/mailer.py b/synapse/push/mailer.py index 90f2520008db..d75b42c69a14 100644 --- a/synapse/push/mailer.py +++ b/synapse/push/mailer.py @@ -195,7 +195,7 @@ def send_add_threepid_mail(self, email_address, token, client_secret, sid): link = ( self.hs.config.public_baseurl + "_matrix/client/unstable/add_threepid/email/submit_token?%s" - % urllib.parse.urlencode(params), + % urllib.parse.urlencode(params) ) template_vars = {"link": link} From f62a27cafeca905a7c56ce9975ac52b7aa8b6b95 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 20 Sep 2019 11:11:26 +0100 Subject: [PATCH 35/43] Pull if validation_session out of helper method --- synapse/rest/client/v2_alpha/account.py | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 7b9a3cf33831..ae79791c41e8 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -655,7 +655,8 @@ def on_POST(self, request): "email", client_secret, sid=sid, validated=True ) - if self._add_threepid_to_account(user_id, validation_session): + if validation_session: + self._add_threepid_to_account(user_id, validation_session) return 200, {} # Try to validate as msisdn @@ -665,7 +666,8 @@ def on_POST(self, request): self.hs.config.account_threepid_delegate_msisdn, threepid_creds ) - if self._add_threepid_to_account(user_id, validation_session): + if validation_session: + self._add_threepid_to_account(user_id, validation_session) return 200, {} raise SynapseError( @@ -673,7 +675,7 @@ def on_POST(self, request): ) def _add_threepid_to_account(self, user_id, validation_session): - """Attempt to add a threepid wrapped in a validation_session dict to an account + """Add a threepid wrapped in a validation_session dict to an account Args: user_id (str): The mxid of the user to add this 3PID to @@ -684,13 +686,7 @@ def _add_threepid_to_account(self, user_id, validation_session): * validated_at - timestamp of when the validation occurred If validation_session is None, this method will return False - - Returns: - A boolean stating whether adding the threepid was successful """ - if not validation_session: - return False - yield self.auth_handler.add_threepid( user_id, validation_session["medium"], @@ -698,8 +694,6 @@ def _add_threepid_to_account(self, user_id, validation_session): validation_session["validated_at"], ) - return True - class ThreepidUnbindRestServlet(RestServlet): PATTERNS = client_patterns("/account/3pid/unbind$") From ac92a58be96daef770e72f4a42730488d4b9fab2 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 20 Sep 2019 11:18:51 +0100 Subject: [PATCH 36/43] this confused me for so long --- synapse/rest/client/v2_alpha/account.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index ae79791c41e8..8a00c9f1e3bf 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -656,7 +656,7 @@ def on_POST(self, request): ) if validation_session: - self._add_threepid_to_account(user_id, validation_session) + yield self._add_threepid_to_account(user_id, validation_session) return 200, {} # Try to validate as msisdn @@ -667,13 +667,14 @@ def on_POST(self, request): ) if validation_session: - self._add_threepid_to_account(user_id, validation_session) + yield self._add_threepid_to_account(user_id, validation_session) return 200, {} raise SynapseError( 400, "No validated 3pid session found", Codes.THREEPID_AUTH_FAILED ) + @defer.inlineCallbacks def _add_threepid_to_account(self, user_id, validation_session): """Add a threepid wrapped in a validation_session dict to an account From 38a543c214ed77c8f0e3e1ec1dfbce037a2c8491 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 20 Sep 2019 11:24:40 +0100 Subject: [PATCH 37/43] Don't force https on account_threepid_delegates --- synapse/handlers/identity.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py index 512f38e5a6f5..572dcc8f7806 100644 --- a/synapse/handlers/identity.py +++ b/synapse/handlers/identity.py @@ -114,10 +114,11 @@ def threepid_from_creds(self, id_server, creds): query_params = {"sid": session_id, "client_secret": client_secret} - url = "https://%s%s" % ( - id_server, - "/_matrix/identity/api/v1/3pid/getValidated3pid", - ) + if not id_server.startswith("http"): + # Prefix identity server URL with https + id_server = "https://" + id_server + + url = "%s%s" % (id_server, "/_matrix/identity/api/v1/3pid/getValidated3pid") data = yield self.http_client.get_json(url, query_params) return data if "medium" in data else None From b12e51aaab712012403b1e4bc66c0f4ac5e1f313 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 20 Sep 2019 12:05:47 +0100 Subject: [PATCH 38/43] Ensure we catch HttpResponseException when calling to id servers --- synapse/rest/client/v2_alpha/account.py | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 213a7f28f273..1ec1232ae7bb 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -21,7 +21,12 @@ from twisted.internet import defer from synapse.api.constants import LoginType -from synapse.api.errors import Codes, SynapseError, ThreepidValidationError +from synapse.api.errors import ( + Codes, + HttpResponseException, + SynapseError, + ThreepidValidationError, +) from synapse.config.emailconfig import ThreepidBehaviour from synapse.http.server import finish_request from synapse.http.servlet import ( @@ -629,9 +634,12 @@ def on_POST(self, request): # Try to validate as email if self.hs.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE: # Ask our delegated email identity server - validation_session = yield self.identity_handler.threepid_from_creds( - self.hs.config.account_threepid_delegate_email, threepid_creds - ) + try: + validation_session = yield self.identity_handler.threepid_from_creds( + self.hs.config.account_threepid_delegate_email, threepid_creds + ) + except HttpResponseException: + pass elif self.hs.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL: # Get a validated session matching these details validation_session = yield self.datastore.get_threepid_validation_session( @@ -645,9 +653,12 @@ def on_POST(self, request): # Try to validate as msisdn if self.hs.config.account_threepid_delegate_msisdn: # Ask our delegated msisdn identity server - validation_session = yield self.identity_handler.threepid_from_creds( - self.hs.config.account_threepid_delegate_msisdn, threepid_creds - ) + try: + validation_session = yield self.identity_handler.threepid_from_creds( + self.hs.config.account_threepid_delegate_msisdn, threepid_creds + ) + except HttpResponseException: + pass if validation_session: yield self._add_threepid_to_account(user_id, validation_session) From 1267abd3970912b885ce23b4878aeba8e4a4389f Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 20 Sep 2019 12:13:32 +0100 Subject: [PATCH 39/43] Unpack response from identity server to check for errors --- synapse/rest/client/v2_alpha/account.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 1ec1232ae7bb..58f89514e1a0 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -646,7 +646,7 @@ def on_POST(self, request): "email", client_secret, sid=sid, validated=True ) - if validation_session: + if validation_session and "error" not in validation_session: yield self._add_threepid_to_account(user_id, validation_session) return 200, {} @@ -660,7 +660,7 @@ def on_POST(self, request): except HttpResponseException: pass - if validation_session: + if validation_session and "error" not in validation_session: yield self._add_threepid_to_account(user_id, validation_session) return 200, {} From b3bf43021daf6c5928beb2a1809cfd0cff50bc48 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 20 Sep 2019 13:08:52 +0100 Subject: [PATCH 40/43] Remove prefix nonsense --- synapse/handlers/identity.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py index 572dcc8f7806..778133133f2a 100644 --- a/synapse/handlers/identity.py +++ b/synapse/handlers/identity.py @@ -114,10 +114,6 @@ def threepid_from_creds(self, id_server, creds): query_params = {"sid": session_id, "client_secret": client_secret} - if not id_server.startswith("http"): - # Prefix identity server URL with https - id_server = "https://" + id_server - url = "%s%s" % (id_server, "/_matrix/identity/api/v1/3pid/getValidated3pid") data = yield self.http_client.get_json(url, query_params) From 227ec3338f6c3c9264b3cec8583aa2f92c4d4cad Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 20 Sep 2019 13:10:22 +0100 Subject: [PATCH 41/43] Factor out password_reset trailing slash change --- synapse/rest/client/v2_alpha/account.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 58f89514e1a0..035e5bd3ec74 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -198,7 +198,7 @@ class PasswordResetSubmitTokenServlet(RestServlet): """Handles 3PID validation token submission""" PATTERNS = client_patterns( - "/password_reset/(?P[^/]*)/submit_token$", releases=(), unstable=True + "/password_reset/(?P[^/]*)/submit_token/*$", releases=(), unstable=True ) def __init__(self, hs): From 4efc10881c14ee6412255128c1abb9fa6d254ab7 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 20 Sep 2019 14:56:16 +0100 Subject: [PATCH 42/43] Address review comments --- synapse/handlers/identity.py | 14 +++----------- synapse/rest/client/v2_alpha/account.py | 21 +++++++++++++++++---- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py index 778133133f2a..156719e3087d 100644 --- a/synapse/handlers/identity.py +++ b/synapse/handlers/identity.py @@ -81,11 +81,10 @@ def threepid_from_creds(self, id_server, creds): given identity server Args: - id_server (str|None): The identity server to validate 3PIDs against. If None, - we will attempt to extract id_server creds + id_server (str): The identity server to validate 3PIDs against. Must be a + complete URL including the protocol (http(s)://) creds (dict[str, str]): Dictionary containing the following keys: - * id_server|idServer: An optional domain name of an identity server * client_secret|clientSecret: A unique secret str provided by the client * sid: The ID of the validation session @@ -104,17 +103,10 @@ def threepid_from_creds(self, id_server, creds): raise SynapseError( 400, "Missing param session_id in creds", errcode=Codes.MISSING_PARAM ) - if not id_server: - # Attempt to get the id_server from the creds dict - id_server = creds.get("id_server") or creds.get("idServer") - if not id_server: - raise SynapseError( - 400, "Missing param id_server in creds", errcode=Codes.MISSING_PARAM - ) query_params = {"sid": session_id, "client_secret": client_secret} - url = "%s%s" % (id_server, "/_matrix/identity/api/v1/3pid/getValidated3pid") + url = id_server + "/_matrix/identity/api/v1/3pid/getValidated3pid" data = yield self.http_client.get_json(url, query_params) return data if "medium" in data else None diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 035e5bd3ec74..c121b16728c7 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -639,13 +639,22 @@ def on_POST(self, request): self.hs.config.account_threepid_delegate_email, threepid_creds ) except HttpResponseException: - pass + logger.debug( + "%s reported non-validated threepid: %s", + self.hs.config.account_threepid_delegate_email, + threepid_creds, + ) elif self.hs.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL: # Get a validated session matching these details validation_session = yield self.datastore.get_threepid_validation_session( "email", client_secret, sid=sid, validated=True ) + # Old versions of Sydent return a 200 http code even on a failed validation check. + # Thus, in addition to the HttpResponseException check above (which checks for + # non-200 errors), we need to make sure validation_session isn't actually an error, + # identified by containing an "error" key + # See https://github.com/matrix-org/sydent/issues/215 for details if validation_session and "error" not in validation_session: yield self._add_threepid_to_account(user_id, validation_session) return 200, {} @@ -658,8 +667,14 @@ def on_POST(self, request): self.hs.config.account_threepid_delegate_msisdn, threepid_creds ) except HttpResponseException: - pass + logger.debug( + "%s reported non-validated threepid: %s", + self.hs.config.account_threepid_delegate_email, + threepid_creds, + ) + # Check that validation_session isn't actually an error due to old Sydent instances + # See explanatory comment above if validation_session and "error" not in validation_session: yield self._add_threepid_to_account(user_id, validation_session) return 200, {} @@ -679,8 +694,6 @@ def _add_threepid_to_account(self, user_id, validation_session): * medium - medium of the threepid * address - address of the threepid * validated_at - timestamp of when the validation occurred - - If validation_session is None, this method will return False """ yield self.auth_handler.add_threepid( user_id, From 7c351df7f590414ad2458278999620108449aecc Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 20 Sep 2019 15:06:05 +0100 Subject: [PATCH 43/43] validation_session cannot be None --- synapse/rest/client/v2_alpha/account.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index c121b16728c7..cc0960e6bfbb 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -690,7 +690,7 @@ def _add_threepid_to_account(self, user_id, validation_session): Args: user_id (str): The mxid of the user to add this 3PID to - validation_session (dict|None): A dict containing the following: + validation_session (dict): A dict containing the following: * medium - medium of the threepid * address - address of the threepid * validated_at - timestamp of when the validation occurred