rewrite based on PR feedback:
* [ ] split config options into allowed_local_3pids and registrations_require_3pid * [ ] simplify and comment logic for picking registration flows * [ ] fix docstring and move check_3pid_allowed into a new util module * [ ] use check_3pid_allowed everywhere @erikjohnston PTAL
This commit is contained in:
parent
9d332e0f79
commit
447f4f0d5f
|
@ -32,6 +32,7 @@ class RegistrationConfig(Config):
|
|||
)
|
||||
|
||||
self.registrations_require_3pid = config.get("registrations_require_3pid", [])
|
||||
self.allowed_local_3pids = config.get("allowed_local_3pids", [])
|
||||
self.registration_shared_secret = config.get("registration_shared_secret")
|
||||
|
||||
self.bcrypt_rounds = config.get("bcrypt_rounds", 12)
|
||||
|
@ -53,11 +54,16 @@ class RegistrationConfig(Config):
|
|||
# Enable registration for new users.
|
||||
enable_registration: False
|
||||
|
||||
# Mandate that registrations require a 3PID which matches one or more
|
||||
# of these 3PIDs. N.B. regexp escape backslashes are doubled (once for
|
||||
# YAML and once for the regexp itself)
|
||||
# The user must provide all of the below types of 3PID when registering.
|
||||
#
|
||||
# registrations_require_3pid:
|
||||
# - email
|
||||
# - msisdn
|
||||
|
||||
# Mandate that users are only allowed to associate certain formats of
|
||||
# 3PIDs with accounts on this server.
|
||||
#
|
||||
# allowed_local_3pids:
|
||||
# - medium: email
|
||||
# pattern: ".*@matrix\\.org"
|
||||
# - medium: email
|
||||
|
|
|
@ -15,7 +15,6 @@
|
|||
|
||||
"""Contains functions for registering clients."""
|
||||
import logging
|
||||
import re
|
||||
|
||||
from twisted.internet import defer
|
||||
|
||||
|
@ -26,6 +25,7 @@ from synapse.http.client import CaptchaServerHttpClient
|
|||
from synapse import types
|
||||
from synapse.types import UserID
|
||||
from synapse.util.async import run_on_reactor
|
||||
from synapse.util.threepids import check_3pid_allowed
|
||||
from ._base import BaseHandler
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
@ -308,15 +308,10 @@ class RegistrationHandler(BaseHandler):
|
|||
logger.info("got threepid with medium '%s' and address '%s'",
|
||||
threepid['medium'], threepid['address'])
|
||||
|
||||
for constraint in self.hs.config.registrations_require_3pid:
|
||||
if (
|
||||
constraint['medium'] == 'email' and
|
||||
threepid['medium'] == 'email' and
|
||||
re.match(constraint['pattern'], threepid['address'])
|
||||
):
|
||||
raise RegistrationError(
|
||||
403, "Third party identifier is not allowed"
|
||||
)
|
||||
if not check_3pid_allowed(self.hs, threepid['medium'], threepid['address']):
|
||||
raise RegistrationError(
|
||||
403, "Third party identifier is not allowed"
|
||||
)
|
||||
|
||||
@defer.inlineCallbacks
|
||||
def bind_emails(self, user_id, threepidCreds):
|
||||
|
|
|
@ -71,22 +71,13 @@ class RegisterRestServlet(ClientV1RestServlet):
|
|||
|
||||
def on_GET(self, request):
|
||||
|
||||
require_email = False
|
||||
require_msisdn = False
|
||||
for constraint in self.hs.config.registrations_require_3pid:
|
||||
if constraint['medium'] == 'email':
|
||||
require_email = True
|
||||
elif constraint['medium'] == 'msisdn':
|
||||
require_msisdn = True
|
||||
else:
|
||||
logger.warn(
|
||||
"Unrecognised 3PID medium %s in registrations_require_3pid" %
|
||||
constraint['medium']
|
||||
)
|
||||
require_email = 'email' in self.hs.config.registrations_require_3pid
|
||||
require_msisdn = 'msisdn' in self.hs.config.registrations_require_3pid
|
||||
|
||||
flows = []
|
||||
if self.hs.config.enable_registration_captcha:
|
||||
if require_email or not require_msisdn:
|
||||
# only support the email-only flow if we don't require MSISDN 3PIDs
|
||||
if not require_msisdn:
|
||||
flows.extend([
|
||||
{
|
||||
"type": LoginType.RECAPTCHA,
|
||||
|
@ -97,6 +88,7 @@ class RegisterRestServlet(ClientV1RestServlet):
|
|||
]
|
||||
},
|
||||
])
|
||||
# only support 3PIDless registration if no 3PIDs are required
|
||||
if not require_email and not require_msisdn:
|
||||
flows.extend([
|
||||
{
|
||||
|
@ -105,6 +97,7 @@ class RegisterRestServlet(ClientV1RestServlet):
|
|||
}
|
||||
])
|
||||
else:
|
||||
# only support the email-only flow if we don't require MSISDN 3PIDs
|
||||
if require_email or not require_msisdn:
|
||||
flows.extend([
|
||||
{
|
||||
|
@ -114,6 +107,7 @@ class RegisterRestServlet(ClientV1RestServlet):
|
|||
]
|
||||
}
|
||||
])
|
||||
# only support 3PIDless registration if no 3PIDs are required
|
||||
if not require_email and not require_msisdn:
|
||||
flows.extend([
|
||||
{
|
||||
|
|
|
@ -60,27 +60,6 @@ def set_timeline_upper_limit(filter_json, filter_timeline_limit):
|
|||
filter_timeline_limit)
|
||||
|
||||
|
||||
def check_3pid_allowed(hs, medium, address):
|
||||
# check whether the HS has whitelisted the given 3PID
|
||||
|
||||
allow = False
|
||||
if hs.config.registrations_require_3pid:
|
||||
for constraint in hs.config.registrations_require_3pid:
|
||||
logger.debug("Checking 3PID %s (%s) against %s (%s)" % (
|
||||
address, medium, constraint['pattern'], constraint['medium']
|
||||
))
|
||||
if (
|
||||
medium == constraint['medium'] and
|
||||
re.match(constraint['pattern'], address)
|
||||
):
|
||||
allow = True
|
||||
break
|
||||
else:
|
||||
allow = True
|
||||
|
||||
return allow
|
||||
|
||||
|
||||
def interactive_auth_handler(orig):
|
||||
"""Wraps an on_POST method to handle InteractiveAuthIncompleteErrors
|
||||
|
||||
|
|
|
@ -26,7 +26,8 @@ from synapse.http.servlet import (
|
|||
)
|
||||
from synapse.util.async import run_on_reactor
|
||||
from synapse.util.msisdn import phone_number_to_msisdn
|
||||
from ._base import client_v2_patterns, interactive_auth_handler, check_3pid_allowed
|
||||
from synapse.util.threepids import check_3pid_allowed
|
||||
from ._base import client_v2_patterns, interactive_auth_handler
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
|
|
@ -26,11 +26,11 @@ from synapse.http.servlet import (
|
|||
RestServlet, parse_json_object_from_request, assert_params_in_request, parse_string
|
||||
)
|
||||
from synapse.util.msisdn import phone_number_to_msisdn
|
||||
from synapse.util.threepids import check_3pid_allowed
|
||||
|
||||
from ._base import client_v2_patterns, interactive_auth_handler, check_3pid_allowed
|
||||
from ._base import client_v2_patterns, interactive_auth_handler
|
||||
|
||||
import logging
|
||||
import re
|
||||
import hmac
|
||||
from hashlib import sha1
|
||||
from synapse.util.async import run_on_reactor
|
||||
|
@ -316,41 +316,41 @@ class RegisterRestServlet(RestServlet):
|
|||
if 'x_show_msisdn' in body and body['x_show_msisdn']:
|
||||
show_msisdn = True
|
||||
|
||||
require_email = False
|
||||
require_msisdn = False
|
||||
for constraint in self.hs.config.registrations_require_3pid:
|
||||
if constraint['medium'] == 'email':
|
||||
require_email = True
|
||||
elif constraint['medium'] == 'msisdn':
|
||||
require_msisdn = True
|
||||
else:
|
||||
logger.warn(
|
||||
"Unrecognised 3PID medium %s in registrations_require_3pid" %
|
||||
constraint['medium']
|
||||
)
|
||||
# FIXME: need a better error than "no auth flow found" for scenarios
|
||||
# where we required 3PID for registration but the user didn't give one
|
||||
require_email = 'email' in self.hs.config.registrations_require_3pid
|
||||
require_msisdn = 'msisdn' in self.hs.config.registrations_require_3pid
|
||||
|
||||
flows = []
|
||||
if self.hs.config.enable_registration_captcha:
|
||||
# only support 3PIDless registration if no 3PIDs are required
|
||||
if not require_email and not require_msisdn:
|
||||
flows.extend([[LoginType.RECAPTCHA]])
|
||||
if require_email or not require_msisdn:
|
||||
# only support the email-only flow if we don't require MSISDN 3PIDs
|
||||
if not require_msisdn:
|
||||
flows.extend([[LoginType.EMAIL_IDENTITY, LoginType.RECAPTCHA]])
|
||||
|
||||
if show_msisdn:
|
||||
if not require_email or require_msisdn:
|
||||
# only support the MSISDN-only flow if we don't require email 3PIDs
|
||||
if not require_email:
|
||||
flows.extend([[LoginType.MSISDN, LoginType.RECAPTCHA]])
|
||||
# always let users provide both MSISDN & email
|
||||
flows.extend([
|
||||
[LoginType.MSISDN, LoginType.EMAIL_IDENTITY, LoginType.RECAPTCHA],
|
||||
])
|
||||
else:
|
||||
# only support 3PIDless registration if no 3PIDs are required
|
||||
if not require_email and not require_msisdn:
|
||||
flows.extend([[LoginType.DUMMY]])
|
||||
if require_email or not require_msisdn:
|
||||
# only support the email-only flow if we don't require MSISDN 3PIDs
|
||||
if not require_msisdn:
|
||||
flows.extend([[LoginType.EMAIL_IDENTITY]])
|
||||
|
||||
if show_msisdn:
|
||||
# only support the MSISDN-only flow if we don't require email 3PIDs
|
||||
if not require_email or require_msisdn:
|
||||
flows.extend([[LoginType.MSISDN]])
|
||||
# always let users provide both MSISDN & email
|
||||
flows.extend([
|
||||
[LoginType.MSISDN, LoginType.EMAIL_IDENTITY]
|
||||
])
|
||||
|
@ -359,30 +359,23 @@ class RegisterRestServlet(RestServlet):
|
|||
flows, body, self.hs.get_ip_from_request(request)
|
||||
)
|
||||
|
||||
# doublecheck that we're not trying to register an denied 3pid.
|
||||
# the user-facing checks should already have happened when we requested
|
||||
# a 3PID token to validate them in /register/email/requestToken etc
|
||||
# Check that we're not trying to register a denied 3pid.
|
||||
#
|
||||
# the user-facing checks will probably already have happened in
|
||||
# /register/email/requestToken when we requested a 3pid, but that's not
|
||||
# guaranteed.
|
||||
|
||||
for constraint in self.hs.config.registrations_require_3pid:
|
||||
if (
|
||||
constraint['medium'] == 'email' and
|
||||
auth_result and LoginType.EMAIL_IDENTITY in auth_result and
|
||||
re.match(
|
||||
constraint['pattern'],
|
||||
auth_result[LoginType.EMAIL_IDENTITY].threepid.address
|
||||
)
|
||||
):
|
||||
raise SynapseError(
|
||||
403, "Third party identifier is not allowed", Codes.THREEPID_DENIED
|
||||
)
|
||||
elif (
|
||||
constraint['medium'] == 'msisdn' and
|
||||
auth_result and LoginType.MSISDN in auth_result and
|
||||
re.match(
|
||||
constraint['pattern'],
|
||||
auth_result[LoginType.MSISDN].threepid.address
|
||||
)
|
||||
):
|
||||
if (
|
||||
auth_result and
|
||||
(
|
||||
LoginType.EMAIL_IDENTITY in auth_result or
|
||||
LoginType.EMAIL_MSISDN in auth_result
|
||||
)
|
||||
):
|
||||
medium = auth_result[LoginType.EMAIL_IDENTITY].threepid['medium']
|
||||
address = auth_result[LoginType.EMAIL_IDENTITY].threepid['address']
|
||||
|
||||
if not check_3pid_allowed(self.hs, medium, address):
|
||||
raise SynapseError(
|
||||
403, "Third party identifier is not allowed", Codes.THREEPID_DENIED
|
||||
)
|
||||
|
|
|
@ -0,0 +1,45 @@
|
|||
# -*- coding: utf-8 -*-
|
||||
# Copyright 2018 New Vector Ltd
|
||||
#
|
||||
# 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.
|
||||
|
||||
import logging
|
||||
import re
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def check_3pid_allowed(hs, medium, address):
|
||||
"""Checks whether a given format of 3PID is allowed to be used on this HS
|
||||
|
||||
Args:
|
||||
hs (synapse.server.HomeServer): server
|
||||
medium (str): 3pid medium - e.g. email, msisdn
|
||||
address (str): address within that medium (e.g. "wotan@matrix.org")
|
||||
msisdns need to first have been canonicalised
|
||||
"""
|
||||
|
||||
if hs.config.allowed_local_3pids:
|
||||
for constraint in hs.config.allowed_local_3pids:
|
||||
logger.debug("Checking 3PID %s (%s) against %s (%s)" % (
|
||||
address, medium, constraint['pattern'], constraint['medium']
|
||||
))
|
||||
if (
|
||||
medium == constraint['medium'] and
|
||||
re.match(constraint['pattern'], address)
|
||||
):
|
||||
return True
|
||||
else:
|
||||
return True
|
||||
|
||||
return False
|
Loading…
Reference in New Issue