From e0da3f6a00a5795ad827fd2608c227cd31403bca Mon Sep 17 00:00:00 2001
From: Andrew Morgan <andrew@amorgan.xyz>
Date: Mon, 22 Jun 2020 18:54:06 +0100
Subject: [PATCH 01/10] Add config option to selectively delegate 3pid
 validation

And update sample_config.yaml.
---
 docs/sample_config.yaml        | 16 +++++++++++++---
 synapse/config/registration.py | 31 ++++++++++++++++++++++++++++---
 2 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml
index 05e7bf215ac8..f498cdd1a7b9 100644
--- a/docs/sample_config.yaml
+++ b/docs/sample_config.yaml
@@ -1160,9 +1160,9 @@ account_validity:
 #  - matrix.org
 #  - vector.im
 
-# Handle threepid (email/phone etc) registration and password resets through a set of
-# *trusted* identity servers. Note that this allows the configured identity server to
-# reset passwords for accounts!
+# Handle threepid (email/phone etc) registration and/or password resets through a set of
+# *trusted* identity servers. Note that this potentially allows the configured external
+# service to reset passwords for accounts!
 #
 # Be aware that if `email` is not set, and SMTP options have not been
 # configured in the email config block, registration and user password resets via
@@ -1186,6 +1186,16 @@ account_threepid_delegates:
     #email: https://example.com     # Delegate email sending to example.com
     #msisdn: http://localhost:8090  # Delegate SMS sending to this local process
 
+    # Choose which services are delegated. These apply to both email and msisdn
+    # identities.
+    #
+    # * adding_threepid: when adding an email or phone number to your homeserver
+    # * password_resets: when verifying your email or phone number during password reset
+    #
+    # Default is delegating for all services, or [adding_threepid, password_resets].
+    #
+    #delegate_for: [adding_threepid]
+
 # Whether users are allowed to change their displayname after it has
 # been initially set. Useful when provisioning users based on the
 # contents of a third-party directory.
diff --git a/synapse/config/registration.py b/synapse/config/registration.py
index fecced2d57ed..2ac513e344c8 100644
--- a/synapse/config/registration.py
+++ b/synapse/config/registration.py
@@ -19,6 +19,7 @@
 import pkg_resources
 
 from synapse.config._base import Config, ConfigError
+from synapse.config.emailconfig import ThreepidService
 from synapse.types import RoomAlias
 from synapse.util.stringutils import random_string_with_symbols
 
@@ -117,6 +118,20 @@ def read_config(self, config, **kwargs):
                 "configure `public_baseurl`."
             )
 
+        delegate_for = account_threepid_delegates.get(
+            "delegate_for",
+            [ThreepidService.ADDING_THREEPID, ThreepidService.PASSWORD_RESET],
+        )
+        try:
+            self.account_threepid_delegate_delegate_for = [
+                ThreepidService(service_type) for service_type in delegate_for
+            ]
+        except ValueError as e:
+            # This will be raised if a provided service_type does not exist
+            raise ConfigError(
+                "Option account_threepid_delegates.delegate_for: %s" % (e,)
+            )
+
         self.default_identity_server = config.get("default_identity_server")
         self.allow_guest_access = config.get("allow_guest_access", False)
 
@@ -307,9 +322,9 @@ def generate_config_section(self, generate_secrets=False, **kwargs):
         #  - matrix.org
         #  - vector.im
 
-        # Handle threepid (email/phone etc) registration and password resets through a set of
-        # *trusted* identity servers. Note that this allows the configured identity server to
-        # reset passwords for accounts!
+        # Handle threepid (email/phone etc) registration and/or password resets through a set of
+        # *trusted* identity servers. Note that this potentially allows the configured external
+        # service to reset passwords for accounts!
         #
         # Be aware that if `email` is not set, and SMTP options have not been
         # configured in the email config block, registration and user password resets via
@@ -333,6 +348,16 @@ def generate_config_section(self, generate_secrets=False, **kwargs):
             #email: https://example.com     # Delegate email sending to example.com
             #msisdn: http://localhost:8090  # Delegate SMS sending to this local process
 
+            # Choose which services are delegated. These apply to both email and msisdn
+            # identities.
+            #
+            # * adding_threepid: when adding an email or phone number to your homeserver
+            # * password_resets: when verifying your email or phone number during password reset
+            #
+            # Default is delegating for all services, or [adding_threepid, password_resets].
+            #
+            #delegate_for: [adding_threepid]
+
         # Whether users are allowed to change their displayname after it has
         # been initially set. Useful when provisioning users based on the
         # contents of a third-party directory.

From 89b8f648b8a09d766b78795483808e65d1c7bb91 Mon Sep 17 00:00:00 2001
From: Andrew Morgan <andrew@amorgan.xyz>
Date: Mon, 22 Jun 2020 19:03:38 +0100
Subject: [PATCH 02/10] Break up threepid_behaviour_email into
 add_threepid/password_reset

So we initially had a variable called `threepid_behaviour_email`
attached to the config object which was an instance of
ThreepidBehaviour, the latter having three possible values - REMOTE,
LOCAL or OFF. The rest of the codebase would basically use this to
determine whether to try and do email stuff itself, or to delegate the
operation to a third-party service (an identity server usually).

Well, now that the sysadmin can choose whether to do this for adding a
threepid vs. password reset, we now need two ThreepidBehaviour values.
The code can then check either the one for adding a threepid, or the one
for password reset, depending on what it's doing.

This commit is breaking that up and also cleaning up the email template
loading code such that templates are only loaded if necessary i.e. we'll
be sending emails with that template.

Oh, we also introduce ThreepidService, which is another enum like
ThreepidBehaviour, but this one specifies which service you're
validating (whether it's password reset or adding a threepid). I chose
and enum again instead of a straight string as then we can easily avoid
typos.
---
 synapse/config/emailconfig.py | 269 ++++++++++++++++++++++------------
 1 file changed, 176 insertions(+), 93 deletions(-)

diff --git a/synapse/config/emailconfig.py b/synapse/config/emailconfig.py
index ca61214454f8..8608c853c18f 100644
--- a/synapse/config/emailconfig.py
+++ b/synapse/config/emailconfig.py
@@ -21,7 +21,7 @@
 import email.utils
 import os
 from enum import Enum
-from typing import Optional
+from typing import List, Optional
 
 import pkg_resources
 
@@ -86,15 +86,6 @@ def read_config(self, config, **kwargs):
         account_validity_config = config.get("account_validity") or {}
         account_validity_renewal_enabled = account_validity_config.get("renew_at")
 
-        self.threepid_behaviour_email = (
-            # Have Synapse handle the email sending if account_threepid_delegates.email
-            # is not defined
-            # msisdn is currently always remote while Synapse does not support any method of
-            # sending SMS messages
-            ThreepidBehaviour.REMOTE
-            if self.account_threepid_delegate_email
-            else ThreepidBehaviour.LOCAL
-        )
         # Prior to Synapse v1.4.0, there was another option that defined whether Synapse would
         # use an identity server to password reset tokens on its behalf. We now warn the user
         # if they have this set and tell them to use the updated option, while using a default
@@ -123,16 +114,43 @@ def read_config(self, config, **kwargs):
                     '"trusted_third_party_id_servers" but it is empty.'
                 )
 
