Change account_threepid_delegate to a dictionary (#5969)

anoadragon453 · web-flow · commit 891afb57cbdf · 2019-09-04T15:02:55.000+01:00
`account_threepid_delegate` was an option added as part of this privacy sprint for the homeserver admin to declare which identity server (or any server handling third-party verification requests) they'd like to use to send email/sms messages on their behalf for the purposes of user registration and password resets.

We realized however, that while admins would want to set this option to `""` (allow Synapse to handle email sending itself), some homeservers have users with bound phone numbers, and setting `account_threepid_delegate` to `""` would prevent them from having any phone number verification, since Synapse does not at this time support sending SMS messages.

So, seeing as a common use case would be to have Synapse handle email verification, but an external server handle MSISDN verification, we split `account_threepid_delegate` into a dictionary, and called it `account_threepid_delegates` instead. This contains two keys as of present, `email` and `msisdn`. You can then set either to an external server of your choice, or `""` for Synapse to attempt to handle it.
diff --git a/UPGRADE.rst b/UPGRADE.rst
@@ -66,21 +66,23 @@ its own, phone-based password resets and registration will be disabled. For Syna
 emails, the ``email`` block of the config must be filled out. If not, then password resets and
 registration via email will be disabled entirely.
 
-This release also deprecates the ``email.trust_identity_server_for_password_resets`` option
-and replaces it with ``account_threepid_delegate``. This option defines whether the homeserver
-should delegate an external server (typically an `identity server
-<https://matrix.org/docs/spec/identity_service/r0.2.1>`_) to handle sending password reset
-or registration messages via email or SMS.
-
-If ``email.trust_identity_server_for_password_resets`` was changed from its default to
-``true``, and ``account_threepid_delegate`` is not set to an identity server domain, then the
-server handling password resets and registration via third-party addresses will be set to the
-first entry in the Synapse config's ``trusted_third_party_id_servers`` entry. If no domains are
-configured, Synapse will throw an error on startup.
-
-If ``email.trust_identity_server_for_password_resets`` is not set to ``true`` and
-``account_threepid_delegate`` is not set to a domain, then Synapse will attempt to send
-password reset and registration messages itself.
+This release also deprecates the ``email.trust_identity_server_for_password_resets`` option and
+replaces it with the ``account_threepid_delegates`` dictionary. This option defines whether the
+homeserver should delegate an external server (typically an `identity server
+<https://matrix.org/docs/spec/identity_service/r0.2.1>`_) to handle sending password reset or
+registration messages via email and SMS.
+
+Specifically for email, if ``email.trust_identity_server_for_password_resets`` was changed from
+its default to ``true``, and ``account_threepid_delegates.email`` is not set, then the server
+handling password resets and registration via third-party addresses will be set to the first
+entry in the Synapse config's ``trusted_third_party_id_servers`` entry. This is to ensure that
+people who set up an external server for handling these tasks before v1.4.0 will not have their
+setups mysteriously stop working. However, if no trusted identity server domains are
+configured, Synapse will throw an error.
+
+If ``email.trust_identity_server_for_password_resets`` is not set to ``true`` and a type in
+``account_threepid_delegates`` is not set to a domain, then Synapse will attempt to send
+password reset and registration messages itself for that type.
 
 Email templates
 ---------------
diff --git a/changelog.d/5876.feature b/changelog.d/5876.feature
@@ -1 +1 @@
-Replace `trust_identity_server_for_password_resets` config option with `account_threepid_delegate`.
+Replace `trust_identity_server_for_password_resets` config option with `account_threepid_delegates`.
diff --git a/changelog.d/5969.feature b/changelog.d/5969.feature
@@ -0,0 +1 @@
+Replace `trust_identity_server_for_password_resets` config option with `account_threepid_delegates`.
diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml
@@ -903,19 +903,29 @@ uploads_path: "DATADIR/uploads"
 #  - matrix.org
 #  - vector.im
 
-# Handle threepid (email/phone etc) registration and password resets
-# through a *trusted* identity server. Note that this allows the configured
-# identity server to reset passwords for accounts.
+# 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!
 #
-# If this option is not defined and SMTP options have not been
-# configured, registration by email and resetting user passwords via
-# email will be disabled
+# 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
+# email will be globally disabled.
 #
-# Otherwise, to enable set this option to the reachable domain name, including protocol
-# definition, for an identity server
-# (e.g "https://matrix.org", "http://localhost:8090")
+# Additionally, if `msisdn` is not set, registration and password resets via msisdn
+# will be disabled regardless. This is due to Synapse currently not supporting any
+# method of sending SMS messages on its own.
 #
-#account_threepid_delegate: ""
+# To enable using an identity server for operations regarding a particular third-party
+# identifier type, set the value to the URL of that identity server as shown in the
+# examples below.
+#
+# Servers handling the these requests must answer the `/requestToken` endpoints defined
+# by the Matrix Identity Service API specification:
+# https://matrix.org/docs/spec/identity_service/latest
+#
+account_threepid_delegates:
+    #email: https://example.com     # Delegate email sending to matrix.org
+    #msisdn: http://localhost:8090  # Delegate SMS sending to this local process
 
 # Users who register on this homeserver will automatically be joined
 # to these rooms
diff --git a/synapse/config/emailconfig.py b/synapse/config/emailconfig.py
@@ -75,11 +75,13 @@ def read_config(self, config, **kwargs):
             "renew_at"
         )
 
