From ab8229479bddd89546ab486152344e80f01be820 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 16 Mar 2015 00:17:25 +0000 Subject: [PATCH 1/8] Respect ban membership --- synapse/api/auth.py | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index b176db8ce..96963d743 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -166,6 +166,7 @@ class Auth(object): target = auth_events.get(key) target_in_room = target and target.membership == Membership.JOIN + target_banned = target and target.membership == Membership.BAN key = (EventTypes.JoinRules, "", ) join_rule_event = auth_events.get(key) @@ -194,6 +195,7 @@ class Auth(object): { "caller_in_room": caller_in_room, "caller_invited": caller_invited, + "target_banned": target_banned, "target_in_room": target_in_room, "membership": membership, "join_rule": join_rule, @@ -202,6 +204,11 @@ class Auth(object): } ) + if ban_level: + ban_level = int(ban_level) + else: + ban_level = 50 # FIXME (erikj): What should we do here? + if Membership.INVITE == membership: # TODO (erikj): We should probably handle this more intelligently # PRIVATE join rules. @@ -212,6 +219,10 @@ class Auth(object): 403, "%s not in room %s." % (event.user_id, event.room_id,) ) + elif target_banned: + raise AuthError( + 403, "%s is banned from the room" % (target_user_id,) + ) elif target_in_room: # the target is already in the room. raise AuthError(403, "%s is already in the room." % target_user_id) @@ -221,6 +232,8 @@ class Auth(object): # joined: It's a NOOP if event.user_id != target_user_id: raise AuthError(403, "Cannot force another user to join.") + elif target_banned: + raise AuthError(403, "You are banned from this room") elif join_rule == JoinRules.PUBLIC: pass elif join_rule == JoinRules.INVITE: @@ -238,6 +251,10 @@ class Auth(object): 403, "%s not in room %s." % (target_user_id, event.room_id,) ) + elif target_banned and user_level < ban_level: + raise AuthError( + 403, "You cannot unban user &s." % (target_user_id,) + ) elif target_user_id != event.user_id: if kick_level: kick_level = int(kick_level) @@ -249,11 +266,6 @@ class Auth(object): 403, "You cannot kick user %s." % target_user_id ) elif Membership.BAN == membership: - if ban_level: - ban_level = int(ban_level) - else: - ban_level = 50 # FIXME (erikj): What should we do here? - if user_level < ban_level: raise AuthError(403, "You don't have permission to ban") else: From ea8590cf6626364e9532860548a5f1ae3b172d80 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 16 Mar 2015 00:18:08 +0000 Subject: [PATCH 2/8] Make context.auth_events grap auth events from current state. Otherwise auth is wrong. --- synapse/api/auth.py | 8 +++++++- synapse/state.py | 22 +++++----------------- 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 96963d743..4873cf9d1 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -28,6 +28,12 @@ import logging logger = logging.getLogger(__name__) +AuthEventTypes = ( + EventTypes.Create, EventTypes.Member, EventTypes.PowerLevels, + EventTypes.JoinRules, +) + + class Auth(object): def __init__(self, hs): @@ -427,7 +433,7 @@ class Auth(object): context.auth_events = { k: v for k, v in context.current_state.items() - if v.event_id in auth_ids + if v.type in AuthEventTypes } def compute_auth_events(self, event, current_state): diff --git a/synapse/state.py b/synapse/state.py index 80cced351..345046cd8 100644 --- a/synapse/state.py +++ b/synapse/state.py @@ -21,6 +21,7 @@ from synapse.util.async import run_on_reactor from synapse.util.expiringcache import ExpiringCache from synapse.api.constants import EventTypes from synapse.api.errors import AuthError +from synapse.api.auth import AuthEventTypes from synapse.events.snapshot import EventContext from collections import namedtuple @@ -38,12 +39,6 @@ def _get_state_key_from_event(event): KeyStateTuple = namedtuple("KeyStateTuple", ("context", "type", "state_key")) -AuthEventTypes = ( - EventTypes.Create, EventTypes.Member, EventTypes.PowerLevels, - EventTypes.JoinRules, -) - - SIZE_OF_CACHE = 1000 EVICTION_TIMEOUT_SECONDS = 20 @@ -187,17 +182,10 @@ class StateHandler(object): replaces = context.current_state[key] event.unsigned["replaces_state"] = replaces.event_id - if hasattr(event, "auth_events") and event.auth_events: - auth_ids = self.hs.get_auth().compute_auth_events( - event, context.current_state - ) - context.auth_events = { - k: v - for k, v in context.current_state.items() - if v.event_id in auth_ids - } - else: - context.auth_events = {} + context.auth_events = { + k: e for k, e in context.current_state.items() + if k[0] in AuthEventTypes + } context.prev_state_events = prev_state defer.returnValue(context) From 758d114cbce3b71f4253bf49669ec366185bfde9 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 16 Mar 2015 00:27:59 +0000 Subject: [PATCH 3/8] Send all membership events to the remote homeserver --- synapse/handlers/_base.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/synapse/handlers/_base.py b/synapse/handlers/_base.py index 1773fa20a..349a52b85 100644 --- a/synapse/handlers/_base.py +++ b/synapse/handlers/_base.py @@ -133,10 +133,9 @@ class BaseHandler(object): for k, s in context.current_state.items(): try: if k[0] == EventTypes.Member: - if s.content["membership"] == Membership.JOIN: - destinations.add( - UserID.from_string(s.state_key).domain - ) + destinations.add( + UserID.from_string(s.state_key).domain + ) except SynapseError: logger.warn( "Failed to get destination from event %s", s.event_id From e7ce5d8b062561b17df0441f5b1be38026b0d2b3 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 16 Mar 2015 00:30:59 +0000 Subject: [PATCH 4/8] Fix test --- tests/handlers/test_room.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/handlers/test_room.py b/tests/handlers/test_room.py index 6417f7330..0da7eb877 100644 --- a/tests/handlers/test_room.py +++ b/tests/handlers/test_room.py @@ -219,7 +219,7 @@ class RoomMemberHandlerTestCase(unittest.TestCase): yield room_handler.change_membership(event, context) self.federation.handle_new_event.assert_called_once_with( - event, destinations=set() + event, destinations=set(['red']) ) self.datastore.persist_event.assert_called_once_with( From b2e6ee5b43ebcd9e7ba37a56ed22b26c63b01370 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 16 Mar 2015 13:06:23 +0000 Subject: [PATCH 5/8] Remove concept of context.auth_events, instead use context.current_state --- synapse/api/auth.py | 6 ------ synapse/events/snapshot.py | 3 +-- synapse/handlers/_base.py | 6 +++--- synapse/handlers/federation.py | 8 +++----- synapse/state.py | 17 ----------------- 5 files changed, 7 insertions(+), 33 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 4873cf9d1..90f9eb684 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -430,12 +430,6 @@ class Auth(object): builder.auth_events = auth_events_entries - context.auth_events = { - k: v - for k, v in context.current_state.items() - if v.type in AuthEventTypes - } - def compute_auth_events(self, event, current_state): if event.type == EventTypes.Create: return [] diff --git a/synapse/events/snapshot.py b/synapse/events/snapshot.py index 7e98bdef2..4ecadf087 100644 --- a/synapse/events/snapshot.py +++ b/synapse/events/snapshot.py @@ -16,8 +16,7 @@ class EventContext(object): - def __init__(self, current_state=None, auth_events=None): + def __init__(self, current_state=None): self.current_state = current_state - self.auth_events = auth_events self.state_group = None self.rejected = False diff --git a/synapse/handlers/_base.py b/synapse/handlers/_base.py index 349a52b85..261335b27 100644 --- a/synapse/handlers/_base.py +++ b/synapse/handlers/_base.py @@ -90,8 +90,8 @@ class BaseHandler(object): event = builder.build() logger.debug( - "Created event %s with auth_events: %s, current state: %s", - event.event_id, context.auth_events, context.current_state, + "Created event %s with current state: %s", + event.event_id, context.current_state, ) defer.returnValue( @@ -106,7 +106,7 @@ class BaseHandler(object): # We now need to go and hit out to wherever we need to hit out to. if not suppress_auth: - self.auth.check(event, auth_events=context.auth_events) + self.auth.check(event, auth_events=context.current_state) yield self.store.persist_event(event, context=context) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index ae4e9b316..65cfacba2 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -464,11 +464,9 @@ class FederationHandler(BaseHandler): builder=builder, ) - self.auth.check(event, auth_events=context.auth_events) + self.auth.check(event, auth_events=context.current_state) - pdu = event - - defer.returnValue(pdu) + defer.returnValue(event) @defer.inlineCallbacks @log_function @@ -705,7 +703,7 @@ class FederationHandler(BaseHandler): ) if not auth_events: - auth_events = context.auth_events + auth_events = context.current_state logger.debug( "_handle_new_event: %s, auth_events: %s", diff --git a/synapse/state.py b/synapse/state.py index 345046cd8..ba2500d61 100644 --- a/synapse/state.py +++ b/synapse/state.py @@ -134,18 +134,6 @@ class StateHandler(object): } context.state_group = None - if hasattr(event, "auth_events") and event.auth_events: - auth_ids = self.hs.get_auth().compute_auth_events( - event, context.current_state - ) - context.auth_events = { - k: v - for k, v in context.current_state.items() - if v.event_id in auth_ids - } - else: - context.auth_events = {} - if event.is_state(): key = (event.type, event.state_key) if key in context.current_state: @@ -182,11 +170,6 @@ class StateHandler(object): replaces = context.current_state[key] event.unsigned["replaces_state"] = replaces.event_id - context.auth_events = { - k: e for k, e in context.current_state.items() - if k[0] in AuthEventTypes - } - context.prev_state_events = prev_state defer.returnValue(context) From 857810d2dd5e3ca6fe39b3bec7d76d75cb0c94ec Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 16 Mar 2015 15:12:47 +0000 Subject: [PATCH 6/8] Revert incorrect changes to where we send events --- synapse/handlers/_base.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/synapse/handlers/_base.py b/synapse/handlers/_base.py index 261335b27..2a9d9ec13 100644 --- a/synapse/handlers/_base.py +++ b/synapse/handlers/_base.py @@ -133,9 +133,10 @@ class BaseHandler(object): for k, s in context.current_state.items(): try: if k[0] == EventTypes.Member: - destinations.add( - UserID.from_string(s.state_key).domain - ) + if s.content["membership"] == Membership.JOIN: + destinations.add( + UserID.from_string(s.state_key).domain + ) except SynapseError: logger.warn( "Failed to get destination from event %s", s.event_id From f1d2b94e0b6fbdde811a7deef3ab4ab7386a207f Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 16 Mar 2015 15:13:05 +0000 Subject: [PATCH 7/8] Copy dict of context.current_state before changing it. --- synapse/storage/state.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/state.py b/synapse/storage/state.py index 71db16d0e..456e4bd45 100644 --- a/synapse/storage/state.py +++ b/synapse/storage/state.py @@ -82,7 +82,7 @@ class StateStore(SQLBaseStore): if context.current_state is None: return - state_events = context.current_state + state_events = dict(context.current_state) if event.is_state(): state_events[(event.type, event.state_key)] = event From 6df319b6f07f2acce0c1b9aa19fd9f6005aee4ac Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 16 Mar 2015 15:15:14 +0000 Subject: [PATCH 8/8] Fix tests --- tests/handlers/test_room.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/handlers/test_room.py b/tests/handlers/test_room.py index 0da7eb877..6417f7330 100644 --- a/tests/handlers/test_room.py +++ b/tests/handlers/test_room.py @@ -219,7 +219,7 @@ class RoomMemberHandlerTestCase(unittest.TestCase): yield room_handler.change_membership(event, context) self.federation.handle_new_event.assert_called_once_with( - event, destinations=set(['red']) + event, destinations=set() ) self.datastore.persist_event.assert_called_once_with(