+        self.threepid_behaviour_email_add_threepid = (
+            # Only delegate email sending if a delegate is set and a corresponding ThreepidService
+            # type appears in delegate_for
+            #
+            # msisdn is currently always remote as Synapse does not currently support any
+            # method of sending SMS messages
+            ThreepidBehaviour.REMOTE
+            if self.account_threepid_delegate_email
+            and ThreepidService.ADDING_THREEPID
+            in self.account_threepid_delegate_delegate_for
+            else ThreepidBehaviour.LOCAL
+        )
+        self.threepid_behaviour_email_password_reset = (
+            # Same as above
+            ThreepidBehaviour.REMOTE
+            if self.account_threepid_delegate_email
+            and ThreepidService.PASSWORD_RESET
+            in self.account_threepid_delegate_delegate_for
+            else ThreepidBehaviour.LOCAL
+        )
+
+        # If a service is intended to be provided by the local Synapse instance, check that
+        # the config exists for us to do so.
+        # If not, disable the service and warn the user why it has happened
         self.local_threepid_handling_disabled_due_to_email_config = False
         if (
-            self.threepid_behaviour_email == ThreepidBehaviour.LOCAL
-            and email_config == {}
-        ):
+            self.threepid_behaviour_email_add_threepid == ThreepidBehaviour.LOCAL
+            or self.threepid_behaviour_email_password_reset == ThreepidBehaviour.LOCAL
+        ) and email_config == {}:
             # We cannot warn the user this has happened here
             # Instead do so when a user attempts to reset their password
             self.local_threepid_handling_disabled_due_to_email_config = True
 
-            self.threepid_behaviour_email = ThreepidBehaviour.OFF
+            if self.threepid_behaviour_email_add_threepid == ThreepidBehaviour.LOCAL:
+                self.threepid_behaviour_email_add_threepid = ThreepidBehaviour.OFF
+            if self.threepid_behaviour_email_password_reset == ThreepidBehaviour.LOCAL:
+                self.threepid_behaviour_email_password_reset = ThreepidBehaviour.OFF
 
         # Get lifetime of a validation token in milliseconds
         self.email_validation_token_lifetime = self.parse_duration(
@@ -142,7 +160,8 @@ def read_config(self, config, **kwargs):
         if (
             self.email_enable_notifs
             or account_validity_renewal_enabled
-            or self.threepid_behaviour_email == ThreepidBehaviour.LOCAL
+            or self.threepid_behaviour_email_add_threepid == ThreepidBehaviour.LOCAL
+            or self.threepid_behaviour_email_password_reset == ThreepidBehaviour.LOCAL
         ):
             # make sure we can import the required deps
             import jinja2
@@ -152,7 +171,73 @@ def read_config(self, config, **kwargs):
             jinja2
             bleach
 
-        if self.threepid_behaviour_email == ThreepidBehaviour.LOCAL:
+        if self.threepid_behaviour_email_add_threepid == ThreepidBehaviour.LOCAL:
+            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_add_threepid_template_failure_html = email_config.get(
+                "add_threepid_template_failure_html", "add_threepid_failure.html"
+            )
+            email_add_threepid_template_success_html = email_config.get(
+                "add_threepid_template_success_html", "add_threepid_success.html"
+            )
+
+            self._check_templates_exist(
+                [
+                    self.email_add_threepid_template_html,
+                    self.email_add_threepid_template_text,
+                    self.email_add_threepid_template_failure_html,
+                    email_add_threepid_template_success_html,
+                ]
+            )
+
+            self.email_add_threepid_template_success_html_content = self._read_template_with_name(
+                email_add_threepid_template_success_html,
+                "email.add_threepid_template_success_html",
+            )
+
+        if self.threepid_behaviour_email_password_reset == ThreepidBehaviour.LOCAL:
+            # These templates have placeholders in them, and thus must be
+            # parsed using a templating engine during a request
+            self.email_password_reset_template_html = email_config.get(
+                "password_reset_template_html", "password_reset.html"
+            )
+            self.email_password_reset_template_text = email_config.get(
+                "password_reset_template_text", "password_reset.txt"
+            )
+            self.email_password_reset_template_failure_html = email_config.get(
+                "password_reset_template_failure_html", "password_reset_failure.html"
+            )
+
+            # These templates do not support any placeholder variables, so we
+            # will read them from disk once during setup
+            email_password_reset_template_success_html = email_config.get(
+                "password_reset_template_success_html", "password_reset_success.html"
+            )
+
+            self._check_templates_exist(
+                [
+                    self.email_password_reset_template_html,
+                    self.email_password_reset_template_text,
+                    self.email_password_reset_template_failure_html,
+                    email_password_reset_template_success_html,
+                ]
+            )
+
+            # This is a static template. Load its contents here instead of at runtime
+            self.email_password_reset_template_success_html = self._read_template_with_name(
+                email_password_reset_template_success_html,
+                "email.password_reset_template_success_html",
+            )
+
+        # Required config for threepid
+        if (
+            self.threepid_behaviour_email_add_threepid == ThreepidBehaviour.LOCAL
+            or self.threepid_behaviour_email_password_reset == ThreepidBehaviour.LOCAL
+        ):
             missing = []
             if not self.email_notif_from:
                 missing.append("email.notif_from")
@@ -169,84 +254,32 @@ def read_config(self, config, **kwargs):
 
             # These email templates have placeholders in them, and thus must be
             # parsed using a templating engine during a request
-            self.email_password_reset_template_html = email_config.get(
-                "password_reset_template_html", "password_reset.html"
-            )
-            self.email_password_reset_template_text = email_config.get(
-                "password_reset_template_text", "password_reset.txt"
-            )
             self.email_registration_template_html = email_config.get(
                 "registration_template_html", "registration.html"
             )
             self.email_registration_template_text = email_config.get(
                 "registration_template_text", "registration.txt"
             )
-            self.email_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
-            email_password_reset_template_success_html = email_config.get(
-                "password_reset_template_success_html", "password_reset_success.html"
-            )
             email_registration_template_success_html = email_config.get(
                 "registration_template_success_html", "registration_success.html"
             )
-            email_add_threepid_template_success_html = email_config.get(
-                "add_threepid_template_success_html", "add_threepid_success.html"
+
+            self._check_templates_exist(
+                [
+                    self.email_registration_template_html,
+                    self.email_registration_template_text,
+                    self.email_registration_template_failure_html,
+                    email_registration_template_success_html,
+                ]
             )
 
-            # Check templates exist
-            for f in [
-                self.email_password_reset_template_html,
-                self.email_password_reset_template_text,
-                self.email_registration_template_html,
-                self.email_registration_template_text,
-                self.email_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,
+            # This is a static template. Load its contents here instead of at runtime
+            self.email_registration_template_success_html_content = self._read_template_with_name(
                 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):
-                    raise ConfigError("Unable to find template file %s" % (p,))
-
-            # Retrieve content of web templates
-            filepath = os.path.join(
-                self.email_template_dir, email_password_reset_template_success_html
-            )
-            self.email_password_reset_template_success_html = self.read_file(
-                filepath, "email.password_reset_template_success_html"
-            )
-            filepath = os.path.join(
-                self.email_template_dir, email_registration_template_success_html
-            )
-            self.email_registration_template_success_html_content = self.read_file(
-                filepath, "email.registration_template_success_html"
-            )
-            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"
+                "email.registration_template_success_html",
             )
 
         if self.email_enable_notifs:
@@ -263,6 +296,15 @@ def read_config(self, config, **kwargs):
                     % (", ".join(missing),)
                 )
 
+            self.email_notif_for_new_users = email_config.get(
+                "notif_for_new_users", True
+            )
+            self.email_riot_base_url = email_config.get(
+                "client_base_url", email_config.get("riot_base_url", None)
+            )
+
+            # These email templates have placeholders in them, and thus must be
+            # parsed using a templating engine during a request
             self.email_notif_template_html = email_config.get(
                 "notif_template_html", "notif_mail.html"
             )
@@ -270,19 +312,13 @@ def read_config(self, config, **kwargs):
                 "notif_template_text", "notif_mail.txt"
             )
 
