Merge pull request #3777 from matrix-org/neilj/fix_register_user_registration

fix bug where preserved threepid user comes to sign up and server is …
This commit is contained in:
Neil Johnson 2018-08-31 16:31:48 +00:00 committed by GitHub
commit 99178f8602
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 78 additions and 5 deletions

1
changelog.d/3777.bugfix Normal file
View File

@ -0,0 +1 @@
Fix bug where preserved threepid user comes to sign up and server is mau blocked

View File

@ -26,6 +26,7 @@ import synapse.types
from synapse import event_auth from synapse import event_auth
from synapse.api.constants import EventTypes, JoinRules, Membership from synapse.api.constants import EventTypes, JoinRules, Membership
from synapse.api.errors import AuthError, Codes, ResourceLimitError from synapse.api.errors import AuthError, Codes, ResourceLimitError
from synapse.config.server import is_threepid_reserved
from synapse.types import UserID from synapse.types import UserID
from synapse.util.caches import CACHE_SIZE_FACTOR, register_cache from synapse.util.caches import CACHE_SIZE_FACTOR, register_cache
from synapse.util.caches.lrucache import LruCache from synapse.util.caches.lrucache import LruCache
@ -775,13 +776,19 @@ class Auth(object):
) )
@defer.inlineCallbacks @defer.inlineCallbacks
def check_auth_blocking(self, user_id=None): def check_auth_blocking(self, user_id=None, threepid=None):
"""Checks if the user should be rejected for some external reason, """Checks if the user should be rejected for some external reason,
such as monthly active user limiting or global disable flag such as monthly active user limiting or global disable flag
Args: Args:
user_id(str|None): If present, checks for presence against existing user_id(str|None): If present, checks for presence against existing
MAU cohort MAU cohort
threepid(dict|None): If present, checks for presence against configured
reserved threepid. Used in cases where the user is trying register
with a MAU blocked server, normally they would be rejected but their
threepid is on the reserved list. user_id and
threepid should never be set at the same time.
""" """
# Never fail an auth check for the server notices users # Never fail an auth check for the server notices users
@ -797,6 +804,8 @@ class Auth(object):
limit_type=self.hs.config.hs_disabled_limit_type limit_type=self.hs.config.hs_disabled_limit_type
) )
if self.hs.config.limit_usage_by_mau is True: if self.hs.config.limit_usage_by_mau is True:
assert not (user_id and threepid)
# If the user is already part of the MAU cohort or a trial user # If the user is already part of the MAU cohort or a trial user
if user_id: if user_id:
timestamp = yield self.store.user_last_seen_monthly_active(user_id) timestamp = yield self.store.user_last_seen_monthly_active(user_id)
@ -806,12 +815,16 @@ class Auth(object):
is_trial = yield self.store.is_trial_user(user_id) is_trial = yield self.store.is_trial_user(user_id)
if is_trial: if is_trial:
return return
elif threepid:
# If the user does not exist yet, but is signing up with a
# reserved threepid then pass auth check
if is_threepid_reserved(self.hs.config, threepid):
return
# Else if there is no room in the MAU bucket, bail # Else if there is no room in the MAU bucket, bail
current_mau = yield self.store.get_monthly_active_count() current_mau = yield self.store.get_monthly_active_count()
if current_mau >= self.hs.config.max_mau_value: if current_mau >= self.hs.config.max_mau_value:
raise ResourceLimitError( raise ResourceLimitError(
403, "Monthly Active User Limit Exceeded", 403, "Monthly Active User Limit Exceeded",
admin_contact=self.hs.config.admin_contact, admin_contact=self.hs.config.admin_contact,
errcode=Codes.RESOURCE_LIMIT_EXCEEDED, errcode=Codes.RESOURCE_LIMIT_EXCEEDED,
limit_type="monthly_active_user" limit_type="monthly_active_user"

View File

@ -404,6 +404,23 @@ class ServerConfig(Config):
" service on the given port.") " service on the given port.")
def is_threepid_reserved(config, threepid):
"""Check the threepid against the reserved threepid config
Args:
config(ServerConfig) - to access server config attributes
threepid(dict) - The threepid to test for
Returns:
boolean Is the threepid undertest reserved_user
"""
for tp in config.mau_limits_reserved_threepids:
if (threepid['medium'] == tp['medium']
and threepid['address'] == tp['address']):
return True
return False
def read_gc_thresholds(thresholds): def read_gc_thresholds(thresholds):
"""Reads the three integer thresholds for garbage collection. Ensures that """Reads the three integer thresholds for garbage collection. Ensures that
the thresholds are integers if thresholds are supplied. the thresholds are integers if thresholds are supplied.

View File

@ -125,6 +125,7 @@ class RegistrationHandler(BaseHandler):
guest_access_token=None, guest_access_token=None,
make_guest=False, make_guest=False,
admin=False, admin=False,
threepid=None,
): ):
"""Registers a new client on the server. """Registers a new client on the server.
@ -145,7 +146,7 @@ class RegistrationHandler(BaseHandler):
RegistrationError if there was a problem registering. RegistrationError if there was a problem registering.
""" """
yield self.auth.check_auth_blocking() yield self.auth.check_auth_blocking(threepid=threepid)
password_hash = None password_hash = None
if password: if password:
password_hash = yield self.auth_handler().hash(password) password_hash = yield self.auth_handler().hash(password)

View File

