Merge pull request #3096 from matrix-org/matthew/derive-mxid-from-3pid

add the register_mxid_from_3pid setting (untested)

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.
         #

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;
+        """target_user is the UserID whose displayname is to be changed;
-        auth_user is the user attempting to make this change."""
+        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)

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):

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

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):

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,)
         )

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",

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"](

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,
         )