-        self.threepid_behaviour = (
-            # Have Synapse handle the email sending if account_threepid_delegate
+        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
+            if self.account_threepid_delegate_email
             else ThreepidBehaviour.LOCAL
         )
         # Prior to Synapse v1.4.0, there was another option that defined whether Synapse would
@@ -88,14 +90,16 @@ def read_config(self, config, **kwargs):
         # identity server in the process.
         self.using_identity_server_from_trusted_list = False
         if (
-            not self.account_threepid_delegate
+            not self.account_threepid_delegate_email
             and config.get("trust_identity_server_for_password_resets", False) is True
         ):
             # Use the first entry in self.trusted_third_party_id_servers instead
             if self.trusted_third_party_id_servers:
-                # XXX: It's a little confusing that account_threepid_delegate is modifed
+                # XXX: It's a little confusing that account_threepid_delegate_email is modified
                 # both in RegistrationConfig and here. We should factor this bit out
-                self.account_threepid_delegate = self.trusted_third_party_id_servers[0]
+                self.account_threepid_delegate_email = self.trusted_third_party_id_servers[
+                    0
+                ]
                 self.using_identity_server_from_trusted_list = True
             else:
                 raise ConfigError(
@@ -104,12 +108,15 @@ def read_config(self, config, **kwargs):
                 )
 
         self.local_threepid_handling_disabled_due_to_email_config = False
