Remove access-token support from RegistrationHandler.register (#5641)

Nothing uses this now, so we can remove the dead code, and clean up the
API.

Since we're changing the shape of the return value anyway, we take the
opportunity to give the method a better name.

changelog.d/5641.misc

@@ -0,0 +1 @@
+Remove access-token support from RegistrationHandler.register, and rename it.

synapse/handlers/register.py

@@ -138,11 +138,10 @@ class RegistrationHandler(BaseHandler):
                 )
 
     @defer.inlineCallbacks
-    def register(
+    def register_user(
         self,
         localpart=None,
         password=None,
-        generate_token=True,
         guest_access_token=None,
         make_guest=False,
         admin=False,
@@ -160,11 +159,6 @@ class RegistrationHandler(BaseHandler):
             password (unicode) : The password to assign to this user so they can
               login again. This can be None which means they cannot login again
               via a password (e.g. the user is an application service user).
-            generate_token (bool): Whether a new access token should be
-              generated. Having this be True should be considered deprecated,
-              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.
             user_type (str|None): type of user. One of the values from
               api.constants.UserTypes, or None for a normal user.
             default_display_name (unicode|None): if set, the new user's displayname
@@ -172,7 +166,7 @@ class RegistrationHandler(BaseHandler):
             address (str|None): the IP address used to perform the registration.
             bind_emails (List[str]): list of emails to bind to this account.
         Returns:
-            A tuple of (user_id, access_token).
+            Deferred[str]: user_id
         Raises:
             RegistrationError if there was a problem registering.
         """
@@ -206,12 +200,8 @@ class RegistrationHandler(BaseHandler):
             elif default_display_name is None:
                 default_display_name = localpart
 
-            token = None
-            if generate_token:
-                token = self.macaroon_gen.generate_access_token(user_id)
             yield self.register_with_store(
                 user_id=user_id,
-                token=token,
                 password_hash=password_hash,
                 was_guest=was_guest,
                 make_guest=make_guest,
@@ -230,21 +220,17 @@ class RegistrationHandler(BaseHandler):
         else:
             # autogen a sequential user ID
             attempts = 0
-            token = None
             user = None
             while not user:
                 localpart = yield self._generate_user_id(attempts > 0)
                 user = UserID(localpart, self.hs.hostname)
                 user_id = user.to_string()
                 yield self.check_user_id_not_appservice_exclusive(user_id)
-                if generate_token:
-                    token = self.macaroon_gen.generate_access_token(user_id)
                 if default_display_name is None:
                     default_display_name = localpart
                 try:
                     yield self.register_with_store(
                         user_id=user_id,
-                        token=token,
                         password_hash=password_hash,
                         make_guest=make_guest,
                         create_profile_with_displayname=default_display_name,
@@ -254,7 +240,6 @@ class RegistrationHandler(BaseHandler):
                     # if user id is taken, just generate another
                     user = None
                     user_id = None
-                    token = None
                     attempts += 1
 
         if not self.hs.config.user_consent_at_registration:
@@ -278,7 +263,7 @@ class RegistrationHandler(BaseHandler):
             # Bind email to new account
             yield self._register_email_threepid(user_id, threepid_dict, None, False)
 
-        defer.returnValue((user_id, token))
+        defer.returnValue(user_id)
 
     @defer.inlineCallbacks
     def _auto_join_rooms(self, user_id):
@@ -541,7 +526,6 @@ class RegistrationHandler(BaseHandler):
     def register_with_store(
         self,
         user_id,
-        token=None,
         password_hash=None,
         was_guest=False,
         make_guest=False,
@@ -555,9 +539,6 @@ class RegistrationHandler(BaseHandler):
 
         Args:
             user_id (str): The desired user ID to register.
-            token (str): The desired access token to use for this user. If this
-                is not None, the given access token is associated with the user
-                id.
             password_hash (str|None): Optional. The password hash for this user.
             was_guest (bool): Optional. Whether this is a guest account being
                 upgraded to a non-guest account.
@@ -593,7 +574,6 @@ class RegistrationHandler(BaseHandler):
         if self.hs.config.worker_app:
             return self._register_client(
                 user_id=user_id,
-                token=token,
                 password_hash=password_hash,
                 was_guest=was_guest,
                 make_guest=make_guest,
@@ -606,7 +586,6 @@ class RegistrationHandler(BaseHandler):
         else:
             return self.store.register(
                 user_id=user_id,
-                token=token,
                 password_hash=password_hash,
                 was_guest=was_guest,
                 make_guest=make_guest,

synapse/module_api/__init__.py

@@ -103,7 +103,6 @@ class ModuleApi(object):
         _, access_token = yield self.register_device(user_id)
         defer.returnValue((user_id, access_token))
 
-    @defer.inlineCallbacks
     def register_user(self, localpart, displayname=None, emails=[]):
         """Registers a new user with given localpart and optional displayname, emails.
 
@@ -115,15 +114,10 @@ class ModuleApi(object):
         Returns:
             Deferred[str]: user_id
         """
-        user_id, _ = yield self.hs.get_registration_handler().register(
+        return self.hs.get_registration_handler().register_user(
-            localpart=localpart,
+            localpart=localpart, default_display_name=displayname, bind_emails=emails
-            default_display_name=displayname,
-            bind_emails=emails,
-            generate_token=False,
         )
 
-        defer.returnValue(user_id)
-
     def register_device(self, user_id, device_id=None, initial_display_name=None):
         """Register a device for a user and generate an access token.
 

synapse/replication/http/register.py

@@ -38,7 +38,6 @@ class ReplicationRegisterServlet(ReplicationEndpoint):
     @staticmethod
     def _serialize_payload(
         user_id,
-        token,
         password_hash,
         was_guest,
         make_guest,
@@ -51,9 +50,6 @@ class ReplicationRegisterServlet(ReplicationEndpoint):
         """
         Args:
             user_id (str): The desired user ID to register.
-            token (str): The desired access token to use for this user. If this
-                is not None, the given access token is associated with the user
-                id.
             password_hash (str|None): Optional. The password hash for this user.
             was_guest (bool): Optional. Whether this is a guest account being
                 upgraded to a non-guest account.
@@ -68,7 +64,6 @@ class ReplicationRegisterServlet(ReplicationEndpoint):
             address (str|None): the IP address used to perform the regitration.
         """
         return {
-            "token": token,
             "password_hash": password_hash,
             "was_guest": was_guest,
             "make_guest": make_guest,
@@ -85,7 +80,6 @@ class ReplicationRegisterServlet(ReplicationEndpoint):
 
         yield self.registration_handler.register_with_store(
             user_id=user_id,
-            token=content["token"],
             password_hash=content["password_hash"],
             was_guest=content["was_guest"],
             make_guest=content["make_guest"],

synapse/rest/admin/__init__.py

@@ -219,11 +219,10 @@ class UserRegisterServlet(RestServlet):
 
         register = RegisterRestServlet(self.hs)
 
-        (user_id, _) = yield register.registration_handler.register(
+        user_id = yield register.registration_handler.register_user(
             localpart=body["username"].lower(),
             password=body["password"],
             admin=bool(admin),
-            generate_token=False,
             user_type=user_type,
         )
 

synapse/rest/client/v1/login.py

@@ -314,10 +314,8 @@ class LoginRestServlet(RestServlet):
 
         registered_user_id = yield self.auth_handler.check_user_exists(user_id)
         if not registered_user_id:
-            registered_user_id, _ = (
+            registered_user_id = yield self.registration_handler.register_user(
-                yield self.registration_handler.register(
+                localpart=user
-                    localpart=user, generate_token=False
-                )
             )
 
         result = yield self._register_device_with_callback(
@@ -505,12 +503,8 @@ class SSOAuthHandler(object):
         user_id = UserID(localpart, self._hostname).to_string()
         registered_user_id = yield self._auth_handler.check_user_exists(user_id)
         if not registered_user_id:
-            registered_user_id, _ = (
+            registered_user_id = yield self._registration_handler.register_user(
-                yield self._registration_handler.register(
+                localpart=localpart, default_display_name=user_display_name
-                    localpart=localpart,
-                    generate_token=False,
-                    default_display_name=user_display_name,
-                )
             )
 
         login_token = self._macaroon_gen.generate_short_term_login_token(

synapse/rest/client/v2_alpha/register.py

@@ -464,11 +464,10 @@ class RegisterRestServlet(RestServlet):
                                 Codes.THREEPID_IN_USE,
                             )
 
-            (registered_user_id, _) = yield self.registration_handler.register(
+            registered_user_id = yield self.registration_handler.register_user(
                 localpart=desired_username,
                 password=new_password,
                 guest_access_token=guest_access_token,
-                generate_token=False,
                 threepid=threepid,
                 address=client_addr,
             )
@@ -542,8 +541,8 @@ class RegisterRestServlet(RestServlet):
         if not compare_digest(want_mac, got_mac):
             raise SynapseError(403, "HMAC incorrect")
 
-        (user_id, _) = yield self.registration_handler.register(
+        user_id = yield self.registration_handler.register_user(
-            localpart=username, password=password, generate_token=False
+            localpart=username, password=password
         )
 
         result = yield self._create_registration_details(user_id, body)
@@ -577,8 +576,8 @@ class RegisterRestServlet(RestServlet):
     def _do_guest_registration(self, params, address=None):
         if not self.hs.config.allow_guest_access:
             raise SynapseError(403, "Guest access is disabled")
-        user_id, _ = yield self.registration_handler.register(
+        user_id = yield self.registration_handler.register_user(
-            generate_token=False, make_guest=True, address=address
+            make_guest=True, address=address
         )
 
         # we don't allow guests to specify their own device_id, because

tests/handlers/test_register.py

@@ -129,21 +129,21 @@ class RegistrationTestCase(unittest.HomeserverTestCase):
             return_value=defer.succeed(self.lots_of_users)
         )
         self.get_failure(
-            self.handler.register(localpart="local_part"), ResourceLimitError
+            self.handler.register_user(localpart="local_part"), ResourceLimitError
         )
 
         self.store.get_monthly_active_count = Mock(
             return_value=defer.succeed(self.hs.config.max_mau_value)
         )
         self.get_failure(
-            self.handler.register(localpart="local_part"), ResourceLimitError
+            self.handler.register_user(localpart="local_part"), ResourceLimitError
         )
 
     def test_auto_create_auto_join_rooms(self):
         room_alias_str = "#room:test"
         self.hs.config.auto_join_rooms = [room_alias_str]
-        res = self.get_success(self.handler.register(localpart="jeff"))
+        user_id = self.get_success(self.handler.register_user(localpart="jeff"))
-        rooms = self.get_success(self.store.get_rooms_for_user(res[0]))
+        rooms = self.get_success(self.store.get_rooms_for_user(user_id))
         directory_handler = self.hs.get_handlers().directory_handler
         room_alias = RoomAlias.from_string(room_alias_str)
         room_id = self.get_success(directory_handler.get_association(room_alias))
@@ -154,25 +154,25 @@ class RegistrationTestCase(unittest.HomeserverTestCase):
     def test_auto_create_auto_join_rooms_with_no_rooms(self):
         self.hs.config.auto_join_rooms = []
         frank = UserID.from_string("@frank:test")
-        res = self.get_success(self.handler.register(frank.localpart))
+        user_id = self.get_success(self.handler.register_user(frank.localpart))
-        self.assertEqual(res[0], frank.to_string())
+        self.assertEqual(user_id, frank.to_string())
-        rooms = self.get_success(self.store.get_rooms_for_user(res[0]))
+        rooms = self.get_success(self.store.get_rooms_for_user(user_id))
         self.assertEqual(len(rooms), 0)
 
     def test_auto_create_auto_join_where_room_is_another_domain(self):
         self.hs.config.auto_join_rooms = ["#room:another"]
         frank = UserID.from_string("@frank:test")
-        res = self.get_success(self.handler.register(frank.localpart))
+        user_id = self.get_success(self.handler.register_user(frank.localpart))
-        self.assertEqual(res[0], frank.to_string())
+        self.assertEqual(user_id, frank.to_string())
-        rooms = self.get_success(self.store.get_rooms_for_user(res[0]))
+        rooms = self.get_success(self.store.get_rooms_for_user(user_id))
         self.assertEqual(len(rooms), 0)
 
     def test_auto_create_auto_join_where_auto_create_is_false(self):
         self.hs.config.autocreate_auto_join_rooms = False
         room_alias_str = "#room:test"
         self.hs.config.auto_join_rooms = [room_alias_str]
-        res = self.get_success(self.handler.register(localpart="jeff"))
+        user_id = self.get_success(self.handler.register_user(localpart="jeff"))
-        rooms = self.get_success(self.store.get_rooms_for_user(res[0]))
+        rooms = self.get_success(self.store.get_rooms_for_user(user_id))
         self.assertEqual(len(rooms), 0)
 
     def test_auto_create_auto_join_rooms_when_support_user_exists(self):
@@ -180,8 +180,8 @@ class RegistrationTestCase(unittest.HomeserverTestCase):
         self.hs.config.auto_join_rooms = [room_alias_str]
 
         self.store.is_support_user = Mock(return_value=True)
-        res = self.get_success(self.handler.register(localpart="support"))
+        user_id = self.get_success(self.handler.register_user(localpart="support"))
-        rooms = self.get_success(self.store.get_rooms_for_user(res[0]))
+        rooms = self.get_success(self.store.get_rooms_for_user(user_id))
         self.assertEqual(len(rooms), 0)
         directory_handler = self.hs.get_handlers().directory_handler
         room_alias = RoomAlias.from_string(room_alias_str)
@@ -209,27 +209,31 @@ class RegistrationTestCase(unittest.HomeserverTestCase):
 
         # When:-
         #   * the user is registered and post consent actions are called
-        res = self.get_success(self.handler.register(localpart="jeff"))
+        user_id = self.get_success(self.handler.register_user(localpart="jeff"))
-        self.get_success(self.handler.post_consent_actions(res[0]))
+        self.get_success(self.handler.post_consent_actions(user_id))
 
         # Then:-
         #   * Ensure that they have not been joined to the room
-        rooms = self.get_success(self.store.get_rooms_for_user(res[0]))
+        rooms = self.get_success(self.store.get_rooms_for_user(user_id))
         self.assertEqual(len(rooms), 0)
 
     def test_register_support_user(self):
-        res = self.get_success(
+        user_id = self.get_success(
-            self.handler.register(localpart="user", user_type=UserTypes.SUPPORT)
+            self.handler.register_user(localpart="user", user_type=UserTypes.SUPPORT)
         )
-        self.assertTrue(self.store.is_support_user(res[0]))
+        d = self.store.is_support_user(user_id)
+        self.assertTrue(self.get_success(d))
 
     def test_register_not_support_user(self):
-        res = self.get_success(self.handler.register(localpart="user"))
+        user_id = self.get_success(self.handler.register_user(localpart="user"))
-        self.assertFalse(self.store.is_support_user(res[0]))
+        d = self.store.is_support_user(user_id)
+        self.assertFalse(self.get_success(d))
 
     def test_invalid_user_id_length(self):
         invalid_user_id = "x" * 256
-        self.get_failure(self.handler.register(localpart=invalid_user_id), SynapseError)
+        self.get_failure(
+            self.handler.register_user(localpart=invalid_user_id), SynapseError
+        )
 
     @defer.inlineCallbacks
     def get_or_create_user(self, requester, localpart, displayname, password_hash=None):
@@ -267,13 +271,12 @@ class RegistrationTestCase(unittest.HomeserverTestCase):
         if need_register:
             yield self.handler.register_with_store(
                 user_id=user_id,
-                token=token,
                 password_hash=password_hash,
                 create_profile_with_displayname=user.localpart,
             )
         else:
             yield self.hs.get_auth_handler().delete_access_tokens_for_user(user_id)
-            yield self.store.add_access_token_to_user(user_id=user_id, token=token)
+        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)