fix test_auto_create_auto_join_where_no_consent (#4886)

5 files changed, 39 insertions, 5 deletions

diff --git a/changelog.d/4886.bugfix b/changelog.d/4886.bugfix
new file mode 100644
index 0000000000..b17aa92485
--- /dev/null
+++ b/changelog.d/4886.bugfix
@@ -0,0 +1 @@
+fix test_auto_create_auto_join_where_no_consent.
diff --git a/changelog.d/4886.misc b/changelog.d/4886.misc
new file mode 100644
index 0000000000..b17aa92485
--- /dev/null
+++ b/changelog.d/4886.misc
@@ -0,0 +1 @@
+fix test_auto_create_auto_join_where_no_consent.
diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py
index c762b58902..55787563c0 100644
--- a/synapse/handlers/message.py
+++ b/synapse/handlers/message.py
@@ -243,7 +243,14 @@ class EventCreationHandler(object):
 
         self.spam_checker = hs.get_spam_checker()
 
-        if self.config.block_events_without_consent_error is not None:
+        self._block_events_without_consent_error = (
+            self.config.block_events_without_consent_error
+        )
+
+        # we need to construct a ConsentURIBuilder here, as it checks that the necessary
+        # config options, but *only* if we have a configuration for which we are
+        # going to need it.
+        if self._block_events_without_consent_error:
             self._consent_uri_builder = ConsentURIBuilder(self.config)
 
     @defer.inlineCallbacks
@@ -378,7 +385,7 @@ class EventCreationHandler(object):
         Raises:
             ConsentNotGivenError: if the user has not given consent yet
         """
-        if self.config.block_events_without_consent_error is None:
+        if self._block_events_without_consent_error is None:
             return
 
         # exempt AS users from needing consent
@@ -405,7 +412,7 @@ class EventCreationHandler(object):
         consent_uri = self._consent_uri_builder.build_user_consent_uri(
             requester.user.localpart,
         )
-        msg = self.config.block_events_without_consent_error % {
+        msg = self._block_events_without_consent_error % {
             'consent_uri': consent_uri,
         }
         raise ConsentNotGivenError(
diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py
index 0ec16b1d2e..68f73d3793 100644
--- a/synapse/handlers/register.py
+++ b/synapse/handlers/register.py
@@ -23,6 +23,7 @@ from synapse.api.constants import LoginType
 from synapse.api.errors import (
     AuthError,
     Codes,
+    ConsentNotGivenError,
     InvalidCaptchaError,
     LimitExceededError,
     RegistrationError,
@@ -311,6 +312,10 @@ class RegistrationHandler(BaseHandler):
                         )
                 else:
                     yield self._join_user_to_room(fake_requester, r)
+            except ConsentNotGivenError as e:
+                # Technically not necessary to pull out this error though
+                # moving away from bare excepts is a good thing to do.
+                logger.error("Failed to join new user to %r: %r", r, e)
             except Exception as e:
                 logger.error("Failed to join new user to %r: %r", r, e)
 
diff --git a/tests/handlers/test_register.py b/tests/handlers/test_register.py
index c9c1506273..010e65829e 100644
--- a/tests/handlers/test_register.py
+++ b/tests/handlers/test_register.py
@@ -187,12 +187,32 @@ class RegistrationTestCase(unittest.TestCase):
 
     @defer.inlineCallbacks
     def test_auto_create_auto_join_where_no_consent(self):
-        self.hs.config.user_consent_at_registration = True
-        self.hs.config.block_events_without_consent_error = "Error"
+        """Test to ensure that the first user is not auto-joined to a room if
+        they have not given general consent.
+        """
+
+        # Given:-
+        #    * a user must give consent,
+        #    * they have not given that consent
+        #    * The server is configured to auto-join to a room
+        # (and autocreate if necessary)
+
+        event_creation_handler = self.hs.get_event_creation_handler()
+        # (Messing with the internals of event_creation_handler is fragile
+        # but can't see a better way to do this. One option could be to subclass
+        # the test with custom config.)
+        event_creation_handler._block_events_without_consent_error = ("Error")
+        event_creation_handler._consent_uri_builder = Mock()
         room_alias_str = "#room:test"
         self.hs.config.auto_join_rooms = [room_alias_str]
+
+        # When:-
+        #   * the user is registered and post consent actions are called
         res = yield self.handler.register(localpart='jeff')
         yield self.handler.post_consent_actions(res[0])
+
+        # Then:-
+        #   * Ensure that they have not been joined to the room
         rooms = yield self.store.get_rooms_for_user(res[0])
         self.assertEqual(len(rooms), 0)