-            for f in self.email_notif_template_text, self.email_notif_template_html:
-                p = os.path.join(self.email_template_dir, f)
-                if not os.path.isfile(p):
-                    raise ConfigError("Unable to find email template file %s" % (p,))
-
-            self.email_notif_for_new_users = email_config.get(
-                "notif_for_new_users", True
-            )
-            self.email_riot_base_url = email_config.get(
-                "client_base_url", email_config.get("riot_base_url", None)
+            self._check_templates_exist(
+                [self.email_notif_template_text, self.email_notif_template_html]
             )
 
         if account_validity_renewal_enabled:
+            # These email templates have placeholders in them, and thus must be
+            # parsed using a templating engine during a request
             self.email_expiry_template_html = email_config.get(
                 "expiry_template_html", "notice_expiry.html"
             )
@@ -290,10 +326,9 @@ def read_config(self, config, **kwargs):
                 "expiry_template_text", "notice_expiry.txt"
             )
 
-            for f in self.email_expiry_template_text, self.email_expiry_template_html:
-                p = os.path.join(self.email_template_dir, f)
-                if not os.path.isfile(p):
-                    raise ConfigError("Unable to find email template file %s" % (p,))
+            self._check_templates_exist(
+                [self.email_expiry_template_text, self.email_expiry_template_html]
+            )
 
     def generate_config_section(self, config_dir_path, server_name, **kwargs):
         return """\
@@ -404,6 +439,38 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs):
           #template_dir: "res/templates"
         """
 
