summary refs log tree commit diff
diff options
context:
space:
mode:
-rw-r--r--changelog.d/5637.misc1
-rw-r--r--changelog.d/5641.misc1
-rw-r--r--changelog.d/5643.misc1
-rw-r--r--synapse/handlers/register.py34
-rw-r--r--synapse/module_api/__init__.py10
-rw-r--r--synapse/replication/http/register.py6
-rw-r--r--synapse/rest/admin/__init__.py3
-rw-r--r--synapse/rest/client/v1/login.py14
-rw-r--r--synapse/rest/client/v2_alpha/register.py11
-rw-r--r--sytest-blacklist4
-rw-r--r--tests/handlers/test_register.py53
11 files changed, 53 insertions, 85 deletions
diff --git a/changelog.d/5637.misc b/changelog.d/5637.misc
new file mode 100644
index 0000000000..f18d6197e5
--- /dev/null
+++ b/changelog.d/5637.misc
@@ -0,0 +1 @@
+Unblacklist some user_directory sytests.
diff --git a/changelog.d/5641.misc b/changelog.d/5641.misc
new file mode 100644
index 0000000000..1899bc963d
--- /dev/null
+++ b/changelog.d/5641.misc
@@ -0,0 +1 @@
+Remove access-token support from RegistrationHandler.register, and rename it.
diff --git a/changelog.d/5643.misc b/changelog.d/5643.misc
new file mode 100644
index 0000000000..2b2316469e
--- /dev/null
+++ b/changelog.d/5643.misc
@@ -0,0 +1 @@
+Improve logging for auto-join when a new user is created.
diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py
index 853020180b..a3e553d5f5 100644
--- a/synapse/handlers/register.py
+++ b/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,10 +240,15 @@ 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:
             yield self._auto_join_rooms(user_id)
+        else:
+            logger.info(
+                "Skipping auto-join for %s because consent is required at registration",
+                user_id,
+            )
 
         # Bind any specified emails to this account
         current_time = self.hs.get_clock().time_msec()
@@ -272,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):
@@ -298,6 +289,7 @@ class RegistrationHandler(BaseHandler):
             count = yield self.store.count_all_users()
             should_auto_create_rooms = count == 1
         for r in self.hs.config.auto_join_rooms:
+            logger.info("Auto-joining %s to %s", user_id, r)
             try:
                 if should_auto_create_rooms:
                     room_alias = RoomAlias.from_string(r)
@@ -534,7 +526,6 @@ class RegistrationHandler(BaseHandler):
     def register_with_store(
         self,
         user_id,
-        token=None,
         password_hash=None,
         was_guest=False,
         make_guest=False,
@@ -548,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.
@@ -586,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,
@@ -599,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,
diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py
index a0be2c5ca3..7bb020cb45 100644
--- a/synapse/module_api/__init__.py
+++ b/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(
-            localpart=localpart,
-            default_display_name=displayname,
-            bind_emails=emails,
-            generate_token=False,
+        return self.hs.get_registration_handler().register_user(
+            localpart=localpart, default_display_name=displayname, bind_emails=emails
         )
 
-        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.
 
diff --git a/synapse/replication/http/register.py b/synapse/replication/http/register.py
index f81a0f1b8f..2bf2173895 100644
--- a/synapse/replication/http/register.py
+++ b/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"],
diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py
index 9843a902c6..6888ae5590 100644
--- a/synapse/rest/admin/__init__.py
+++ b/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,
         )
 
diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py
index b13043cc64..0d05945f0a 100644
--- a/synapse/rest/client/v1/login.py
+++ b/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, _ = (
-                yield self.registration_handler.register(
-                    localpart=user, generate_token=False
-                )
+            registered_user_id = yield self.registration_handler.register_user(
+                localpart=user
             )
 
         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, _ = (
-                yield self._registration_handler.register(
-                    localpart=localpart,
-                    generate_token=False,
-                    default_display_name=user_display_name,
-                )
+            registered_user_id = yield self._registration_handler.register_user(
+                localpart=localpart, default_display_name=user_display_name
             )
 
         login_token = self._macaroon_gen.generate_short_term_login_token(
diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py
index 5c120e4dd5..f327999e59 100644
--- a/synapse/rest/client/v2_alpha/register.py
+++ b/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(
-            localpart=username, password=password, generate_token=False
+        user_id = yield self.registration_handler.register_user(
+            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(
-            generate_token=False, make_guest=True, address=address
+        user_id = yield self.registration_handler.register_user(
+            make_guest=True, address=address
         )
 
         # we don't allow guests to specify their own device_id, because
diff --git a/sytest-blacklist b/sytest-blacklist
index b760a48c57..11785fd43f 100644
--- a/sytest-blacklist
+++ b/sytest-blacklist
@@ -24,10 +24,6 @@ Newly created users see their own presence in /initialSync (SYT-34)
 # Blacklisted due to https://github.com/matrix-org/synapse/issues/1396
 Should reject keys claiming to belong to a different user
 
-# Blacklisted due to https://github.com/matrix-org/synapse/issues/2306
-Users appear/disappear from directory when join_rules are changed
-Users appear/disappear from directory when history_visibility are changed
-
 # Blacklisted due to https://github.com/matrix-org/synapse/issues/1531
 Enabling an unknown default rule fails with 404
 
diff --git a/tests/handlers/test_register.py b/tests/handlers/test_register.py
index 1c7ded7397..8197f26d4f 100644
--- a/tests/handlers/test_register.py
+++ b/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"))
-        rooms = self.get_success(self.store.get_rooms_for_user(res[0]))
+        user_id = self.get_success(self.handler.register_user(localpart="jeff"))
+        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))
-        self.assertEqual(res[0], frank.to_string())
-        rooms = self.get_success(self.store.get_rooms_for_user(res[0]))
+        user_id = self.get_success(self.handler.register_user(frank.localpart))
+        self.assertEqual(user_id, frank.to_string())
+        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))
-        self.assertEqual(res[0], frank.to_string())
-        rooms = self.get_success(self.store.get_rooms_for_user(res[0]))
+        user_id = self.get_success(self.handler.register_user(frank.localpart))
+        self.assertEqual(user_id, frank.to_string())
+        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"))
-        rooms = self.get_success(self.store.get_rooms_for_user(res[0]))
+        user_id = self.get_success(self.handler.register_user(localpart="jeff"))
+        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"))
-        rooms = self.get_success(self.store.get_rooms_for_user(res[0]))
+        user_id = self.get_success(self.handler.register_user(localpart="support"))
+        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"))
-        self.get_success(self.handler.post_consent_actions(res[0]))
+        user_id = self.get_success(self.handler.register_user(localpart="jeff"))
+        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(
-            self.handler.register(localpart="user", user_type=UserTypes.SUPPORT)
+        user_id = self.get_success(
+            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"))
-        self.assertFalse(self.store.is_support_user(res[0]))
+        user_id = self.get_success(self.handler.register_user(localpart="user"))
+        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)