diff options
| -rw-r--r-- | changelog.d/9922.feature | 1 | ||||
| -rw-r--r-- | synapse/federation/transport/server.py | 2 | ||||
| -rw-r--r-- | synapse/handlers/event_auth.py | 104 | ||||
| -rw-r--r-- | synapse/handlers/space_summary.py | 201 |
4 files changed, 254 insertions, 54 deletions
diff --git a/changelog.d/9922.feature b/changelog.d/9922.feature new file mode 100644 index 0000000000..2c655350c0 --- /dev/null +++ b/changelog.d/9922.feature @@ -0,0 +1 @@ +Experimental support to allow a user who could join a restricted room to view it in the spaces summary. diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py index e1b7462474..c17a085a4f 100644 --- a/synapse/federation/transport/server.py +++ b/synapse/federation/transport/server.py @@ -1428,7 +1428,7 @@ class FederationSpaceSummaryServlet(BaseFederationServlet): ) return 200, await self.handler.federation_space_summary( - room_id, suggested_only, max_rooms_per_space, exclude_rooms + origin, room_id, suggested_only, max_rooms_per_space, exclude_rooms ) diff --git a/synapse/handlers/event_auth.py b/synapse/handlers/event_auth.py index 5b2fe103e7..a0df16a32f 100644 --- a/synapse/handlers/event_auth.py +++ b/synapse/handlers/event_auth.py @@ -11,7 +11,7 @@ # 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 TYPE_CHECKING, Optional +from typing import TYPE_CHECKING, Collection, Optional from synapse.api.constants import EventTypes, JoinRules, Membership from synapse.api.errors import AuthError @@ -59,32 +59,76 @@ class EventAuthHandler: ): return + # This is not a room with a restricted join rule, so we don't need to do the + # restricted room specific checks. + # + # Note: We'll be applying the standard join rule checks later, which will + # catch the cases of e.g. trying to join private rooms without an invite. + if not await self.has_restricted_join_rules(state_ids, room_version): + return + + # Get the spaces which allow access to this room and check if the user is + # in any of them. + allowed_spaces = await self.get_spaces_that_allow_join(state_ids) + if not await self.is_user_in_rooms(allowed_spaces, user_id): + raise AuthError( + 403, + "You do not belong to any of the required spaces to join this room.", + ) + + async def has_restricted_join_rules( + self, state_ids: StateMap[str], room_version: RoomVersion + ) -> bool: + """ + Return if the room has the proper join rules set for access via spaces. + + Args: + state_ids: The state of the room as it currently is. + room_version: The room version of the room to query. + + Returns: + True if the proper room version and join rules are set for restricted access. + """ # This only applies to room versions which support the new join rule. if not room_version.msc3083_join_rules: - return + return False # If there's no join rule, then it defaults to invite (so this doesn't apply). join_rules_event_id = state_ids.get((EventTypes.JoinRules, ""), None) if not join_rules_event_id: - return + return False + + # If the join rule is not restricted, this doesn't apply. + join_rules_event = await self._store.get_event(join_rules_event_id) + return join_rules_event.content.get("join_rule") == JoinRules.MSC3083_RESTRICTED + + async def get_spaces_that_allow_join( + self, state_ids: StateMap[str] + ) -> Collection[str]: + """ + Generate a list of spaces which allow access to a room. + + Args: + state_ids: The state of the room as it currently is. + + Returns: + A collection of spaces which provide membership to the room. + """ + # If there's no join rule, then it defaults to invite (so this doesn't apply). + join_rules_event_id = state_ids.get((EventTypes.JoinRules, ""), None) + if not join_rules_event_id: + return () # If the join rule is not restricted, this doesn't apply. join_rules_event = await self._store.get_event(join_rules_event_id) - if join_rules_event.content.get("join_rule") != JoinRules.MSC3083_RESTRICTED: - return # If allowed is of the wrong form, then only allow invited users. allowed_spaces = join_rules_event.content.get("allow", []) if not isinstance(allowed_spaces, list): - allowed_spaces = () - - # Get the list of joined rooms and see if there's an overlap. - if allowed_spaces: - joined_rooms = await self._store.get_rooms_for_user(user_id) - else: - joined_rooms = () + return () # Pull out the other room IDs, invalid data gets filtered. + result = [] for space in allowed_spaces: if not isinstance(space, dict): continue @@ -93,13 +137,31 @@ class EventAuthHandler: if not isinstance(space_id, str): continue - # The user was joined to one of the spaces specified, they can join - # this room! - if space_id in joined_rooms: - return + result.append(space_id) + + return result + + async def is_user_in_rooms(self, room_ids: Collection[str], user_id: str) -> bool: + """ + Check whether a user is a member of any of the provided rooms. + + Args: + room_ids: The rooms to check for membership. + user_id: The user to check. + + Returns: + True if the user is in any of the rooms, false otherwise. + """ + if not room_ids: + return False + + # Get the list of joined rooms and see if there's an overlap. + joined_rooms = await self._store.get_rooms_for_user(user_id) + + # Check each room and see if the user is in it. + for room_id in room_ids: + if room_id in joined_rooms: + return True - # The user was not in any of the required spaces. - raise AuthError( - 403, - "You do not belong to any of the required spaces to join this room.", - ) + # The user was not in any of the rooms. + return False diff --git a/synapse/handlers/space_summary.py b/synapse/handlers/space_summary.py index eb80a5ad67..8d49ba8164 100644 --- a/synapse/handlers/space_summary.py +++ b/synapse/handlers/space_summary.py @@ -16,11 +16,16 @@ import itertools import logging import re from collections import deque -from typing import TYPE_CHECKING, Iterable, List, Optional, Sequence, Set, Tuple, cast +from typing import TYPE_CHECKING, Iterable, List, Optional, Sequence, Set, Tuple import attr -from synapse.api.constants import EventContentFields, EventTypes, HistoryVisibility +from synapse.api.constants import ( + EventContentFields, + EventTypes, + HistoryVisibility, + Membership, +) from synapse.api.errors import AuthError from synapse.events import EventBase from synapse.events.utils import format_event_for_client_v2 @@ -47,6 +52,7 @@ class SpaceSummaryHandler: self._auth = hs.get_auth() self._room_list_handler = hs.get_room_list_handler() self._state_handler = hs.get_state_handler() + self._event_auth_handler = hs.get_event_auth_handler() self._store = hs.get_datastore() self._event_serializer = hs.get_event_client_serializer() self._server_name = hs.hostname @@ -111,28 +117,88 @@ class SpaceSummaryHandler: max_children = max_rooms_per_space if processed_rooms else None if is_in_room: - rooms, events = await self._summarize_local_room( - requester, room_id, suggested_only, max_children + room, events = await self._summarize_local_room( + requester, None, room_id, suggested_only, max_children ) + + logger.debug( + "Query of local room %s returned events %s", + room_id, + ["%s->%s" % (ev["room_id"], ev["state_key"]) for ev in events], + ) + + if room: + rooms_result.append(room) else: - rooms, events = await self._summarize_remote_room( + fed_rooms, fed_events = await self._summarize_remote_room( queue_entry, suggested_only, max_children, exclude_rooms=processed_rooms, ) - logger.debug( - "Query of %s returned rooms %s, events %s", - queue_entry.room_id, - [room.get("room_id") for room in rooms], - ["%s->%s" % (ev["room_id"], ev["state_key"]) for ev in events], - ) - - rooms_result.extend(rooms) - - # any rooms returned don't need visiting again - processed_rooms.update(cast(str, room.get("room_id")) for room in rooms) + # The results over federation might include rooms that the we, + # as the requesting server, are allowed to see, but the requesting + # user is not permitted see. + # + # Filter the returned results to only what is accessible to the user. + room_ids = set() + events = [] + for room in fed_rooms: + fed_room_id = room.get("room_id") + if not fed_room_id or not isinstance(fed_room_id, str): + continue + + # The room should only be included in the summary if: + # a. the user is in the room; + # b. the room is world readable; or + # c. the user is in a space that has been granted access to + # the room. + # + # Note that we know the user is not in the root room (which is + # why the remote call was made in the first place), but the user + # could be in one of the children rooms and we just didn't know + # about the link. + include_room = room.get("world_readable") is True + + # Check if the user is a member of any of the allowed spaces + # from the response. + allowed_spaces = room.get("allowed_spaces") + if ( + not include_room + and allowed_spaces + and isinstance(allowed_spaces, list) + ): + include_room = await self._event_auth_handler.is_user_in_rooms( + allowed_spaces, requester + ) + + # Finally, if this isn't the requested room, check ourselves + # if we can access the room. + if not include_room and fed_room_id != queue_entry.room_id: + include_room = await self._is_room_accessible( + fed_room_id, requester, None + ) + + # The user can see the room, include it! + if include_room: + rooms_result.append(room) + room_ids.add(fed_room_id) + + # All rooms returned don't need visiting again (even if the user + # didn't have access to them). + processed_rooms.add(fed_room_id) + + for event in fed_events: + if event.get("room_id") in room_ids: + events.append(event) + + logger.debug( + "Query of %s returned rooms %s, events %s", + room_id, + [room.get("room_id") for room in fed_rooms], + ["%s->%s" % (ev["room_id"], ev["state_key"]) for ev in fed_events], + ) # the room we queried may or may not have been returned, but don't process # it again, anyway. @@ -158,10 +224,16 @@ class SpaceSummaryHandler: ) processed_events.add(ev_key) + # Before returning to the client, remove the allowed_spaces key for any + # rooms. + for room in rooms_result: + room.pop("allowed_spaces", None) + return {"rooms": rooms_result, "events": events_result} async def federation_space_summary( self, + origin: str, room_id: str, suggested_only: bool, max_rooms_per_space: Optional[int], @@ -171,6 +243,8 @@ class SpaceSummaryHandler: Implementation of the space summary Federation API Args: + origin: The server requesting the spaces summary. + room_id: room id to start the summary at suggested_only: whether we should only return children with the "suggested" @@ -205,14 +279,15 @@ class SpaceSummaryHandler: logger.debug("Processing room %s", room_id) - rooms, events = await self._summarize_local_room( - None, room_id, suggested_only, max_rooms_per_space + room, events = await self._summarize_local_room( + None, origin, room_id, suggested_only, max_rooms_per_space ) processed_rooms.add(room_id) - rooms_result.extend(rooms) - events_result.extend(events) + if room: + rooms_result.append(room) + events_result.extend(events) # add any children to the queue room_queue.extend(edge_event["state_key"] for edge_event in events) @@ -222,10 +297,11 @@ class SpaceSummaryHandler: async def _summarize_local_room( self, requester: Optional[str], + origin: Optional[str], room_id: str, suggested_only: bool, max_children: Optional[int], - ) -> Tuple[Sequence[JsonDict], Sequence[JsonDict]]: + ) -> Tuple[Optional[JsonDict], Sequence[JsonDict]]: """ Generate a room entry and a list of event entries for a given room. @@ -233,6 +309,9 @@ class SpaceSummaryHandler: requester: The user requesting the summary, if it is a local request. None if this is a federation request. + origin: + The server requesting the summary, if it is a federation request. + None if this is a local request. room_id: The room ID to summarize. suggested_only: True if only suggested children should be returned. Otherwise, all children are returned. @@ -247,8 +326,8 @@ class SpaceSummaryHandler: An iterable of the sorted children events. This may be limited to a maximum size or may include all children. """ - if not await self._is_room_accessible(room_id, requester): - return (), () + if not await self._is_room_accessible(room_id, requester, origin): + return None, () room_entry = await self._build_room_entry(room_id) @@ -272,7 +351,7 @@ class SpaceSummaryHandler: event_format=format_event_for_client_v2, ) ) - return (room_entry,), events_result + return room_entry, events_result async def _summarize_remote_room( self, @@ -332,13 +411,17 @@ class SpaceSummaryHandler: or ev.event_type == EventTypes.SpaceChild ) - async def _is_room_accessible(self, room_id: str, requester: Optional[str]) -> bool: + async def _is_room_accessible( + self, room_id: str, requester: Optional[str], origin: Optional[str] + ) -> bool: """ Calculate whether the room should be shown in the spaces summary. It should be included if: * The requester is joined or invited to the room. + * The requester can join without an invite (per MSC3083). + * The origin server has any user that is joined or invited to the room. * The history visibility is set to world readable. Args: @@ -346,31 +429,75 @@ class SpaceSummaryHandler: requester: The user requesting the summary, if it is a local request. None if this is a federation request. + origin: + The server requesting the summary, if it is a federation request. + None if this is a local request. Returns: True if the room should be included in the spaces summary. """ + state_ids = await self._store.get_current_state_ids(room_id) + + # If there's no state for the room, it isn't known. + if not state_ids: + logger.info("room %s is unknown, omitting from summary", room_id) + return False + + room_version = await self._store.get_room_version(room_id) # if we have an authenticated requesting user, first check if they are able to view # stripped state in the room. if requester: + member_event_id = state_ids.get((EventTypes.Member, requester), None) + + # If they're in the room they can see info on it. + member_event = None + if member_event_id: + member_event = await self._store.get_event(member_event_id) + if member_event.membership in (Membership.JOIN, Membership.INVITE): + return True + + # Otherwise, check if they should be allowed access via membership in a space. try: - await self._auth.check_user_in_room(room_id, requester) - return True + await self._event_auth_handler.check_restricted_join_rules( + state_ids, room_version, requester, member_event + ) except AuthError: + # The user doesn't have access due to spaces, but might have access + # another way. Keep trying. pass + else: + return True + + # If this is a request over federation, check if the host is in the room or + # is in one of the spaces specified via the join rules. + elif origin: + if await self._auth.check_host_in_room(room_id, origin): + return True + + # Alternately, if the host has a user in any of the spaces specified + # for access, then the host can see this room (and should do filtering + # if the requester cannot see it). + if await self._event_auth_handler.has_restricted_join_rules( + state_ids, room_version + ): + allowed_spaces = ( + await self._event_auth_handler.get_spaces_that_allow_join(state_ids) + ) + for space_id in allowed_spaces: + if await self._auth.check_host_in_room(space_id, origin): + return True # otherwise, check if the room is peekable - hist_vis_ev = await self._state_handler.get_current_state( - room_id, EventTypes.RoomHistoryVisibility, "" - ) - if hist_vis_ev: + hist_vis_event_id = state_ids.get((EventTypes.RoomHistoryVisibility, ""), None) + if hist_vis_event_id: + hist_vis_ev = await self._store.get_event(hist_vis_event_id) hist_vis = hist_vis_ev.content.get("history_visibility") if hist_vis == HistoryVisibility.WORLD_READABLE: return True logger.info( - "room %s is unpeekable and user %s is not a member, omitting from summary", + "room %s is unpeekable and user %s is not a member / not allowed to join, omitting from summary", room_id, requester, ) @@ -395,6 +522,15 @@ class SpaceSummaryHandler: if not room_type: room_type = create_event.content.get(EventContentFields.MSC1772_ROOM_TYPE) + room_version = await self._store.get_room_version(room_id) + allowed_spaces = None + if await self._event_auth_handler.has_restricted_join_rules( + current_state_ids, room_version + ): + allowed_spaces = await self._event_auth_handler.get_spaces_that_allow_join( + current_state_ids + ) + entry = { "room_id": stats["room_id"], "name": stats["name"], @@ -408,6 +544,7 @@ class SpaceSummaryHandler: "guest_can_join": stats["guest_access"] == "can_join", "creation_ts": create_event.origin_server_ts, "room_type": room_type, + "allowed_spaces": allowed_spaces, } # Filter out Nones – rather omit the field altogether |