improve auto room join logic, comments and tests

4 files changed, 36 insertions, 9 deletions

diff --git a/changelog.d/3975.feature b/changelog.d/3975.feature
index 5cd8ad6cca..496ba4f4a0 100644
--- a/changelog.d/3975.feature
+++ b/changelog.d/3975.feature
@@ -1 +1 @@
-First user should autocreate autojoin rooms
+Servers with auto join rooms, should autocreate those rooms when first user registers
diff --git a/synapse/config/registration.py b/synapse/config/registration.py
index dcf2374ed2..43ff20a637 100644
--- a/synapse/config/registration.py
+++ b/synapse/config/registration.py
@@ -15,6 +15,8 @@
 
 from distutils.util import strtobool
 
+from synapse.config._base import ConfigError
+from synapse.types import RoomAlias
 from synapse.util.stringutils import random_string_with_symbols
 
 from ._base import Config
@@ -44,6 +46,9 @@ class RegistrationConfig(Config):
         )
 
         self.auto_join_rooms = config.get("auto_join_rooms", [])
+        for room_alias in self.auto_join_rooms:
+            if not RoomAlias.is_valid(room_alias):
+                raise ConfigError('Invalid auto_join_rooms entry %s' % room_alias)
         self.autocreate_auto_join_rooms = config.get("autocreate_auto_join_rooms", True)
 
     def default_config(self, **kwargs):
@@ -100,7 +105,11 @@ class RegistrationConfig(Config):
         #auto_join_rooms:
         #    - "#example:example.com"
 
-        # Have first user on server autocreate autojoin rooms
+        # Where auto_join_rooms are specified, setting this flag ensures that the
+        # the rooms exists by creating them when the first user on the
+        # homeserver registers.
+        # Setting to false means that if the rooms are not manually created,
+        # users cannot be auto joined since they do not exist.
         autocreate_auto_join_rooms: true
         """ % locals()
 
diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py
index 01cf7ab58e..2b269ab692 100644
--- a/synapse/handlers/register.py
+++ b/synapse/handlers/register.py
@@ -26,6 +26,7 @@ from synapse.api.errors import (
     RegistrationError,
     SynapseError,
 )
+from synapse.config._base import ConfigError
 from synapse.http.client import CaptchaServerHttpClient
 from synapse.types import RoomAlias, RoomID, UserID, create_requester
 from synapse.util.async_helpers import Linearizer
@@ -222,14 +223,19 @@ class RegistrationHandler(BaseHandler):
         fake_requester = create_requester(user_id)
 
         # try to create the room if we're the first user on the server
+        should_auto_create_rooms = False
         if self.hs.config.autocreate_auto_join_rooms:
             count = yield self.store.count_all_users()
-            auto_create_rooms = count == 1
+            should_auto_create_rooms = count == 1
 
         for r in self.hs.config.auto_join_rooms:
             try:
-                if auto_create_rooms and RoomAlias.is_valid(r):
+                if should_auto_create_rooms:
                     room_creation_handler = self.hs.get_room_creation_handler()
+                    if self.hs.hostname != RoomAlias.from_string(r).domain:
+                        raise ConfigError(
+                            'Cannot create room alias %s, it does not match server domain'
+                        )
                     # create room expects the localpart of the room alias
                     room_alias_localpart = RoomAlias.from_string(r).localpart
                     yield room_creation_handler.create_room(
@@ -531,7 +537,6 @@ class RegistrationHandler(BaseHandler):
 
     @defer.inlineCallbacks
     def _join_user_to_room(self, requester, room_identifier):
-
         room_id = None
         room_member_handler = self.hs.get_room_member_handler()
         if RoomID.is_valid(room_identifier):
diff --git a/tests/handlers/test_register.py b/tests/handlers/test_register.py
index a150a897ac..3e9a190727 100644
--- a/tests/handlers/test_register.py
+++ b/tests/handlers/test_register.py
@@ -47,7 +47,6 @@ class RegistrationTestCase(unittest.TestCase):
             generate_access_token=Mock(return_value='secret')
         )
         self.hs.get_macaroon_generator = Mock(return_value=self.macaroon_generator)
-        # self.hs.handlers = RegistrationHandlers(self.hs)
         self.handler = self.hs.get_handlers().registration_handler
         self.store = self.hs.get_datastore()
         self.hs.config.max_mau_value = 50
@@ -148,9 +147,7 @@ class RegistrationTestCase(unittest.TestCase):
     @defer.inlineCallbacks
     def test_auto_create_auto_join_rooms(self):
         room_alias_str = "#room:test"
-        self.hs.config.autocreate_auto_join_rooms = True
         self.hs.config.auto_join_rooms = [room_alias_str]
-
         res = yield self.handler.register(localpart='jeff')
         rooms = yield self.store.get_rooms_for_user(res[0])
 
@@ -163,11 +160,27 @@ class RegistrationTestCase(unittest.TestCase):
 
     @defer.inlineCallbacks
     def test_auto_create_auto_join_rooms_with_no_rooms(self):
-        self.hs.config.autocreate_auto_join_rooms = True
         self.hs.config.auto_join_rooms = []
         frank = UserID.from_string("@frank:test")
         res = yield self.handler.register(frank.localpart)
         self.assertEqual(res[0], frank.to_string())
         rooms = yield self.store.get_rooms_for_user(res[0])
+        self.assertEqual(len(rooms), 0)
+
+    @defer.inlineCallbacks
+    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 = yield self.handler.register(frank.localpart)
+        self.assertEqual(res[0], frank.to_string())
+        rooms = yield self.store.get_rooms_for_user(res[0])
+        self.assertEqual(len(rooms), 0)
 
+    @defer.inlineCallbacks
+    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 = yield self.handler.register(localpart='jeff')
+        rooms = yield self.store.get_rooms_for_user(res[0])
         self.assertEqual(len(rooms), 0)