From ac6bfcd52f03e9574324978f83a281cf35f4ea89 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 18 May 2021 12:17:04 -0400 Subject: Refactor checking restricted join rules (#10007) To be more consistent with similar code. The check now automatically raises an AuthError instead of passing back a boolean. It also absorbs some shared logic between callers. --- synapse/handlers/event_auth.py | 51 ++++++++++++++++++++++++++++------------- synapse/handlers/federation.py | 29 ++++++++--------------- synapse/handlers/room_member.py | 20 ++++------------ 3 files changed, 49 insertions(+), 51 deletions(-) (limited to 'synapse') diff --git a/synapse/handlers/event_auth.py b/synapse/handlers/event_auth.py index eff639f407..5b2fe103e7 100644 --- a/synapse/handlers/event_auth.py +++ b/synapse/handlers/event_auth.py @@ -11,10 +11,12 @@ # 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 +from typing import TYPE_CHECKING, Optional -from synapse.api.constants import EventTypes, JoinRules +from synapse.api.constants import EventTypes, JoinRules, Membership +from synapse.api.errors import AuthError from synapse.api.room_versions import RoomVersion +from synapse.events import EventBase from synapse.types import StateMap if TYPE_CHECKING: @@ -29,44 +31,58 @@ class EventAuthHandler: def __init__(self, hs: "HomeServer"): self._store = hs.get_datastore() - async def can_join_without_invite( - self, state_ids: StateMap[str], room_version: RoomVersion, user_id: str - ) -> bool: + async def check_restricted_join_rules( + self, + state_ids: StateMap[str], + room_version: RoomVersion, + user_id: str, + prev_member_event: Optional[EventBase], + ) -> None: """ - Check whether a user can join a room without an invite. + Check whether a user can join a room without an invite due to restricted join rules. When joining a room with restricted joined rules (as defined in MSC3083), - the membership of spaces must be checked during join. + the membership of spaces must be checked during a room join. Args: state_ids: The state of the room as it currently is. room_version: The room version of the room being joined. user_id: The user joining the room. + prev_member_event: The current membership event for this user. - Returns: - True if the user can join the room, false otherwise. + Raises: + AuthError if the user cannot join the room. """ + # If the member is invited or currently joined, then nothing to do. + if prev_member_event and ( + prev_member_event.membership in (Membership.JOIN, Membership.INVITE) + ): + return + # This only applies to room versions which support the new join rule. if not room_version.msc3083_join_rules: - return True + return # 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 True + 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 True + 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): - return False + allowed_spaces = () # Get the list of joined rooms and see if there's an overlap. - joined_rooms = await self._store.get_rooms_for_user(user_id) + if allowed_spaces: + joined_rooms = await self._store.get_rooms_for_user(user_id) + else: + joined_rooms = () # Pull out the other room IDs, invalid data gets filtered. for space in allowed_spaces: @@ -80,7 +96,10 @@ class EventAuthHandler: # The user was joined to one of the spaces specified, they can join # this room! if space_id in joined_rooms: - return True + return # The user was not in any of the required spaces. - return False + raise AuthError( + 403, + "You do not belong to any of the required spaces to join this room.", + ) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 798ed75b30..678f6b7707 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -1668,28 +1668,17 @@ class FederationHandler(BaseHandler): # Check if the user is already in the room or invited to the room. user_id = event.state_key prev_member_event_id = prev_state_ids.get((EventTypes.Member, user_id), None) - newly_joined = True - user_is_invited = False + prev_member_event = None if prev_member_event_id: prev_member_event = await self.store.get_event(prev_member_event_id) - newly_joined = prev_member_event.membership != Membership.JOIN - user_is_invited = prev_member_event.membership == Membership.INVITE - - # If the member is not already in the room, and not invited, check if - # they should be allowed access via membership in a space. - if ( - newly_joined - and not user_is_invited - and not await self._event_auth_handler.can_join_without_invite( - prev_state_ids, - event.room_version, - user_id, - ) - ): - raise AuthError( - 403, - "You do not belong to any of the required spaces to join this room.", - ) + + # Check if the member should be allowed access via membership in a space. + await self._event_auth_handler.check_restricted_join_rules( + prev_state_ids, + event.room_version, + user_id, + prev_member_event, + ) # Persist the event. await self._auth_and_persist_event(origin, event, context) diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 9a092da715..d6fc43e798 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -260,25 +260,15 @@ class RoomMemberHandler(metaclass=abc.ABCMeta): if event.membership == Membership.JOIN: newly_joined = True - user_is_invited = False + prev_member_event = None if prev_member_event_id: prev_member_event = await self.store.get_event(prev_member_event_id) newly_joined = prev_member_event.membership != Membership.JOIN - user_is_invited = prev_member_event.membership == Membership.INVITE - # If the member is not already in the room and is not accepting an invite, - # check if they should be allowed access via membership in a space. - if ( - newly_joined - and not user_is_invited - and not await self.event_auth_handler.can_join_without_invite( - prev_state_ids, event.room_version, user_id - ) - ): - raise AuthError( - 403, - "You do not belong to any of the required spaces to join this room.", - ) + # Check if the member should be allowed access via membership in a space. + await self.event_auth_handler.check_restricted_join_rules( + prev_state_ids, event.room_version, user_id, prev_member_event + ) # Only rate-limit if the user actually joined the room, otherwise we'll end # up blocking profile updates. -- cgit 1.4.1