Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Stop advertising unsupported flows for registration (#6107)
Browse files Browse the repository at this point in the history
If email or msisdn verification aren't supported, let's stop advertising them
for registration.

Fixes #6100.
  • Loading branch information
richvdh authored Sep 25, 2019
1 parent 2cd9881 commit 990928a
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 16 deletions.
1 change: 1 addition & 0 deletions changelog.d/6107.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Ensure that servers which are not configured to support email address verification do not offer it in the registration flows.
11 changes: 10 additions & 1 deletion synapse/handlers/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ def __init__(self, hs):
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

Expand Down Expand Up @@ -156,6 +157,14 @@ def validate_user_via_ui_auth(self, requester, request_body, clientip):

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):
"""
Expand Down
26 changes: 26 additions & 0 deletions synapse/handlers/ui_auth/checkers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -51,13 +58,19 @@ def check_auth(self, authdict, clientip):
class DummyAuthChecker(UserInteractiveAuthChecker):
AUTH_TYPE = LoginType.DUMMY

def is_enabled(self):
return True

def check_auth(self, authdict, clientip):
return defer.succeed(True)


class TermsAuthChecker(UserInteractiveAuthChecker):
AUTH_TYPE = LoginType.TERMS

def is_enabled(self):
return True

def check_auth(self, authdict, clientip):
return defer.succeed(True)

Expand All @@ -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:
Expand Down Expand Up @@ -191,6 +208,12 @@ def __init__(self, hs):
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)

Expand All @@ -202,6 +225,9 @@ def __init__(self, hs):
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)

Expand Down
32 changes: 29 additions & 3 deletions synapse/rest/client/v2_alpha/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,14 @@
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,
Expand Down Expand Up @@ -375,7 +377,9 @@ def __init__(self, hs):
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
Expand Down Expand Up @@ -664,11 +668,13 @@ def _do_guest_registration(self, params, address=None):
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
"""
Expand All @@ -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
Expand All @@ -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
Expand Down
29 changes: 17 additions & 12 deletions tests/rest/client/v2_alpha/test_register.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,16 +198,8 @@ def test_advertised_flows(self):
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(
{
Expand All @@ -217,9 +209,13 @@ def test_advertised_flows(self):
"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)
Expand All @@ -241,7 +237,16 @@ def test_advertised_flows_captcha_and_terms(self):
)

@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"{}")
Expand Down

0 comments on commit 990928a

Please sign in to comment.