Allow HS to send emails when adding an email to the HS (#6042)

12 files changed, 385 insertions, 81 deletions

diff --git a/changelog.d/6042.feature b/changelog.d/6042.feature
new file mode 100644
index 0000000000..a737760363
--- /dev/null
+++ b/changelog.d/6042.feature
@@ -0,0 +1 @@
+Allow homeserver to handle or delegate email validation when adding an email to a user's account.
diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml
index bd486d97b6..722f111ad8 100644
--- a/docs/sample_config.yaml
+++ b/docs/sample_config.yaml
@@ -1448,6 +1448,12 @@ password_config:
 #   #registration_template_html: registration.html
 #   #registration_template_text: registration.txt
 #
+#   # Templates for validation emails sent by the homeserver when adding an email to
+#   # your user account
+#   #
+#   #add_threepid_template_html: add_threepid.html
+#   #add_threepid_template_text: add_threepid.txt
+#
 #   # Templates for password reset success and failure pages that a user
 #   # will see after attempting to reset their password
 #   #
@@ -1459,6 +1465,12 @@ password_config:
 #   #
 #   #registration_template_success_html: registration_success.html
 #   #registration_template_failure_html: registration_failure.html
+#
+#   # Templates for success and failure pages that a user will see after attempting
+#   # to add an email or phone to their account
+#   #
+#   #add_threepid_success_html: add_threepid_success.html
+#   #add_threepid_failure_html: add_threepid_failure.html
 
 
 #password_providers:
diff --git a/synapse/config/emailconfig.py b/synapse/config/emailconfig.py
index e5de768b0c..d9b43de660 100644
--- a/synapse/config/emailconfig.py
+++ b/synapse/config/emailconfig.py
@@ -169,12 +169,22 @@ class EmailConfig(Config):
             self.email_registration_template_text = email_config.get(
                 "registration_template_text", "registration.txt"
             )
+            self.email_add_threepid_template_html = email_config.get(
+                "add_threepid_template_html", "add_threepid.html"
+            )
+            self.email_add_threepid_template_text = email_config.get(
+                "add_threepid_template_text", "add_threepid.txt"
+            )
+
             self.email_password_reset_template_failure_html = email_config.get(
                 "password_reset_template_failure_html", "password_reset_failure.html"
             )
             self.email_registration_template_failure_html = email_config.get(
                 "registration_template_failure_html", "registration_failure.html"
             )
+            self.email_add_threepid_template_failure_html = email_config.get(
+                "add_threepid_template_failure_html", "add_threepid_failure.html"
+            )
 
             # These templates do not support any placeholder variables, so we
             # will read them from disk once during setup
@@ -184,6 +194,9 @@ class EmailConfig(Config):
             email_registration_template_success_html = email_config.get(
                 "registration_template_success_html", "registration_success.html"
             )
+            email_add_threepid_template_success_html = email_config.get(
+                "add_threepid_template_success_html", "add_threepid_success.html"
+            )
 
             # Check templates exist
             for f in [
@@ -191,9 +204,14 @@ class EmailConfig(Config):
                 self.email_password_reset_template_text,
                 self.email_registration_template_html,
                 self.email_registration_template_text,
+                self.email_add_threepid_template_html,
+                self.email_add_threepid_template_text,
                 self.email_password_reset_template_failure_html,
+                self.email_registration_template_failure_html,
+                self.email_add_threepid_template_failure_html,
                 email_password_reset_template_success_html,
                 email_registration_template_success_html,
+                email_add_threepid_template_success_html,
             ]:
                 p = os.path.join(self.email_template_dir, f)
                 if not os.path.isfile(p):
@@ -212,6 +230,12 @@ class EmailConfig(Config):
             self.email_registration_template_success_html_content = self.read_file(
                 filepath, "email.registration_template_success_html"
             )
+            filepath = os.path.join(
+                self.email_template_dir, email_add_threepid_template_success_html
+            )
+            self.email_add_threepid_template_success_html_content = self.read_file(
+                filepath, "email.add_threepid_template_success_html"
+            )
 
         if self.email_enable_notifs:
             required = [
@@ -328,6 +352,12 @@ class EmailConfig(Config):
         #   #registration_template_html: registration.html
         #   #registration_template_text: registration.txt
         #
+        #   # Templates for validation emails sent by the homeserver when adding an email to
+        #   # your user account
+        #   #
+        #   #add_threepid_template_html: add_threepid.html
+        #   #add_threepid_template_text: add_threepid.txt
+        #
         #   # Templates for password reset success and failure pages that a user
         #   # will see after attempting to reset their password
         #   #
@@ -339,6 +369,12 @@ class EmailConfig(Config):
         #   #
         #   #registration_template_success_html: registration_success.html
         #   #registration_template_failure_html: registration_failure.html
+        #
+        #   # Templates for success and failure pages that a user will see after attempting
+        #   # to add an email or phone to their account
+        #   #
+        #   #add_threepid_success_html: add_threepid_success.html
+        #   #add_threepid_failure_html: add_threepid_failure.html
         """
 
 
diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py
index 894b2e0c9d..7d04a0f199 100644
--- a/synapse/handlers/identity.py
+++ b/synapse/handlers/identity.py
@@ -93,11 +93,10 @@ class IdentityHandler(BaseHandler):
         given identity server
 
         Args:
-            id_server (str|None): The identity server to validate 3PIDs against. If None,
-                we will attempt to extract id_server creds
+            id_server (str): The identity server to validate 3PIDs against. Must be a
+                complete URL including the protocol (http(s)://)
 
             creds (dict[str, str]): Dictionary containing the following keys:
-                * id_server|idServer: An optional domain name of an identity server
                 * client_secret|clientSecret: A unique secret str provided by the client
                 * sid: The ID of the validation session
 
@@ -116,13 +115,6 @@ class IdentityHandler(BaseHandler):
             raise SynapseError(
                 400, "Missing param session_id in creds", errcode=Codes.MISSING_PARAM
             )
-        if not id_server:
-            # Attempt to get the id_server from the creds dict
-            id_server = creds.get("id_server") or creds.get("idServer")
-            if not id_server:
-                raise SynapseError(
-                    400, "Missing param id_server in creds", errcode=Codes.MISSING_PARAM
-                )
 
         query_params = {"sid": session_id, "client_secret": client_secret}
 
diff --git a/synapse/push/mailer.py b/synapse/push/mailer.py
index 2437235dc4..5a4fc78b4c 100644
--- a/synapse/push/mailer.py
+++ b/synapse/push/mailer.py
@@ -180,6 +180,35 @@ class Mailer(object):
         )
 
     @defer.inlineCallbacks
+    def send_add_threepid_mail(self, email_address, token, client_secret, sid):
+        """Send an email with a validation link to a user for adding a 3pid to their account
+
+        Args:
+            email_address (str): Email address we're sending the validation link to
+
+            token (str): Unique token generated by the server to verify the email was received
+
+            client_secret (str): Unique token generated by the client to group together
+                multiple email sending attempts
+
+            sid (str): The generated session ID
+        """
+        params = {"token": token, "client_secret": client_secret, "sid": sid}
+        link = (
+            self.hs.config.public_baseurl
+            + "_matrix/client/unstable/add_threepid/email/submit_token?%s"
+            % urllib.parse.urlencode(params)
+        )
+
+        template_vars = {"link": link}
+
+        yield self.send_email(
+            email_address,
+            "[%s] Validate Your Email" % self.hs.config.server_name,
+            template_vars,
+        )
+
+    @defer.inlineCallbacks
     def send_notification_mail(
         self, app_id, user_id, email_address, push_actions, reason
     ):
diff --git a/synapse/res/templates/add_threepid.html b/synapse/res/templates/add_threepid.html
new file mode 100644
index 0000000000..cc4ab07e09
--- /dev/null
+++ b/synapse/res/templates/add_threepid.html
@@ -0,0 +1,9 @@
+<html>
+<body>
+    <p>A request to add an email address to your Matrix account has been received. If this was you, please click the link below to confirm adding this email:</p>
+
+    <a href="{{ link }}">{{ link }}</a>
+
+    <p>If this was not you, you can safely ignore this email. Thank you.</p>
+</body>
+</html>
diff --git a/synapse/res/templates/add_threepid.txt b/synapse/res/templates/add_threepid.txt
new file mode 100644
index 0000000000..a60c1ff659
--- /dev/null
+++ b/synapse/res/templates/add_threepid.txt
@@ -0,0 +1,6 @@
+A request to add an email address to your Matrix account has been received. If this was you,
+please click the link below to confirm adding this email:
+
+{{ link }}
+
+If this was not you, you can safely ignore this email. Thank you.
diff --git a/synapse/res/templates/add_threepid_failure.html b/synapse/res/templates/add_threepid_failure.html
new file mode 100644
index 0000000000..441d11c846
--- /dev/null
+++ b/synapse/res/templates/add_threepid_failure.html
@@ -0,0 +1,8 @@
+<html>
+<head></head>
+<body>
+<p>The request failed for the following reason: {{ failure_reason }}.</p>
+
+<p>No changes have been made to your account.</p>
+</body>
+</html>
diff --git a/synapse/res/templates/add_threepid_success.html b/synapse/res/templates/add_threepid_success.html
new file mode 100644
index 0000000000..fbd6e4018f
--- /dev/null
+++ b/synapse/res/templates/add_threepid_success.html
@@ -0,0 +1,6 @@
+<html>
+<head></head>
+<body>
+<p>Your email has now been validated, please return to your client. You may now close this window.</p>
+</body>
+</html>
diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py
index f17858101e..0f64461197 100644
--- a/synapse/rest/client/v2_alpha/account.py
+++ b/synapse/rest/client/v2_alpha/account.py
@@ -22,7 +22,12 @@ from six.moves import http_client
 from twisted.internet import defer
 
 from synapse.api.constants import LoginType
-from synapse.api.errors import Codes, SynapseError, ThreepidValidationError
+from synapse.api.errors import (
+    Codes,
+    HttpResponseException,
+    SynapseError,
+    ThreepidValidationError,
+)
 from synapse.config.emailconfig import ThreepidBehaviour
 from synapse.http.server import finish_request
 from synapse.http.servlet import (
@@ -108,16 +113,9 @@ class EmailPasswordRequestTokenRestServlet(RestServlet):
             raise SynapseError(400, "Email not found", Codes.THREEPID_NOT_FOUND)
 
         if self.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE:
-            # Have the configured identity server handle the request
-            if not self.hs.config.account_threepid_delegate_email:
-                logger.warn(
-                    "No upstream email account_threepid_delegate configured on the server to "
-                    "handle this request"
-                )
-                raise SynapseError(
-                    400, "Password reset by email is not supported on this homeserver"
-                )
+            assert self.hs.config.account_threepid_delegate_email
 
+            # Have the configured identity server handle the request
             ret = yield self.identity_handler.requestEmailToken(
                 self.hs.config.account_threepid_delegate_email,
                 email,
@@ -221,6 +219,11 @@ class PasswordResetSubmitTokenServlet(RestServlet):
         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, = load_jinja2_templates(
+                self.config.email_template_dir,
+                [self.config.email_password_reset_template_failure_html],
+            )
 
     @defer.inlineCallbacks
     def on_GET(self, request, medium):
@@ -271,13 +274,8 @@ class PasswordResetSubmitTokenServlet(RestServlet):
             request.setResponseCode(e.code)
 
             # Show a failure page with a reason
-            html_template, = load_jinja2_templates(
-                self.config.email_template_dir,
-                [self.config.email_password_reset_template_failure_html],
-            )
-
             template_vars = {"failure_reason": e.msg}
-            html = html_template.render(**template_vars)
+            html = self.failure_email_template.render(**template_vars)
 
         request.write(html.encode("utf-8"))
         finish_request(request)
@@ -432,13 +430,35 @@ class EmailThreepidRequestTokenRestServlet(RestServlet):
         self.identity_handler = hs.get_handlers().identity_handler
         self.store = self.hs.get_datastore()
 
+        if self.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL:
+            template_html, template_text = load_jinja2_templates(
+                self.config.email_template_dir,
+                [
+                    self.config.email_add_threepid_template_html,
+                    self.config.email_add_threepid_template_text,
+                ],
+                public_baseurl=self.config.public_baseurl,
+            )
+            self.mailer = Mailer(
+                hs=self.hs,
+                app_name=self.config.email_app_name,
+                template_html=template_html,
+                template_text=template_text,
+            )
+
     @defer.inlineCallbacks
     def on_POST(self, request):
+        if self.config.threepid_behaviour_email == ThreepidBehaviour.OFF:
+            if self.config.local_threepid_handling_disabled_due_to_email_config:
+                logger.warn(
+                    "Adding emails have been disabled due to lack of an email config"
+                )
+            raise SynapseError(
+                400, "Adding an email to your account is disabled on this server"
+            )
+
         body = parse_json_object_from_request(request)
-        assert_params_in_dict(
-            body, ["id_server", "client_secret", "email", "send_attempt"]
-        )
-        id_server = "https://" + body["id_server"]  # Assume https
+        assert_params_in_dict(body, ["client_secret", "email", "send_attempt"])
         client_secret = body["client_secret"]
         email = body["email"]
         send_attempt = body["send_attempt"]
@@ -460,9 +480,30 @@ class EmailThreepidRequestTokenRestServlet(RestServlet):
         if existing_user_id is not None:
             raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE)
 
-        ret = yield self.identity_handler.requestEmailToken(
-            id_server, email, client_secret, send_attempt, next_link
-        )
+        if self.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE:
+            assert self.hs.config.account_threepid_delegate_email
+
+            # Have the configured identity server handle the request
+            ret = yield self.identity_handler.requestEmailToken(
+                self.hs.config.account_threepid_delegate_email,
+                email,
+                client_secret,
+                send_attempt,
+                next_link,
+            )
+        else:
+            # Send threepid validation emails from Synapse
+            sid = yield self.identity_handler.send_threepid_validation(
+                email,
+                client_secret,
+                send_attempt,
+                self.mailer.send_add_threepid_mail,
+                next_link,
+            )
+
+            # Wrap the session id in a JSON object
+            ret = {"sid": sid}
+
         return 200, ret
 
 
@@ -508,9 +549,86 @@ class MsisdnThreepidRequestTokenRestServlet(RestServlet):
         ret = yield self.identity_handler.requestMsisdnToken(
             id_server, country, phone_number, client_secret, send_attempt, next_link
         )
+
         return 200, ret
 
 
+class AddThreepidSubmitTokenServlet(RestServlet):
+    """Handles 3PID validation token submission for adding an email to a user's account"""
+
+    PATTERNS = client_patterns(
+        "/add_threepid/email/submit_token$", releases=(), unstable=True
+    )
+
+    def __init__(self, hs):
+        """
+        Args:
+            hs (synapse.server.HomeServer): server
+        """
+        super().__init__()
+        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, = load_jinja2_templates(
+                self.config.email_template_dir,
+                [self.config.email_add_threepid_template_failure_html],
+            )
+
+    @defer.inlineCallbacks
+    def on_GET(self, request):
+        if self.config.threepid_behaviour_email == ThreepidBehaviour.OFF:
+            if self.config.local_threepid_handling_disabled_due_to_email_config:
+                logger.warn(
+                    "Adding emails have been disabled due to lack of an email config"
+                )
+            raise SynapseError(
+                400, "Adding an email to your account is disabled on this server"
+            )
+        elif self.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE:
+            raise SynapseError(
+                400,
+                "This homeserver is not validating threepids. Use an identity server "
+                "instead.",
+            )
+
+        sid = parse_string(request, "sid", required=True)
+        client_secret = parse_string(request, "client_secret", required=True)
+        token = parse_string(request, "token", required=True)
+
+        # Attempt to validate a 3PID session
+        try:
+            # Mark the session as valid
+            next_link = yield self.store.validate_threepid_session(
+                sid, client_secret, token, self.clock.time_msec()
+            )
+
+            # Perform a 302 redirect if next_link is set
+            if next_link:
+                if next_link.startswith("file:///"):
+                    logger.warn(
+                        "Not redirecting to next_link as it is a local file: address"
+                    )
+                else:
+                    request.setResponseCode(302)
+                    request.setHeader("Location", next_link)
+                    finish_request(request)
+                    return None
+
+            # Otherwise show the success template
+            html = self.config.email_add_threepid_template_success_html_content
+            request.setResponseCode(200)
+        except ThreepidValidationError as e:
+            request.setResponseCode(e.code)
+
+            # Show a failure page with a reason
+            template_vars = {"failure_reason": e.msg}
+            html = self.failure_email_template.render(**template_vars)
+
+        request.write(html.encode("utf-8"))
+        finish_request(request)
+
+
 class ThreepidRestServlet(RestServlet):
     PATTERNS = client_patterns("/account/3pid$")
 
@@ -536,51 +654,126 @@ class ThreepidRestServlet(RestServlet):
         if self.hs.config.disable_3pid_changes:
             raise SynapseError(400, "3PID changes disabled on this server")
 
-        body = parse_json_object_from_request(request)
-
         requester = yield self.auth.get_user_by_req(request)
         user_id = requester.user.to_string()
+        body = parse_json_object_from_request(request)
 
         # skip validation if this is a shadow 3PID from an AS
-        if not requester.app_service:
-            threepid_creds = body.get("threePidCreds") or body.get("three_pid_creds")
-            if threepid_creds is None:
-                raise SynapseError(400, "Missing param", Codes.MISSING_PARAM)
-
-            requester = yield self.auth.get_user_by_req(request)
-            user_id = requester.user.to_string()
-
-            # Specify None as the identity server to retrieve it from the request body instead
-            threepid = yield self.identity_handler.threepid_from_creds(None, threepid_creds)
-
-            if not threepid:
-                raise SynapseError(
-                    400, "Failed to auth 3pid", Codes.THREEPID_AUTH_FAILED
-                )
-
-            for reqd in ["medium", "address", "validated_at"]:
-                if reqd not in threepid:
-                    logger.warn("Couldn't add 3pid: invalid response from ID server")
-                    raise SynapseError(500, "Invalid response from ID Server")
-        else:
+        if requester.app_service:
             # XXX: ASes pass in a validated threepid directly to bypass the IS.
             # This makes the API entirely change shape when we have an AS token;
             # it really should be an entirely separate API - perhaps
             # /account/3pid/replicate or something.
             threepid = body.get("threepid")
 
+            yield self.auth_handler.add_threepid(
+                user_id, threepid["medium"], threepid["address"], threepid["validated_at"]
+            )
+
+            if self.hs.config.shadow_server:
+                shadow_user = UserID(
+                    requester.user.localpart, self.hs.config.shadow_server.get("hs")
+                )
+                self.shadow_3pid({"threepid": threepid}, shadow_user.to_string())
+
+            return 200, {}
+
+        threepid_creds = body.get("threePidCreds") or body.get("three_pid_creds")
+        if threepid_creds is None:
+            raise SynapseError(
+                400, "Missing param three_pid_creds", Codes.MISSING_PARAM
+            )
+        assert_params_in_dict(threepid_creds, ["client_secret", "sid"])
+
+        client_secret = threepid_creds["client_secret"]
+        sid = threepid_creds["sid"]
+
+        # We don't actually know which medium this 3PID is. Thus we first assume it's email,
+        # and if validation fails we try msisdn
+        validation_session = None
+
+        # Try to validate as email
+        if self.hs.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE:
+            # Ask our delegated email identity server
+            try:
+                validation_session = yield self.identity_handler.threepid_from_creds(
+                    self.hs.config.account_threepid_delegate_email, threepid_creds
+                )
+            except HttpResponseException:
+                logger.debug(
+                    "%s reported non-validated threepid: %s",
+                    self.hs.config.account_threepid_delegate_email,
+                    threepid_creds,
+                )
+        elif self.hs.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL:
+            # Get a validated session matching these details
+            validation_session = yield self.datastore.get_threepid_validation_session(
+                "email", client_secret, sid=sid, validated=True
+            )
+
+        # Old versions of Sydent return a 200 http code even on a failed validation check.
+        # Thus, in addition to the HttpResponseException check above (which checks for
+        # non-200 errors), we need to make sure validation_session isn't actually an error,
+        # identified by containing an "error" key
+        # See https://github.com/matrix-org/sydent/issues/215 for details
+        if validation_session and "error" not in validation_session:
+            yield self._add_threepid_to_account(user_id, validation_session)
+            return 200, {}
+
+        # Try to validate as msisdn
+        if self.hs.config.account_threepid_delegate_msisdn:
+            # Ask our delegated msisdn identity server
+            try:
+                validation_session = yield self.identity_handler.threepid_from_creds(
+                    self.hs.config.account_threepid_delegate_msisdn, threepid_creds
+                )
+            except HttpResponseException:
+                logger.debug(
+                    "%s reported non-validated threepid: %s",
+                    self.hs.config.account_threepid_delegate_email,
+                    threepid_creds,
+                )
+
+            # Check that validation_session isn't actually an error due to old Sydent instances
+            # See explanatory comment above
+            if validation_session and "error" not in validation_session:
+                yield self._add_threepid_to_account(user_id, validation_session)
+                return 200, {}
+
+        raise SynapseError(
+            400, "No validated 3pid session found", Codes.THREEPID_AUTH_FAILED
+        )
+
+    @defer.inlineCallbacks
+    def _add_threepid_to_account(self, requester, validation_session):
+        """Add a threepid wrapped in a validation_session dict to an account
+
+        Args:
+            requester: The Requester object of the user to add this 3PID to
+
+            validation_session (dict): A dict containing the following:
+                * medium       - medium of the threepid
+                * address      - address of the threepid
+                * validated_at - timestamp of when the validation occurred
+        """
         yield self.auth_handler.add_threepid(
-            user_id, threepid["medium"], threepid["address"], threepid["validated_at"]
+            requester.user.to_string(),
+            validation_session["medium"],
+            validation_session["address"],
+            validation_session["validated_at"],
         )
 
         if self.hs.config.shadow_server:
             shadow_user = UserID(
                 requester.user.localpart, self.hs.config.shadow_server.get("hs")
             )
+            threepid = {
+                "medium": validation_session["medium"],
+                "address": validation_session["address"],
+                "validated_at": validation_session["validated_at"],
+            }
             self.shadow_3pid({"threepid": threepid}, shadow_user.to_string())
 
-        return 200, {}
-
     @defer.inlineCallbacks
     def shadow_3pid(self, body, user_id):
         # TODO: retries
@@ -765,6 +958,7 @@ def register_servlets(hs, http_server):
     DeactivateAccountRestServlet(hs).register(http_server)
     EmailThreepidRequestTokenRestServlet(hs).register(http_server)
     MsisdnThreepidRequestTokenRestServlet(hs).register(http_server)
+    AddThreepidSubmitTokenServlet(hs).register(http_server)
     ThreepidRestServlet(hs).register(http_server)
     ThreepidUnbindRestServlet(hs).register(http_server)
     ThreepidDeleteRestServlet(hs).register(http_server)
diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py
index c48cf017a3..e01b11181c 100644
--- a/synapse/rest/client/v2_alpha/register.py
+++ b/synapse/rest/client/v2_alpha/register.py
@@ -134,15 +134,9 @@ class EmailRegisterRequestTokenRestServlet(RestServlet):
             raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE)
 
         if self.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE:
-            if not self.hs.config.account_threepid_delegate_email:
-                logger.warn(
-                    "No upstream email account_threepid_delegate configured on the server to "
-                    "handle this request"
-                )
-                raise SynapseError(
-                    400, "Registration by email is not supported on this homeserver"
-                )
+            assert self.hs.config.account_threepid_delegate_email
 
+            # Have the configured identity server handle the request
             ret = yield self.identity_handler.requestEmailToken(
                 self.hs.config.account_threepid_delegate_email,
                 email,
@@ -251,6 +245,12 @@ class RegistrationSubmitTokenServlet(RestServlet):
         self.clock = hs.get_clock()
         self.store = hs.get_datastore()
 
+        if self.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL:
+            self.failure_email_template, = load_jinja2_templates(
+                self.config.email_template_dir,
+                [self.config.email_registration_template_failure_html],
+            )
+
     @defer.inlineCallbacks
     def on_GET(self, request, medium):
         if medium != "email":
@@ -294,17 +294,11 @@ class RegistrationSubmitTokenServlet(RestServlet):
 
             request.setResponseCode(200)
         except ThreepidValidationError as e:
-            # Show a failure page with a reason
             request.setResponseCode(e.code)
 
             # Show a failure page with a reason
-            html_template, = load_jinja2_templates(
-                self.config.email_template_dir,
-                [self.config.email_registration_template_failure_html],
-            )
-
             template_vars = {"failure_reason": e.msg}
-            html = html_template.render(**template_vars)
+            html = self.failure_email_template.render(**template_vars)
 
         request.write(html.encode("utf-8"))
         finish_request(request)
diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py
index f0932d8317..f6b926eb0a 100644
--- a/synapse/storage/registration.py
+++ b/synapse/storage/registration.py
@@ -24,7 +24,7 @@ from six.moves import range
 from twisted.internet import defer
 
 from synapse.api.constants import UserTypes
-from synapse.api.errors import Codes, StoreError, ThreepidValidationError
+from synapse.api.errors import Codes, StoreError, SynapseError, ThreepidValidationError
 from synapse.metrics.background_process_metrics import run_as_background_process
 from synapse.storage import background_updates
 from synapse.storage._base import SQLBaseStore
@@ -683,18 +683,31 @@ class RegistrationWorkerStore(SQLBaseStore):
             medium (str|None): The medium of the 3PID
             address (str|None): The address of the 3PID
             sid (str|None): The ID of the validation session
-            client_secret (str|None): A unique string provided by the client to
-                help identify this validation attempt
+            client_secret (str): A unique string provided by the client to help identify this
+                validation attempt
             validated (bool|None): Whether sessions should be filtered by
                 whether they have been validated already or not. None to
                 perform no filtering
 
         Returns:
-            deferred {str, int}|None: A dict containing the
-                latest session_id and send_attempt count for this 3PID.
-                Otherwise None if there hasn't been a previous attempt
+            Deferred[dict|None]: A dict containing the following:
+                * address - address of the 3pid
+                * medium - medium of the 3pid
+                * client_secret - a secret provided by the client for this validation session
+                * session_id - ID of the validation session
+                * send_attempt - a number serving to dedupe send attempts for this session
+                * validated_at - timestamp of when this session was validated if so
+
+                Otherwise None if a validation session is not found
         """
-        keyvalues = {"medium": medium, "client_secret": client_secret}
+        if not client_secret:
+            raise SynapseError(
+                400, "Missing parameter: client_secret", errcode=Codes.MISSING_PARAM
+            )
+
+        keyvalues = {"client_secret": client_secret}
+        if medium:
+            keyvalues["medium"] = medium
         if address:
             keyvalues["address"] = address
         if sid:
@@ -1231,6 +1244,10 @@ class RegistrationStore(
             current_ts (int): The current unix time in milliseconds. Used for
                 checking token expiry status
 
+        Raises:
+            ThreepidValidationError: if a matching validation token was not found or has
+                expired
+
         Returns:
             deferred str|None: A str representing a link to redirect the user
             to if there is one.