+    def _check_templates_exist(self, template_filepaths: List[str]):
+        """Checks that the given filepaths exist
+
+        Raises:
+            ConfigError: if at least one filepath does not exist or can't otherwise be read
+        """
+        for filepath in template_filepaths:
+            absolute_path = os.path.join(self.email_template_dir, filepath)
+            if not os.path.isfile(absolute_path):
+                raise ConfigError("Unable to find template file %s" % (absolute_path,))
+
+    def _read_template_with_name(
+        self, template_filename: str, config_option: str
+    ) -> str:
+        """Returns the contents of a template given the filename
+
+        Args:
+            template_filename: The name of the template file. The contents of
+                self.email_template_dir will be prepended to create the final filepath
+
+            config_option: The config option name that defined the template name. Used for
+                warning the sysadmin when a template file cannot be found
+
+        Raises:
+            ConfigError: if the file's path is incorrect or otherwise cannot be read
+
+        Returns:
+            The contents of the template
+        """
+        filepath = os.path.join(self.email_template_dir, template_filename)
+        return self.read_file(filepath, config_option)
+
 
 class ThreepidBehaviour(Enum):
     """
@@ -418,3 +485,19 @@ class ThreepidBehaviour(Enum):
     REMOTE = "remote"
     LOCAL = "local"
     OFF = "off"
+
+
+class ThreepidService(Enum):
+    """
+    Services regarding threepids that the homeserver can either provide locally or by proxying
+    to an external server. The server may choose to do this if they cannot access an SMTP
+    server from the Synapse box for some reason. Or if they are accepting phone numbers,
+    which Synapse cannot verify itself yet
+
+    ADDING_THREEPID = the user is adding a threepid to their account information. This is
+        also used when threepids are added during registration
+    PASSWORD_RESET = the user is validating a threepid in order to reset their password
+    """
+
+    ADDING_THREEPID = "adding_threepid"
+    PASSWORD_RESET = "password_reset"

From 6448a3ab860b5a9a39e77e9e1268f7f3428f7b21 Mon Sep 17 00:00:00 2001
From: Andrew Morgan <andrew@amorgan.xyz>
Date: Mon, 22 Jun 2020 19:04:46 +0100
Subject: [PATCH 03/10] Validate emails locally or remotely based on new
 ThreepidBehaviours

Now we start using the different `threepid_behaviour_email.*` variants for different account/registration operations.
---
 synapse/rest/client/v2_alpha/account.py  | 39 ++++++++++++++++--------
 synapse/rest/client/v2_alpha/register.py | 23 ++++++++++----
 2 files changed, 44 insertions(+), 18 deletions(-)

diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py
index 923bcb9f853d..1f24202e6875 100644
--- a/synapse/rest/client/v2_alpha/account.py
+++ b/synapse/rest/client/v2_alpha/account.py
@@ -47,7 +47,10 @@ def __init__(self, hs):
         self.config = hs.config
         self.identity_handler = hs.get_handlers().identity_handler
 
-        if self.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL:
+        if (
+            self.config.threepid_behaviour_email_password_reset
+            == ThreepidBehaviour.LOCAL
+        ):
             template_html, template_text = load_jinja2_templates(
                 self.config.email_template_dir,
                 [
@@ -66,7 +69,7 @@ def __init__(self, hs):
             )
 
     async def on_POST(self, request):
-        if self.config.threepid_behaviour_email == ThreepidBehaviour.OFF:
+        if self.config.threepid_behaviour_email_password_reset == ThreepidBehaviour.OFF:
             if self.config.local_threepid_handling_disabled_due_to_email_config:
                 logger.warning(
                     "User password resets have been disabled due to lack of email config"
@@ -106,7 +109,10 @@ async def on_POST(self, request):
 
             raise SynapseError(400, "Email not found", Codes.THREEPID_NOT_FOUND)
 
-        if self.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE:
+        if (
+            self.config.threepid_behaviour_email_password_reset
+            == ThreepidBehaviour.REMOTE
+        ):
             assert self.hs.config.account_threepid_delegate_email
 
             # Have the configured identity server handle the request
@@ -151,7 +157,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:
+        if (
+            self.config.threepid_behaviour_email_password_reset
+            == ThreepidBehaviour.LOCAL
+        ):
             (self.failure_email_template,) = load_jinja2_templates(
                 self.config.email_template_dir,
                 [self.config.email_password_reset_template_failure_html],
@@ -163,7 +172,7 @@ 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.threepid_behaviour_email_password_reset == ThreepidBehaviour.OFF:
             if self.config.local_threepid_handling_disabled_due_to_email_config:
                 logger.warning(
                     "Password reset emails have been disabled due to lack of an email config"
@@ -327,7 +336,7 @@ async def on_POST(self, request):
 
         requester = await self.auth.get_user_by_req(request)
 
-        # allow ASes to dectivate their own users
+        # allow ASes to deactivate their own users
         if requester.app_service:
             await self._deactivate_account_handler.deactivate_account(
                 requester.user.to_string(), erase
@@ -362,7 +371,7 @@ def __init__(self, hs):
         self.identity_handler = hs.get_handlers().identity_handler
         self.store = self.hs.get_datastore()
 
-        if self.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL:
+        if self.config.threepid_behaviour_email_add_threepid == ThreepidBehaviour.LOCAL:
             template_html, template_text = load_jinja2_templates(
                 self.config.email_template_dir,
                 [
@@ -379,7 +388,7 @@ def __init__(self, hs):
             )
 
     async def on_POST(self, request):
-        if self.config.threepid_behaviour_email == ThreepidBehaviour.OFF:
+        if self.config.threepid_behaviour_email_add_threepid == ThreepidBehaviour.OFF:
             if self.config.local_threepid_handling_disabled_due_to_email_config:
                 logger.warning(
                     "Adding emails have been disabled due to lack of an email config"
@@ -416,7 +425,10 @@ async def on_POST(self, request):
 
             raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE)
 
-        if self.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE:
+        if (
+            self.config.threepid_behaviour_email_add_threepid
+            == ThreepidBehaviour.REMOTE
+        ):
             assert self.hs.config.account_threepid_delegate_email
 
             # Have the configured identity server handle the request
@@ -522,14 +534,14 @@ 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:
+        if self.config.threepid_behaviour_email_add_threepid == ThreepidBehaviour.LOCAL:
             (self.failure_email_template,) = load_jinja2_templates(
                 self.config.email_template_dir,
                 [self.config.email_add_threepid_template_failure_html],
             )
 
     async def on_GET(self, request):
-        if self.config.threepid_behaviour_email == ThreepidBehaviour.OFF:
+        if self.config.threepid_behaviour_email_add_threepid == ThreepidBehaviour.OFF:
             if self.config.local_threepid_handling_disabled_due_to_email_config:
                 logger.warning(
                     "Adding emails have been disabled due to lack of an email config"
@@ -537,7 +549,10 @@ async def on_GET(self, request):
             raise SynapseError(
                 400, "Adding an email to your account is disabled on this server"
             )
-        elif self.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE:
+        elif (
+            self.config.threepid_behaviour_email_add_threepid
+            == ThreepidBehaviour.REMOTE
+        ):
             raise SynapseError(
                 400,
                 "This homeserver is not validating threepids. Use an identity server "
diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py
index 141a3f5fac37..f218c6e197ea 100644
--- a/synapse/rest/client/v2_alpha/register.py
+++ b/synapse/rest/client/v2_alpha/register.py
@@ -79,7 +79,12 @@ def __init__(self, hs):
         self.identity_handler = hs.get_handlers().identity_handler
         self.config = hs.config
 
-        if self.hs.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL:
+        # Only load resources to validate registration emails if smtp details are configured
+        # on the homeserver
+        if (
+            self.hs.config.threepid_behaviour_email_add_threepid
+            == ThreepidBehaviour.LOCAL
+        ):
             from synapse.push.mailer import Mailer, load_jinja2_templates
 
             template_html, template_text = load_jinja2_templates(
@@ -100,7 +105,10 @@ def __init__(self, hs):
             )
 
     async def on_POST(self, request):
-        if self.hs.config.threepid_behaviour_email == ThreepidBehaviour.OFF:
+        if (
+            self.hs.config.threepid_behaviour_email_add_threepid
+            == ThreepidBehaviour.OFF
+        ):
             if self.hs.config.local_threepid_handling_disabled_due_to_email_config:
                 logger.warning(
                     "Email registration has been disabled due to lack of email config"
@@ -139,7 +147,10 @@ async def on_POST(self, request):
 
             raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE)
 
-        if self.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE:
+        if (
+            self.config.threepid_behaviour_email_add_threepid
+            == ThreepidBehaviour.REMOTE
+        ):
             assert self.hs.config.account_threepid_delegate_email
 
             # Have the configured identity server handle the request
@@ -253,13 +264,13 @@ def __init__(self, hs):
         self.clock = hs.get_clock()
         self.store = hs.get_datastore()
 
-        if self.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL:
+        if self.config.threepid_behaviour_email_add_threepid == ThreepidBehaviour.LOCAL:
             (self.failure_email_template,) = load_jinja2_templates(
                 self.config.email_template_dir,
                 [self.config.email_registration_template_failure_html],
             )
 
-        if self.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL:
+        if self.config.threepid_behaviour_email_add_threepid == ThreepidBehaviour.LOCAL:
             (self.failure_email_template,) = load_jinja2_templates(
                 self.config.email_template_dir,
                 [self.config.email_registration_template_failure_html],
@@ -270,7 +281,7 @@ async def on_GET(self, request, medium):
             raise SynapseError(
                 400, "This medium is currently not supported for registration"
             )
-        if self.config.threepid_behaviour_email == ThreepidBehaviour.OFF:
+        if self.config.threepid_behaviour_email_add_threepid == ThreepidBehaviour.OFF:
             if self.config.local_threepid_handling_disabled_due_to_email_config:
                 logger.warning(
                     "User registration via email has been disabled due to lack of email config"

From 74260e234cfcd33aea7da87950bda03ba989aeab Mon Sep 17 00:00:00 2001
From: Andrew Morgan <andrew@amorgan.xyz>
Date: Tue, 16 Jun 2020 14:12:16 +0100
Subject: [PATCH 04/10] Modify validate_threepid_session to check add_threepid.

Some additional refactoring for:
  * docstrings
  * lessening config option polling
---
 synapse/handlers/identity.py | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py
index 4ba004276890..daa2a2204e47 100644
--- a/synapse/handlers/identity.py
+++ b/synapse/handlers/identity.py
@@ -19,6 +19,7 @@
 
 import logging
 import urllib.parse
+from typing import Dict, Optional, Union
 
 from canonicaljson import json
 from signedjson.key import decode_verify_key_bytes
@@ -59,6 +60,10 @@ def __init__(self, hs):
         self.federation_http_client = hs.get_http_client()
         self.hs = hs
 
+        self._threepid_behaviour_email_add_threepid = (
+            self.hs.config.threepid_behaviour_email_add_threepid
+        )
+
     async def threepid_from_creds(self, id_server, creds):
         """
         Retrieve and validate a threepid identifier from a "credentials" dictionary against a
