From 9ad68163bd3224dcbf8b7dd7c7e65a35b5cd3f73 Mon Sep 17 00:00:00 2001
From: Neil Johnson <neil@matrix.org>
Date: Tue, 19 Mar 2019 10:19:53 +0000
Subject: [PATCH] fix test_auto_create_auto_join_where_no_consent

---
 synapse/handlers/register.py    |  6 ++++++
 tests/handlers/test_register.py | 24 ++++++++++++++++++------
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py
index 0ec16b1d2e..eb81556080 100644
--- a/synapse/handlers/register.py
+++ b/synapse/handlers/register.py
@@ -23,11 +23,13 @@ from synapse.api.constants import LoginType
 from synapse.api.errors import (
     AuthError,
     Codes,
+    ConsentNotGivenError,
     InvalidCaptchaError,
     LimitExceededError,
     RegistrationError,
     SynapseError,
 )
+
 from synapse.config.server import is_threepid_reserved
 from synapse.http.client import CaptchaServerHttpClient
 from synapse.http.servlet import assert_params_in_dict
@@ -311,6 +313,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 ef6ad127fc..001761f2b1 100644
--- a/tests/handlers/test_register.py
+++ b/tests/handlers/test_register.py
@@ -21,7 +21,7 @@ from synapse.api.constants import UserTypes
 from synapse.api.errors import ResourceLimitError, SynapseError
 from synapse.handlers.register import RegistrationHandler
 from synapse.types import RoomAlias, UserID, create_requester
-
+from synapse.api.urls import ConsentURIBuilder
 from tests.utils import setup_test_homeserver
 
 from .. import unittest
@@ -187,18 +187,30 @@ class RegistrationTestCase(unittest.TestCase):
 
     @defer.inlineCallbacks
     def test_auto_create_auto_join_where_no_consent(self):
-        """XXX what is this trying to test? I *think* it is trying to test
-        that we are not auto-joined to the server notices room if
-        block_events_without_consent_error is set, but (a) that doesn't seem to be
-        a sensible thing to test for, and (b) it doesn't work anyway because you can't
-        change the config after the EventCreationHandler has been instantiated.
+        """Test to ensure that the first user is not auto-joined to a room if
+        they have not given general consent.
         """
         self.hs.config.user_consent_at_registration = True
         self.hs.config.block_events_without_consent_error = "Error"
+
+        # 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()
+        event_creation_handler._block_events_without_consent_error = (
+            self.hs.config.block_events_without_consent_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])
+
+        # 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)