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)