@@ -480,17 +485,27 @@ async def requestMsisdnToken(
         )
         return data
 
-    async def validate_threepid_session(self, client_secret, sid):
+    async def validate_threepid_session(
+        self, client_secret: str, sid: str
+    ) -> Optional[Dict[str, Union[str, int]]]:
         """Validates a threepid session with only the client secret and session ID
         Tries validating against any configured account_threepid_delegates as well as locally.
 
         Args:
-            client_secret (str): A secret provided by the client
+            client_secret: A secret provided by the client
 
-            sid (str): The ID of the session
+            sid: The ID of the session
 
         Returns:
-            Dict[str, str|int] if validation was successful, otherwise None
+            If validation succeeds, a dict containing:
+                * 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
         """
         # XXX: We shouldn't need to keep wrapping and unwrapping this value
         threepid_creds = {"client_secret": client_secret, "sid": sid}
@@ -500,12 +515,17 @@ async def validate_threepid_session(self, client_secret, sid):
         validation_session = None
 
         # Try to validate as email
-        if self.hs.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE:
+
+        # Note that this method is only used by /register and /account/3pid* currently,
+        # so there's no need to consult config.threepid_behaviour_email_password_reset
+        threepid_behaviour = self._threepid_behaviour_email_add_threepid
+
+        if threepid_behaviour == ThreepidBehaviour.REMOTE:
             # Ask our delegated email identity server
             validation_session = await self.threepid_from_creds(
                 self.hs.config.account_threepid_delegate_email, threepid_creds
             )
