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

Stop advertising unsupported flows for registration #6107

Merged
merged 2 commits into from
Sep 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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