@ -23,6 +23,7 @@ from twisted.internet import defer
import synapse.util.stringutils as stringutils import synapse.util.stringutils as stringutils
from synapse.api.constants import LoginType from synapse.api.constants import LoginType
from synapse.api.errors import Codes, SynapseError from synapse.api.errors import Codes, SynapseError
from synapse.config.server import is_threepid_reserved
from synapse.http.servlet import assert_params_in_dict, parse_json_object_from_request from synapse.http.servlet import assert_params_in_dict, parse_json_object_from_request
from synapse.rest.client.v1.base import ClientV1RestServlet from synapse.rest.client.v1.base import ClientV1RestServlet
from synapse.types import create_requester from synapse.types import create_requester
@ -281,12 +282,20 @@ class RegisterRestServlet(ClientV1RestServlet):
register_json["user"].encode("utf-8") register_json["user"].encode("utf-8")
if "user" in register_json else None if "user" in register_json else None
) )
threepid = None
if session.get(LoginType.EMAIL_IDENTITY):
threepid = session["threepidCreds"]
handler = self.handlers.registration_handler handler = self.handlers.registration_handler
(user_id, token) = yield handler.register( (user_id, token) = yield handler.register(
localpart=desired_user_id, localpart=desired_user_id,
password=password password=password,
threepid=threepid,
) )
# Necessary due to auth checks prior to the threepid being
# written to the db
if is_threepid_reserved(self.hs.config, threepid):
yield self.store.upsert_monthly_active_user(user_id)
if session[LoginType.EMAIL_IDENTITY]: if session[LoginType.EMAIL_IDENTITY]:
logger.debug("Binding emails %s to %s" % ( logger.debug("Binding emails %s to %s" % (

View File

@ -26,6 +26,7 @@ import synapse
import synapse.types import synapse.types
from synapse.api.constants import LoginType from synapse.api.constants import LoginType
from synapse.api.errors import Codes, SynapseError, UnrecognizedRequestError from synapse.api.errors import Codes, SynapseError, UnrecognizedRequestError
from synapse.config.server import is_threepid_reserved
from synapse.http.servlet import ( from synapse.http.servlet import (
RestServlet, RestServlet,
assert_params_in_dict, assert_params_in_dict,
@ -395,12 +396,21 @@ class RegisterRestServlet(RestServlet):
if desired_username is not None: if desired_username is not None:
desired_username = desired_username.lower() desired_username = desired_username.lower()
threepid = None
if auth_result:
threepid = auth_result.get(LoginType.EMAIL_IDENTITY)
(registered_user_id, _) = yield self.registration_handler.register( (registered_user_id, _) = yield self.registration_handler.register(
localpart=desired_username, localpart=desired_username,
password=new_password, password=new_password,
guest_access_token=guest_access_token, guest_access_token=guest_access_token,
generate_token=False, generate_token=False,
threepid=threepid,
) )
# Necessary due to auth checks prior to the threepid being
# written to the db
if is_threepid_reserved(self.hs.config, threepid):
yield self.store.upsert_monthly_active_user(registered_user_id)
# remember that we've now registered that user account, and with # remember that we've now registered that user account, and with
# what user ID (since the user may not have specified) # what user ID (since the user may not have specified)

View File

@ -36,7 +36,6 @@ class MonthlyActiveUsersStore(SQLBaseStore):
@defer.inlineCallbacks @defer.inlineCallbacks
def initialise_reserved_users(self, threepids): def initialise_reserved_users(self, threepids):
# TODO Why can't I do this in init?
store = self.hs.get_datastore() store = self.hs.get_datastore()
reserved_user_list = [] reserved_user_list = []

View File

@ -467,6 +467,23 @@ class AuthTestCase(unittest.TestCase):
) )
yield self.auth.check_auth_blocking() yield self.auth.check_auth_blocking()
@defer.inlineCallbacks
def test_reserved_threepid(self):
self.hs.config.limit_usage_by_mau = True
self.hs.config.max_mau_value = 1
threepid = {'medium': 'email', 'address': 'reserved@server.com'}
unknown_threepid = {'medium': 'email', 'address': 'unreserved@server.com'}
self.hs.config.mau_limits_reserved_threepids = [threepid]
yield self.store.register(user_id='user1', token="123", password_hash=None)
with self.assertRaises(ResourceLimitError):
yield self.auth.check_auth_blocking()
with self.assertRaises(ResourceLimitError):
yield self.auth.check_auth_blocking(threepid=unknown_threepid)
yield self.auth.check_auth_blocking(threepid=threepid)
@defer.inlineCallbacks @defer.inlineCallbacks
def test_hs_disabled(self): def test_hs_disabled(self):
self.hs.config.hs_disabled = True self.hs.config.hs_disabled = True

View File

@ -26,6 +26,7 @@ from twisted.internet import defer, reactor
from synapse.api.constants import EventTypes from synapse.api.constants import EventTypes
from synapse.api.errors import CodeMessageException, cs_error from synapse.api.errors import CodeMessageException, cs_error
from synapse.config.server import ServerConfig
from synapse.federation.transport import server from synapse.federation.transport import server
from synapse.http.server import HttpServer from synapse.http.server import HttpServer
from synapse.server import HomeServer from synapse.server import HomeServer
@ -158,6 +159,11 @@ def setup_test_homeserver(
# background, which upsets the test runner. # background, which upsets the test runner.
config.update_user_directory = False config.update_user_directory = False
def is_threepid_reserved(threepid):
return ServerConfig.is_threepid_reserved(config, threepid)
config.is_threepid_reserved.side_effect = is_threepid_reserved
config.use_frozen_dicts = True config.use_frozen_dicts = True
config.ldap_enabled = False config.ldap_enabled = False