-        elif self.hs.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL:
+        elif threepid_behaviour == ThreepidBehaviour.LOCAL:
             # Get a validated session matching these details
             validation_session = await self.store.get_threepid_validation_session(
                 "email", client_secret, sid=sid, validated=True

From 689cdee9bb1d46d91185abbe5aff5d6feb1e84f5 Mon Sep 17 00:00:00 2001
From: Andrew Morgan <andrew@amorgan.xyz>
Date: Thu, 18 Jun 2020 16:01:35 +0100
Subject: [PATCH 05/10] Database migration to add 'service' to
 'threepid_validation_session'

We have a problem whereby during threepid (email) validation we need to know whether to handle the validation token ourselves or via an account threepid delegate.

Earlier we could just check whether we were delegating email or not, but now that email can be either Synapse or a threepid delegate depending on which service (adding threepids, password reset) the validation is for, we need a way to find that out.

So we store that information along with the validation session in the DB.
---
 .../04threepid_validation_session_service.sql | 39 +++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 synapse/storage/data_stores/main/schema/delta/58/04threepid_validation_session_service.sql

diff --git a/synapse/storage/data_stores/main/schema/delta/58/04threepid_validation_session_service.sql b/synapse/storage/data_stores/main/schema/delta/58/04threepid_validation_session_service.sql
new file mode 100644
index 000000000000..c24fd9252111
--- /dev/null
+++ b/synapse/storage/data_stores/main/schema/delta/58/04threepid_validation_session_service.sql
@@ -0,0 +1,39 @@
+/* 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.
+ */
+
+/* We would like to be able to classify threepid validation sessions
+ * by service.
+ */
+
+/* We also need to drop the existing sessions as it's not possible to reliably
+ * classify them. This will result in UIA sessions needing to be restarted after
+ * homeserver upgrade, but the impact of this is rather minimal.
+ */
+
+DROP TABLE threepid_validation_session;
+
+/* We choose to recreate the table instead of adding a column as SQLite does not
+ * support adding a new NOT NULL column to a table, even if it is empty.
+ * https://stackoverflow.com/q/3170634
+ */
+CREATE TABLE threepid_validation_session (
+    session_id TEXT PRIMARY KEY,
+    medium TEXT NOT NULL,
+    address TEXT NOT NULL,
+    client_secret TEXT NOT NULL,
+    last_send_attempt BIGINT NOT NULL,
+    validated_at BIGINT,
+    service TEXT NOT NULL  -- New column
+);

From 3364cd0a8cb61f9607ba867935a211a869076959 Mon Sep 17 00:00:00 2001
From: Andrew Morgan <andrew@amorgan.xyz>
Date: Mon, 22 Jun 2020 17:30:23 +0100
Subject: [PATCH 06/10] Store a ThreepidService alongside each validation
 session

Store the type of service so that we can reference it during token validation.
---
 synapse/storage/data_stores/main/registration.py | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/synapse/storage/data_stores/main/registration.py b/synapse/storage/data_stores/main/registration.py
index 587d4b91c1be..173362548ae3 100644
--- a/synapse/storage/data_stores/main/registration.py
+++ b/synapse/storage/data_stores/main/registration.py
@@ -24,6 +24,7 @@
 
 from synapse.api.constants import UserTypes
 from synapse.api.errors import Codes, StoreError, SynapseError, ThreepidValidationError
+from synapse.config.emailconfig import ThreepidService  # noqa: F401
 from synapse.metrics.background_process_metrics import run_as_background_process
 from synapse.storage._base import SQLBaseStore
 from synapse.storage.database import Database
@@ -727,6 +728,7 @@ def get_threepid_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
+                * service - the ThreepidService type that this session is authorising
 
                 Otherwise None if a validation session is not found
         """
@@ -748,7 +750,7 @@ def get_threepid_validation_session(
         def get_threepid_validation_session_txn(txn):
             sql = """
                 SELECT address, session_id, medium, client_secret,
-                last_send_attempt, validated_at
+                last_send_attempt, validated_at, service
                 FROM threepid_validation_session WHERE %s
                 """ % (
                 " AND ".join("%s = ?" % k for k in keyvalues.keys()),
@@ -1426,6 +1428,7 @@ def start_or_continue_validation_session(
         next_link,
         token,
         token_expires,
+        service,
     ):
         """Creates a new threepid validation session if it does not already
         exist and associates a new validation token with it
@@ -1442,6 +1445,8 @@ def start_or_continue_validation_session(
             token (str): The validation token
             token_expires (int): The timestamp for which after the token
                 will no longer be valid
+            service (ThreepidService): The type of threepid service that
+                this validation session will authorise
         """
 
         def start_or_continue_validation_session_txn(txn):
@@ -1455,6 +1460,7 @@ def start_or_continue_validation_session_txn(txn):
                     "medium": medium,
                     "address": address,
                     "client_secret": client_secret,
+                    "service": service.value,
                 },
             )
 

From 5af5b07bce2a9d02324a650edc2392c5124f46f2 Mon Sep 17 00:00:00 2001
From: Andrew Morgan <andrew@amorgan.xyz>
Date: Mon, 22 Jun 2020 17:30:56 +0100
Subject: [PATCH 07/10] Specify the ThreepidService when creating a threepid
 validation session.

The changes to identity.py are just threading the service through to `start_or_continue_validation_session`.
---
 synapse/handlers/identity.py             | 6 +++++-
 synapse/rest/client/v2_alpha/account.py  | 8 +++++---
 synapse/rest/client/v2_alpha/register.py | 5 +++--
 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py
index daa2a2204e47..9732f97debf5 100644
--- a/synapse/handlers/identity.py
+++ b/synapse/handlers/identity.py
@@ -35,7 +35,7 @@
     HttpResponseException,
     SynapseError,
 )
-from synapse.config.emailconfig import ThreepidBehaviour
+from synapse.config.emailconfig import ThreepidBehaviour, ThreepidService  # noqa: F401
 from synapse.http.client import SimpleHttpClient
 from synapse.util.hash import sha256_and_url_safe_base64
 from synapse.util.stringutils import assert_valid_client_secret, random_string
@@ -296,6 +296,7 @@ async def send_threepid_validation(
         client_secret,
         send_attempt,
         send_email_func,
+        service,
         next_link=None,
     ):
         """Send a threepid validation email for password reset or
@@ -308,6 +309,8 @@ async def send_threepid_validation(
             send_email_func (func): A function that takes an email address, token,
                                     client_secret and session_id, sends an email
                                     and returns a Deferred.
+            service (ThreepidService): The type of threepid service that
+                this validation session will authorise
             next_link (str|None): The URL to redirect the user to after validation
 
         Returns:
@@ -372,6 +375,7 @@ async def send_threepid_validation(
             next_link,
             token,
             token_expires,
+            service,
         )
 
         return session_id
diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py
index 1f24202e6875..17c26e525bca 100644
--- a/synapse/rest/client/v2_alpha/account.py
+++ b/synapse/rest/client/v2_alpha/account.py
@@ -19,7 +19,7 @@
 
 from synapse.api.constants import LoginType
 from synapse.api.errors import Codes, SynapseError, ThreepidValidationError
-from synapse.config.emailconfig import ThreepidBehaviour
+from synapse.config.emailconfig import ThreepidBehaviour, ThreepidService
 from synapse.http.server import finish_request
 from synapse.http.servlet import (
     RestServlet,
@@ -130,7 +130,8 @@ async def on_POST(self, request):
                 client_secret,
                 send_attempt,
                 self.mailer.send_password_reset_mail,
-                next_link,
+                service=ThreepidService.PASSWORD_RESET,
+                next_link=next_link,
             )
 
             # Wrap the session id in a JSON object
@@ -446,7 +447,8 @@ async def on_POST(self, request):
                 client_secret,
                 send_attempt,
                 self.mailer.send_add_threepid_mail,
-                next_link,
+                service=ThreepidService.ADDING_THREEPID,
+                next_link=next_link,
             )
 
             # Wrap the session id in a JSON object
diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py
index f218c6e197ea..2031caed0dcf 100644
--- a/synapse/rest/client/v2_alpha/register.py
+++ b/synapse/rest/client/v2_alpha/register.py
@@ -31,7 +31,7 @@
 from synapse.config import ConfigError
 from synapse.config.captcha import CaptchaConfig
 from synapse.config.consent_config import ConsentConfig
-from synapse.config.emailconfig import ThreepidBehaviour
+from synapse.config.emailconfig import ThreepidBehaviour, ThreepidService
 from synapse.config.ratelimiting import FederationRateLimitConfig
 from synapse.config.registration import RegistrationConfig
 from synapse.config.server import is_threepid_reserved
@@ -168,7 +168,8 @@ async def on_POST(self, request):
                 client_secret,
                 send_attempt,
                 self.mailer.send_registration_mail,
-                next_link,
+                service=ThreepidService.ADDING_THREEPID,
+                next_link=next_link,
             )
 
             # Wrap the session id in a JSON object

From b7d695704de57fe2c13f577bd2f13f5b3ab9aa83 Mon Sep 17 00:00:00 2001
From: Andrew Morgan <andrew@amorgan.xyz>
Date: Mon, 22 Jun 2020 17:33:10 +0100
Subject: [PATCH 08/10] Check the validation session's service type when a
 token is submitted.

If a service is specified, check with our new `delegate_for` option for
whether this should be handled by an account threepid delegate, or
remotely.

If there isn't a threepid session found with the details given by the
client, assume that this is a remote session.

Also modify the `is_enabled` method to check both `add_threepid` and `password_reset` services. If any of them are configured, then this checker is enabled.

And again, refactor a little bit by pulling config option polling into the constructor.
---
 synapse/handlers/ui_auth/checkers.py | 72 ++++++++++++++++++++++++----
 1 file changed, 63 insertions(+), 9 deletions(-)

diff --git a/synapse/handlers/ui_auth/checkers.py b/synapse/handlers/ui_auth/checkers.py
index 8b24a7331927..4e0dcd72207f 100644
--- a/synapse/handlers/ui_auth/checkers.py
+++ b/synapse/handlers/ui_auth/checkers.py
@@ -13,6 +13,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 import logging
+from typing import Any, Dict, Optional
 
 from canonicaljson import json
 
@@ -21,7 +22,7 @@
 
 from synapse.api.constants import LoginType
 from synapse.api.errors import Codes, LoginError, SynapseError
-from synapse.config.emailconfig import ThreepidBehaviour
+from synapse.config.emailconfig import ThreepidBehaviour, ThreepidService
 
 logger = logging.getLogger(__name__)
 
@@ -138,7 +139,21 @@ def __init__(self, hs):
         self.hs = hs
         self.store = hs.get_datastore()
 
-    async def _check_threepid(self, medium, authdict):
+        self._account_threepid_delegate_email = (
+            self.hs.config.account_threepid_delegate_email
+        )
+        self._account_threepid_delegate_msisdn = (
+            self.hs.config.account_threepid_delegate_msisdn
+        )
+
+        self._threepid_behaviour_email_add_threepid = (
+            self.hs.config.threepid_behaviour_email_add_threepid
+        )
+        self._threepid_behaviour_email_password_reset = (
+            self.hs.config.threepid_behaviour_email_password_reset
+        )
+
+    async def _check_threepid(self, medium: str, authdict: Dict[str, Any]):
         if "threepid_creds" not in authdict:
             raise LoginError(400, "Missing threepid_creds", Codes.MISSING_PARAM)
 
@@ -150,20 +165,54 @@ async def _check_threepid(self, medium, authdict):
 
         # msisdns are currently always ThreepidBehaviour.REMOTE
         if medium == "msisdn":
-            if not self.hs.config.account_threepid_delegate_msisdn:
+            if not self._account_threepid_delegate_msisdn:
                 raise SynapseError(
                     400, "Phone number verification is not enabled on this homeserver"
                 )
             threepid = await identity_handler.threepid_from_creds(
-                self.hs.config.account_threepid_delegate_msisdn, threepid_creds
+                self._account_threepid_delegate_msisdn, threepid_creds
             )
         elif medium == "email":
-            if self.hs.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE:
+            # Determine why this check is happening. This will help us decide whether
+            # Synapse or an account threepid delegate should complete the validation
+            service = None  # type: Optional[ThreepidService]
+
+            session = await self.store.get_threepid_validation_session(
+                medium,
+                threepid_creds["client_secret"],
+                sid=threepid_creds["sid"],
+                validated=True,
+            )
+
+            if session:
+                # We found a local session.
+                # Determine which service this was intended to authorise
+                service = ThreepidService(session["service"])
+
+                if service == ThreepidService.ADDING_THREEPID:
+                    threepid_behaviour = self._threepid_behaviour_email_add_threepid
+                elif service == ThreepidService.PASSWORD_RESET:
+                    threepid_behaviour = self._threepid_behaviour_email_password_reset
+                else:
+                    raise SynapseError(500, "Unknown threepid service")
+            else:
+                # We can't find a local, matching session.
+                # Do we have a threepid delegate configured?
+                if not self.hs.config.account_threepid_delegate_email:
+                    raise LoginError(401, "", errcode=Codes.UNAUTHORIZED)
+
+                # We do. Presume that this is a remote session
+                threepid_behaviour = ThreepidBehaviour.REMOTE
+
+            if threepid_behaviour == ThreepidBehaviour.REMOTE:
                 assert self.hs.config.account_threepid_delegate_email
+
+                # Ask our threepid delegate about this validation attempt
                 threepid = await identity_handler.threepid_from_creds(
                     self.hs.config.account_threepid_delegate_email, threepid_creds
                 )
-            elif self.hs.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL:
+            elif threepid_behaviour == ThreepidBehaviour.LOCAL:
+                # Attempt to validate locally
                 threepid = None
                 row = await self.store.get_threepid_validation_session(
                     medium,
@@ -213,10 +262,15 @@ def __init__(self, hs):
         _BaseThreepidAuthChecker.__init__(self, hs)
 
     def is_enabled(self):
-        return self.hs.config.threepid_behaviour_email in (
-            ThreepidBehaviour.REMOTE,
-            ThreepidBehaviour.LOCAL,
+        add_threepid_enabled = (
+            self.hs.config.threepid_behaviour_email_add_threepid
+            != ThreepidBehaviour.OFF
+        )
+        password_reset_enabled = (
+            self.hs.config.threepid_behaviour_email_password_reset
+            != ThreepidBehaviour.OFF
         )
+        return add_threepid_enabled or password_reset_enabled
 
     def check_auth(self, authdict, clientip):
         return defer.ensureDeferred(self._check_threepid("email", authdict))

From 0ba9328dd91cf132fcfcc685abb81b0f7ab7a33b Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Mon, 1 Jun 2020 15:36:57 +0200
Subject: [PATCH 09/10] Add test cases for each possible configuration

---
 tests/rest/client/v2_alpha/test_register.py | 184 ++++++++++++++++++++
 1 file changed, 184 insertions(+)

diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py
index 7deaf5b24a48..df2f0c6d22fd 100644
--- a/tests/rest/client/v2_alpha/test_register.py
+++ b/tests/rest/client/v2_alpha/test_register.py
@@ -18,13 +18,19 @@
 import datetime
 import json
 import os
+from typing import List, Optional
+
+from mock import Mock
 
 import pkg_resources
 
+from twisted.internet import defer
+
 import synapse.rest.admin
 from synapse.api.constants import LoginType
 from synapse.api.errors import Codes
 from synapse.appservice import ApplicationService
+from synapse.config.emailconfig import ThreepidService
 from synapse.rest.client.v1 import login, logout
 from synapse.rest.client.v2_alpha import account, account_validity, register, sync
 
@@ -35,6 +41,7 @@
 class RegisterRestServletTestCase(unittest.HomeserverTestCase):
 
     servlets = [
+        account.register_servlets,
         login.register_servlets,
         register.register_servlets,
         synapse.rest.admin.register_servlets,
@@ -302,6 +309,183 @@ def test_request_token_existing_email_inhibit_error(self):
 
         self.assertIsNotNone(channel.json_body.get("sid"))
 
+    def _test_delegate_for_option(
+        self, delegating_for: Optional[List[ThreepidService]]
+    ):
+        """Tests the account_threepid_delegates.delegate_for option"""
+        possible_delegated_services = [
+            ThreepidService.ADDING_THREEPID,
+            ThreepidService.PASSWORD_RESET,
+        ]
+
+        if delegating_for is None:
+            # The default value of delegate_for should be delegating *all* services
+            delegating_for = possible_delegated_services
+
+            # In contrast, an empty list means delegating *no* services
+
+        # Create a user and associated email address
+        email = "kermit@example.com"
+        user_id = self.register_user("kermit", "monkey")
+
+        identity_handler = self.hs.get_handlers().identity_handler
+
+        # Mock this method to learn when we attempt to validate a threepid via an account
+        # threepid delegate
+        identity_handler.requestEmailToken = Mock(
+            return_value=defer.succeed({"sid": 1234})
+        )
+        # We expect this many calls to be delegated to the account threepid delegate
+        expected_delegated_calls = len(delegating_for)
+
+        # Mock this method to learn when a local validation session is started, and
+        # with what service type
+        identity_handler.send_threepid_validation = Mock(
+            return_value=defer.succeed(1234)
+        )
+        # We expect this many calls to be handled by Synapse itself
+        expected_send_threepid_validation_calls = (
+            len(possible_delegated_services) - expected_delegated_calls
+        )
+
+        # Try to add a threepid to our account
+        body = {"client_secret": "somesecret", "email": email, "send_attempt": 0}
+        request, channel = self.make_request(
+            "POST", "/account/3pid/email/requestToken", body,
+        )
+        self.render(request)
+        self.assertEqual(channel.code, 200, channel.json_body)
+
+        if ThreepidService.ADDING_THREEPID not in delegating_for:
+            # Check that this is being handled locally, and was classified as the
+            # correct service
+            call_args_list = identity_handler.send_threepid_validation.call_args_list
+            args, kwargs = call_args_list[len(call_args_list) - 1]  # Get last call
+
+            self.assertEquals(kwargs["service"], ThreepidService.ADDING_THREEPID)
+
+        # Add a threepid that will be used for asking for our reset password
+        self.get_success(
+            self.hs.get_datastore().user_add_threepid(
+                user_id=user_id,
+                medium="email",
+                address=email,
+                validated_at=0,
+                added_at=0,
+            )
+        )
+
+        # Now let's test password reset
+        body = {"client_secret": "somesecret", "email": email, "send_attempt": 0}
+        request, channel = self.make_request(
+            "POST", "/account/password/email/requestToken", body,
+        )
+        self.render(request)
+        self.assertEqual(channel.code, 200, channel.json_body)
+
+        if ThreepidService.PASSWORD_RESET not in delegating_for:
+            # Check that this is being handled locally, and was classified as the
+            # correct service
+            call_args_list = identity_handler.send_threepid_validation.call_args_list
+            args, kwargs = call_args_list[len(call_args_list) - 1]  # Get last call
+
+            self.assertEquals(kwargs["service"], ThreepidService.PASSWORD_RESET)
+
+        # Check that we got the expected number of calls to each of our mocked methods
+        self.assertEqual(
+            len(identity_handler.requestEmailToken.mock_calls),
+            expected_delegated_calls,
+            "Expected calls to delegated threepid handler did not match actual call count",
+        )
+        self.assertEqual(
+            len(identity_handler.send_threepid_validation.mock_calls),
+            expected_send_threepid_validation_calls,
+            "Expected local threepid validation calls did not match actual call count",
+        )
+
+    @unittest.override_config(
+        {
+            "account_threepid_delegates": {
+                "email": "https://id_server",
+                "msisdn": "https://id_server",
+                "delegate_for": ["adding_threepid"],
+            },
+            "public_baseurl": "https://test_server",
+            "email": {
+                "smtp_host": "mail_server",
+                "smtp_port": 2525,
+                "notif_from": "sender@host",
+            },
+        }
+    )
+    def test_threepid_delegate_request_token_adding_only(self):
+        """Tests that only adding a threepid to an account is delegated to an account
+        threepid delegate, and Synapse still handles other validation tasks itself
+        """
+        self._test_delegate_for_option(delegating_for=[ThreepidService.ADDING_THREEPID])
+
+    @unittest.override_config(
+        {
+            "account_threepid_delegates": {
+                "email": "https://id_server",
+                "msisdn": "https://id_server",
+                "delegate_for": ["password_reset"],
+            },
+            "public_baseurl": "https://test_server",
+            "email": {
+                "smtp_host": "mail_server",
+                "smtp_port": 2525,
+                "notif_from": "sender@host",
+            },
+        }
+    )
+    def test_threepid_delegate_request_token_password_reset_only(self):
+        """Tests that only password reset is delegated to an account threepid delegate,
+        and Synapse still handles other validation tasks itself
+        """
+        self._test_delegate_for_option(delegating_for=[ThreepidService.PASSWORD_RESET])
+
+    @unittest.override_config(
+        {
+            "account_threepid_delegates": {
+                "email": "https://id_server",
+                "msisdn": "https://id_server",
+            },
+            "public_baseurl": "https://test_server",
+            "email": {
+                "smtp_host": "mail_server",
+                "smtp_port": 2525,
+                "notif_from": "sender@host",
+            },
+        }
+    )
+    def test_threepid_delegate_request_token_default(self):
+        """Test the default values for account_threepid_delegates.delegate_for
+        (which should result in delegating all services to an identity server)
+        """
+        self._test_delegate_for_option(delegating_for=None)
+
+    @unittest.override_config(
+        {
+            "account_threepid_delegates": {
+                "email": "https://id_server",
+                "msisdn": "https://id_server",
+                "delegate_for": [],
+            },
+            "public_baseurl": "https://test_server",
+            "email": {
+                "smtp_host": "mail_server",
+                "smtp_port": 2525,
+                "notif_from": "sender@host",
+            },
+        }
+    )
+    def test_threepid_delegate_request_token_none(self):
+        """Tests that providing no services in delegate_for results in Synapse executing
+        all threepid validation tasks itself
+        """
+        self._test_delegate_for_option(delegating_for=[])
+
 
 class AccountValidityTestCase(unittest.HomeserverTestCase):
 

From e446977197894651c8077d4b5f79b1df2e6dc535 Mon Sep 17 00:00:00 2001
From: Andrew Morgan <andrew@amorgan.xyz>
Date: Mon, 22 Jun 2020 17:39:01 +0100
Subject: [PATCH 10/10] Changelog

---
 changelog.d/7611.feature | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 changelog.d/7611.feature

diff --git a/changelog.d/7611.feature b/changelog.d/7611.feature
new file mode 100644
index 000000000000..fc84bc37c191
--- /dev/null
+++ b/changelog.d/7611.feature
@@ -0,0 +1 @@
+Add a `delegate_for` configuration flag to `account_threepid_delegates` to allow selectively delegating certain 3PID validation services.