Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Prevent showing non-fed rooms in fed /publicRooms #4736

Merged
merged 15 commits into from
Feb 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion synapse/federation/transport/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -736,7 +736,8 @@ def on_GET(self, origin, content, query):

data = yield self.handler.get_local_public_room_list(
limit, since_token,
network_tuple=network_tuple
network_tuple=network_tuple,
from_federation=True,
)
defer.returnValue((200, data))

Expand Down
6 changes: 2 additions & 4 deletions synapse/groups/groups_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,7 @@ def get_group_summary(self, group_id, requester_user_id):
room_id = room_entry["room_id"]
joined_users = yield self.store.get_users_in_room(room_id)
entry = yield 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=True,
)
entry = dict(entry) # so we don't change whats cached
entry.pop("room_id", None)
Expand Down Expand Up @@ -544,8 +543,7 @@ def get_rooms_in_group(self, group_id, requester_user_id):

joined_users = yield self.store.get_users_in_room(room_id)
entry = yield 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=True,
)

if not entry:
Expand Down
76 changes: 65 additions & 11 deletions synapse/handlers/room_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,19 +47,21 @@ def __init__(self, hs):
self.response_cache = ResponseCache(hs, "room_list")
self.remote_response_cache = ResponseCache(hs, "remote_room_list",
timeout_ms=30 * 1000)
self.config = hs.get_config()

def get_local_public_room_list(self, limit=None, since_token=None,
search_filter=None,
network_tuple=EMPTY_THIRD_PARTY_ID,):
network_tuple=EMPTY_THIRD_PARTY_ID,
from_federation=False):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docstring pls

"""Generate a local public room list.

There are multiple different lists: the main one plus one per third
party network. A client can ask for a specific list or to return all.

Args:
limit (int)
since_token (str)
search_filter (dict)
limit (int|None)
since_token (str|None)
search_filter (dict|None)
network_tuple (ThirdPartyInstanceID): Which public list to use.
This can be (None, None) to indicate the main list, or a particular
appservice and network id to use an appservice specific one.
Expand Down Expand Up @@ -87,14 +89,31 @@ def get_local_public_room_list(self, limit=None, since_token=None,
return self.response_cache.wrap(
key,
self._get_public_room_list,
limit, since_token, network_tuple=network_tuple,
limit, since_token,
network_tuple=network_tuple, from_federation=from_federation,
)

@defer.inlineCallbacks
def _get_public_room_list(self, limit=None, since_token=None,
search_filter=None,
network_tuple=EMPTY_THIRD_PARTY_ID,
from_federation=False,
timeout=None,):
"""Generate a public room list.

Args:
limit (int|None): Maximum amount of rooms to return.
since_token (str|None)
search_filter (dict|None): Dictionary to filter rooms by.
network_tuple (ThirdPartyInstanceID): Which public list to use.
This can be (None, None) to indicate the main list, or a particular
appservice and network id to use an appservice specific one.
Setting to None returns all public rooms across all lists.
from_federation (bool): Whether this request originated from a
federating server or a client. Used for room filtering.
timeout (int|None): Amount of seconds to wait for a response before
timing out.
"""
if since_token and since_token != "END":
since_token = RoomListNextBatch.from_token(since_token)
else:
Expand Down Expand Up @@ -217,7 +236,8 @@ def get_order_for_room(room_id):
yield concurrently_execute(
lambda r: self._append_room_entry_to_chunk(
r, rooms_to_num_joined[r],
chunk, limit, search_filter
chunk, limit, search_filter,
from_federation=from_federation,
),
batch, 5,
)
Expand Down Expand Up @@ -288,23 +308,51 @@ def get_order_for_room(room_id):

@defer.inlineCallbacks
def _append_room_entry_to_chunk(self, room_id, num_joined_users, chunk, limit,
search_filter):
search_filter, from_federation=False):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docstring for from_federation wouldn't hurt.

"""Generate the entry for a room in the public room list and append it
to the `chunk` if it matches the search filter

Args:
room_id (str): The ID of the room.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, if you're going to add these, please get the types right

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

room_id (str)

Are they supposed to be RoomIDs?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you meant adding |None. Have done so for the other types yes.

num_joined_users (int): The number of joined users in the room.
chunk (list)
limit (int|None): Maximum amount of rooms to display. Function will
return if length of chunk is greater than limit + 1.
search_filter (dict|None)
from_federation (bool): Whether this request originated from a
federating server or a client. Used for room filtering.
"""
if limit and len(chunk) > limit + 1:
# We've already got enough, so lets just drop it.
return

result = yield self.generate_room_entry(room_id, num_joined_users)
if not result:
return

if from_federation and not result.get("m.federate", True):
# This is a room that other servers cannot join. Do not show them
# this room.
return

if result and _matches_room_entry(result, search_filter):
if _matches_room_entry(result, search_filter):
chunk.append(result)

@cachedInlineCallbacks(num_args=1, cache_context=True)
def generate_room_entry(self, room_id, num_joined_users, cache_context,
with_alias=True, allow_private=False):
@cachedInlineCallbacks(num_args=2, cache_context=True)
def generate_room_entry(self, room_id, num_joined_users,
cache_context, with_alias=True, allow_private=False):
"""Returns the entry for a room

Args:
room_id (str): The room's ID.
num_joined_users (int): Number of users in the room.
cache_context: Information for cached responses.
with_alias (bool): Whether to return the room's aliases in the result.
allow_private (bool): Whether invite-only rooms should be shown.

Returns:
Deferred[dict|None]: Returns a room entry as a dictionary, or None if this
room was determined not to be shown publicly.
"""
result = {
"room_id": room_id,
Expand All @@ -318,6 +366,7 @@ def generate_room_entry(self, room_id, num_joined_users, cache_context,
event_map = yield self.store.get_events([
event_id for key, event_id in iteritems(current_state_ids)
if key[0] in (
EventTypes.Create,
EventTypes.JoinRules,
EventTypes.Name,
EventTypes.Topic,
Expand All @@ -334,12 +383,17 @@ def generate_room_entry(self, room_id, num_joined_users, cache_context,
}

# Double check that this is actually a public room.

join_rules_event = current_state.get((EventTypes.JoinRules, ""))
if join_rules_event:
join_rule = join_rules_event.content.get("join_rule", None)
if not allow_private and join_rule and join_rule != JoinRules.PUBLIC:
defer.returnValue(None)

# Return whether this room is open to federation users or not
create_event = current_state.get((EventTypes.Create, ""))
result["m.federate"] = create_event.content.get("m.federate", True)

if with_alias:
aliases = yield self.store.get_aliases_for_room(
room_id, on_invalidate=cache_context.invalidate
Expand Down
2 changes: 1 addition & 1 deletion synapse/server.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ import synapse.handlers.auth
import synapse.handlers.deactivate_account
import synapse.handlers.device
import synapse.handlers.e2e_keys
import synapse.handlers.message
import synapse.handlers.room
import synapse.handlers.room_member
import synapse.handlers.message
import synapse.handlers.set_password
import synapse.rest.media.v1.media_repository
import synapse.server_notices.server_notices_manager
Expand Down