summary refs log tree commit diff
diff options
context:
space:
mode:
authorRichard van der Hoff <richard@matrix.org>2021-08-31 11:30:16 +0100
committerRichard van der Hoff <richard@matrix.org>2021-08-31 11:30:16 +0100
commit619cc1d23055aa9b201220830878df051ada4f19 (patch)
tree4ab8020f1d473cc7bfc3b9723311c678a596cbba
parentRemove some redundant patches from the hotfixes branch (diff)
parentMerge pull request from GHSA-jj53-8fmw-f2w2 (diff)
downloadsynapse-619cc1d23055aa9b201220830878df051ada4f19.tar.xz
Merge branch 'release-v1.41' into matrix-org-hotfixes
-rw-r--r--CHANGES.md20
-rw-r--r--changelog.d/10571.feature1
-rw-r--r--changelog.d/10713.bugfix1
-rw-r--r--debian/changelog6
-rw-r--r--mypy.ini1
-rw-r--r--synapse/__init__.py2
-rw-r--r--synapse/groups/groups_server.py18
-rw-r--r--synapse/handlers/message.py23
-rw-r--r--synapse/handlers/send_email.py65
-rw-r--r--tests/handlers/test_send_email.py112
-rw-r--r--tests/rest/client/v1/test_rooms.py84
-rw-r--r--tests/rest/client/v2_alpha/test_groups.py43
-rw-r--r--tests/server.py15
13 files changed, 359 insertions, 32 deletions
diff --git a/CHANGES.md b/CHANGES.md
index cad9423ebd..f8da8771aa 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -1,3 +1,17 @@
+Synapse 1.41.0 (2021-08-24)
+===========================
+
+This release adds support for Debian 12 (Bookworm), but **removes support for Ubuntu 20.10 (Groovy Gorilla)**, which reached End of Life last month.
+
+Note that when using workers the `/_synapse/admin/v1/users/{userId}/media` must now be handled by media workers. See the [upgrade notes](https://matrix-org.github.io/synapse/latest/upgrade.html) for more information.
+
+
+Features
+--------
+
+- Enable room capabilities ([MSC3244](https://github.com/matrix-org/matrix-doc/pull/3244)) by default and set room version 8 as the preferred room version when creating restricted rooms. ([\#10571](https://github.com/matrix-org/synapse/issues/10571))
+
+
 Synapse 1.41.0rc1 (2021-08-18)
 ==============================
 
@@ -7,7 +21,7 @@ Features
 - Add `get_userinfo_by_id` method to ModuleApi. ([\#9581](https://github.com/matrix-org/synapse/issues/9581))
 - Initial local support for [MSC3266](https://github.com/matrix-org/synapse/pull/10394), Room Summary over the unstable `/rooms/{roomIdOrAlias}/summary` API. ([\#10394](https://github.com/matrix-org/synapse/issues/10394))
 - Experimental support for [MSC3288](https://github.com/matrix-org/matrix-doc/pull/3288), sending `room_type` to the identity server for 3pid invites over the `/store-invite` API. ([\#10435](https://github.com/matrix-org/synapse/issues/10435))
-- Add support for sending federation requests through a proxy. Contributed by @Bubu and @dklimpel. ([\#10475](https://github.com/matrix-org/synapse/issues/10475))
+- Add support for sending federation requests through a proxy. Contributed by @Bubu and @dklimpel. See the [upgrade notes](https://matrix-org.github.io/synapse/latest/upgrade.html) for more information. ([\#10596](https://github.com/matrix-org/synapse/issues/10596)). ([\#10475](https://github.com/matrix-org/synapse/issues/10475))
 - Add support for "marker" events which makes historical events discoverable for servers that already have all of the scrollback history (part of [MSC2716](https://github.com/matrix-org/matrix-doc/pull/2716)). ([\#10498](https://github.com/matrix-org/synapse/issues/10498))
 - Add a configuration setting for the time a `/sync` response is cached for. ([\#10513](https://github.com/matrix-org/synapse/issues/10513))
 - The default logging handler for new installations is now `PeriodicallyFlushingMemoryHandler`, a buffered logging handler which periodically flushes itself. ([\#10518](https://github.com/matrix-org/synapse/issues/10518))
@@ -29,7 +43,7 @@ Bugfixes
 - Add some clarification to the sample config file. Contributed by @Kentokamoto. ([\#10129](https://github.com/matrix-org/synapse/issues/10129))
 - Fix a long-standing bug where protocols which are not implemented by any appservices were incorrectly returned via `GET /_matrix/client/r0/thirdparty/protocols`. ([\#10532](https://github.com/matrix-org/synapse/issues/10532))
 - Fix exceptions in logs when failing to get remote room list. ([\#10541](https://github.com/matrix-org/synapse/issues/10541))
-- Fix longstanding bug which caused the user "status" to be reset when the user went offline. Contributed by @dklimpel. ([\#10550](https://github.com/matrix-org/synapse/issues/10550))
+- Fix longstanding bug which caused the user's presence "status message" to be reset when the user went offline. Contributed by @dklimpel. ([\#10550](https://github.com/matrix-org/synapse/issues/10550))
 - Allow public rooms to be previewed in the spaces summary APIs from [MSC2946](https://github.com/matrix-org/matrix-doc/pull/2946). ([\#10580](https://github.com/matrix-org/synapse/issues/10580))
 - Fix a bug introduced in v1.37.1 where an error could occur in the asynchronous processing of PDUs when the queue was empty. ([\#10592](https://github.com/matrix-org/synapse/issues/10592))
 - Fix errors on /sync when read receipt data is a string. Only affects homeservers with the experimental flag for [MSC2285](https://github.com/matrix-org/matrix-doc/pull/2285) enabled. Contributed by @SimonBrandner. ([\#10606](https://github.com/matrix-org/synapse/issues/10606))
@@ -40,7 +54,7 @@ Bugfixes
 Improved Documentation
 ----------------------
 
-- Add documentation for configuration a forward proxy. ([\#10443](https://github.com/matrix-org/synapse/issues/10443))
+- Add documentation for configuring a forward proxy. ([\#10443](https://github.com/matrix-org/synapse/issues/10443))
 - Updated the reverse proxy documentation to highlight the homserver configuration that is needed to make Synapse aware that is is intentionally reverse proxied. ([\#10551](https://github.com/matrix-org/synapse/issues/10551))
 - Update CONTRIBUTING.md to fix index links and the instructions for SyTest in docker. ([\#10599](https://github.com/matrix-org/synapse/issues/10599))
 
diff --git a/changelog.d/10571.feature b/changelog.d/10571.feature
deleted file mode 100644
index 0da318cd5b..0000000000
--- a/changelog.d/10571.feature
+++ /dev/null
@@ -1 +0,0 @@
-Enable room capabilities ([MSC3244](https://github.com/matrix-org/matrix-doc/pull/3244)) by default and set room version 8 as the preferred room version for restricted rooms.
diff --git a/changelog.d/10713.bugfix b/changelog.d/10713.bugfix
new file mode 100644
index 0000000000..e8caf3d23a
--- /dev/null
+++ b/changelog.d/10713.bugfix
@@ -0,0 +1 @@
+Fix a regression introduced in Synapse 1.41 which broke email transmission on Systems using older versions of the Twisted library.
diff --git a/debian/changelog b/debian/changelog
index 68f309b0b2..4da4bc018c 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+matrix-synapse-py3 (1.41.0) stable; urgency=medium
+
+  * New synapse release 1.41.0.
+
+ -- Synapse Packaging team <packages@matrix.org>  Tue, 24 Aug 2021 15:31:45 +0100
+
 matrix-synapse-py3 (1.41.0~rc1) stable; urgency=medium
 
   * New synapse release 1.41.0~rc1.
diff --git a/mypy.ini b/mypy.ini
index e1b9405daa..349efe37bb 100644
--- a/mypy.ini
+++ b/mypy.ini
@@ -87,6 +87,7 @@ files =
   tests/test_utils,
   tests/handlers/test_password_providers.py,
   tests/handlers/test_room_summary.py,
+  tests/handlers/test_send_email.py,
   tests/rest/client/v1/test_login.py,
   tests/rest/client/v2_alpha/test_auth.py,
   tests/util/test_itertools.py,
diff --git a/synapse/__init__.py b/synapse/__init__.py
index 6ada20a77f..ef3770262e 100644
--- a/synapse/__init__.py
+++ b/synapse/__init__.py
@@ -47,7 +47,7 @@ try:
 except ImportError:
     pass
 
-__version__ = "1.41.0rc1"
+__version__ = "1.41.0"
 
 if bool(os.environ.get("SYNAPSE_TEST_PATCH_LOG_CONTEXTS", False)):
     # We import here so that we don't have to install a bunch of deps when
diff --git a/synapse/groups/groups_server.py b/synapse/groups/groups_server.py
index 3dc55ab861..d6b75ac27f 100644
--- a/synapse/groups/groups_server.py
+++ b/synapse/groups/groups_server.py
@@ -332,6 +332,13 @@ class GroupsServerWorkerHandler:
             requester_user_id, group_id
         )
 
+        # Note! room_results["is_public"] is about whether the room is considered
+        # public from the group's point of view. (i.e. whether non-group members
+        # should be able to see the room is in the group).
+        # This is not the same as whether the room itself is public (in the sense
+        # of being visible in the room directory).
+        # As such, room_results["is_public"] itself is not sufficient to determine
+        # whether any given user is permitted to see the room's metadata.
         room_results = await self.store.get_rooms_in_group(
             group_id, include_private=is_user_in_group
         )
@@ -341,8 +348,15 @@ class GroupsServerWorkerHandler:
             room_id = room_result["room_id"]
 
             joined_users = await self.store.get_users_in_room(room_id)
+
+            # check the user is actually allowed to see the room before showing it to them
+            allow_private = requester_user_id in joined_users
+
             entry = await self.room_list_handler.generate_room_entry(
-                room_id, len(joined_users), with_alias=False, allow_private=True
+                room_id,
+                len(joined_users),
+                with_alias=False,
+                allow_private=allow_private,
             )
 
             if not entry:
@@ -354,7 +368,7 @@ class GroupsServerWorkerHandler:
 
         chunk.sort(key=lambda e: -e["num_joined_members"])
 
-        return {"chunk": chunk, "total_room_count_estimate": len(room_results)}
+        return {"chunk": chunk, "total_room_count_estimate": len(chunk)}
 
 
 class GroupsServerHandler(GroupsServerWorkerHandler):
diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py
index cf0359556a..54d0af9940 100644
--- a/synapse/handlers/message.py
+++ b/synapse/handlers/message.py
@@ -183,20 +183,37 @@ class MessageHandler:
 
             if not last_events:
                 raise NotFoundError("Can't find event for token %s" % (at_token,))
+            last_event = last_events[0]
+
+            # check whether the user is in the room at that time to determine
+            # whether they should be treated as peeking.
+            state_map = await self.state_store.get_state_for_event(
+                last_event.event_id,
+                StateFilter.from_types([(EventTypes.Member, user_id)]),
+            )
+
+            joined = False
+            membership_event = state_map.get((EventTypes.Member, user_id))
+            if membership_event:
+                joined = membership_event.membership == Membership.JOIN
+
+            is_peeking = not joined
 
             visible_events = await filter_events_for_client(
                 self.storage,
                 user_id,
                 last_events,
                 filter_send_to_client=False,
+                is_peeking=is_peeking,
             )
 
-            event = last_events[0]
             if visible_events:
                 room_state_events = await self.state_store.get_state_for_events(
-                    [event.event_id], state_filter=state_filter
+                    [last_event.event_id], state_filter=state_filter
                 )
-                room_state: Mapping[Any, EventBase] = room_state_events[event.event_id]
+                room_state: Mapping[Any, EventBase] = room_state_events[
+                    last_event.event_id
+                ]
             else:
                 raise AuthError(
                     403,
diff --git a/synapse/handlers/send_email.py b/synapse/handlers/send_email.py
index dda9659c11..a31fe3e3c7 100644
--- a/synapse/handlers/send_email.py
+++ b/synapse/handlers/send_email.py
@@ -19,9 +19,12 @@ from email.mime.text import MIMEText
 from io import BytesIO
 from typing import TYPE_CHECKING, Optional
 
+from pkg_resources import parse_version
+
+import twisted
 from twisted.internet.defer import Deferred
-from twisted.internet.interfaces import IReactorTCP
-from twisted.mail.smtp import ESMTPSenderFactory
+from twisted.internet.interfaces import IOpenSSLContextFactory, IReactorTCP
+from twisted.mail.smtp import ESMTPSender, ESMTPSenderFactory
 
 from synapse.logging.context import make_deferred_yieldable
 
@@ -30,6 +33,19 @@ if TYPE_CHECKING:
 
 logger = logging.getLogger(__name__)
 
+_is_old_twisted = parse_version(twisted.__version__) < parse_version("21")
+
+
+class _NoTLSESMTPSender(ESMTPSender):
+    """Extend ESMTPSender to disable TLS
+
+    Unfortunately, before Twisted 21.2, ESMTPSender doesn't give an easy way to disable
+    TLS, so we override its internal method which it uses to generate a context factory.
+    """
+
+    def _getContextFactory(self) -> Optional[IOpenSSLContextFactory]:
+        return None
+
 
 async def _sendmail(
     reactor: IReactorTCP,
@@ -42,7 +58,7 @@ async def _sendmail(
     password: Optional[bytes] = None,
     require_auth: bool = False,
     require_tls: bool = False,
-    tls_hostname: Optional[str] = None,
+    enable_tls: bool = True,
 ) -> None:
     """A simple wrapper around ESMTPSenderFactory, to allow substitution in tests
 
@@ -57,24 +73,37 @@ async def _sendmail(
         password: password to give when authenticating
         require_auth: if auth is not offered, fail the request
         require_tls: if TLS is not offered, fail the reqest
-        tls_hostname: TLS hostname to check for. None to disable TLS.
+        enable_tls: True to enable TLS. If this is False and require_tls is True,
+           the request will fail.
     """
     msg = BytesIO(msg_bytes)
-
     d: "Deferred[object]" = Deferred()
 
-    factory = ESMTPSenderFactory(
-        username,
-        password,
-        from_addr,
-        to_addr,
-        msg,
-        d,
-        heloFallback=True,
-        requireAuthentication=require_auth,
-        requireTransportSecurity=require_tls,
-        hostname=tls_hostname,
-    )
+    def build_sender_factory(**kwargs) -> ESMTPSenderFactory:
+        return ESMTPSenderFactory(
+            username,
+            password,
+            from_addr,
+            to_addr,
+            msg,
+            d,
+            heloFallback=True,
+            requireAuthentication=require_auth,
+            requireTransportSecurity=require_tls,
+            **kwargs,
+        )
+
+    if _is_old_twisted:
+        # before twisted 21.2, we have to override the ESMTPSender protocol to disable
+        # TLS
+        factory = build_sender_factory()
+
+        if not enable_tls:
+            factory.protocol = _NoTLSESMTPSender
+    else:
+        # for twisted 21.2 and later, there is a 'hostname' parameter which we should
+        # set to enable TLS.
+        factory = build_sender_factory(hostname=smtphost if enable_tls else None)
 
     # the IReactorTCP interface claims host has to be a bytes, which seems to be wrong
     reactor.connectTCP(smtphost, smtpport, factory, timeout=30, bindAddress=None)  # type: ignore[arg-type]
@@ -154,5 +183,5 @@ class SendEmailHandler:
             password=self._smtp_pass,
             require_auth=self._smtp_user is not None,
             require_tls=self._require_transport_security,
-            tls_hostname=self._smtp_host if self._enable_tls else None,
+            enable_tls=self._enable_tls,
         )
diff --git a/tests/handlers/test_send_email.py b/tests/handlers/test_send_email.py
new file mode 100644
index 0000000000..6f77b1237c
--- /dev/null
+++ b/tests/handlers/test_send_email.py
@@ -0,0 +1,112 @@
+# Copyright 2021 The Matrix.org Foundation C.I.C.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+
+from typing import List, Tuple
+
+from zope.interface import implementer
+
+from twisted.internet import defer
+from twisted.internet.address import IPv4Address
+from twisted.internet.defer import ensureDeferred
+from twisted.mail import interfaces, smtp
+
+from tests.server import FakeTransport
+from tests.unittest import HomeserverTestCase
+
+
+@implementer(interfaces.IMessageDelivery)
+class _DummyMessageDelivery:
+    def __init__(self):
+        # (recipient, message) tuples
+        self.messages: List[Tuple[smtp.Address, bytes]] = []
+
+    def receivedHeader(self, helo, origin, recipients):
+        return None
+
+    def validateFrom(self, helo, origin):
+        return origin
+
+    def record_message(self, recipient: smtp.Address, message: bytes):
+        self.messages.append((recipient, message))
+
+    def validateTo(self, user: smtp.User):
+        return lambda: _DummyMessage(self, user)
+
+
+@implementer(interfaces.IMessageSMTP)
+class _DummyMessage:
+    """IMessageSMTP implementation which saves the message delivered to it
+    to the _DummyMessageDelivery object.
+    """
+
+    def __init__(self, delivery: _DummyMessageDelivery, user: smtp.User):
+        self._delivery = delivery
+        self._user = user
+        self._buffer: List[bytes] = []
+
+    def lineReceived(self, line):
+        self._buffer.append(line)
+
+    def eomReceived(self):
+        message = b"\n".join(self._buffer) + b"\n"
+        self._delivery.record_message(self._user.dest, message)
+        return defer.succeed(b"saved")
+
+    def connectionLost(self):
+        pass
+
+
+class SendEmailHandlerTestCase(HomeserverTestCase):
+    def test_send_email(self):
+        """Happy-path test that we can send email to a non-TLS server."""
+        h = self.hs.get_send_email_handler()
+        d = ensureDeferred(
+            h.send_email(
+                "foo@bar.com", "test subject", "Tests", "HTML content", "Text content"
+            )
+        )
+        # there should be an attempt to connect to localhost:25
+        self.assertEqual(len(self.reactor.tcpClients), 1)
+        (host, port, client_factory, _timeout, _bindAddress) = self.reactor.tcpClients[
+            0
+        ]
+        self.assertEqual(host, "localhost")
+        self.assertEqual(port, 25)
+
+        # wire it up to an SMTP server
+        message_delivery = _DummyMessageDelivery()
+        server_protocol = smtp.ESMTP()
+        server_protocol.delivery = message_delivery
+        # make sure that the server uses the test reactor to set timeouts
+        server_protocol.callLater = self.reactor.callLater  # type: ignore[assignment]
+
+        client_protocol = client_factory.buildProtocol(None)
+        client_protocol.makeConnection(FakeTransport(server_protocol, self.reactor))
+        server_protocol.makeConnection(
+            FakeTransport(
+                client_protocol,
+                self.reactor,
+                peer_address=IPv4Address("TCP", "127.0.0.1", 1234),
+            )
+        )
+
+        # the message should now get delivered
+        self.get_success(d, by=0.1)
+
+        # check it arrived
+        self.assertEqual(len(message_delivery.messages), 1)
+        user, msg = message_delivery.messages.pop()
+        self.assertEqual(str(user), "foo@bar.com")
+        self.assertIn(b"Subject: test subject", msg)
diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py
index 0c9cbb9aff..50100a5ae4 100644
--- a/tests/rest/client/v1/test_rooms.py
+++ b/tests/rest/client/v1/test_rooms.py
@@ -29,7 +29,7 @@ from synapse.api.constants import EventContentFields, EventTypes, Membership
 from synapse.api.errors import HttpResponseException
 from synapse.handlers.pagination import PurgeStatus
 from synapse.rest import admin
-from synapse.rest.client import account, directory, login, profile, room
+from synapse.rest.client import account, directory, login, profile, room, sync
 from synapse.types import JsonDict, RoomAlias, UserID, create_requester
 from synapse.util.stringutils import random_string
 
@@ -381,6 +381,8 @@ class RoomPermissionsTestCase(RoomBase):
 class RoomsMemberListTestCase(RoomBase):
     """Tests /rooms/$room_id/members/list REST events."""
 
+    servlets = RoomBase.servlets + [sync.register_servlets]
+
     user_id = "@sid1:red"
 
     def test_get_member_list(self):
@@ -397,6 +399,86 @@ class RoomsMemberListTestCase(RoomBase):
         channel = self.make_request("GET", "/rooms/%s/members" % room_id)
         self.assertEquals(403, channel.code, msg=channel.result["body"])
 
+    def test_get_member_list_no_permission_with_at_token(self):
+        """
+        Tests that a stranger to the room cannot get the member list
+        (in the case that they use an at token).
+        """
+        room_id = self.helper.create_room_as("@someone.else:red")
+
+        # first sync to get an at token
+        channel = self.make_request("GET", "/sync")
+        self.assertEquals(200, channel.code)
+        sync_token = channel.json_body["next_batch"]
+
+        # check that permission is denied for @sid1:red to get the
+        # memberships of @someone.else:red's room.
+        channel = self.make_request(
+            "GET",
+            f"/rooms/{room_id}/members?at={sync_token}",
+        )
+        self.assertEquals(403, channel.code, msg=channel.result["body"])
+
+    def test_get_member_list_no_permission_former_member(self):
+        """
+        Tests that a former member of the room can not get the member list.
+        """
+        # create a room, invite the user and the user joins
+        room_id = self.helper.create_room_as("@alice:red")
+        self.helper.invite(room_id, "@alice:red", self.user_id)
+        self.helper.join(room_id, self.user_id)
+
+        # check that the user can see the member list to start with
+        channel = self.make_request("GET", "/rooms/%s/members" % room_id)
+        self.assertEquals(200, channel.code, msg=channel.result["body"])
+
+        # ban the user
+        self.helper.change_membership(room_id, "@alice:red", self.user_id, "ban")
+
+        # check the user can no longer see the member list
+        channel = self.make_request("GET", "/rooms/%s/members" % room_id)
+        self.assertEquals(403, channel.code, msg=channel.result["body"])
+
+    def test_get_member_list_no_permission_former_member_with_at_token(self):
+        """
+        Tests that a former member of the room can not get the member list
+        (in the case that they use an at token).
+        """
+        # create a room, invite the user and the user joins
+        room_id = self.helper.create_room_as("@alice:red")
+        self.helper.invite(room_id, "@alice:red", self.user_id)
+        self.helper.join(room_id, self.user_id)
+
+        # sync to get an at token
+        channel = self.make_request("GET", "/sync")
+        self.assertEquals(200, channel.code)
+        sync_token = channel.json_body["next_batch"]
+
+        # check that the user can see the member list to start with
+        channel = self.make_request(
+            "GET", "/rooms/%s/members?at=%s" % (room_id, sync_token)
+        )
+        self.assertEquals(200, channel.code, msg=channel.result["body"])
+
+        # ban the user (Note: the user is actually allowed to see this event and
+        # state so that they know they're banned!)
+        self.helper.change_membership(room_id, "@alice:red", self.user_id, "ban")
+
+        # invite a third user and let them join
+        self.helper.invite(room_id, "@alice:red", "@bob:red")
+        self.helper.join(room_id, "@bob:red")
+
+        # now, with the original user, sync again to get a new at token
+        channel = self.make_request("GET", "/sync")
+        self.assertEquals(200, channel.code)
+        sync_token = channel.json_body["next_batch"]
+
+        # check the user can no longer see the updated member list
+        channel = self.make_request(
+            "GET", "/rooms/%s/members?at=%s" % (room_id, sync_token)
+        )
+        self.assertEquals(403, channel.code, msg=channel.result["body"])
+
     def test_get_member_list_mixed_memberships(self):
         room_creator = "@some_other_guy:red"
         room_id = self.helper.create_room_as(room_creator)
diff --git a/tests/rest/client/v2_alpha/test_groups.py b/tests/rest/client/v2_alpha/test_groups.py
new file mode 100644
index 0000000000..bfa9336baa
--- /dev/null
+++ b/tests/rest/client/v2_alpha/test_groups.py
@@ -0,0 +1,43 @@
+from synapse.rest.client.v1 import room
+from synapse.rest.client.v2_alpha import groups
+
+from tests import unittest
+from tests.unittest import override_config
+
+
+class GroupsTestCase(unittest.HomeserverTestCase):
+    user_id = "@alice:test"
+    room_creator_user_id = "@bob:test"
+
+    servlets = [room.register_servlets, groups.register_servlets]
+
+    @override_config({"enable_group_creation": True})
+    def test_rooms_limited_by_visibility(self):
+        group_id = "+spqr:test"
+
+        # Alice creates a group
+        channel = self.make_request("POST", "/create_group", {"localpart": "spqr"})
+        self.assertEquals(channel.code, 200, msg=channel.text_body)
+        self.assertEquals(channel.json_body, {"group_id": group_id})
+
+        # Bob creates a private room
+        room_id = self.helper.create_room_as(self.room_creator_user_id, is_public=False)
+        self.helper.auth_user_id = self.room_creator_user_id
+        self.helper.send_state(
+            room_id, "m.room.name", {"name": "bob's secret room"}, tok=None
+        )
+        self.helper.auth_user_id = self.user_id
+
+        # Alice adds the room to her group.
+        channel = self.make_request(
+            "PUT", f"/groups/{group_id}/admin/rooms/{room_id}", {}
+        )
+        self.assertEquals(channel.code, 200, msg=channel.text_body)
+        self.assertEquals(channel.json_body, {})
+
+        # Alice now tries to retrieve the room list of the space.
+        channel = self.make_request("GET", f"/groups/{group_id}/rooms")
+        self.assertEquals(channel.code, 200, msg=channel.text_body)
+        self.assertEquals(
+            channel.json_body, {"chunk": [], "total_room_count_estimate": 0}
+        )
diff --git a/tests/server.py b/tests/server.py
index 6fddd3b305..b861c7b866 100644
--- a/tests/server.py
+++ b/tests/server.py
@@ -10,9 +10,10 @@ from zope.interface import implementer
 
 from twisted.internet import address, threads, udp
 from twisted.internet._resolver import SimpleResolverComplexifier
-from twisted.internet.defer import Deferred, fail, succeed
+from twisted.internet.defer import Deferred, fail, maybeDeferred, succeed
 from twisted.internet.error import DNSLookupError
 from twisted.internet.interfaces import (
+    IAddress,
     IHostnameResolver,
     IProtocol,
     IPullProducer,
@@ -511,6 +512,9 @@ class FakeTransport:
     will get called back for connectionLost() notifications etc.
     """
 
+    _peer_address: Optional[IAddress] = attr.ib(default=None)
+    """The value to be returend by getPeer"""
+
     disconnecting = False
     disconnected = False
     connected = True
@@ -519,7 +523,7 @@ class FakeTransport:
     autoflush = attr.ib(default=True)
 
     def getPeer(self):
-        return None
+        return self._peer_address
 
     def getHost(self):
         return None
@@ -572,7 +576,12 @@ class FakeTransport:
         self.producerStreaming = streaming
 
         def _produce():
-            d = self.producer.resumeProducing()
+            if not self.producer:
+                # we've been unregistered
+                return
+            # some implementations of IProducer (for example, FileSender)
+            # don't return a deferred.
+            d = maybeDeferred(self.producer.resumeProducing)
             d.addCallback(lambda x: self._reactor.callLater(0.1, _produce))
 
         if not streaming: