From 9003eb4bcdc1c3e7803e937ff486e1138d06d055 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 21 Aug 2020 11:19:22 +0100 Subject: [PATCH 01/32] Add confirmation page template --- .../templates/password_reset_confirmation.html | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 synapse/res/templates/password_reset_confirmation.html diff --git a/synapse/res/templates/password_reset_confirmation.html b/synapse/res/templates/password_reset_confirmation.html new file mode 100644 index 000000000000..41a16693ece5 --- /dev/null +++ b/synapse/res/templates/password_reset_confirmation.html @@ -0,0 +1,16 @@ + + + + +
+ + + + +

You have requested to reset your Matrix account password. Click the link below to confirm this action.

+ If you did not mean to do this, please close this page and your password will not be changed.

+

+
+ + + From 5f7a834a50eb9c9eec9284afa4bef22cd9ccf550 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 30 Jul 2020 14:45:16 -0700 Subject: [PATCH 02/32] Load the new template --- docs/sample_config.yaml | 10 +++++++--- synapse/config/emailconfig.py | 15 ++++++++++++--- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index f168853f67f8..7bd8dcf45d4f 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -2015,9 +2015,13 @@ email: # * The contents of password reset emails sent by the homeserver: # 'password_reset.html' and 'password_reset.txt' # - # * HTML pages for success and failure that a user will see when they follow - # the link in the password reset email: 'password_reset_success.html' and - # 'password_reset_failure.html' + # * An HTML page that a user will see when they follow the link in the password + # reset email. The user will be asked to confirm the action before their + # password is reset: 'password_reset_confirmation.html' + # + # * HTML pages for success and failure that a user will see when they confirm + # the password reset flow using the page above: 'password_reset_success.html' + # and 'password_reset_failure.html' # # * The contents of address verification emails sent during registration: # 'registration.html' and 'registration.txt' diff --git a/synapse/config/emailconfig.py b/synapse/config/emailconfig.py index 7a796996c056..019b78509b6f 100644 --- a/synapse/config/emailconfig.py +++ b/synapse/config/emailconfig.py @@ -198,6 +198,9 @@ def read_config(self, config, **kwargs): "add_threepid_template_text", "add_threepid.txt" ) + password_reset_template_confirmation_html = ( + "password_reset_confirmation.html" + ) password_reset_template_failure_html = email_config.get( "password_reset_template_failure_html", "password_reset_failure.html" ) @@ -228,6 +231,7 @@ def read_config(self, config, **kwargs): self.email_registration_template_text, self.email_add_threepid_template_html, self.email_add_threepid_template_text, + self.email_password_reset_template_confirmation_html, self.email_password_reset_template_failure_html, self.email_registration_template_failure_html, self.email_add_threepid_template_failure_html, @@ -242,6 +246,7 @@ def read_config(self, config, **kwargs): registration_template_text, add_threepid_template_html, add_threepid_template_text, + password_reset_template_confirmation_html, password_reset_template_failure_html, registration_template_failure_html, add_threepid_template_failure_html, @@ -404,9 +409,13 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs): # * The contents of password reset emails sent by the homeserver: # 'password_reset.html' and 'password_reset.txt' # - # * HTML pages for success and failure that a user will see when they follow - # the link in the password reset email: 'password_reset_success.html' and - # 'password_reset_failure.html' + # * An HTML page that a user will see when they follow the link in the password + # reset email. The user will be asked to confirm the action before their + # password is reset: 'password_reset_confirmation.html' + # + # * HTML pages for success and failure that a user will see when they confirm + # the password reset flow using the page above: 'password_reset_success.html' + # and 'password_reset_failure.html' # # * The contents of address verification emails sent during registration: # 'registration.html' and 'registration.txt' From d9a19fc696f37270353ca7811fa49f7596995a41 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 21 Aug 2020 11:00:10 +0100 Subject: [PATCH 03/32] Create new password reset confirmation endpoint Creates a new implementation-specific endpoint for accepting confirmation of password resets. This endpoint should be hit with the same parameters as the regular password reset submit_token endpoint. This endpoint will now complete the password resets, whereas the existing endpoint will now just show the confirmation template page. --- synapse/rest/client/v2_alpha/account.py | 54 +++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 3 deletions(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 203e76b9f226..44011f675d61 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -158,9 +158,10 @@ def __init__(self, hs): self.config = hs.config self.clock = hs.get_clock() self.store = hs.get_datastore() + if self.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL: - self._failure_email_template = ( - self.config.email_password_reset_template_failure_html + self._confirmation_email_template = ( + self.config.email_password_reset_template_confirmation_html ) async def on_GET(self, request, medium): @@ -183,6 +184,52 @@ async def on_GET(self, request, medium): client_secret = parse_string(request, "client_secret", required=True) assert_valid_client_secret(client_secret) + # Show a confirmation page, just in case someone accidentally clicked this link when + # they didn't mean to + template_vars = { + "sid": sid, + "token": token, + "client_secret": client_secret, + "medium": medium, + } + respond_with_html( + request, 200, self._confirmation_email_template.render(**template_vars) + ) + + +class PasswordResetConfirmationSubmitTokenServlet(RestServlet): + """Handles confirmation of 3PID validation token submission. + + A user will land on PasswordResetSubmitTokenServlet, confirm the password reset, then + submit the same parameters to this servlet. + """ + + PATTERNS = client_patterns( + "/password_reset/email/submit_token_confirm$", releases=(), unstable=True, + ) + + def __init__(self, hs): + """ + Args: + hs (synapse.server.HomeServer): server + """ + super(PasswordResetConfirmationSubmitTokenServlet, self).__init__() + self.auth = hs.get_auth() + self.clock = hs.get_clock() + self.store = hs.get_datastore() + if hs.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL: + self._failure_email_template = ( + hs.config.email_password_reset_template_failure_html + ) + self._email_password_reset_template_success_html = ( + hs.config.email_password_reset_template_success_html_content + ) + + async def on_POST(self, request): + sid = parse_string(request, "sid", required=True) + token = parse_string(request, "token", required=True) + client_secret = parse_string(request, "client_secret", required=True) + # Attempt to validate a 3PID session try: # Mark the session as valid @@ -203,7 +250,7 @@ async def on_GET(self, request, medium): return None # Otherwise show the success template - html = self.config.email_password_reset_template_success_html_content + html = self._email_password_reset_template_success_html status_code = 200 except ThreepidValidationError as e: status_code = e.code @@ -881,6 +928,7 @@ async def on_GET(self, request): def register_servlets(hs, http_server): EmailPasswordRequestTokenRestServlet(hs).register(http_server) PasswordResetSubmitTokenServlet(hs).register(http_server) + PasswordResetConfirmationSubmitTokenServlet(hs).register(http_server) PasswordRestServlet(hs).register(http_server) DeactivateAccountRestServlet(hs).register(http_server) EmailThreepidRequestTokenRestServlet(hs).register(http_server) From 6140b32397620da391dda9647eb54d533b4b2b5c Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 21 Aug 2020 11:00:53 +0100 Subject: [PATCH 04/32] Add confirmation as a step to the password reset unit tests --- tests/rest/client/v2_alpha/test_account.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/rest/client/v2_alpha/test_account.py b/tests/rest/client/v2_alpha/test_account.py index 152a5182fa39..f336671e2c98 100644 --- a/tests/rest/client/v2_alpha/test_account.py +++ b/tests/rest/client/v2_alpha/test_account.py @@ -250,10 +250,23 @@ def _validate_token(self, link): # Remove the host path = link.replace("https://example.com", "") + # Load the password reset confirmation page request, channel = self.make_request("GET", path, shorthand=False) self.render(request) self.assertEquals(200, channel.code, channel.result) + # Replace the path with the confirmation path + path = re.sub( + "^/_matrix.*submit_token", + "/_matrix/client/unstable/password_reset/email/submit_token_confirm", + path, + ) + + # Confirm the password reset + request, channel = self.make_request("POST", path, shorthand=False) + self.render(request) + self.assertEquals(200, channel.code, channel.result) + def _get_link_from_email(self): assert self.email_attempts, "No emails have been sent" From 622946e8818838972e3c5dfb5b145a87cbad2236 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 21 Aug 2020 11:01:25 +0100 Subject: [PATCH 05/32] Remind people about the new template in the upgrade notes --- UPGRADE.rst | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/UPGRADE.rst b/UPGRADE.rst index 6492fa011f4a..c1020ade4af8 100644 --- a/UPGRADE.rst +++ b/UPGRADE.rst @@ -75,6 +75,30 @@ for example: wget https://packages.matrix.org/debian/pool/main/m/matrix-synapse-py3/matrix-synapse-py3_1.3.0+stretch1_amd64.deb dpkg -i matrix-synapse-py3_1.3.0+stretch1_amd64.deb +Upgrading to v1.20.0 +==================== + +New HTML templates +------------------ + +A new HTML template, +`password_reset_confirmation.html `_, +has been added to the ``synapse/res/templates`` directory. If you are using a +custom template directory, you may want to copy the template over and modify it. + +Note that as of v1.20.0, templates do not need to be included in custom template +directories for Synapse to start. The default templates will be used if a custom +template cannot be found. + +This page will appear to the user after clicking a password reset link that has +been emailed to them. + +To complete password reset, the page must include a way to make a `POST` +request to +``/_matrix/client/unstable/password_reset/{medium}/submit_token_confirm`` +with the query parameters from the original link. See the file itself for more +details. + Upgrading to v1.18.0 ==================== From 3568e1897ccdaa40f01cc8d5a993a11914075225 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 30 Jul 2020 19:53:09 -0700 Subject: [PATCH 06/32] Add changelog --- changelog.d/8004.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/8004.feature diff --git a/changelog.d/8004.feature b/changelog.d/8004.feature new file mode 100644 index 000000000000..a91b75e0e0fd --- /dev/null +++ b/changelog.d/8004.feature @@ -0,0 +1 @@ +Require the user to confirm that their password should be reset after clicking the email confirmation link. \ No newline at end of file From 8cc8ee44486ed44341f8a6d64bf3612f63ad9b11 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 21 Aug 2020 17:14:36 +0100 Subject: [PATCH 07/32] Remove unused fields --- synapse/rest/client/v2_alpha/account.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 44011f675d61..a3dd0117888b 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -153,11 +153,7 @@ def __init__(self, hs): hs (synapse.server.HomeServer): server """ super(PasswordResetSubmitTokenServlet, self).__init__() - self.hs = hs - self.auth = hs.get_auth() self.config = hs.config - self.clock = hs.get_clock() - self.store = hs.get_datastore() if self.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL: self._confirmation_email_template = ( From b41b0512f9e8df7eb67e41b8f0821d93e033e1eb Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 21 Aug 2020 17:18:42 +0100 Subject: [PATCH 08/32] typing --- synapse/rest/client/v2_alpha/account.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index a3dd0117888b..ab771db408fd 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -16,6 +16,7 @@ # limitations under the License. import logging from http import HTTPStatus +from typing import TYPE_CHECKING from synapse.api.constants import LoginType from synapse.api.errors import ( @@ -37,6 +38,9 @@ from synapse.util.stringutils import assert_valid_client_secret, random_string from synapse.util.threepids import canonicalise_email, check_3pid_allowed +if TYPE_CHECKING: + from synapse.server import HomeServer + from ._base import client_patterns, interactive_auth_handler logger = logging.getLogger(__name__) @@ -204,10 +208,10 @@ class PasswordResetConfirmationSubmitTokenServlet(RestServlet): "/password_reset/email/submit_token_confirm$", releases=(), unstable=True, ) - def __init__(self, hs): + def __init__(self, hs: "HomeServer"): """ Args: - hs (synapse.server.HomeServer): server + hs: server """ super(PasswordResetConfirmationSubmitTokenServlet, self).__init__() self.auth = hs.get_auth() From 0a2b11f36187ceeef13627b7d3e16e161e50f6e5 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 21 Aug 2020 17:21:02 +0100 Subject: [PATCH 09/32] Ensure we don't fail a request due to the config --- synapse/rest/client/v2_alpha/account.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index ab771db408fd..e91db435cf0f 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -214,6 +214,7 @@ def __init__(self, hs: "HomeServer"): hs: server """ super(PasswordResetConfirmationSubmitTokenServlet, self).__init__() + self.hs = hs self.auth = hs.get_auth() self.clock = hs.get_clock() self.store = hs.get_datastore() @@ -226,6 +227,15 @@ def __init__(self, hs: "HomeServer"): ) async 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.warning( + "Password reset emails have been disabled due to lack of an email config" + ) + raise SynapseError( + 400, "Email-based password resets are disabled on this server" + ) + sid = parse_string(request, "sid", required=True) token = parse_string(request, "token", required=True) client_secret = parse_string(request, "client_secret", required=True) From 1d79f7b22bc31cfb0cd5d23710678b77a6989b7d Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 21 Aug 2020 17:24:44 +0100 Subject: [PATCH 10/32] Remove another unused class var --- 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 e91db435cf0f..158ef7ec8ccb 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -215,7 +215,6 @@ def __init__(self, hs: "HomeServer"): """ super(PasswordResetConfirmationSubmitTokenServlet, self).__init__() self.hs = hs - self.auth = hs.get_auth() self.clock = hs.get_clock() self.store = hs.get_datastore() if hs.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL: From d6addba84e86a8f3197a8be9efec99981b5b2d71 Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Fri, 28 Aug 2020 14:15:25 +0100 Subject: [PATCH 11/32] Update synapse/rest/client/v2_alpha/account.py Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- 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 72ec0960cb35..67b5bf049cef 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -217,7 +217,7 @@ def __init__(self, hs: "HomeServer"): Args: hs: server """ - super(PasswordResetConfirmationSubmitTokenServlet, self).__init__() + super().__init__() self.hs = hs self.clock = hs.get_clock() self.store = hs.get_datastore() From 990178f58b341b61e33b32619963f43d89955ce2 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 28 Aug 2020 14:59:05 +0100 Subject: [PATCH 12/32] Pull things from, instead of copying the entirety of, the config --- synapse/rest/client/v2_alpha/account.py | 30 +++++++++++++++---------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 67b5bf049cef..746f34153a7c 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -161,11 +161,14 @@ def __init__(self, hs): hs (synapse.server.HomeServer): server """ super(PasswordResetSubmitTokenServlet, self).__init__() - self.config = hs.config - if self.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL: + self._threepid_behaviour_email = hs.config.threepid_behaviour_email + self._local_threepid_handling_disabled_due_to_email_config = ( + hs.config.local_threepid_handling_disabled_due_to_email_config + ) + if self._threepid_behaviour_email == ThreepidBehaviour.LOCAL: self._confirmation_email_template = ( - self.config.email_password_reset_template_confirmation_html + hs.config.email_password_reset_template_confirmation_html ) async def on_GET(self, request, medium): @@ -174,8 +177,8 @@ async def on_GET(self, request, medium): raise SynapseError( 400, "This medium is currently not supported for password resets" ) - if self.config.threepid_behaviour_email == ThreepidBehaviour.OFF: - if self.config.local_threepid_handling_disabled_due_to_email_config: + if self._threepid_behaviour_email == ThreepidBehaviour.OFF: + if self._local_threepid_handling_disabled_due_to_email_config: logger.warning( "Password reset emails have been disabled due to lack of an email config" ) @@ -218,20 +221,23 @@ def __init__(self, hs: "HomeServer"): hs: server """ super().__init__() - self.hs = hs self.clock = hs.get_clock() self.store = hs.get_datastore() - if hs.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL: - self._failure_email_template = ( - hs.config.email_password_reset_template_failure_html - ) + self._threepid_behaviour_email = hs.config.threepid_behaviour_email + self._local_threepid_handling_disabled_due_to_email_config = ( + hs.config.local_threepid_handling_disabled_due_to_email_config + ) + if self._threepid_behaviour_email == ThreepidBehaviour.LOCAL: self._email_password_reset_template_success_html = ( hs.config.email_password_reset_template_success_html_content ) + self._failure_email_template = ( + hs.config.email_password_reset_template_failure_html + ) async 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: + if self._threepid_behaviour_email == ThreepidBehaviour.OFF: + if self._local_threepid_handling_disabled_due_to_email_config: logger.warning( "Password reset emails have been disabled due to lack of an email config" ) From 1b4458ed26cc02bae741175558ca238473333dde Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 28 Aug 2020 15:17:30 +0100 Subject: [PATCH 13/32] Return 400 when accessing submit_token/_confirm with REMOTE behaviour --- synapse/rest/client/v2_alpha/account.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 746f34153a7c..a309cf532d89 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -185,6 +185,11 @@ async def on_GET(self, request, medium): raise SynapseError( 400, "Email-based password resets are disabled on this server" ) + elif self._threepid_behaviour_email == ThreepidBehaviour.REMOTE: + raise SynapseError( + 400, + "Password resets for this homeserver are handled by a separate program", + ) sid = parse_string(request, "sid", required=True) token = parse_string(request, "token", required=True) @@ -244,6 +249,11 @@ async def on_POST(self, request): raise SynapseError( 400, "Email-based password resets are disabled on this server" ) + elif self._threepid_behaviour_email == ThreepidBehaviour.REMOTE: + raise SynapseError( + 400, + "Password resets for this homeserver are handled by a separate program", + ) sid = parse_string(request, "sid", required=True) token = parse_string(request, "token", required=True) From 0608fe197936af46524b0fc7f281735d420914e6 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 28 Aug 2020 18:10:06 +0100 Subject: [PATCH 14/32] Convert confirmation from request args to HTML form data To correctly emulate a POSTed URL-encoded form submission from the browser, we needed to do a few things. * URL-encode the request arguments and place their 'key=value' pairs, separated by '&' characters, into a byte string. * Place this byte string in the request body instead of the query parameters. * Set the request's 'Content-Type' header to 'application/x-www-form-urlencoded', instead of 'application/json', which was previously the default when making a request in the unit tests. Additionally, @clokep noticed that we needed to seek to the end of the request content before passing it to twisted, else things blew up. --- UPGRADE.rst | 4 ++-- tests/rest/client/v2_alpha/test_account.py | 23 +++++++++++++++------- tests/server.py | 15 ++++++++++++-- tests/unittest.py | 4 ++++ 4 files changed, 35 insertions(+), 11 deletions(-) diff --git a/UPGRADE.rst b/UPGRADE.rst index c1020ade4af8..5cb04b815e60 100644 --- a/UPGRADE.rst +++ b/UPGRADE.rst @@ -96,8 +96,8 @@ been emailed to them. To complete password reset, the page must include a way to make a `POST` request to ``/_matrix/client/unstable/password_reset/{medium}/submit_token_confirm`` -with the query parameters from the original link. See the file itself for more -details. +with the query parameters from the original link, encoded as a URL-encoded form. See the file +itself for more details. Upgrading to v1.18.0 ==================== diff --git a/tests/rest/client/v2_alpha/test_account.py b/tests/rest/client/v2_alpha/test_account.py index f336671e2c98..8eddf6619269 100644 --- a/tests/rest/client/v2_alpha/test_account.py +++ b/tests/rest/client/v2_alpha/test_account.py @@ -14,11 +14,11 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. - import json import os import re from email.parser import Parser +from urllib.parse import urlencode import pkg_resources @@ -256,14 +256,23 @@ def _validate_token(self, link): self.assertEquals(200, channel.code, channel.result) # Replace the path with the confirmation path - path = re.sub( - "^/_matrix.*submit_token", - "/_matrix/client/unstable/password_reset/email/submit_token_confirm", - path, - ) + path = "/_matrix/client/unstable/password_reset/email/submit_token_confirm" + + # Send arguments as url-encoded form data, matching the template's behaviour + form_args = [] + for key, value_list in request.args.items(): + for value in value_list: + arg = (key, value) + form_args.append(arg) # Confirm the password reset - request, channel = self.make_request("POST", path, shorthand=False) + request, channel = self.make_request( + "POST", + path, + content=urlencode(form_args).encode("utf8"), + shorthand=False, + content_is_form=True, + ) self.render(request) self.assertEquals(200, channel.code, channel.result) diff --git a/tests/server.py b/tests/server.py index b6e0b14e78f9..936f739ca222 100644 --- a/tests/server.py +++ b/tests/server.py @@ -1,6 +1,6 @@ import json import logging -from io import BytesIO +from io import SEEK_END, BytesIO import attr from zope.interface import implementer @@ -135,6 +135,7 @@ def make_request( request=SynapseRequest, shorthand=True, federation_auth_origin=None, + content_is_form=False, ): """ Make a web request using the given method and path, feed it the @@ -150,6 +151,8 @@ def make_request( with the usual REST API path, if it doesn't contain it. federation_auth_origin (bytes|None): if set to not-None, we will add a fake Authorization header pretenting to be the given server name. + content_is_form: Whether the content is URL encoded form data. Adds the + 'Content-Type': 'application/x-www-form-urlencoded' header. Returns: Tuple[synapse.http.site.SynapseRequest, channel] @@ -181,6 +184,8 @@ def make_request( req = request(channel) req.process = lambda: b"" req.content = BytesIO(content) + # Twisted expects to be at the end of the content when parsing the request. + req.content.seek(SEEK_END) req.postpath = list(map(unquote, path[1:].split(b"/"))) if access_token: @@ -195,7 +200,13 @@ def make_request( ) if content: - req.requestHeaders.addRawHeader(b"Content-Type", b"application/json") + if content_is_form: + req.requestHeaders.addRawHeader( + b"Content-Type", b"application/x-www-form-urlencoded" + ) + else: + # Assume the body is JSON + req.requestHeaders.addRawHeader(b"Content-Type", b"application/json") req.requestReceived(method, path, b"1.1") diff --git a/tests/unittest.py b/tests/unittest.py index 7b80999a7413..ca8720710beb 100644 --- a/tests/unittest.py +++ b/tests/unittest.py @@ -353,6 +353,7 @@ def make_request( request: Type[T] = SynapseRequest, shorthand: bool = True, federation_auth_origin: str = None, + content_is_form: bool = False, ) -> Tuple[T, FakeChannel]: """ Create a SynapseRequest at the path using the method and containing the @@ -368,6 +369,8 @@ def make_request( with the usual REST API path, if it doesn't contain it. federation_auth_origin (bytes|None): if set to not-None, we will add a fake Authorization header pretenting to be the given server name. + content_is_form: Whether the content is URL encoded form data. Adds the + 'Content-Type': 'application/x-www-form-urlencoded' header. Returns: Tuple[synapse.http.site.SynapseRequest, channel] @@ -384,6 +387,7 @@ def make_request( request, shorthand, federation_auth_origin, + content_is_form, ) def render(self, request): From d9f5b4c123cf2c3a7bfec9fd21740fd9e6a58b1d Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 28 Aug 2020 21:06:53 +0100 Subject: [PATCH 15/32] Combine the submit_token and submit_token_confirm endpoints They are now simply GET vs POST /submit_token. Thanks to @richvdh for the idea. This commit also removes the endpoint from handling .../msisdn/... calls, as Synapse currently doesn't support handling password resets via this medium. There shouldn't be any cases where it is called. --- UPGRADE.rst | 2 +- .../password_reset_confirmation.html | 2 +- synapse/rest/client/v2_alpha/account.py | 109 +++++++----------- tests/rest/client/v2_alpha/test_account.py | 4 +- 4 files changed, 43 insertions(+), 74 deletions(-) diff --git a/UPGRADE.rst b/UPGRADE.rst index 5cb04b815e60..faa58df72405 100644 --- a/UPGRADE.rst +++ b/UPGRADE.rst @@ -95,7 +95,7 @@ been emailed to them. To complete password reset, the page must include a way to make a `POST` request to -``/_matrix/client/unstable/password_reset/{medium}/submit_token_confirm`` +``/_matrix/client/unstable/password_reset/{medium}/submit_token`` with the query parameters from the original link, encoded as a URL-encoded form. See the file itself for more details. diff --git a/synapse/res/templates/password_reset_confirmation.html b/synapse/res/templates/password_reset_confirmation.html index 41a16693ece5..25c80a026e68 100644 --- a/synapse/res/templates/password_reset_confirmation.html +++ b/synapse/res/templates/password_reset_confirmation.html @@ -2,7 +2,7 @@ -
+ diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index a309cf532d89..ee00bfc7b861 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -152,45 +152,37 @@ class PasswordResetSubmitTokenServlet(RestServlet): """Handles 3PID validation token submission""" PATTERNS = client_patterns( - "/password_reset/(?P[^/]*)/submit_token$", releases=(), unstable=True + "/password_reset/email/submit_token$", releases=(), unstable=True ) - def __init__(self, hs): + def __init__(self, hs: "HomeServer"): """ Args: - hs (synapse.server.HomeServer): server + hs: server """ - super(PasswordResetSubmitTokenServlet, self).__init__() + super().__init__() + + self.clock = hs.get_clock() + self.store = hs.get_datastore() - self._threepid_behaviour_email = hs.config.threepid_behaviour_email self._local_threepid_handling_disabled_due_to_email_config = ( hs.config.local_threepid_handling_disabled_due_to_email_config ) + self._threepid_behaviour_email = hs.config.threepid_behaviour_email if self._threepid_behaviour_email == ThreepidBehaviour.LOCAL: self._confirmation_email_template = ( hs.config.email_password_reset_template_confirmation_html ) - - async 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._threepid_behaviour_email == ThreepidBehaviour.OFF: - if self._local_threepid_handling_disabled_due_to_email_config: - logger.warning( - "Password reset emails have been disabled due to lack of an email config" - ) - raise SynapseError( - 400, "Email-based password resets are disabled on this server" + self._email_password_reset_template_success_html = ( + hs.config.email_password_reset_template_success_html_content ) - elif self._threepid_behaviour_email == ThreepidBehaviour.REMOTE: - raise SynapseError( - 400, - "Password resets for this homeserver are handled by a separate program", + self._failure_email_template = ( + hs.config.email_password_reset_template_failure_html ) + async def on_GET(self, request): + self._check_threepid_behaviour() + sid = parse_string(request, "sid", required=True) token = parse_string(request, "token", required=True) client_secret = parse_string(request, "client_secret", required=True) @@ -202,58 +194,14 @@ async def on_GET(self, request, medium): "sid": sid, "token": token, "client_secret": client_secret, - "medium": medium, + "medium": "email", } respond_with_html( request, 200, self._confirmation_email_template.render(**template_vars) ) - -class PasswordResetConfirmationSubmitTokenServlet(RestServlet): - """Handles confirmation of 3PID validation token submission. - - A user will land on PasswordResetSubmitTokenServlet, confirm the password reset, then - submit the same parameters to this servlet. - """ - - PATTERNS = client_patterns( - "/password_reset/email/submit_token_confirm$", releases=(), unstable=True, - ) - - def __init__(self, hs: "HomeServer"): - """ - Args: - hs: server - """ - super().__init__() - self.clock = hs.get_clock() - self.store = hs.get_datastore() - self._threepid_behaviour_email = hs.config.threepid_behaviour_email - self._local_threepid_handling_disabled_due_to_email_config = ( - hs.config.local_threepid_handling_disabled_due_to_email_config - ) - if self._threepid_behaviour_email == ThreepidBehaviour.LOCAL: - self._email_password_reset_template_success_html = ( - hs.config.email_password_reset_template_success_html_content - ) - self._failure_email_template = ( - hs.config.email_password_reset_template_failure_html - ) - async def on_POST(self, request): - if self._threepid_behaviour_email == ThreepidBehaviour.OFF: - if self._local_threepid_handling_disabled_due_to_email_config: - logger.warning( - "Password reset emails have been disabled due to lack of an email config" - ) - raise SynapseError( - 400, "Email-based password resets are disabled on this server" - ) - elif self._threepid_behaviour_email == ThreepidBehaviour.REMOTE: - raise SynapseError( - 400, - "Password resets for this homeserver are handled by a separate program", - ) + self._check_threepid_behaviour() sid = parse_string(request, "sid", required=True) token = parse_string(request, "token", required=True) @@ -290,6 +238,28 @@ async def on_POST(self, request): respond_with_html(request, status_code, html) + def _check_threepid_behaviour(self): + """ + Ensure that ThreepidBehaviour is set to LOCAL, and handle cases where it is not + + Raises: + SynapseError: if threepid_behaviour_email is not set to LOCAL + """ + # We currently only handle threepid token submissions for email + if self._threepid_behaviour_email == ThreepidBehaviour.OFF: + if self._local_threepid_handling_disabled_due_to_email_config: + logger.warning( + "Password reset emails have been disabled due to lack of an email config" + ) + raise SynapseError( + 400, "Email-based password resets are disabled on this server" + ) + elif self._threepid_behaviour_email == ThreepidBehaviour.REMOTE: + raise SynapseError( + 400, + "Password resets for this homeserver are handled by a separate program", + ) + class PasswordRestServlet(RestServlet): PATTERNS = client_patterns("/account/password$") @@ -963,7 +933,6 @@ async def on_GET(self, request): def register_servlets(hs, http_server): EmailPasswordRequestTokenRestServlet(hs).register(http_server) PasswordResetSubmitTokenServlet(hs).register(http_server) - PasswordResetConfirmationSubmitTokenServlet(hs).register(http_server) PasswordRestServlet(hs).register(http_server) DeactivateAccountRestServlet(hs).register(http_server) EmailThreepidRequestTokenRestServlet(hs).register(http_server) diff --git a/tests/rest/client/v2_alpha/test_account.py b/tests/rest/client/v2_alpha/test_account.py index 8eddf6619269..45ea46421d6a 100644 --- a/tests/rest/client/v2_alpha/test_account.py +++ b/tests/rest/client/v2_alpha/test_account.py @@ -255,8 +255,8 @@ def _validate_token(self, link): self.render(request) self.assertEquals(200, channel.code, channel.result) - # Replace the path with the confirmation path - path = "/_matrix/client/unstable/password_reset/email/submit_token_confirm" + # Now POST to the same endpoint, mimicking the same behaviour as clicking the + # password reset confirm button # Send arguments as url-encoded form data, matching the template's behaviour form_args = [] From f25209ce9d3d2e5692c80530de107c88c93096f0 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 28 Aug 2020 21:22:12 +0100 Subject: [PATCH 16/32] Switch from an unstable /_matrix/client prefix to /_synapse/client --- UPGRADE.rst | 2 +- synapse/api/urls.py | 1 + synapse/push/mailer.py | 2 +- .../templates/password_reset_confirmation.html | 2 +- synapse/rest/client/v2_alpha/_base.py | 15 ++++++++++++++- synapse/rest/client/v2_alpha/account.py | 6 ++---- 6 files changed, 20 insertions(+), 8 deletions(-) diff --git a/UPGRADE.rst b/UPGRADE.rst index faa58df72405..b44e4666123c 100644 --- a/UPGRADE.rst +++ b/UPGRADE.rst @@ -95,7 +95,7 @@ been emailed to them. To complete password reset, the page must include a way to make a `POST` request to -``/_matrix/client/unstable/password_reset/{medium}/submit_token`` +``/_synapse/client/password_reset/{medium}/submit_token`` with the query parameters from the original link, encoded as a URL-encoded form. See the file itself for more details. diff --git a/synapse/api/urls.py b/synapse/api/urls.py index bd03ebca5a31..619d06224fd1 100644 --- a/synapse/api/urls.py +++ b/synapse/api/urls.py @@ -21,6 +21,7 @@ from synapse.config import ConfigError +SYNAPSE_CLIENT_API_PREFIX = "/_synapse/client" CLIENT_API_PREFIX = "/_matrix/client" FEDERATION_PREFIX = "/_matrix/federation" FEDERATION_V1_PREFIX = FEDERATION_PREFIX + "/v1" diff --git a/synapse/push/mailer.py b/synapse/push/mailer.py index c38e03728104..cb1797866b54 100644 --- a/synapse/push/mailer.py +++ b/synapse/push/mailer.py @@ -123,7 +123,7 @@ async def send_password_reset_mail(self, email_address, token, client_secret, si params = {"token": token, "client_secret": client_secret, "sid": sid} link = ( self.hs.config.public_baseurl - + "_matrix/client/unstable/password_reset/email/submit_token?%s" + + "_synapse/client/password_reset/email/submit_token?%s" % urllib.parse.urlencode(params) ) diff --git a/synapse/res/templates/password_reset_confirmation.html b/synapse/res/templates/password_reset_confirmation.html index 25c80a026e68..ea9a4284e609 100644 --- a/synapse/res/templates/password_reset_confirmation.html +++ b/synapse/res/templates/password_reset_confirmation.html @@ -2,7 +2,7 @@ - + diff --git a/synapse/rest/client/v2_alpha/_base.py b/synapse/rest/client/v2_alpha/_base.py index f016b4f1bd41..7f34ef367163 100644 --- a/synapse/rest/client/v2_alpha/_base.py +++ b/synapse/rest/client/v2_alpha/_base.py @@ -20,7 +20,7 @@ from typing import Iterable, Pattern from synapse.api.errors import InteractiveAuthIncompleteError -from synapse.api.urls import CLIENT_API_PREFIX +from synapse.api.urls import CLIENT_API_PREFIX, SYNAPSE_CLIENT_API_PREFIX from synapse.types import JsonDict logger = logging.getLogger(__name__) @@ -59,6 +59,19 @@ def client_patterns( return patterns +def synapse_client_patterns(path_regex: str) -> Iterable[Pattern]: + """Creates a regex compiled client path with the correct synapse client + path prefix. + + Args: + path_regex: The regex string to match. This should NOT have a ^ + as this will be prefixed. + Returns: + An iterable of patterns. + """ + return [re.compile("^" + SYNAPSE_CLIENT_API_PREFIX + path_regex)] + + def set_timeline_upper_limit(filter_json: JsonDict, filter_timeline_limit: int) -> None: """ Enforces a maximum limit of a timeline query. diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index ee00bfc7b861..68398bdf62d9 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -42,7 +42,7 @@ if TYPE_CHECKING: from synapse.server import HomeServer -from ._base import client_patterns, interactive_auth_handler +from ._base import client_patterns, interactive_auth_handler, synapse_client_patterns logger = logging.getLogger(__name__) @@ -151,9 +151,7 @@ async def on_POST(self, request): class PasswordResetSubmitTokenServlet(RestServlet): """Handles 3PID validation token submission""" - PATTERNS = client_patterns( - "/password_reset/email/submit_token$", releases=(), unstable=True - ) + PATTERNS = synapse_client_patterns("/password_reset/email/submit_token$") def __init__(self, hs: "HomeServer"): """ From 5060a8e3a4edfb7a4319156dd94f03877f43fefc Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Tue, 1 Sep 2020 19:00:49 +0100 Subject: [PATCH 17/32] Update UPGRADE.rst --- UPGRADE.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UPGRADE.rst b/UPGRADE.rst index b44e4666123c..965548d9876f 100644 --- a/UPGRADE.rst +++ b/UPGRADE.rst @@ -96,7 +96,7 @@ been emailed to them. To complete password reset, the page must include a way to make a `POST` request to ``/_synapse/client/password_reset/{medium}/submit_token`` -with the query parameters from the original link, encoded as a URL-encoded form. See the file +with the query parameters from the original link, presented as a URL-encoded form. See the file itself for more details. Upgrading to v1.18.0 From 9d90642b1935603c0fffa83dfa19634f0a12dbe6 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 2 Sep 2020 18:31:55 +0100 Subject: [PATCH 18/32] Add new resource to password reset unit tests --- tests/rest/client/v2_alpha/test_account.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/rest/client/v2_alpha/test_account.py b/tests/rest/client/v2_alpha/test_account.py index 45ea46421d6a..9c9fe3611f2c 100644 --- a/tests/rest/client/v2_alpha/test_account.py +++ b/tests/rest/client/v2_alpha/test_account.py @@ -27,6 +27,7 @@ from synapse.api.errors import Codes from synapse.rest.client.v1 import login, room from synapse.rest.client.v2_alpha import account, register +from synapse.rest.synapse.client import password_reset from tests import unittest @@ -38,6 +39,7 @@ class PasswordResetTestCase(unittest.HomeserverTestCase): synapse.rest.admin.register_servlets_for_client_rest_resource, register.register_servlets, login.register_servlets, + password_reset.register_servlets, ] def make_homeserver(self, reactor, clock): From 7073605dd003953e77902d92370428fddc505d23 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 2 Sep 2020 18:32:56 +0100 Subject: [PATCH 19/32] Move PasswordResetSubmitTokenServlet to new synapse/client resource --- synapse/rest/client/v2_alpha/account.py | 118 +------------- synapse/rest/synapse/client/password_reset.py | 144 ++++++++++++++++++ 2 files changed, 145 insertions(+), 117 deletions(-) create mode 100644 synapse/rest/synapse/client/password_reset.py diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 68398bdf62d9..26e60548a8e7 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -17,7 +17,6 @@ import logging import random from http import HTTPStatus -from typing import TYPE_CHECKING from synapse.api.constants import LoginType from synapse.api.errors import ( @@ -39,10 +38,7 @@ from synapse.util.stringutils import assert_valid_client_secret, random_string from synapse.util.threepids import canonicalise_email, check_3pid_allowed -if TYPE_CHECKING: - from synapse.server import HomeServer - -from ._base import client_patterns, interactive_auth_handler, synapse_client_patterns +from ._base import client_patterns, interactive_auth_handler logger = logging.getLogger(__name__) @@ -148,117 +144,6 @@ async def on_POST(self, request): return 200, ret -class PasswordResetSubmitTokenServlet(RestServlet): - """Handles 3PID validation token submission""" - - PATTERNS = synapse_client_patterns("/password_reset/email/submit_token$") - - def __init__(self, hs: "HomeServer"): - """ - Args: - hs: server - """ - super().__init__() - - self.clock = hs.get_clock() - self.store = hs.get_datastore() - - self._local_threepid_handling_disabled_due_to_email_config = ( - hs.config.local_threepid_handling_disabled_due_to_email_config - ) - self._threepid_behaviour_email = hs.config.threepid_behaviour_email - if self._threepid_behaviour_email == ThreepidBehaviour.LOCAL: - self._confirmation_email_template = ( - hs.config.email_password_reset_template_confirmation_html - ) - self._email_password_reset_template_success_html = ( - hs.config.email_password_reset_template_success_html_content - ) - self._failure_email_template = ( - hs.config.email_password_reset_template_failure_html - ) - - async def on_GET(self, request): - self._check_threepid_behaviour() - - sid = parse_string(request, "sid", required=True) - token = parse_string(request, "token", required=True) - client_secret = parse_string(request, "client_secret", required=True) - assert_valid_client_secret(client_secret) - - # Show a confirmation page, just in case someone accidentally clicked this link when - # they didn't mean to - template_vars = { - "sid": sid, - "token": token, - "client_secret": client_secret, - "medium": "email", - } - respond_with_html( - request, 200, self._confirmation_email_template.render(**template_vars) - ) - - async def on_POST(self, request): - self._check_threepid_behaviour() - - sid = parse_string(request, "sid", required=True) - token = parse_string(request, "token", required=True) - client_secret = parse_string(request, "client_secret", required=True) - - # Attempt to validate a 3PID session - try: - # Mark the session as valid - next_link = await 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.warning( - "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._email_password_reset_template_success_html - status_code = 200 - except ThreepidValidationError as e: - status_code = e.code - - # Show a failure page with a reason - template_vars = {"failure_reason": e.msg} - html = self._failure_email_template.render(**template_vars) - - respond_with_html(request, status_code, html) - - def _check_threepid_behaviour(self): - """ - Ensure that ThreepidBehaviour is set to LOCAL, and handle cases where it is not - - Raises: - SynapseError: if threepid_behaviour_email is not set to LOCAL - """ - # We currently only handle threepid token submissions for email - if self._threepid_behaviour_email == ThreepidBehaviour.OFF: - if self._local_threepid_handling_disabled_due_to_email_config: - logger.warning( - "Password reset emails have been disabled due to lack of an email config" - ) - raise SynapseError( - 400, "Email-based password resets are disabled on this server" - ) - elif self._threepid_behaviour_email == ThreepidBehaviour.REMOTE: - raise SynapseError( - 400, - "Password resets for this homeserver are handled by a separate program", - ) - - class PasswordRestServlet(RestServlet): PATTERNS = client_patterns("/account/password$") @@ -930,7 +815,6 @@ async def on_GET(self, request): def register_servlets(hs, http_server): EmailPasswordRequestTokenRestServlet(hs).register(http_server) - PasswordResetSubmitTokenServlet(hs).register(http_server) PasswordRestServlet(hs).register(http_server) DeactivateAccountRestServlet(hs).register(http_server) EmailThreepidRequestTokenRestServlet(hs).register(http_server) diff --git a/synapse/rest/synapse/client/password_reset.py b/synapse/rest/synapse/client/password_reset.py new file mode 100644 index 000000000000..53b974b6620b --- /dev/null +++ b/synapse/rest/synapse/client/password_reset.py @@ -0,0 +1,144 @@ +# -*- coding: utf-8 -*- +# Copyright 2020 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. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import logging +from typing import TYPE_CHECKING + +from synapse.api.errors import SynapseError, ThreepidValidationError +from synapse.config.emailconfig import ThreepidBehaviour +from synapse.http.server import finish_request, respond_with_html +from synapse.http.servlet import RestServlet, parse_string +from synapse.util.stringutils import assert_valid_client_secret + +if TYPE_CHECKING: + from synapse.server import HomeServer + +from ._base import synapse_client_patterns + +logger = logging.getLogger(__name__) + + +class PasswordResetSubmitTokenServlet(RestServlet): + """Handles 3PID validation token submission""" + + PATTERNS = synapse_client_patterns("/password_reset/email/submit_token$") + + def __init__(self, hs: "HomeServer"): + """ + Args: + hs: server + """ + super().__init__() + + self.clock = hs.get_clock() + self.store = hs.get_datastore() + + self._local_threepid_handling_disabled_due_to_email_config = ( + hs.config.local_threepid_handling_disabled_due_to_email_config + ) + self._threepid_behaviour_email = hs.config.threepid_behaviour_email + if self._threepid_behaviour_email == ThreepidBehaviour.LOCAL: + self._confirmation_email_template = ( + hs.config.email_password_reset_template_confirmation_html + ) + self._email_password_reset_template_success_html = ( + hs.config.email_password_reset_template_success_html_content + ) + self._failure_email_template = ( + hs.config.email_password_reset_template_failure_html + ) + + async def on_GET(self, request): + self._check_threepid_behaviour() + + sid = parse_string(request, "sid", required=True) + token = parse_string(request, "token", required=True) + client_secret = parse_string(request, "client_secret", required=True) + assert_valid_client_secret(client_secret) + + # Show a confirmation page, just in case someone accidentally clicked this link when + # they didn't mean to + template_vars = { + "sid": sid, + "token": token, + "client_secret": client_secret, + "medium": "email", + } + respond_with_html( + request, 200, self._confirmation_email_template.render(**template_vars) + ) + + async def on_POST(self, request): + self._check_threepid_behaviour() + + sid = parse_string(request, "sid", required=True) + token = parse_string(request, "token", required=True) + client_secret = parse_string(request, "client_secret", required=True) + + # Attempt to validate a 3PID session + try: + # Mark the session as valid + next_link = await 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.warning( + "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._email_password_reset_template_success_html + status_code = 200 + except ThreepidValidationError as e: + status_code = e.code + + # Show a failure page with a reason + template_vars = {"failure_reason": e.msg} + html = self._failure_email_template.render(**template_vars) + + respond_with_html(request, status_code, html) + + def _check_threepid_behaviour(self): + """ + Ensure that ThreepidBehaviour is set to LOCAL, and handle cases where it is not + + Raises: + SynapseError: if threepid_behaviour_email is not set to LOCAL + """ + # We currently only handle threepid token submissions for email + if self._threepid_behaviour_email == ThreepidBehaviour.OFF: + if self._local_threepid_handling_disabled_due_to_email_config: + logger.warning( + "Password reset emails have been disabled due to lack of an email config" + ) + raise SynapseError( + 400, "Email-based password resets are disabled on this server" + ) + elif self._threepid_behaviour_email == ThreepidBehaviour.REMOTE: + raise SynapseError( + 400, + "Password resets for this homeserver are handled by a separate program", + ) + + +def register_servlets(hs, http_server): + PasswordResetSubmitTokenServlet(hs).register(http_server) From 8efa7acbe317a02e6f561a840ad04f2778a9c179 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 2 Sep 2020 18:33:46 +0100 Subject: [PATCH 20/32] Move synapse_client_patterns to synapse/client resource folder --- synapse/rest/client/v2_alpha/_base.py | 15 +---------- synapse/rest/synapse/client/_base.py | 37 +++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 14 deletions(-) create mode 100644 synapse/rest/synapse/client/_base.py diff --git a/synapse/rest/client/v2_alpha/_base.py b/synapse/rest/client/v2_alpha/_base.py index 7f34ef367163..f016b4f1bd41 100644 --- a/synapse/rest/client/v2_alpha/_base.py +++ b/synapse/rest/client/v2_alpha/_base.py @@ -20,7 +20,7 @@ from typing import Iterable, Pattern from synapse.api.errors import InteractiveAuthIncompleteError -from synapse.api.urls import CLIENT_API_PREFIX, SYNAPSE_CLIENT_API_PREFIX +from synapse.api.urls import CLIENT_API_PREFIX from synapse.types import JsonDict logger = logging.getLogger(__name__) @@ -59,19 +59,6 @@ def client_patterns( return patterns -def synapse_client_patterns(path_regex: str) -> Iterable[Pattern]: - """Creates a regex compiled client path with the correct synapse client - path prefix. - - Args: - path_regex: The regex string to match. This should NOT have a ^ - as this will be prefixed. - Returns: - An iterable of patterns. - """ - return [re.compile("^" + SYNAPSE_CLIENT_API_PREFIX + path_regex)] - - def set_timeline_upper_limit(filter_json: JsonDict, filter_timeline_limit: int) -> None: """ Enforces a maximum limit of a timeline query. diff --git a/synapse/rest/synapse/client/_base.py b/synapse/rest/synapse/client/_base.py new file mode 100644 index 000000000000..250fa63e8120 --- /dev/null +++ b/synapse/rest/synapse/client/_base.py @@ -0,0 +1,37 @@ +# -*- coding: utf-8 -*- +# Copyright 2020 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. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""This module contains base REST classes for constructing client v1 servlets. +""" +import logging +import re +from typing import Iterable, Pattern + +from synapse.api.urls import SYNAPSE_CLIENT_API_PREFIX + +logger = logging.getLogger(__name__) + + +def synapse_client_patterns(path_regex: str) -> Iterable[Pattern]: + """Creates a regex compiled client path with the correct synapse client + path prefix. + + Args: + path_regex: The regex string to match. This should NOT have a ^ + as this will be prefixed. + Returns: + An iterable of patterns. + """ + return [re.compile("^" + SYNAPSE_CLIENT_API_PREFIX + path_regex)] From ad231e349c6622da9ea97f2b4ad1ef19bcff38dc Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 2 Sep 2020 18:34:12 +0100 Subject: [PATCH 21/32] Add new PasswordResetRestResource JsonResource --- synapse/rest/synapse/client/__init__.py | 37 +++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 synapse/rest/synapse/client/__init__.py diff --git a/synapse/rest/synapse/client/__init__.py b/synapse/rest/synapse/client/__init__.py new file mode 100644 index 000000000000..b4f030eac0b2 --- /dev/null +++ b/synapse/rest/synapse/client/__init__.py @@ -0,0 +1,37 @@ +# -*- coding: utf-8 -*- +# Copyright 2020 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. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from typing import TYPE_CHECKING + +from synapse.http.server import JsonResource +from synapse.rest.synapse.client import password_reset + +if TYPE_CHECKING: + from synapse.server import HomeServer + + +class PasswordResetRestResource(JsonResource): + """Synapse Internal Resource for password reset functionality + + This resource gets mounted under /_synapse/client + """ + + def __init__(self, hs: "HomeServer"): + JsonResource.__init__(self, hs, canonical_json=False) + self.register_servlets(self, hs) + + @staticmethod + def register_servlets(synapse_client_resource: JsonResource, hs: "HomeServer"): + password_reset.register_servlets(hs, synapse_client_resource) From c1b9ad2360e14e3ce5a112b4c75454c75bf99f32 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 2 Sep 2020 18:36:20 +0100 Subject: [PATCH 22/32] Add PasswordResetRestResource to HomeServer --- synapse/app/homeserver.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 98d0d14a124b..01eb901e8b2d 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -48,6 +48,7 @@ from synapse.app import _base from synapse.app._base import listen_ssl, listen_tcp, quit_with_error from synapse.config._base import ConfigError +from synapse.config.emailconfig import ThreepidBehaviour from synapse.config.homeserver import HomeServerConfig from synapse.config.server import ListenerConfig from synapse.federation.transport.server import TransportLayerServer @@ -209,6 +210,12 @@ def _configure_named_resource(self, name, compress=False): resources["/_matrix/saml2"] = SAML2Resource(self) + if self.get_config().threepid_behaviour_email == ThreepidBehaviour.LOCAL: + from synapse.rest.synapse.client import PasswordResetRestResource + + password_reset = PasswordResetRestResource(self) + resources["/_synapse/client/password_reset"] = password_reset + if name == "consent": from synapse.rest.consent.consent_resource import ConsentResource From cb7c903e45a8a0f1eaaa7239f1522ab5c23a762f Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 3 Sep 2020 11:15:55 +0100 Subject: [PATCH 23/32] Convert synapse.rest.synapse to a python package --- synapse/rest/synapse/__init__.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 synapse/rest/synapse/__init__.py diff --git a/synapse/rest/synapse/__init__.py b/synapse/rest/synapse/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 From 76e90003c44bf95362ba19cc671bbe9af7f971bf Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 3 Sep 2020 11:41:33 +0100 Subject: [PATCH 24/32] Rename synapse module to internal due to conflicts --- synapse/app/homeserver.py | 2 +- synapse/rest/{synapse => internal}/__init__.py | 0 synapse/rest/{synapse => internal}/client/__init__.py | 2 +- synapse/rest/{synapse => internal}/client/_base.py | 0 synapse/rest/{synapse => internal}/client/password_reset.py | 0 tests/rest/client/v2_alpha/test_account.py | 2 +- 6 files changed, 3 insertions(+), 3 deletions(-) rename synapse/rest/{synapse => internal}/__init__.py (100%) rename synapse/rest/{synapse => internal}/client/__init__.py (95%) rename synapse/rest/{synapse => internal}/client/_base.py (100%) rename synapse/rest/{synapse => internal}/client/password_reset.py (100%) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 01eb901e8b2d..5285981157e8 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -211,7 +211,7 @@ def _configure_named_resource(self, name, compress=False): resources["/_matrix/saml2"] = SAML2Resource(self) if self.get_config().threepid_behaviour_email == ThreepidBehaviour.LOCAL: - from synapse.rest.synapse.client import PasswordResetRestResource + from synapse.rest.internal.client import PasswordResetRestResource password_reset = PasswordResetRestResource(self) resources["/_synapse/client/password_reset"] = password_reset diff --git a/synapse/rest/synapse/__init__.py b/synapse/rest/internal/__init__.py similarity index 100% rename from synapse/rest/synapse/__init__.py rename to synapse/rest/internal/__init__.py diff --git a/synapse/rest/synapse/client/__init__.py b/synapse/rest/internal/client/__init__.py similarity index 95% rename from synapse/rest/synapse/client/__init__.py rename to synapse/rest/internal/client/__init__.py index b4f030eac0b2..be8118628a1d 100644 --- a/synapse/rest/synapse/client/__init__.py +++ b/synapse/rest/internal/client/__init__.py @@ -16,7 +16,7 @@ from typing import TYPE_CHECKING from synapse.http.server import JsonResource -from synapse.rest.synapse.client import password_reset +from synapse.rest.internal.client import password_reset if TYPE_CHECKING: from synapse.server import HomeServer diff --git a/synapse/rest/synapse/client/_base.py b/synapse/rest/internal/client/_base.py similarity index 100% rename from synapse/rest/synapse/client/_base.py rename to synapse/rest/internal/client/_base.py diff --git a/synapse/rest/synapse/client/password_reset.py b/synapse/rest/internal/client/password_reset.py similarity index 100% rename from synapse/rest/synapse/client/password_reset.py rename to synapse/rest/internal/client/password_reset.py diff --git a/tests/rest/client/v2_alpha/test_account.py b/tests/rest/client/v2_alpha/test_account.py index 9c9fe3611f2c..bf72e6424334 100644 --- a/tests/rest/client/v2_alpha/test_account.py +++ b/tests/rest/client/v2_alpha/test_account.py @@ -27,7 +27,7 @@ from synapse.api.errors import Codes from synapse.rest.client.v1 import login, room from synapse.rest.client.v2_alpha import account, register -from synapse.rest.synapse.client import password_reset +from synapse.rest.internal.client import password_reset from tests import unittest From 717961ddb3c7308cb41083a7188352821ebd90ce Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 3 Sep 2020 13:42:26 +0100 Subject: [PATCH 25/32] Remove action call from HTML template, remove medium placeholder --- synapse/res/templates/password_reset_confirmation.html | 2 +- synapse/rest/internal/client/password_reset.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/synapse/res/templates/password_reset_confirmation.html b/synapse/res/templates/password_reset_confirmation.html index ea9a4284e609..def4b5162b90 100644 --- a/synapse/res/templates/password_reset_confirmation.html +++ b/synapse/res/templates/password_reset_confirmation.html @@ -2,7 +2,7 @@ - + diff --git a/synapse/rest/internal/client/password_reset.py b/synapse/rest/internal/client/password_reset.py index 53b974b6620b..9e5a0544fb31 100644 --- a/synapse/rest/internal/client/password_reset.py +++ b/synapse/rest/internal/client/password_reset.py @@ -73,7 +73,6 @@ async def on_GET(self, request): "sid": sid, "token": token, "client_secret": client_secret, - "medium": "email", } respond_with_html( request, 200, self._confirmation_email_template.render(**template_vars) From 775e310c42a5e0b8a63ad139340e06fd30dddd3c Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 3 Sep 2020 13:44:11 +0100 Subject: [PATCH 26/32] Update docstring --- synapse/rest/internal/client/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/rest/internal/client/__init__.py b/synapse/rest/internal/client/__init__.py index be8118628a1d..6cfbd1123b8b 100644 --- a/synapse/rest/internal/client/__init__.py +++ b/synapse/rest/internal/client/__init__.py @@ -25,7 +25,7 @@ class PasswordResetRestResource(JsonResource): """Synapse Internal Resource for password reset functionality - This resource gets mounted under /_synapse/client + This resource gets mounted under /_synapse/client/password_reset/email/submit_token """ def __init__(self, hs: "HomeServer"): From 5ed7cf731b8dd66014a0a6fda158d36e1ad17446 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 3 Sep 2020 15:37:56 +0100 Subject: [PATCH 27/32] Switch from serlvets to base resources --- synapse/app/homeserver.py | 9 +++-- synapse/rest/internal/__init__.py | 14 +++++++ synapse/rest/internal/client/__init__.py | 23 ------------ synapse/rest/internal/client/_base.py | 37 ------------------- .../rest/internal/client/password_reset.py | 27 +++++++------- tests/rest/client/v2_alpha/test_account.py | 10 +++-- 6 files changed, 40 insertions(+), 80 deletions(-) delete mode 100644 synapse/rest/internal/client/_base.py diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 5285981157e8..95dcd970816a 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -211,10 +211,13 @@ def _configure_named_resource(self, name, compress=False): resources["/_matrix/saml2"] = SAML2Resource(self) if self.get_config().threepid_behaviour_email == ThreepidBehaviour.LOCAL: - from synapse.rest.internal.client import PasswordResetRestResource + from synapse.rest.internal.client.password_reset import ( + PasswordResetSubmitTokenResource, + ) - password_reset = PasswordResetRestResource(self) - resources["/_synapse/client/password_reset"] = password_reset + resources[ + "/_synapse/client/password_reset/email/submit_token" + ] = PasswordResetSubmitTokenResource(self) if name == "consent": from synapse.rest.consent.consent_resource import ConsentResource diff --git a/synapse/rest/internal/__init__.py b/synapse/rest/internal/__init__.py index e69de29bb2d1..c0b733488b5f 100644 --- a/synapse/rest/internal/__init__.py +++ b/synapse/rest/internal/__init__.py @@ -0,0 +1,14 @@ +# -*- coding: utf-8 -*- +# Copyright 2020 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. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. diff --git a/synapse/rest/internal/client/__init__.py b/synapse/rest/internal/client/__init__.py index 6cfbd1123b8b..c0b733488b5f 100644 --- a/synapse/rest/internal/client/__init__.py +++ b/synapse/rest/internal/client/__init__.py @@ -12,26 +12,3 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. - -from typing import TYPE_CHECKING - -from synapse.http.server import JsonResource -from synapse.rest.internal.client import password_reset - -if TYPE_CHECKING: - from synapse.server import HomeServer - - -class PasswordResetRestResource(JsonResource): - """Synapse Internal Resource for password reset functionality - - This resource gets mounted under /_synapse/client/password_reset/email/submit_token - """ - - def __init__(self, hs: "HomeServer"): - JsonResource.__init__(self, hs, canonical_json=False) - self.register_servlets(self, hs) - - @staticmethod - def register_servlets(synapse_client_resource: JsonResource, hs: "HomeServer"): - password_reset.register_servlets(hs, synapse_client_resource) diff --git a/synapse/rest/internal/client/_base.py b/synapse/rest/internal/client/_base.py deleted file mode 100644 index 250fa63e8120..000000000000 --- a/synapse/rest/internal/client/_base.py +++ /dev/null @@ -1,37 +0,0 @@ -# -*- coding: utf-8 -*- -# Copyright 2020 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. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -"""This module contains base REST classes for constructing client v1 servlets. -""" -import logging -import re -from typing import Iterable, Pattern - -from synapse.api.urls import SYNAPSE_CLIENT_API_PREFIX - -logger = logging.getLogger(__name__) - - -def synapse_client_patterns(path_regex: str) -> Iterable[Pattern]: - """Creates a regex compiled client path with the correct synapse client - path prefix. - - Args: - path_regex: The regex string to match. This should NOT have a ^ - as this will be prefixed. - Returns: - An iterable of patterns. - """ - return [re.compile("^" + SYNAPSE_CLIENT_API_PREFIX + path_regex)] diff --git a/synapse/rest/internal/client/password_reset.py b/synapse/rest/internal/client/password_reset.py index 9e5a0544fb31..e38f563b5c95 100644 --- a/synapse/rest/internal/client/password_reset.py +++ b/synapse/rest/internal/client/password_reset.py @@ -17,22 +17,27 @@ from synapse.api.errors import SynapseError, ThreepidValidationError from synapse.config.emailconfig import ThreepidBehaviour -from synapse.http.server import finish_request, respond_with_html -from synapse.http.servlet import RestServlet, parse_string +from synapse.http.server import ( + DirectServeHtmlResource, + finish_request, + respond_with_html, +) +from synapse.http.servlet import parse_string from synapse.util.stringutils import assert_valid_client_secret if TYPE_CHECKING: from synapse.server import HomeServer -from ._base import synapse_client_patterns - logger = logging.getLogger(__name__) -class PasswordResetSubmitTokenServlet(RestServlet): - """Handles 3PID validation token submission""" +class PasswordResetSubmitTokenResource(DirectServeHtmlResource): + """Handles 3PID validation token submission + + This resource gets mounted under /_synapse/client/password_reset/email/submit_token + """ - PATTERNS = synapse_client_patterns("/password_reset/email/submit_token$") + isLeaf = 1 def __init__(self, hs: "HomeServer"): """ @@ -59,7 +64,7 @@ def __init__(self, hs: "HomeServer"): hs.config.email_password_reset_template_failure_html ) - async def on_GET(self, request): + async def _async_render_GET(self, request): self._check_threepid_behaviour() sid = parse_string(request, "sid", required=True) @@ -78,7 +83,7 @@ async def on_GET(self, request): request, 200, self._confirmation_email_template.render(**template_vars) ) - async def on_POST(self, request): + async def _async_render_POST(self, request): self._check_threepid_behaviour() sid = parse_string(request, "sid", required=True) @@ -137,7 +142,3 @@ def _check_threepid_behaviour(self): 400, "Password resets for this homeserver are handled by a separate program", ) - - -def register_servlets(hs, http_server): - PasswordResetSubmitTokenServlet(hs).register(http_server) diff --git a/tests/rest/client/v2_alpha/test_account.py b/tests/rest/client/v2_alpha/test_account.py index bf72e6424334..8e8c05d6ec34 100644 --- a/tests/rest/client/v2_alpha/test_account.py +++ b/tests/rest/client/v2_alpha/test_account.py @@ -27,7 +27,7 @@ from synapse.api.errors import Codes from synapse.rest.client.v1 import login, room from synapse.rest.client.v2_alpha import account, register -from synapse.rest.internal.client import password_reset +from synapse.rest.internal.client.password_reset import PasswordResetSubmitTokenResource from tests import unittest @@ -39,7 +39,6 @@ class PasswordResetTestCase(unittest.HomeserverTestCase): synapse.rest.admin.register_servlets_for_client_rest_resource, register.register_servlets, login.register_servlets, - password_reset.register_servlets, ] def make_homeserver(self, reactor, clock): @@ -71,6 +70,7 @@ async def sendmail(smtphost, from_addr, to_addrs, msg, **kwargs): def prepare(self, reactor, clock, hs): self.store = hs.get_datastore() + self.submit_token_resource = PasswordResetSubmitTokenResource(hs) def test_basic_password_reset(self): """Test basic password reset flow @@ -254,7 +254,8 @@ def _validate_token(self, link): # Load the password reset confirmation page request, channel = self.make_request("GET", path, shorthand=False) - self.render(request) + request.render(self.submit_token_resource) + self.pump() self.assertEquals(200, channel.code, channel.result) # Now POST to the same endpoint, mimicking the same behaviour as clicking the @@ -275,7 +276,8 @@ def _validate_token(self, link): shorthand=False, content_is_form=True, ) - self.render(request) + request.render(self.submit_token_resource) + self.pump() self.assertEquals(200, channel.code, channel.result) def _get_link_from_email(self): From c3f6ff57c34b01d01b91c5be6cac80529cc13164 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 9 Sep 2020 13:44:50 +0100 Subject: [PATCH 28/32] Inline password reset confirmation template filename --- synapse/config/emailconfig.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/synapse/config/emailconfig.py b/synapse/config/emailconfig.py index 019b78509b6f..72b42bfd6278 100644 --- a/synapse/config/emailconfig.py +++ b/synapse/config/emailconfig.py @@ -198,9 +198,6 @@ def read_config(self, config, **kwargs): "add_threepid_template_text", "add_threepid.txt" ) - password_reset_template_confirmation_html = ( - "password_reset_confirmation.html" - ) password_reset_template_failure_html = email_config.get( "password_reset_template_failure_html", "password_reset_failure.html" ) @@ -246,7 +243,7 @@ def read_config(self, config, **kwargs): registration_template_text, add_threepid_template_html, add_threepid_template_text, - password_reset_template_confirmation_html, + "password_reset_confirmation.html", password_reset_template_failure_html, registration_template_failure_html, add_threepid_template_failure_html, From 2b6bde7a076f09aef371bfc4b6f1cecedd7b1d64 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 9 Sep 2020 13:46:11 +0100 Subject: [PATCH 29/32] Typing and returning things directly --- .../rest/internal/client/password_reset.py | 42 ++++++++++--------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/synapse/rest/internal/client/password_reset.py b/synapse/rest/internal/client/password_reset.py index e38f563b5c95..480a0286ca0a 100644 --- a/synapse/rest/internal/client/password_reset.py +++ b/synapse/rest/internal/client/password_reset.py @@ -13,15 +13,13 @@ # See the License for the specific language governing permissions and # limitations under the License. import logging -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Tuple + +from twisted.web.http import Request from synapse.api.errors import SynapseError, ThreepidValidationError from synapse.config.emailconfig import ThreepidBehaviour -from synapse.http.server import ( - DirectServeHtmlResource, - finish_request, - respond_with_html, -) +from synapse.http.server import DirectServeHtmlResource from synapse.http.servlet import parse_string from synapse.util.stringutils import assert_valid_client_secret @@ -64,9 +62,8 @@ def __init__(self, hs: "HomeServer"): hs.config.email_password_reset_template_failure_html ) - async def _async_render_GET(self, request): + async def _async_render_GET(self, request: Request) -> Tuple[int, bytes]: self._check_threepid_behaviour() - sid = parse_string(request, "sid", required=True) token = parse_string(request, "token", required=True) client_secret = parse_string(request, "client_secret", required=True) @@ -79,13 +76,13 @@ async def _async_render_GET(self, request): "token": token, "client_secret": client_secret, } - respond_with_html( - request, 200, self._confirmation_email_template.render(**template_vars) + return ( + 200, + self._confirmation_email_template.render(**template_vars).encode("utf-8"), ) - async def _async_render_POST(self, request): + async def _async_render_POST(self, request: Request) -> Tuple[int, bytes]: self._check_threepid_behaviour() - sid = parse_string(request, "sid", required=True) token = parse_string(request, "token", required=True) client_secret = parse_string(request, "client_secret", required=True) @@ -104,22 +101,29 @@ async def _async_render_POST(self, request): "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 + next_link_bytes = next_link.encode("utf-8") + request.setHeader("Location", next_link_bytes) + return ( + 302, + ( + b'You are being redirected to %s.' + % (next_link_bytes, next_link_bytes) + ), + ) # Otherwise show the success template - html = self._email_password_reset_template_success_html + html_bytes = self._email_password_reset_template_success_html.encode("utf-8") status_code = 200 except ThreepidValidationError as e: status_code = e.code # Show a failure page with a reason template_vars = {"failure_reason": e.msg} - html = self._failure_email_template.render(**template_vars) + html_bytes = self._failure_email_template.render(**template_vars).encode( + "utf-8" + ) - respond_with_html(request, status_code, html) + return status_code, html_bytes def _check_threepid_behaviour(self): """ From 0ad075347590d30697713768c062d8cea1504aaa Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 9 Sep 2020 14:15:18 +0100 Subject: [PATCH 30/32] Remove checks for threepid behaviour and add an assert --- .../rest/internal/client/password_reset.py | 53 ++++++------------- 1 file changed, 16 insertions(+), 37 deletions(-) diff --git a/synapse/rest/internal/client/password_reset.py b/synapse/rest/internal/client/password_reset.py index 480a0286ca0a..9e4fbc0cbd08 100644 --- a/synapse/rest/internal/client/password_reset.py +++ b/synapse/rest/internal/client/password_reset.py @@ -17,7 +17,7 @@ from twisted.web.http import Request -from synapse.api.errors import SynapseError, ThreepidValidationError +from synapse.api.errors import ThreepidValidationError from synapse.config.emailconfig import ThreepidBehaviour from synapse.http.server import DirectServeHtmlResource from synapse.http.servlet import parse_string @@ -50,20 +50,20 @@ def __init__(self, hs: "HomeServer"): self._local_threepid_handling_disabled_due_to_email_config = ( hs.config.local_threepid_handling_disabled_due_to_email_config ) - self._threepid_behaviour_email = hs.config.threepid_behaviour_email - if self._threepid_behaviour_email == ThreepidBehaviour.LOCAL: - self._confirmation_email_template = ( - hs.config.email_password_reset_template_confirmation_html - ) - self._email_password_reset_template_success_html = ( - hs.config.email_password_reset_template_success_html_content - ) - self._failure_email_template = ( - hs.config.email_password_reset_template_failure_html - ) + self._confirmation_email_template = ( + hs.config.email_password_reset_template_confirmation_html + ) + self._email_password_reset_template_success_html = ( + hs.config.email_password_reset_template_success_html_content + ) + self._failure_email_template = ( + hs.config.email_password_reset_template_failure_html + ) + + # This resource should not be mounted if threepid behaviour is not LOCAL + assert hs.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL async def _async_render_GET(self, request: Request) -> Tuple[int, bytes]: - self._check_threepid_behaviour() sid = parse_string(request, "sid", required=True) token = parse_string(request, "token", required=True) client_secret = parse_string(request, "client_secret", required=True) @@ -82,7 +82,6 @@ async def _async_render_GET(self, request: Request) -> Tuple[int, bytes]: ) async def _async_render_POST(self, request: Request) -> Tuple[int, bytes]: - self._check_threepid_behaviour() sid = parse_string(request, "sid", required=True) token = parse_string(request, "token", required=True) client_secret = parse_string(request, "client_secret", required=True) @@ -112,7 +111,9 @@ async def _async_render_POST(self, request: Request) -> Tuple[int, bytes]: ) # Otherwise show the success template - html_bytes = self._email_password_reset_template_success_html.encode("utf-8") + html_bytes = self._email_password_reset_template_success_html.encode( + "utf-8" + ) status_code = 200 except ThreepidValidationError as e: status_code = e.code @@ -124,25 +125,3 @@ async def _async_render_POST(self, request: Request) -> Tuple[int, bytes]: ) return status_code, html_bytes - - def _check_threepid_behaviour(self): - """ - Ensure that ThreepidBehaviour is set to LOCAL, and handle cases where it is not - - Raises: - SynapseError: if threepid_behaviour_email is not set to LOCAL - """ - # We currently only handle threepid token submissions for email - if self._threepid_behaviour_email == ThreepidBehaviour.OFF: - if self._local_threepid_handling_disabled_due_to_email_config: - logger.warning( - "Password reset emails have been disabled due to lack of an email config" - ) - raise SynapseError( - 400, "Email-based password resets are disabled on this server" - ) - elif self._threepid_behaviour_email == ThreepidBehaviour.REMOTE: - raise SynapseError( - 400, - "Password resets for this homeserver are handled by a separate program", - ) From c460e3b6450fb0a1a6a0ce82519e83c1cb263b60 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 9 Sep 2020 14:18:43 +0100 Subject: [PATCH 31/32] v1.20.0 -> v1.21.0 --- UPGRADE.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UPGRADE.rst b/UPGRADE.rst index 9090b0912788..1e4da98afe9a 100644 --- a/UPGRADE.rst +++ b/UPGRADE.rst @@ -88,7 +88,7 @@ for example: wget https://packages.matrix.org/debian/pool/main/m/matrix-synapse-py3/matrix-synapse-py3_1.3.0+stretch1_amd64.deb dpkg -i matrix-synapse-py3_1.3.0+stretch1_amd64.deb -Upgrading to v1.20.0 +Upgrading to v1.21.0 ==================== New HTML templates From c5af421a80e404d93cba91b98fff199a27f6436f Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 9 Sep 2020 14:30:12 +0100 Subject: [PATCH 32/32] Rename 'internal' dir to 'synapse' We needed to switch the admin import as else it tries to import relatively, and we end up importing the wrong module. --- synapse/app/homeserver.py | 2 +- synapse/rest/__init__.py | 6 ++---- synapse/rest/{internal => synapse}/__init__.py | 0 synapse/rest/{internal => synapse}/client/__init__.py | 0 synapse/rest/{internal => synapse}/client/password_reset.py | 0 tests/rest/client/v2_alpha/test_account.py | 2 +- 6 files changed, 4 insertions(+), 6 deletions(-) rename synapse/rest/{internal => synapse}/__init__.py (100%) rename synapse/rest/{internal => synapse}/client/__init__.py (100%) rename synapse/rest/{internal => synapse}/client/password_reset.py (100%) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index af5963764678..b08319ca77f6 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -211,7 +211,7 @@ def _configure_named_resource(self, name, compress=False): resources["/_matrix/saml2"] = SAML2Resource(self) if self.get_config().threepid_behaviour_email == ThreepidBehaviour.LOCAL: - from synapse.rest.internal.client.password_reset import ( + from synapse.rest.synapse.client.password_reset import ( PasswordResetSubmitTokenResource, ) diff --git a/synapse/rest/__init__.py b/synapse/rest/__init__.py index 87f927890c5e..40f5c32db2df 100644 --- a/synapse/rest/__init__.py +++ b/synapse/rest/__init__.py @@ -13,8 +13,8 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -import synapse.rest.admin from synapse.http.server import JsonResource +from synapse.rest import admin from synapse.rest.client import versions from synapse.rest.client.v1 import ( directory, @@ -123,9 +123,7 @@ def register_servlets(client_resource, hs): password_policy.register_servlets(hs, client_resource) # moving to /_synapse/admin - synapse.rest.admin.register_servlets_for_client_rest_resource( - hs, client_resource - ) + admin.register_servlets_for_client_rest_resource(hs, client_resource) # unstable shared_rooms.register_servlets(hs, client_resource) diff --git a/synapse/rest/internal/__init__.py b/synapse/rest/synapse/__init__.py similarity index 100% rename from synapse/rest/internal/__init__.py rename to synapse/rest/synapse/__init__.py diff --git a/synapse/rest/internal/client/__init__.py b/synapse/rest/synapse/client/__init__.py similarity index 100% rename from synapse/rest/internal/client/__init__.py rename to synapse/rest/synapse/client/__init__.py diff --git a/synapse/rest/internal/client/password_reset.py b/synapse/rest/synapse/client/password_reset.py similarity index 100% rename from synapse/rest/internal/client/password_reset.py rename to synapse/rest/synapse/client/password_reset.py diff --git a/tests/rest/client/v2_alpha/test_account.py b/tests/rest/client/v2_alpha/test_account.py index c31db7f983fa..93f899d86133 100644 --- a/tests/rest/client/v2_alpha/test_account.py +++ b/tests/rest/client/v2_alpha/test_account.py @@ -28,7 +28,7 @@ from synapse.api.errors import Codes from synapse.rest.client.v1 import login, room from synapse.rest.client.v2_alpha import account, register -from synapse.rest.internal.client.password_reset import PasswordResetSubmitTokenResource +from synapse.rest.synapse.client.password_reset import PasswordResetSubmitTokenResource from tests import unittest from tests.unittest import override_config