diff --git a/synapse/config/registration.py b/synapse/config/registration.py index 8aa3bcb00..3313c6411 100644 --- a/synapse/config/registration.py +++ b/synapse/config/registration.py @@ -41,6 +41,7 @@ class RegistrationConfig(Config): self.disable_3pid_changes = config.get("disable_3pid_changes", False) self.registration_shared_secret = config.get("registration_shared_secret") + self.register_mxid_from_3pid = config.get("register_mxid_from_3pid") self.bcrypt_rounds = config.get("bcrypt_rounds", 12) self.trusted_third_party_id_servers = config["trusted_third_party_id_servers"] @@ -74,6 +75,13 @@ class RegistrationConfig(Config): # - email # - msisdn + # Derive the user's matrix ID from a type of 3PID used when registering. + # This overrides any matrix ID the user proposes when calling /register + # The 3PID type should be present in registrations_require_3pid to avoid + # users failing to register if they don't specify the right kind of 3pid. + # + # register_mxid_from_3pid: email + # Mandate that users are only allowed to associate certain formats of # 3PIDs with accounts on this server. # diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py index 29ccf2384..a9c76f323 100644 --- a/synapse/handlers/profile.py +++ b/synapse/handlers/profile.py @@ -207,12 +207,12 @@ class ProfileHandler(BaseHandler): @defer.inlineCallbacks def set_displayname(self, target_user, requester, new_displayname, by_admin=False): - """target_user is the user whose displayname is to be changed; - auth_user is the user attempting to make this change.""" + """target_user is the UserID whose displayname is to be changed; + requester is the authenticated user attempting to make this change.""" if not self.hs.is_mine(target_user): raise SynapseError(400, "User is not hosted on this Home Server") - if not by_admin and target_user != requester.user: + if not by_admin and requester and target_user != requester.user: raise AuthError(400, "Cannot set another user's displayname") if not by_admin and self.hs.config.disable_set_displayname: @@ -239,7 +239,8 @@ class ProfileHandler(BaseHandler): target_user.to_string(), profile ) - yield self._update_join_states(requester, target_user) + if requester: + yield self._update_join_states(requester, target_user) # start a profile replication push run_in_background(self._replicate_profiles) diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py index 5da03e0a0..523cb6753 100644 --- a/synapse/handlers/register.py +++ b/synapse/handlers/register.py @@ -113,6 +113,7 @@ class RegistrationHandler(BaseHandler): generate_token=True, guest_access_token=None, make_guest=False, + display_name=None, admin=False, ): """Registers a new client on the server. @@ -128,6 +129,7 @@ class RegistrationHandler(BaseHandler): since it offers no means of associating a device_id with the access_token. Instead you should call auth_handler.issue_access_token after registration. + display_name (str): The displayname to set for this user, if any Returns: A tuple of (user_id, access_token). Raises: @@ -165,13 +167,20 @@ class RegistrationHandler(BaseHandler): password_hash=password_hash, was_guest=was_guest, make_guest=make_guest, - create_profile_with_localpart=( - # If the user was a guest then they already have a profile - None if was_guest else user.localpart - ), admin=admin, ) + if display_name is None: + display_name = ( + # If the user was a guest then they already have a profile + None if was_guest else user.localpart + ) + + if display_name: + yield self.profile_handler.set_displayname( + user, None, display_name, by_admin=True, + ) + if self.hs.config.user_directory_search_all_users: profile = yield self.store.get_profileinfo(localpart) yield self.user_directory_handler.handle_local_profile_change( @@ -196,8 +205,12 @@ class RegistrationHandler(BaseHandler): token=token, password_hash=password_hash, make_guest=make_guest, - create_profile_with_localpart=user.localpart, ) + + yield self.profile_handler.set_displayname( + user, None, user.localpart, by_admin=True, + ) + except SynapseError: # if user id is taken, just generate another user = None @@ -241,8 +254,12 @@ class RegistrationHandler(BaseHandler): user_id=user_id, password_hash="", appservice_id=service_id, - create_profile_with_localpart=user.localpart, ) + + yield self.profile_handler.set_displayname( + user, None, user.localpart, by_admin=True, + ) + defer.returnValue(user_id) @defer.inlineCallbacks @@ -288,7 +305,10 @@ class RegistrationHandler(BaseHandler): user_id=user_id, token=token, password_hash=None, - create_profile_with_localpart=user.localpart, + ) + + yield self.profile_handler.set_displayname( + user, None, user.localpart, by_admin=True, ) except Exception as e: yield self.store.add_access_token_to_user(user_id, token) @@ -443,18 +463,15 @@ class RegistrationHandler(BaseHandler): user_id=user_id, token=token, password_hash=password_hash, - create_profile_with_localpart=user.localpart, ) + if displayname is not None: + yield self.profile_handler.set_displayname( + user, None, displayname, by_admin=True, + ) else: yield self._auth_handler.delete_access_tokens_for_user(user_id) yield self.store.add_access_token_to_user(user_id=user_id, token=token) - if displayname is not None: - logger.info("setting user display name: %s -> %s", user_id, displayname) - yield self.profile_handler.set_displayname( - user, requester, displayname, by_admin=True, - ) - defer.returnValue((user_id, token)) def auth_handler(self): diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index 30ed6bef5..31554e787 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -17,7 +17,7 @@ from twisted.internet import defer import synapse -import synapse.types +from synapse import types from synapse.api.auth import get_access_token_from_request, has_access_token from synapse.api.constants import LoginType from synapse.api.errors import SynapseError, Codes, UnrecognizedRequestError @@ -31,6 +31,8 @@ from ._base import client_v2_patterns, interactive_auth_handler import logging import hmac +import re +from string import capwords from hashlib import sha1 from synapse.util.async import run_on_reactor from synapse.util.ratelimitutils import FederationRateLimiter @@ -222,6 +224,8 @@ class RegisterRestServlet(RestServlet): raise SynapseError(400, "Invalid username") desired_username = body['username'] + desired_display_name = None + appservice = None if has_access_token(request): appservice = yield self.auth.get_appservice_by_req(request) @@ -297,13 +301,6 @@ class RegisterRestServlet(RestServlet): session_id, "registered_user_id", None ) - if desired_username is not None: - yield self.registration_handler.check_username( - desired_username, - guest_access_token=guest_access_token, - assigned_user_id=registered_user_id, - ) - # Only give msisdn flows if the x_show_msisdn flag is given: # this is a hack to work around the fact that clients were shipped # that use fallback registration if they see any flows that they don't @@ -376,6 +373,73 @@ class RegisterRestServlet(RestServlet): Codes.THREEPID_DENIED, ) + if self.hs.config.register_mxid_from_3pid: + # override the desired_username based on the 3PID if any. + # reset it first to avoid folks picking their own username. + desired_username = None + + # we should have an auth_result at this point if we're going to progress + # to register the user (i.e. we haven't picked up a registered_user_id + # from our session store), in which case get ready and gen the + # desired_username + if auth_result: + if ( + self.hs.config.register_mxid_from_3pid == 'email' and + LoginType.EMAIL_IDENTITY in auth_result + ): + address = auth_result[LoginType.EMAIL_IDENTITY]['address'] + desired_username = types.strip_invalid_mxid_characters( + address.replace('@', '-').lower() + ) + + # find a unique mxid for the account, suffixing numbers + # if needed + while True: + try: + yield self.registration_handler.check_username( + desired_username, + guest_access_token=guest_access_token, + assigned_user_id=registered_user_id, + ) + # if we got this far we passed the check. + break + except SynapseError as e: + if e.errcode == Codes.USER_IN_USE: + m = re.match(r'^(.*?)(\d+)$', desired_username) + if m: + desired_username = m.group(1) + str( + int(m.group(2)) + 1 + ) + else: + desired_username += "1" + else: + # something else went wrong. + break + + # XXX: a nasty heuristic to turn an email address into + # a displayname, as part of register_mxid_from_3pid + parts = address.replace('.', ' ').split('@') + desired_display_name = ( + capwords(parts[0]) + + " [" + capwords(parts[1].split(' ')[0]) + "]" + ) + elif ( + self.hs.config.register_mxid_from_3pid == 'msisdn' and + LoginType.MSISDN in auth_result + ): + desired_username = auth_result[LoginType.MSISDN]['address'] + else: + raise SynapseError( + 400, "Cannot derive mxid from 3pid; no recognised 3pid" + ) + + if desired_username is not None: + yield self.registration_handler.check_username( + desired_username, + guest_access_token=guest_access_token, + assigned_user_id=registered_user_id, + ) + if registered_user_id is not None: logger.info( "Already registered user ID %r for this session", @@ -390,10 +454,18 @@ class RegisterRestServlet(RestServlet): raise SynapseError(400, "Missing password.", Codes.MISSING_PARAM) - desired_username = params.get("username", None) + if not self.hs.config.register_mxid_from_3pid: + desired_username = params.get("username", None) + else: + # we keep the original desired_username derived from the 3pid above + pass + new_password = params.get("password", None) guest_access_token = params.get("guest_access_token", None) + # XXX: don't we need to validate these for length etc like we did on + # the ones from the JSON body earlier on in the method? + if desired_username is not None: desired_username = desired_username.lower() @@ -402,6 +474,7 @@ class RegisterRestServlet(RestServlet): password=new_password, guest_access_token=guest_access_token, generate_token=False, + display_name=desired_display_name, ) # remember that we've now registered that user account, and with diff --git a/synapse/storage/profile.py b/synapse/storage/profile.py index fcf3af7e5..da1db0eba 100644 --- a/synapse/storage/profile.py +++ b/synapse/storage/profile.py @@ -125,33 +125,28 @@ class ProfileWorkerStore(SQLBaseStore): class ProfileStore(ProfileWorkerStore): - def create_profile(self, user_localpart): - return self._simple_insert( - table="profiles", - values={"user_id": user_localpart}, - desc="create_profile", - ) - def set_profile_displayname(self, user_localpart, new_displayname, batchnum): - return self._simple_update_one( + return self._simple_upsert( table="profiles", keyvalues={"user_id": user_localpart}, - updatevalues={ + values={ "displayname": new_displayname, "batch": batchnum, }, desc="set_profile_displayname", + lock=False # we can do this because user_id has a unique index ) def set_profile_avatar_url(self, user_localpart, new_avatar_url, batchnum): - return self._simple_update_one( + return self._simple_upsert( table="profiles", keyvalues={"user_id": user_localpart}, - updatevalues={ + values={ "avatar_url": new_avatar_url, "batch": batchnum, }, desc="set_profile_avatar_url", + lock=False # we can do this because user_id has a unique index ) def add_remote_profile_cache(self, user_id, displayname, avatar_url): diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py index 6b557ca0c..384c9977c 100644 --- a/synapse/storage/registration.py +++ b/synapse/storage/registration.py @@ -128,7 +128,7 @@ class RegistrationStore(RegistrationWorkerStore, def register(self, user_id, token=None, password_hash=None, was_guest=False, make_guest=False, appservice_id=None, - create_profile_with_localpart=None, admin=False): + admin=False): """Attempts to register an account. Args: @@ -142,8 +142,6 @@ class RegistrationStore(RegistrationWorkerStore, make_guest (boolean): True if the the new user should be guest, false to add a regular user account. appservice_id (str): The ID of the appservice registering the user. - create_profile_with_localpart (str): Optionally create a profile for - the given localpart. Raises: StoreError if the user_id could not be registered. """ @@ -156,7 +154,6 @@ class RegistrationStore(RegistrationWorkerStore, was_guest, make_guest, appservice_id, - create_profile_with_localpart, admin ) @@ -169,7 +166,6 @@ class RegistrationStore(RegistrationWorkerStore, was_guest, make_guest, appservice_id, - create_profile_with_localpart, admin, ): now = int(self.clock.time()) @@ -234,14 +230,6 @@ class RegistrationStore(RegistrationWorkerStore, (next_id, user_id, token,) ) - if create_profile_with_localpart: - # set a default displayname serverside to avoid ugly race - # between auto-joins and clients trying to set displaynames - txn.execute( - "INSERT INTO profiles(user_id, displayname) VALUES (?,?)", - (create_profile_with_localpart, create_profile_with_localpart) - ) - self._invalidate_cache_and_stream( txn, self.get_user_by_id, (user_id,) ) diff --git a/synapse/types.py b/synapse/types.py index cc7c182a7..00e6e1c36 100644 --- a/synapse/types.py +++ b/synapse/types.py @@ -229,6 +229,18 @@ def contains_invalid_mxid_characters(localpart): return any(c not in mxid_localpart_allowed_characters for c in localpart) +def strip_invalid_mxid_characters(localpart): + """Removes any invalid characters from an mxid + + Args: + localpart (basestring): the localpart to be stripped + + Returns: + localpart (basestring): the localpart having been stripped + """ + return filter(lambda c: c in mxid_localpart_allowed_characters, localpart) + + class StreamToken( namedtuple("Token", ( "room_key", diff --git a/tests/handlers/test_profile.py b/tests/handlers/test_profile.py index 8646c4e43..b2fd4b9af 100644 --- a/tests/handlers/test_profile.py +++ b/tests/handlers/test_profile.py @@ -68,8 +68,6 @@ class ProfileTestCase(unittest.TestCase): self.bob = UserID.from_string("@4567:test") self.alice = UserID.from_string("@alice:remote") - yield self.store.create_profile(self.frank.localpart) - self.handler = hs.get_profile_handler() @defer.inlineCallbacks @@ -123,7 +121,6 @@ class ProfileTestCase(unittest.TestCase): @defer.inlineCallbacks def test_incoming_fed_query(self): - yield self.store.create_profile("caroline") yield self.store.set_profile_displayname("caroline", "Caroline", 1) response = yield self.query_handlers["profile"]( diff --git a/tests/storage/test_profile.py b/tests/storage/test_profile.py index 1bfabc15a..d17319578 100644 --- a/tests/storage/test_profile.py +++ b/tests/storage/test_profile.py @@ -35,10 +35,6 @@ class ProfileStoreTestCase(unittest.TestCase): @defer.inlineCallbacks def test_displayname(self): - yield self.store.create_profile( - self.u_frank.localpart - ) - yield self.store.set_profile_displayname( self.u_frank.localpart, "Frank", 1, ) @@ -50,10 +46,6 @@ class ProfileStoreTestCase(unittest.TestCase): @defer.inlineCallbacks def test_avatar_url(self): - yield self.store.create_profile( - self.u_frank.localpart - ) - yield self.store.set_profile_avatar_url( self.u_frank.localpart, "http://my.site/here", 1, )