Pass a list of arg tuples to urlencode instead of request.args

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

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 <https://github.com/matrix-org/synapse/blob/develop/synapse/res/templates/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
 ====================
 

changelog.d/8004.feature

@@ -0,0 +1 @@
+Require the user to confirm that their password should be reset after clicking the email confirmation link.

docs/sample_config.yaml

@@ -2021,9 +2021,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
+  # * An HTML page that a user will see when they follow the link in the password
-  #   the link in the password reset email: 'password_reset_success.html' and
+  #   reset email. The user will be asked to confirm the action before their
-  #   'password_reset_failure.html'
+  #   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'

synapse/config/emailconfig.py

@@ -198,6 +198,9 @@ class EmailConfig(Config):
                 "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 @@ class EmailConfig(Config):
                 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 @@ class EmailConfig(Config):
                     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 @@ class EmailConfig(Config):
           # * 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
+          # * An HTML page that a user will see when they follow the link in the password
-          #   the link in the password reset email: 'password_reset_success.html' and
+          #   reset email. The user will be asked to confirm the action before their
-          #   'password_reset_failure.html'
+          #   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'

synapse/res/templates/password_reset_confirmation.html

@@ -0,0 +1,16 @@
+<html>
+<head></head>
+<body>
+<!--Use a hidden form to resubmit the information necessary to reset the password-->
+<form action="/_matrix/client/unstable/password_reset/{{ medium }}/submit_token_confirm" method="post">
+    <input type="hidden" name="sid" value="{{ sid }}">
+    <input type="hidden" name="token" value="{{ token }}">
+    <input type="hidden" name="client_secret" value="{{ client_secret }}">
+
+    <p>You have requested to <strong>reset your Matrix account password</strong>. Click the link below to confirm this action. <br /><br />
+        If you did not mean to do this, please close this page and your password will not be changed.</p>
+    <p><button type="submit">Confirm changing my password</button></p>
+</form>
+</body>
+</html>
+

synapse/rest/client/v2_alpha/account.py

@@ -17,7 +17,9 @@
 import logging
 import random
 from http import HTTPStatus
+from typing import TYPE_CHECKING
 
+from twisted.web.server import Request
 from synapse.api.constants import LoginType
 from synapse.api.errors import (
     Codes,
@@ -38,6 +40,9 @@ from synapse.util.msisdn import phone_number_to_msisdn
 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__)
@@ -157,14 +162,14 @@ class PasswordResetSubmitTokenServlet(RestServlet):
             hs (synapse.server.HomeServer): server
         """
         super(PasswordResetSubmitTokenServlet, self).__init__()
-        self.hs = hs
+
-        self.auth = hs.get_auth()
+        self._threepid_behaviour_email = hs.config.threepid_behaviour_email
-        self.config = hs.config
+        self._local_threepid_handling_disabled_due_to_email_config = (
-        self.clock = hs.get_clock()
+            hs.config.local_threepid_handling_disabled_due_to_email_config
-        self.store = hs.get_datastore()
+        )
-        if self.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL:
+        if self._threepid_behaviour_email == ThreepidBehaviour.LOCAL:
-            self._failure_email_template = (
+            self._confirmation_email_template = (
-                self.config.email_password_reset_template_failure_html
+                hs.config.email_password_reset_template_confirmation_html
             )
 
     async def on_GET(self, request, medium):
@@ -173,20 +178,91 @@ class PasswordResetSubmitTokenServlet(RestServlet):
             raise SynapseError(
                 400, "This medium is currently not supported for password resets"
             )
-        if self.config.threepid_behaviour_email == ThreepidBehaviour.OFF:
+        if self._threepid_behaviour_email == ThreepidBehaviour.OFF:
-            if self.config.local_threepid_handling_disabled_due_to_email_config:
+            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",
+            )
 
         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": 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: "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: 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",
+            )
+
+        logger.info("ARGS: %s, CONTENT: %s, HEADERS: %s", request.args, request.content,
+                    request.getAllHeaders())
+
+        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
@@ -207,7 +283,7 @@ class PasswordResetSubmitTokenServlet(RestServlet):
                     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
@@ -891,6 +967,7 @@ class WhoamiRestServlet(RestServlet):
 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)

tests/rest/client/v2_alpha/test_account.py

@@ -16,6 +16,7 @@
 # limitations under the License.
 
 import json
+from urllib.parse import urlencode
 import os
 import re
 from email.parser import Parser
@@ -70,6 +71,7 @@ class PasswordResetTestCase(unittest.HomeserverTestCase):
     def prepare(self, reactor, clock, hs):
         self.store = hs.get_datastore()
 
+    @unittest.INFO
     def test_basic_password_reset(self):
         """Test basic password reset flow
         """
@@ -250,10 +252,33 @@ class PasswordResetTestCase(unittest.HomeserverTestCase):
         # 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 = "/_matrix/client/unstable/password_reset/email/submit_token_confirm"
+
+        form_args = []
+        for key, value_list in request.args.items():
+            for value in value_list:
+                arg = (key, value)
+                form_args.append(arg)
+
+        print("form_args:", form_args)
+        print("encoded form_args:", urlencode(form_args))
+
+        # Confirm the password reset
+        request, channel = self.make_request(
+            "POST",
+            path,
+            content=urlencode(form_args).encode("utf8"),
+            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"
 

tests/server.py

@@ -1,6 +1,7 @@
 import json
 import logging
 from io import BytesIO
+from json.decoder import JSONDecodeError
 
 import attr
 from zope.interface import implementer
@@ -195,7 +196,19 @@ def make_request(
         )
 
     if content:
-        req.requestHeaders.addRawHeader(b"Content-Type", b"application/json")
+        content_is_json = True
+        try:
+            json.loads(content)
+        except JSONDecodeError:
+            content_is_json = False
+
+        print("Content is json?", content_is_json, path)
+        if content_is_json:
+            req.requestHeaders.addRawHeader(b"Content-Type", b"application/json")
+        else:
+            req.requestHeaders.addRawHeader(
+                b"Content-Type", b"application/x-www-form-urlencoded"
+            )
 
     req.requestReceived(method, path, b"1.1")
 

tests/test_utils/http.py

@@ -0,0 +1,37 @@
+# -*- coding: utf-8 -*-
+# Copyright 2018 New Vector Ltd
+# 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 twisted.web.server import Request
+
+
+def convert_request_args_to_form_data(request: Request) -> bytes:
+    """Converts query arguments from a request to formatted HTML form data
+
+    Ref: https://developer.mozilla.org/en-US/docs/Learn/Forms/Sending_and_retrieving_form_data
+
+    Args:
+        The request to pull arguments from
+
+    Returns:
+        The HTML form body data representation of the request's arguments
+    """
+    body = b""
+    for key, value in request.args.items():
+        arg = b"%s=%s&" % (key, value[0])
+        body += arg
+
+    # Remove the last '&' sign
+    return body[:-1]