-        if self.threepid_behaviour == ThreepidBehaviour.LOCAL and email_config == {}:
+        if (
+            self.threepid_behaviour_email == 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 = ThreepidBehaviour.OFF
+            self.threepid_behaviour_email = ThreepidBehaviour.OFF
 
         # Get lifetime of a validation token in milliseconds
         self.email_validation_token_lifetime = self.parse_duration(
@@ -119,7 +126,7 @@ def read_config(self, config, **kwargs):
         if (
             self.email_enable_notifs
             or account_validity_renewal_enabled
-            or self.threepid_behaviour == ThreepidBehaviour.LOCAL
+            or self.threepid_behaviour_email == ThreepidBehaviour.LOCAL
         ):
             # make sure we can import the required deps
             import jinja2
@@ -129,7 +136,7 @@ def read_config(self, config, **kwargs):
             jinja2
             bleach
 
-        if self.threepid_behaviour == ThreepidBehaviour.LOCAL:
+        if self.threepid_behaviour_email == ThreepidBehaviour.LOCAL:
             required = ["smtp_host", "smtp_port", "notif_from"]
 
             missing = []
diff --git a/synapse/config/registration.py b/synapse/config/registration.py
@@ -99,7 +99,10 @@ def read_config(self, config, **kwargs):
         self.trusted_third_party_id_servers = config.get(
             "trusted_third_party_id_servers", ["matrix.org", "vector.im"]
         )
-        self.account_threepid_delegate = config.get("account_threepid_delegate")
+        account_threepid_delegates = config.get("account_threepid_delegates") or {}
+        self.account_threepid_delegate_email = account_threepid_delegates.get("email")
+        self.account_threepid_delegate_msisdn = account_threepid_delegates.get("msisdn")
+
         self.default_identity_server = config.get("default_identity_server")
         self.allow_guest_access = config.get("allow_guest_access", False)
 
@@ -270,19 +273,29 @@ def generate_config_section(self, generate_secrets=False, **kwargs):
         #  - matrix.org
         #  - vector.im
 
-        # Handle threepid (email/phone etc) registration and password resets
-        # through a *trusted* identity server. Note that this allows the configured
-        # identity server to reset passwords for accounts.
+        # 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!
+        #
+        # 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
+        # email will be globally disabled.
+        #
+        # Additionally, if `msisdn` is not set, registration and password resets via msisdn
+        # will be disabled regardless. This is due to Synapse currently not supporting any
+        # method of sending SMS messages on its own.
         #
-        # If this option is not defined and SMTP options have not been
-        # configured, registration by email and resetting user passwords via
-        # email will be disabled
+        # To enable using an identity server for operations regarding a particular third-party
+        # identifier type, set the value to the URL of that identity server as shown in the
+        # examples below.
         #
-        # Otherwise, to enable set this option to the reachable domain name, including protocol
-        # definition, for an identity server
-        # (e.g "https://matrix.org", "http://localhost:8090")
+        # Servers handling the these requests must answer the `/requestToken` endpoints defined
+        # by the Matrix Identity Service API specification:
+        # https://matrix.org/docs/spec/identity_service/latest
         #
-        #account_threepid_delegate: ""
+        account_threepid_delegates:
+            #email: https://example.com     # Delegate email sending to matrix.org
+            #msisdn: http://localhost:8090  # Delegate SMS sending to this local process
 
         # Users who register on this homeserver will automatically be joined
         # to these rooms
diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
@@ -461,10 +461,10 @@ def _check_threepid(self, medium, authdict, password_servlet=False, **kwargs):
         logger.info("Getting validated threepid. threepidcreds: %r", (threepid_creds,))
         if (
             not password_servlet
-            or self.hs.config.threepid_behaviour == ThreepidBehaviour.REMOTE
+            or self.hs.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE
         ):
             threepid = yield identity_handler.threepid_from_creds(threepid_creds)
-        elif self.hs.config.threepid_behaviour == ThreepidBehaviour.LOCAL:
+        elif self.hs.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL:
             row = yield self.store.get_threepid_validation_session(
                 medium,
                 threepid_creds["client_secret"],
diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py
@@ -50,7 +50,7 @@ def __init__(self, hs):
         self.config = hs.config
         self.identity_handler = hs.get_handlers().identity_handler
 
-        if self.config.threepid_behaviour == ThreepidBehaviour.LOCAL:
+        if self.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL:
             from synapse.push.mailer import Mailer, load_jinja2_templates
 
             templates = load_jinja2_templates(
@@ -67,7 +67,7 @@ def __init__(self, hs):
 
     @defer.inlineCallbacks
     def on_POST(self, request):
-        if self.config.threepid_behaviour == ThreepidBehaviour.OFF:
+        if self.config.threepid_behaviour_email == ThreepidBehaviour.OFF:
             if self.config.local_threepid_handling_disabled_due_to_email_config:
                 logger.warn(
                     "User password resets have been disabled due to lack of email config"
@@ -100,19 +100,19 @@ def on_POST(self, request):
         if existing_user_id is None:
             raise SynapseError(400, "Email not found", Codes.THREEPID_NOT_FOUND)
 
-        if self.config.threepid_behaviour == ThreepidBehaviour.REMOTE:
+        if self.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE:
             # Have the configured identity server handle the request
-            if not self.hs.config.account_threepid_delegate:
+            if not self.hs.config.account_threepid_delegate_email:
                 logger.warn(
-                    "No upstream account_threepid_delegate configured on the server to handle "
-                    "this request"
+                    "No upstream email account_threepid_delegate configured on the server to "
+                    "handle this request"
                 )
                 raise SynapseError(
                     400, "Password reset by email is not supported on this homeserver"
                 )
 
             ret = yield self.identity_handler.requestEmailToken(
-                self.hs.config.account_threepid_delegate,
+                self.hs.config.account_threepid_delegate_email,
                 email,
                 client_secret,
                 send_attempt,
@@ -172,31 +172,27 @@ def on_POST(self, request):
         if existing_user_id is None:
             raise SynapseError(400, "MSISDN not found", Codes.THREEPID_NOT_FOUND)
 
-        if self.config.threepid_behaviour == ThreepidBehaviour.REMOTE:
-            if not self.hs.config.account_threepid_delegate:
-                logger.warn(
-                    "No upstream account_threepid_delegate configured on the server to handle "
-                    "this request"
-                )
-                raise SynapseError(
-                    400,
-                    "Password reset by phone number is not supported on this homeserver",
-                )
-
-            ret = yield self.identity_handler.requestMsisdnToken(
-                self.config.account_threepid_delegate,
-                country,
-                phone_number,
-                client_secret,
-                send_attempt,
-                next_link,
+        if not self.hs.config.account_threepid_delegate_msisdn:
+            logger.warn(
+                "No upstream msisdn account_threepid_delegate configured on the server to "
+                "handle this request"
+            )
+            raise SynapseError(
+                400,
+                "Password reset by phone number is not supported on this homeserver",
             )
-            return (200, ret)
 
-        raise SynapseError(
-            400, "Password reset by phone number is not supported on this homeserver"
+        ret = yield self.identity_handler.requestMsisdnToken(
+            self.config.account_threepid_delegate_msisdn,
+            country,
+            phone_number,
+            client_secret,
+            send_attempt,
+            next_link,
         )
 
+        return 200, ret
+
 
 class PasswordResetSubmitTokenServlet(RestServlet):
     """Handles 3PID validation token submission"""
@@ -223,7 +219,7 @@ def on_GET(self, request, medium):
             raise SynapseError(
                 400, "This medium is currently not supported for password resets"
             )
-        if self.config.threepid_behaviour == ThreepidBehaviour.OFF:
+        if self.config.threepid_behaviour_email == ThreepidBehaviour.OFF:
             if self.config.local_threepid_handling_disabled_due_to_email_config:
                 logger.warn(
                     "Password reset emails have been disabled due to lack of an email config"
diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py
diff --git a/synapse/rest/client/versions.py b/synapse/rest/client/versions.py

Original file line number	Diff line number	Diff line change
`@@ -1 +1 @@`
`1`		-Replace `trust_identity_server_for_password_resets` config option with `account_threepid_delegate`.
	`1`	+Replace `trust_identity_server_for_password_resets` config option with `account_threepid_delegates`.
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+Replace `trust_identity_server_for_password_resets` config option with `account_threepid_delegates`.