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/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}
+ )
|