From b71ca2b0140fa4d7866ebb10ee49556de7eff44f Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 19 Feb 2016 11:41:02 +0000 Subject: Allow guest users access to messages in rooms they have joined There should be no difference between guest users and non-guest users in terms of access to messages. Define the semantics of the is_peeking argument to filter_events_for_clients (slightly) better; interpret it appropriately, and set it correctly from /sync. --- synapse/handlers/_base.py | 52 +++++++++++++++++++++++++------- synapse/handlers/sync.py | 2 -- synapse/push/bulk_push_rule_evaluator.py | 2 +- 3 files changed, 42 insertions(+), 14 deletions(-) (limited to 'synapse') diff --git a/synapse/handlers/_base.py b/synapse/handlers/_base.py index 064e8723c8..da219184c5 100644 --- a/synapse/handlers/_base.py +++ b/synapse/handlers/_base.py @@ -53,9 +53,15 @@ class BaseHandler(object): self.event_builder_factory = hs.get_event_builder_factory() @defer.inlineCallbacks - def _filter_events_for_clients(self, user_tuples, events, event_id_to_state): + def filter_events_for_clients(self, user_tuples, events, event_id_to_state): """ Returns dict of user_id -> list of events that user is allowed to see. + + :param (str, bool) user_tuples: (user id, is_peeking) for each + user to be checked. is_peeking should be true if: + * the user is not currently a member of the room, and: + * the user has not been a member of the room since the given + events """ forgotten = yield defer.gatherResults([ self.store.who_forgot_in_room( @@ -72,18 +78,20 @@ class BaseHandler(object): def allowed(event, user_id, is_peeking): state = event_id_to_state[event.event_id] + # get the room_visibility at the time of the event. visibility_event = state.get((EventTypes.RoomHistoryVisibility, ""), None) if visibility_event: visibility = visibility_event.content.get("history_visibility", "shared") else: visibility = "shared" + # if it was world_readable, it's easy: everyone can read it if visibility == "world_readable": return True - if is_peeking: - return False - + # get the user's membership at the time of the event. (or rather, + # just *after* the event. Which means that people can see their + # own join events, but not (currently) their own leave events.) membership_event = state.get((EventTypes.Member, user_id), None) if membership_event: if membership_event.event_id in event_id_forgotten: @@ -93,20 +101,32 @@ class BaseHandler(object): else: membership = None + # if the user was a member of the room at the time of the event, + # they can see it. if membership == Membership.JOIN: return True if event.type == EventTypes.RoomHistoryVisibility: - return not is_peeking + # XXX why are m.room.history_visibility events special? + # return True + pass if visibility == "shared": - return True - elif visibility == "joined": - return membership == Membership.JOIN + # user can also see the event if he has become a member since + # the event + # + # XXX: if the user has subsequently joined and then left again, + # ideally we would share history up to the point they left. But + # we don't know when they left. + return not is_peeking elif visibility == "invited": + # user can also see the event if he was *invited* at the time + # of the event. return membership == Membership.INVITE - return True + # presumably visibility is "joined"; we weren't a member at the + # time of the event, so we're done. + return False defer.returnValue({ user_id: [ @@ -119,7 +139,17 @@ class BaseHandler(object): @defer.inlineCallbacks def _filter_events_for_client(self, user_id, events, is_peeking=False): - # Assumes that user has at some point joined the room if not is_guest. + """ + Check which events a user is allowed to see + + :param str user_id: user id to be checked + :param [synapse.events.EventBase] events: list of events to be checked + :param bool is_peeking should be True if: + * the user is not currently a member of the room, and: + * the user has not been a member of the room since the given + events + :rtype [synapse.events.EventBase] + """ types = ( (EventTypes.RoomHistoryVisibility, ""), (EventTypes.Member, user_id), @@ -128,7 +158,7 @@ class BaseHandler(object): frozenset(e.event_id for e in events), types=types ) - res = yield self._filter_events_for_clients( + res = yield self.filter_events_for_clients( [(user_id, is_peeking)], events, event_id_to_state ) defer.returnValue(res.get(user_id, [])) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 1d0f0058a2..f5122b5fb1 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -623,7 +623,6 @@ class SyncHandler(BaseHandler): recents = yield self._filter_events_for_client( sync_config.user.to_string(), recents, - is_peeking=sync_config.is_guest, ) else: recents = [] @@ -645,7 +644,6 @@ class SyncHandler(BaseHandler): loaded_recents = yield self._filter_events_for_client( sync_config.user.to_string(), loaded_recents, - is_peeking=sync_config.is_guest, ) loaded_recents.extend(recents) recents = loaded_recents diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index 8ac5ceb9ef..206b20e15f 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -103,7 +103,7 @@ class BulkPushRuleEvaluator: users_dict = yield self.store.are_guests(self.rules_by_user.keys()) - filtered_by_user = yield handler._filter_events_for_clients( + filtered_by_user = yield handler.filter_events_for_clients( users_dict.items(), [event], {event.event_id: current_state} ) -- cgit 1.4.1 From 6c5b147a39c4c1ee5bc0ec0decacebb9f32de286 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 19 Feb 2016 17:11:11 +0000 Subject: Interpret unknown visibilities the same as shared --- synapse/handlers/_base.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) (limited to 'synapse') diff --git a/synapse/handlers/_base.py b/synapse/handlers/_base.py index f01ab6780b..ef6716002c 100644 --- a/synapse/handlers/_base.py +++ b/synapse/handlers/_base.py @@ -111,22 +111,24 @@ class BaseHandler(object): # return True pass - if visibility == "shared": - # user can also see the event if he has become a member since - # the event - # - # XXX: if the user has subsequently joined and then left again, - # ideally we would share history up to the point they left. But - # we don't know when they left. - return not is_peeking + if visibility == "joined": + # we weren't a member at the time of the event, so we can't + # see this event. + return False + elif visibility == "invited": # user can also see the event if he was *invited* at the time # of the event. return membership == Membership.INVITE - # presumably visibility is "joined"; we weren't a member at the - # time of the event, so we're done. - return False + else: + # visibility is shared: user can also see the event if he has + # become a member since the event + # + # XXX: if the user has subsequently joined and then left again, + # ideally we would share history up to the point they left. But + # we don't know when they left. + return not is_peeking defer.returnValue({ user_id: [ -- cgit 1.4.1 From 5be3944730ccf809b72a4412d599b5f622246589 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 22 Feb 2016 15:27:44 +0000 Subject: address review comments drop commented-out special casing for historyvisibility event s/he/they/ for users --- synapse/handlers/_base.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) (limited to 'synapse') diff --git a/synapse/handlers/_base.py b/synapse/handlers/_base.py index ef6716002c..5613bd2059 100644 --- a/synapse/handlers/_base.py +++ b/synapse/handlers/_base.py @@ -106,23 +106,18 @@ class BaseHandler(object): if membership == Membership.JOIN: return True - if event.type == EventTypes.RoomHistoryVisibility: - # XXX why are m.room.history_visibility events special? - # return True - pass - if visibility == "joined": # we weren't a member at the time of the event, so we can't # see this event. return False elif visibility == "invited": - # user can also see the event if he was *invited* at the time + # user can also see the event if they were *invited* at the time # of the event. return membership == Membership.INVITE else: - # visibility is shared: user can also see the event if he has + # visibility is shared: user can also see the event if they have # become a member since the event # # XXX: if the user has subsequently joined and then left again, -- cgit 1.4.1