Stop advertising unsupported flows for registration (#6107)

If email or msisdn verification aren't supported, let's stop advertising them
for registration.

Fixes #6100.

5 files changed, 83 insertions, 16 deletions

diff --git a/changelog.d/6107.bugfix b/changelog.d/6107.bugfix
new file mode 100644
index 0000000000..d4b9516ac7
--- /dev/null
+++ b/changelog.d/6107.bugfix
@@ -0,0 +1 @@
+Ensure that servers which are not configured to support email address verification do not offer it in the registration flows.
\ No newline at end of file
diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index f920c2f6c1..333eb30625 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -61,7 +61,8 @@ class AuthHandler(BaseHandler):
         self.checkers = {}  # type: dict[str, UserInteractiveAuthChecker]
         for auth_checker_class in INTERACTIVE_AUTH_CHECKERS:
             inst = auth_checker_class(hs)
-            self.checkers[inst.AUTH_TYPE] = inst
+            if inst.is_enabled():
+                self.checkers[inst.AUTH_TYPE] = inst
 
         self.bcrypt_rounds = hs.config.bcrypt_rounds
 
@@ -156,6 +157,14 @@ class AuthHandler(BaseHandler):
 
         return params
 
+    def get_enabled_auth_types(self):
+        """Return the enabled user-interactive authentication types
+
+        Returns the UI-Auth types which are supported by the homeserver's current
+        config.
+        """
+        return self.checkers.keys()
+
     @defer.inlineCallbacks
     def check_auth(self, flows, clientdict, clientip):
         """
diff --git a/synapse/handlers/ui_auth/checkers.py b/synapse/handlers/ui_auth/checkers.py
index fd633b7b0e..ee69223243 100644
--- a/synapse/handlers/ui_auth/checkers.py
+++ b/synapse/handlers/ui_auth/checkers.py
@@ -32,6 +32,13 @@ class UserInteractiveAuthChecker:
     def __init__(self, hs):
         pass
 
+    def is_enabled(self):
+        """Check if the configuration of the homeserver allows this checker to work
+
+        Returns:
+            bool: True if this login type is enabled.
+        """
+
     def check_auth(self, authdict, clientip):
         """Given the authentication dict from the client, attempt to check this step
 
@@ -51,6 +58,9 @@ class UserInteractiveAuthChecker:
 class DummyAuthChecker(UserInteractiveAuthChecker):
     AUTH_TYPE = LoginType.DUMMY
 
+    def is_enabled(self):
+        return True
+
     def check_auth(self, authdict, clientip):
         return defer.succeed(True)
 
@@ -58,6 +68,9 @@ class DummyAuthChecker(UserInteractiveAuthChecker):
 class TermsAuthChecker(UserInteractiveAuthChecker):
     AUTH_TYPE = LoginType.TERMS
 
+    def is_enabled(self):
+        return True
+
     def check_auth(self, authdict, clientip):
         return defer.succeed(True)
 
@@ -67,10 +80,14 @@ class RecaptchaAuthChecker(UserInteractiveAuthChecker):
 
     def __init__(self, hs):
         super().__init__(hs)
+        self._enabled = bool(hs.config.recaptcha_private_key)
         self._http_client = hs.get_simple_http_client()
         self._url = hs.config.recaptcha_siteverify_api
         self._secret = hs.config.recaptcha_private_key
 
+    def is_enabled(self):
+        return self._enabled
+
     @defer.inlineCallbacks
     def check_auth(self, authdict, clientip):
         try:
@@ -191,6 +208,12 @@ class EmailIdentityAuthChecker(UserInteractiveAuthChecker, _BaseThreepidAuthChec
         UserInteractiveAuthChecker.__init__(self, hs)
         _BaseThreepidAuthChecker.__init__(self, hs)
 
+    def is_enabled(self):
+        return self.hs.config.threepid_behaviour_email in (
+            ThreepidBehaviour.REMOTE,
+            ThreepidBehaviour.LOCAL,
+        )
+
     def check_auth(self, authdict, clientip):
         return self._check_threepid("email", authdict)
 
@@ -202,6 +225,9 @@ class MsisdnAuthChecker(UserInteractiveAuthChecker, _BaseThreepidAuthChecker):
         UserInteractiveAuthChecker.__init__(self, hs)
         _BaseThreepidAuthChecker.__init__(self, hs)
 
+    def is_enabled(self):
+        return bool(self.hs.config.account_threepid_delegate_msisdn)
+
     def check_auth(self, authdict, clientip):
         return self._check_threepid("msisdn", authdict)
 
diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py
index e3f3d9126f..4f24a124a6 100644
--- a/synapse/rest/client/v2_alpha/register.py
+++ b/synapse/rest/client/v2_alpha/register.py
@@ -32,12 +32,14 @@ from synapse.api.errors import (
     ThreepidValidationError,
     UnrecognizedRequestError,
 )
+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.ratelimiting import FederationRateLimitConfig
 from synapse.config.registration import RegistrationConfig
 from synapse.config.server import is_threepid_reserved
+from synapse.handlers.auth import AuthHandler
 from synapse.http.server import finish_request
 from synapse.http.servlet import (
     RestServlet,
@@ -375,7 +377,9 @@ class RegisterRestServlet(RestServlet):
         self.ratelimiter = hs.get_registration_ratelimiter()
         self.clock = hs.get_clock()
 
-        self._registration_flows = _calculate_registration_flows(hs.config)
+        self._registration_flows = _calculate_registration_flows(
+            hs.config, self.auth_handler
+        )
 
     @interactive_auth_handler
     @defer.inlineCallbacks
@@ -664,11 +668,13 @@ class RegisterRestServlet(RestServlet):
 def _calculate_registration_flows(
     # technically `config` has to provide *all* of these interfaces, not just one
     config: Union[RegistrationConfig, ConsentConfig, CaptchaConfig],
+    auth_handler: AuthHandler,
 ) -> List[List[str]]:
     """Get a suitable flows list for registration
 
     Args:
         config: server configuration
+        auth_handler: authorization handler
 
     Returns: a list of supported flows
     """
@@ -678,10 +684,29 @@ def _calculate_registration_flows(
     require_msisdn = "msisdn" in config.registrations_require_3pid
 
     show_msisdn = True
+    show_email = True
+
     if config.disable_msisdn_registration:
         show_msisdn = False
         require_msisdn = False
 
+    enabled_auth_types = auth_handler.get_enabled_auth_types()
+    if LoginType.EMAIL_IDENTITY not in enabled_auth_types:
+        show_email = False
+        if require_email:
+            raise ConfigError(
+                "Configuration requires email address at registration, but email "
+                "validation is not configured"
+            )
+
+    if LoginType.MSISDN not in enabled_auth_types:
+        show_msisdn = False
+        if require_msisdn:
+            raise ConfigError(
+                "Configuration requires msisdn at registration, but msisdn "
+                "validation is not configured"
+            )
+
     flows = []
 
     # only support 3PIDless registration if no 3PIDs are required
@@ -693,14 +718,15 @@ def _calculate_registration_flows(
         flows.append([LoginType.DUMMY])
 
     # only support the email-only flow if we don't require MSISDN 3PIDs
-    if not require_msisdn:
+    if show_email and not require_msisdn:
         flows.append([LoginType.EMAIL_IDENTITY])
 
     # only support the MSISDN-only flow if we don't require email 3PIDs
     if show_msisdn and not require_email:
         flows.append([LoginType.MSISDN])
 
-    if show_msisdn:
+    if show_email and show_msisdn:
+        # always let users provide both MSISDN & email
         flows.append([LoginType.MSISDN, LoginType.EMAIL_IDENTITY])
 
     # Prepend m.login.terms to all flows if we're requiring consent
diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py
index bc2dc47973..dab87e5edf 100644
--- a/tests/rest/client/v2_alpha/test_register.py
+++ b/tests/rest/client/v2_alpha/test_register.py
@@ -198,16 +198,8 @@ class RegisterRestServletTestCase(unittest.HomeserverTestCase):
         self.assertEquals(channel.result["code"], b"401", channel.result)
         flows = channel.json_body["flows"]
 
-        # with the stock config, we expect all four combinations of 3pid
-        self.assertCountEqual(
-            [
-                ["m.login.dummy"],
-                ["m.login.email.identity"],
-                ["m.login.msisdn"],
-                ["m.login.msisdn", "m.login.email.identity"],
-            ],
-            (f["stages"] for f in flows),
-        )
+        # with the stock config, we only expect the dummy flow
+        self.assertCountEqual([["m.login.dummy"]], (f["stages"] for f in flows))
 
     @unittest.override_config(
         {
@@ -217,9 +209,13 @@ class RegisterRestServletTestCase(unittest.HomeserverTestCase):
                 "template_dir": "/",
                 "require_at_registration": True,
             },
+            "account_threepid_delegates": {
+                "email": "https://id_server",
+                "msisdn": "https://id_server",
+            },
         }
     )
-    def test_advertised_flows_captcha_and_terms(self):
+    def test_advertised_flows_captcha_and_terms_and_3pids(self):
         request, channel = self.make_request(b"POST", self.url, b"{}")
         self.render(request)
         self.assertEquals(channel.result["code"], b"401", channel.result)
@@ -241,7 +237,16 @@ class RegisterRestServletTestCase(unittest.HomeserverTestCase):
         )
 
     @unittest.override_config(
-        {"registrations_require_3pid": ["email"], "disable_msisdn_registration": True}
+        {
+            "public_baseurl": "https://test_server",
+            "registrations_require_3pid": ["email"],
+            "disable_msisdn_registration": True,
+            "email": {
+                "smtp_host": "mail_server",
+                "smtp_port": 2525,
+                "notif_from": "sender@host",
+            },
+        }
     )
     def test_advertised_flows_no_msisdn_email_required(self):
         request, channel = self.make_request(b"POST", self.url, b"{}")