From b58d17e44f9d9ff7e70578e0f4e328bb9113ec7e Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 18 Feb 2020 23:13:29 +0000 Subject: [PATCH] Refactor the membership check methods in Auth these were getting a bit unwieldy, so let's combine `check_joined_room` and `check_user_was_in_room` into a single `check_user_in_room`. --- synapse/api/auth.py | 80 +++++++++++++++----------------- synapse/handlers/initial_sync.py | 31 ++----------- synapse/handlers/typing.py | 4 +- tests/handlers/test_typing.py | 4 +- 4 files changed, 46 insertions(+), 73 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 8b1277ad02..de7b75ca36 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -14,6 +14,7 @@ # limitations under the License. import logging +from typing import Optional from six import itervalues @@ -35,6 +36,7 @@ from synapse.api.errors import ( ) from synapse.api.room_versions import KNOWN_ROOM_VERSIONS from synapse.config.server import is_threepid_reserved +from synapse.events import EventBase from synapse.types import StateMap, UserID from synapse.util.caches import CACHE_SIZE_FACTOR, register_cache from synapse.util.caches.lrucache import LruCache @@ -92,20 +94,34 @@ class Auth(object): ) @defer.inlineCallbacks - def check_joined_room(self, room_id, user_id, current_state=None): - """Check if the user is currently joined in the room + def check_user_in_room( + self, + room_id: str, + user_id: str, + current_state: Optional[StateMap[EventBase]] = None, + allow_departed_users: bool = False, + ): + """Check if the user is in the room, or was at some point. Args: - room_id(str): The room to check. - user_id(str): The user to check. - current_state(dict): Optional map of the current state of the room. + room_id: The room to check. + + user_id: The user to check. + + current_state: Optional map of the current state of the room. If provided then that map is used to check whether they are a member of the room. Otherwise the current membership is loaded from the database. + + allow_departed_users: if True, accept users that were previously + members but have now departed. + Raises: - AuthError if the user is not in the room. + AuthError if the user is/was not in the room. Returns: - A deferred membership event for the user if the user is in - the room. + Deferred[Optional[EventBase]]: + Membership event for the user if the user was in the + room. This will be the join event if they are currently joined to + the room. This will be the leave event if they have left the room. """ if current_state: member = current_state.get((EventTypes.Member, user_id), None) @@ -113,37 +129,19 @@ class Auth(object): member = yield self.state.get_current_state( room_id=room_id, event_type=EventTypes.Member, state_key=user_id ) - - self._check_joined_room(member, user_id, room_id) - return member - - @defer.inlineCallbacks - def check_user_was_in_room(self, room_id, user_id): - """Check if the user was in the room at some point. - Args: - room_id(str): The room to check. - user_id(str): The user to check. - Raises: - AuthError if the user was never in the room. - Returns: - A deferred membership event for the user if the user was in the - room. This will be the join event if they are currently joined to - the room. This will be the leave event if they have left the room. - """ - member = yield self.state.get_current_state( - room_id=room_id, event_type=EventTypes.Member, state_key=user_id - ) membership = member.membership if member else None - if membership not in (Membership.JOIN, Membership.LEAVE): - raise AuthError(403, "User %s not in room %s" % (user_id, room_id)) + if membership == Membership.JOIN: + return member - if membership == Membership.LEAVE: + # XXX this looks totally bogus. Why do we not allow users who have been banned, + # or those who were members previously and have been re-invited? + if allow_departed_users and membership == Membership.LEAVE: forgot = yield self.store.did_forget(user_id, room_id) - if forgot: - raise AuthError(403, "User %s not in room %s" % (user_id, room_id)) + if not forgot: + return member - return member + raise AuthError(403, "User %s not in room %s" % (user_id, room_id)) @defer.inlineCallbacks def check_host_in_room(self, room_id, host): @@ -151,12 +149,6 @@ class Auth(object): latest_event_ids = yield self.store.is_host_joined(room_id, host) return latest_event_ids - def _check_joined_room(self, member, user_id, room_id): - if not member or member.membership != Membership.JOIN: - raise AuthError( - 403, "User %s not in room %s (%s)" % (user_id, room_id, repr(member)) - ) - def can_federate(self, event, auth_events): creation_event = auth_events.get((EventTypes.Create, "")) @@ -560,7 +552,7 @@ class Auth(object): return True user_id = user.to_string() - yield self.check_joined_room(room_id, user_id) + yield self.check_user_in_room(room_id, user_id) # We currently require the user is a "moderator" in the room. We do this # by checking if they would (theoretically) be able to change the @@ -645,12 +637,14 @@ class Auth(object): """ try: - # check_user_was_in_room will return the most recent membership + # check_user_in_room will return the most recent membership # event for the user if: # * The user is a non-guest user, and was ever in the room # * The user is a guest user, and has joined the room # else it will throw. - member_event = yield self.check_user_was_in_room(room_id, user_id) + member_event = yield self.check_user_in_room( + room_id, user_id, allow_departed_users=True + ) return member_event.membership, member_event.event_id except AuthError: visibility = yield self.state.get_current_state( diff --git a/synapse/handlers/initial_sync.py b/synapse/handlers/initial_sync.py index 2e6755f19c..b7c6a921d9 100644 --- a/synapse/handlers/initial_sync.py +++ b/synapse/handlers/initial_sync.py @@ -18,7 +18,7 @@ import logging from twisted.internet import defer from synapse.api.constants import EventTypes, Membership -from synapse.api.errors import AuthError, Codes, SynapseError +from synapse.api.errors import SynapseError from synapse.events.validator import EventValidator from synapse.handlers.presence import format_user_presence_state from synapse.logging.context import make_deferred_yieldable, run_in_background @@ -274,9 +274,10 @@ class InitialSyncHandler(BaseHandler): user_id = requester.user.to_string() - membership, member_event_id = await self._check_in_room_or_world_readable( - room_id, user_id - ) + ( + membership, + member_event_id, + ) = await self.auth.check_user_in_room_or_world_readable(room_id, user_id) is_peeking = member_event_id is None if membership == Membership.JOIN: @@ -433,25 +434,3 @@ class InitialSyncHandler(BaseHandler): ret["membership"] = membership return ret - - async def _check_in_room_or_world_readable(self, room_id, user_id): - try: - # check_user_was_in_room will return the most recent membership - # event for the user if: - # * The user is a non-guest user, and was ever in the room - # * The user is a guest user, and has joined the room - # else it will throw. - member_event = await self.auth.check_user_was_in_room(room_id, user_id) - return member_event.membership, member_event.event_id - except AuthError: - visibility = await self.state_handler.get_current_state( - room_id, EventTypes.RoomHistoryVisibility, "" - ) - if ( - visibility - and visibility.content["history_visibility"] == "world_readable" - ): - return Membership.JOIN, None - raise AuthError( - 403, "Guest access not allowed", errcode=Codes.GUEST_ACCESS_FORBIDDEN - ) diff --git a/synapse/handlers/typing.py b/synapse/handlers/typing.py index d5ca9cb07b..5406618431 100644 --- a/synapse/handlers/typing.py +++ b/synapse/handlers/typing.py @@ -125,7 +125,7 @@ class TypingHandler(object): if target_user_id != auth_user_id: raise AuthError(400, "Cannot set another user's typing state") - yield self.auth.check_joined_room(room_id, target_user_id) + yield self.auth.check_user_in_room(room_id, target_user_id) logger.debug("%s has started typing in %s", target_user_id, room_id) @@ -155,7 +155,7 @@ class TypingHandler(object): if target_user_id != auth_user_id: raise AuthError(400, "Cannot set another user's typing state") - yield self.auth.check_joined_room(room_id, target_user_id) + yield self.auth.check_user_in_room(room_id, target_user_id) logger.debug("%s has stopped typing in %s", target_user_id, room_id) diff --git a/tests/handlers/test_typing.py b/tests/handlers/test_typing.py index 2767b0497a..140cc0a3c2 100644 --- a/tests/handlers/test_typing.py +++ b/tests/handlers/test_typing.py @@ -122,11 +122,11 @@ class TypingNotificationsTestCase(unittest.HomeserverTestCase): self.room_members = [] - def check_joined_room(room_id, user_id): + def check_user_in_room(room_id, user_id): if user_id not in [u.to_string() for u in self.room_members]: raise AuthError(401, "User is not in the room") - hs.get_auth().check_joined_room = check_joined_room + hs.get_auth().check_user_in_room = check_user_in_room def get_joined_hosts_for_room(room_id): return set(member.domain for member in self.room_members)