From dbeed36dec021df3036e088910c72d5727910dd3 Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Mon, 15 Feb 2016 14:38:27 +0000 Subject: [PATCH 01/16] Merge some room joining codepaths Force joining by alias to go through the send_membership_event checks, rather than bypassing them straight into _do_join. This is the first of many stages of cleanup. --- synapse/handlers/room.py | 14 ++++++++++---- synapse/rest/client/v1/room.py | 2 +- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index b2de2cd0c..89695cc0c 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -455,7 +455,7 @@ class RoomMemberHandler(BaseHandler): yield self.forget(requester.user, room_id) @defer.inlineCallbacks - def send_membership_event(self, event, context, is_guest=False): + def send_membership_event(self, event, context, is_guest=False, room_hosts=None): """ Change the membership status of a user in a room. Args: @@ -490,7 +490,7 @@ class RoomMemberHandler(BaseHandler): if not is_guest_access_allowed: raise AuthError(403, "Guest access not allowed") - yield self._do_join(event, context) + yield self._do_join(event, context, room_hosts=room_hosts) else: if event.membership == Membership.LEAVE: is_host_in_room = yield self.is_host_in_room(room_id, context) @@ -527,7 +527,8 @@ class RoomMemberHandler(BaseHandler): defer.returnValue({"room_id": room_id}) @defer.inlineCallbacks - def join_room_alias(self, joinee, room_alias, content={}): + def join_room_alias(self, requester, room_alias, content={}): + joinee = requester.user directory_handler = self.hs.get_handlers().directory_handler mapping = yield directory_handler.get_association(room_alias) @@ -553,7 +554,12 @@ class RoomMemberHandler(BaseHandler): }) event, context = yield self._create_new_client_event(builder) - yield self._do_join(event, context, room_hosts=hosts) + yield self.send_membership_event( + event, + context, + is_guest=requester.is_guest, + room_hosts=hosts + ) defer.returnValue({"room_id": room_id}) diff --git a/synapse/rest/client/v1/room.py b/synapse/rest/client/v1/room.py index 81bfe377b..76025213d 100644 --- a/synapse/rest/client/v1/room.py +++ b/synapse/rest/client/v1/room.py @@ -246,7 +246,7 @@ class JoinRoomAliasServlet(ClientV1RestServlet): if is_room_alias: handler = self.handlers.room_member_handler ret_dict = yield handler.join_room_alias( - requester.user, + requester, identifier, ) defer.returnValue((200, ret_dict)) From e71095801fc376aac30ff9408ae7f0203684024d Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Mon, 15 Feb 2016 15:39:16 +0000 Subject: [PATCH 02/16] Merge implementation of /join by alias or ID This code is kind of rough (passing the remote servers down a long chain), but is a step towards improvement. --- synapse/handlers/_base.py | 5 ++- synapse/handlers/message.py | 20 ++++++---- synapse/handlers/room.py | 40 ++++++++----------- synapse/rest/client/v1/room.py | 70 ++++++++++++++++------------------ synapse/types.py | 8 ++++ 5 files changed, 72 insertions(+), 71 deletions(-) diff --git a/synapse/handlers/_base.py b/synapse/handlers/_base.py index 064e8723c..8508ecdd4 100644 --- a/synapse/handlers/_base.py +++ b/synapse/handlers/_base.py @@ -188,9 +188,12 @@ class BaseHandler(object): ) @defer.inlineCallbacks - def handle_new_client_event(self, event, context, extra_users=[]): + def handle_new_client_event(self, event, context, ratelimit=True, extra_users=[]): # We now need to go and hit out to wherever we need to hit out to. + if ratelimit: + self.ratelimit(event.sender) + self.auth.check(event, auth_events=context.current_state) yield self.maybe_kick_guest_users(event, context.current_state.values()) diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 82c8cb5f0..a94fad173 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -216,7 +216,7 @@ class MessageHandler(BaseHandler): defer.returnValue((event, context)) @defer.inlineCallbacks - def send_event(self, event, context, ratelimit=True, is_guest=False): + def send_event(self, event, context, ratelimit=True, is_guest=False, room_hosts=None): """ Persists and notifies local clients and federation of an event. @@ -230,9 +230,6 @@ class MessageHandler(BaseHandler): assert self.hs.is_mine(user), "User must be our own: %s" % (user,) - if ratelimit: - self.ratelimit(event.sender) - if event.is_state(): prev_state = context.current_state.get((event.type, event.state_key)) if prev_state and event.user_id == prev_state.user_id: @@ -245,11 +242,18 @@ class MessageHandler(BaseHandler): if event.type == EventTypes.Member: member_handler = self.hs.get_handlers().room_member_handler - yield member_handler.send_membership_event(event, context, is_guest=is_guest) + yield member_handler.send_membership_event( + event, + context, + is_guest=is_guest, + ratelimit=ratelimit, + room_hosts=room_hosts + ) else: yield self.handle_new_client_event( event=event, context=context, + ratelimit=ratelimit, ) if event.type == EventTypes.Message: @@ -259,7 +263,8 @@ class MessageHandler(BaseHandler): @defer.inlineCallbacks def create_and_send_event(self, event_dict, ratelimit=True, - token_id=None, txn_id=None, is_guest=False): + token_id=None, txn_id=None, is_guest=False, + room_hosts=None): """ Creates an event, then sends it. @@ -274,7 +279,8 @@ class MessageHandler(BaseHandler): event, context, ratelimit=ratelimit, - is_guest=is_guest + is_guest=is_guest, + room_hosts=room_hosts, ) defer.returnValue(event) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 89695cc0c..b748e81d2 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -455,7 +455,9 @@ class RoomMemberHandler(BaseHandler): yield self.forget(requester.user, room_id) @defer.inlineCallbacks - def send_membership_event(self, event, context, is_guest=False, room_hosts=None): + def send_membership_event( + self, event, context, is_guest=False, room_hosts=None, ratelimit=True + ): """ Change the membership status of a user in a room. Args: @@ -527,8 +529,17 @@ class RoomMemberHandler(BaseHandler): defer.returnValue({"room_id": room_id}) @defer.inlineCallbacks - def join_room_alias(self, requester, room_alias, content={}): - joinee = requester.user + def lookup_room_alias(self, room_alias): + """ + Get the room ID associated with a room alias. + + Args: + room_alias (RoomAlias): The alias to look up. + Returns: + The room ID as a RoomID object. + Raises: + SynapseError if room alias could not be found. + """ directory_handler = self.hs.get_handlers().directory_handler mapping = yield directory_handler.get_association(room_alias) @@ -540,28 +551,7 @@ class RoomMemberHandler(BaseHandler): if not hosts: raise SynapseError(404, "No known servers") - # If event doesn't include a display name, add one. - yield collect_presencelike_data(self.distributor, joinee, content) - - content.update({"membership": Membership.JOIN}) - builder = self.event_builder_factory.new({ - "type": EventTypes.Member, - "state_key": joinee.to_string(), - "room_id": room_id, - "sender": joinee.to_string(), - "membership": Membership.JOIN, - "content": content, - }) - event, context = yield self._create_new_client_event(builder) - - yield self.send_membership_event( - event, - context, - is_guest=requester.is_guest, - room_hosts=hosts - ) - - defer.returnValue({"room_id": room_id}) + defer.returnValue((RoomID.from_string(room_id), hosts)) @defer.inlineCallbacks def _do_join(self, event, context, room_hosts=None): diff --git a/synapse/rest/client/v1/room.py b/synapse/rest/client/v1/room.py index 76025213d..340c24635 100644 --- a/synapse/rest/client/v1/room.py +++ b/synapse/rest/client/v1/room.py @@ -229,46 +229,40 @@ class JoinRoomAliasServlet(ClientV1RestServlet): allow_guest=True, ) - # the identifier could be a room alias or a room id. Try one then the - # other if it fails to parse, without swallowing other valid - # SynapseErrors. - - identifier = None - is_room_alias = False - try: - identifier = RoomAlias.from_string(room_identifier) - is_room_alias = True - except SynapseError: - identifier = RoomID.from_string(room_identifier) - - # TODO: Support for specifying the home server to join with? - - if is_room_alias: + if RoomID.is_valid(room_identifier): + room_id = room_identifier + room_hosts = None + elif RoomAlias.is_valid(room_identifier): handler = self.handlers.room_member_handler - ret_dict = yield handler.join_room_alias( - requester, - identifier, - ) - defer.returnValue((200, ret_dict)) - else: # room id - msg_handler = self.handlers.message_handler - content = {"membership": Membership.JOIN} - if requester.is_guest: - content["kind"] = "guest" - yield msg_handler.create_and_send_event( - { - "type": EventTypes.Member, - "content": content, - "room_id": identifier.to_string(), - "sender": requester.user.to_string(), - "state_key": requester.user.to_string(), - }, - token_id=requester.access_token_id, - txn_id=txn_id, - is_guest=requester.is_guest, - ) + room_alias = RoomAlias.from_string(room_identifier) + room_id, room_hosts = yield handler.lookup_room_alias(room_alias) + room_id = room_id.to_string() + else: + raise SynapseError(400, "%s was not legal room ID or room alias" % ( + room_identifier, + )) - defer.returnValue((200, {"room_id": identifier.to_string()})) + msg_handler = self.handlers.message_handler + content = {"membership": Membership.JOIN} + if requester.is_guest: + content["kind"] = "guest" + yield msg_handler.create_and_send_event( + { + "type": EventTypes.Member, + "content": content, + "room_id": room_id, + "sender": requester.user.to_string(), + "state_key": requester.user.to_string(), + + "membership": Membership.JOIN, # For backwards compatibility + }, + token_id=requester.access_token_id, + txn_id=txn_id, + is_guest=requester.is_guest, + room_hosts=room_hosts, + ) + + defer.returnValue((200, {"room_id": room_id})) @defer.inlineCallbacks def on_PUT(self, request, room_identifier, txn_id): diff --git a/synapse/types.py b/synapse/types.py index 2095837ba..d5bd95cbd 100644 --- a/synapse/types.py +++ b/synapse/types.py @@ -73,6 +73,14 @@ class DomainSpecificString( """Return a string encoding the fields of the structure object.""" return "%s%s:%s" % (self.SIGIL, self.localpart, self.domain) + @classmethod + def is_valid(cls, s): + try: + cls.from_string(s) + return True + except: + return False + __str__ = to_string @classmethod From f318d4f2a4b45aaab234a124d4db2bee986ba911 Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Mon, 15 Feb 2016 15:57:10 +0000 Subject: [PATCH 03/16] Inline _do_join as it now only has one caller Also, consistently apply rate limiting. Again, ugly, but a step in the right direction. --- synapse/handlers/room.py | 95 ++++++++++++++++++++-------------------- 1 file changed, 47 insertions(+), 48 deletions(-) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index b748e81d2..44bab19c5 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -492,7 +492,50 @@ class RoomMemberHandler(BaseHandler): if not is_guest_access_allowed: raise AuthError(403, "Guest access not allowed") - yield self._do_join(event, context, room_hosts=room_hosts) + room_id = event.room_id + + # XXX: We don't do an auth check if we are doing an invite + # join dance for now, since we're kinda implicitly checking + # that we are allowed to join when we decide whether or not we + # need to do the invite/join dance. + + is_host_in_room = yield self.is_host_in_room(room_id, context) + if is_host_in_room: + should_do_dance = False + elif room_hosts: # TODO: Shouldn't this be remote_room_host? + should_do_dance = True + else: + inviter = yield self.get_inviter(event) + if not inviter: + # return the same error as join_room_alias does + raise SynapseError(404, "No known servers") + should_do_dance = not self.hs.is_mine(inviter) + room_hosts = [inviter.domain] + + if should_do_dance: + handler = self.hs.get_handlers().federation_handler + yield handler.do_invite_join( + room_hosts, + room_id, + event.user_id, + event.content, + ) + else: + logger.debug("Doing normal join") + + yield self._do_local_membership_update( + event, + context=context, + ratelimit=ratelimit, + ) + + prev_state = context.current_state.get((event.type, event.state_key)) + if not prev_state or prev_state.membership != Membership.JOIN: + # Only fire user_joined_room if the user has acutally joined the + # room. Don't bother if the user is just changing their profile + # info. + user = UserID.from_string(event.user_id) + yield user_joined_room(self.distributor, user, room_id) else: if event.membership == Membership.LEAVE: is_host_in_room = yield self.is_host_in_room(room_id, context) @@ -520,6 +563,7 @@ class RoomMemberHandler(BaseHandler): yield self._do_local_membership_update( event, context=context, + ratelimit=ratelimit, ) if prev_state and prev_state.membership == Membership.JOIN: @@ -553,52 +597,6 @@ class RoomMemberHandler(BaseHandler): defer.returnValue((RoomID.from_string(room_id), hosts)) - @defer.inlineCallbacks - def _do_join(self, event, context, room_hosts=None): - room_id = event.room_id - - # XXX: We don't do an auth check if we are doing an invite - # join dance for now, since we're kinda implicitly checking - # that we are allowed to join when we decide whether or not we - # need to do the invite/join dance. - - is_host_in_room = yield self.is_host_in_room(room_id, context) - if is_host_in_room: - should_do_dance = False - elif room_hosts: # TODO: Shouldn't this be remote_room_host? - should_do_dance = True - else: - inviter = yield self.get_inviter(event) - if not inviter: - # return the same error as join_room_alias does - raise SynapseError(404, "No known servers") - should_do_dance = not self.hs.is_mine(inviter) - room_hosts = [inviter.domain] - - if should_do_dance: - handler = self.hs.get_handlers().federation_handler - yield handler.do_invite_join( - room_hosts, - room_id, - event.user_id, - event.content, - ) - else: - logger.debug("Doing normal join") - - yield self._do_local_membership_update( - event, - context=context, - ) - - prev_state = context.current_state.get((event.type, event.state_key)) - if not prev_state or prev_state.membership != Membership.JOIN: - # Only fire user_joined_room if the user has acutally joined the - # room. Don't bother if the user is just changing their profile - # info. - user = UserID.from_string(event.user_id) - yield user_joined_room(self.distributor, user, room_id) - @defer.inlineCallbacks def get_inviter(self, event): # TODO(markjh): get prev_state from snapshot @@ -653,7 +651,7 @@ class RoomMemberHandler(BaseHandler): defer.returnValue(room_ids) @defer.inlineCallbacks - def _do_local_membership_update(self, event, context): + def _do_local_membership_update(self, event, context, ratelimit=True): yield run_on_reactor() target_user = UserID.from_string(event.state_key) @@ -662,6 +660,7 @@ class RoomMemberHandler(BaseHandler): event, context, extra_users=[target_user], + ratelimit=ratelimit, ) @defer.inlineCallbacks From 73e616df2a4e0fb7bc83340f558dede32cc2efc3 Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Mon, 15 Feb 2016 16:02:22 +0000 Subject: [PATCH 04/16] Inline _do_local_membership_update --- synapse/handlers/room.py | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 44bab19c5..d17e5c1b7 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -24,7 +24,6 @@ from synapse.api.constants import ( ) from synapse.api.errors import AuthError, StoreError, SynapseError, Codes from synapse.util import stringutils, unwrapFirstError -from synapse.util.async import run_on_reactor from synapse.util.logcontext import preserve_context_over_fn from signedjson.sign import verify_signed_json @@ -466,6 +465,7 @@ class RoomMemberHandler(BaseHandler): SynapseError if there was a problem changing the membership. """ target_user_id = event.state_key + target_user = UserID.from_string(event.state_key) prev_state = context.current_state.get( (EventTypes.Member, target_user_id), @@ -523,9 +523,10 @@ class RoomMemberHandler(BaseHandler): else: logger.debug("Doing normal join") - yield self._do_local_membership_update( + yield self.handle_new_client_event( event, - context=context, + context, + extra_users=[target_user], ratelimit=ratelimit, ) @@ -560,9 +561,10 @@ class RoomMemberHandler(BaseHandler): defer.returnValue({}) return - yield self._do_local_membership_update( + yield self.handle_new_client_event( event, - context=context, + context, + extra_users=[target_user], ratelimit=ratelimit, ) @@ -650,19 +652,6 @@ class RoomMemberHandler(BaseHandler): defer.returnValue(room_ids) - @defer.inlineCallbacks - def _do_local_membership_update(self, event, context, ratelimit=True): - yield run_on_reactor() - - target_user = UserID.from_string(event.state_key) - - yield self.handle_new_client_event( - event, - context, - extra_users=[target_user], - ratelimit=ratelimit, - ) - @defer.inlineCallbacks def do_3pid_invite( self, From 150fcde0dce02670c2180f9d4657783eb204daa8 Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Mon, 15 Feb 2016 16:16:03 +0000 Subject: [PATCH 05/16] Reuse update_membership from /join --- synapse/handlers/room.py | 12 +++++++++--- synapse/rest/client/v1/room.py | 21 +++++---------------- 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index d17e5c1b7..04916d4e2 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -403,7 +403,9 @@ class RoomMemberHandler(BaseHandler): remotedomains.add(member.domain) @defer.inlineCallbacks - def update_membership(self, requester, target, room_id, action, txn_id=None): + def update_membership( + self, requester, target, room_id, action, txn_id=None, room_hosts=None + ): effective_membership_state = action if action in ["kick", "unban"]: effective_membership_state = "leave" @@ -412,7 +414,7 @@ class RoomMemberHandler(BaseHandler): msg_handler = self.hs.get_handlers().message_handler - content = {"membership": unicode(effective_membership_state)} + content = {"membership": effective_membership_state} if requester.is_guest: content["kind"] = "guest" @@ -423,6 +425,9 @@ class RoomMemberHandler(BaseHandler): "room_id": room_id, "sender": requester.user.to_string(), "state_key": target.to_string(), + + # For backwards compatibility: + "membership": effective_membership_state, }, token_id=requester.access_token_id, txn_id=txn_id, @@ -447,7 +452,8 @@ class RoomMemberHandler(BaseHandler): event, context, ratelimit=True, - is_guest=requester.is_guest + is_guest=requester.is_guest, + room_hosts=room_hosts, ) if action == "forget": diff --git a/synapse/rest/client/v1/room.py b/synapse/rest/client/v1/room.py index 340c24635..f8cd746a8 100644 --- a/synapse/rest/client/v1/room.py +++ b/synapse/rest/client/v1/room.py @@ -242,23 +242,12 @@ class JoinRoomAliasServlet(ClientV1RestServlet): room_identifier, )) - msg_handler = self.handlers.message_handler - content = {"membership": Membership.JOIN} - if requester.is_guest: - content["kind"] = "guest" - yield msg_handler.create_and_send_event( - { - "type": EventTypes.Member, - "content": content, - "room_id": room_id, - "sender": requester.user.to_string(), - "state_key": requester.user.to_string(), - - "membership": Membership.JOIN, # For backwards compatibility - }, - token_id=requester.access_token_id, + yield self.handlers.room_member_handler.update_membership( + requester=requester, + target=requester.user, + room_id=room_id, + action="join", txn_id=txn_id, - is_guest=requester.is_guest, room_hosts=room_hosts, ) From 1bbb67c452f33d14e1d4c5b51cc1a694de53bbb8 Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Mon, 15 Feb 2016 16:40:22 +0000 Subject: [PATCH 06/16] Use update_membership to kick guests --- synapse/handlers/_base.py | 24 ++++++++++-------------- synapse/handlers/room.py | 11 +++++++++-- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/synapse/handlers/_base.py b/synapse/handlers/_base.py index 8508ecdd4..cad37f50e 100644 --- a/synapse/handlers/_base.py +++ b/synapse/handlers/_base.py @@ -18,7 +18,7 @@ from twisted.internet import defer from synapse.api.errors import LimitExceededError, SynapseError, AuthError from synapse.crypto.event_signing import add_hashes_and_signatures from synapse.api.constants import Membership, EventTypes -from synapse.types import UserID, RoomAlias +from synapse.types import UserID, RoomAlias, Requester from synapse.push.action_generator import ActionGenerator from synapse.util.logcontext import PreserveLoggingContext @@ -319,7 +319,8 @@ class BaseHandler(object): if member_event.type != EventTypes.Member: continue - if not self.hs.is_mine(UserID.from_string(member_event.state_key)): + target_user = UserID.from_string(member_event.state_key) + if not self.hs.is_mine(target_user): continue if member_event.content["membership"] not in { @@ -341,18 +342,13 @@ class BaseHandler(object): # and having homeservers have their own users leave keeps more # of that decision-making and control local to the guest-having # homeserver. - message_handler = self.hs.get_handlers().message_handler - yield message_handler.create_and_send_event( - { - "type": EventTypes.Member, - "state_key": member_event.state_key, - "content": { - "membership": Membership.LEAVE, - "kind": "guest" - }, - "room_id": member_event.room_id, - "sender": member_event.state_key - }, + requester = Requester(target_user, "", True) + handler = self.hs.get_handlers().room_member_handler + yield handler.update_membership( + requester, + target_user, + member_event.room_id, + "leave", ratelimit=False, ) except Exception as e: diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 04916d4e2..8c8bacf5d 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -404,7 +404,14 @@ class RoomMemberHandler(BaseHandler): @defer.inlineCallbacks def update_membership( - self, requester, target, room_id, action, txn_id=None, room_hosts=None + self, + requester, + target, + room_id, + action, + txn_id=None, + room_hosts=None, + ratelimit=True, ): effective_membership_state = action if action in ["kick", "unban"]: @@ -451,7 +458,7 @@ class RoomMemberHandler(BaseHandler): yield msg_handler.send_event( event, context, - ratelimit=True, + ratelimit=ratelimit, is_guest=requester.is_guest, room_hosts=room_hosts, ) From 8168341e9bf8aa3c29b3fde9af12df0f1839baf1 Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Mon, 15 Feb 2016 17:14:34 +0000 Subject: [PATCH 07/16] Use update_membership for profile updates --- synapse/handlers/profile.py | 28 ++++++++++++---------------- synapse/handlers/room.py | 4 ---- 2 files changed, 12 insertions(+), 20 deletions(-) diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py index 629e6e359..2850db4a1 100644 --- a/synapse/handlers/profile.py +++ b/synapse/handlers/profile.py @@ -16,8 +16,7 @@ from twisted.internet import defer from synapse.api.errors import SynapseError, AuthError, CodeMessageException -from synapse.api.constants import EventTypes, Membership -from synapse.types import UserID +from synapse.types import UserID, Requester from synapse.util import unwrapFirstError from ._base import BaseHandler @@ -208,21 +207,18 @@ class ProfileHandler(BaseHandler): ) for j in joins: - content = { - "membership": Membership.JOIN, - } - - yield collect_presencelike_data(self.distributor, user, content) - - msg_handler = self.hs.get_handlers().message_handler + handler = self.hs.get_handlers().room_member_handler try: - yield msg_handler.create_and_send_event({ - "type": EventTypes.Member, - "room_id": j.room_id, - "state_key": user.to_string(), - "content": content, - "sender": user.to_string() - }, ratelimit=False) + # Assume the user isn't a guest because we don't let guests set + # profile or avatar data. + requester = Requester(user, "", False) + yield handler.update_membership( + requester, + user, + j.room_id, + "join", # We treat a profile update like a join. + ratelimit=False, + ) except Exception as e: logger.warn( "Failed to update join event for room %s - %s", diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 8c8bacf5d..505fb383e 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -41,10 +41,6 @@ logger = logging.getLogger(__name__) id_server_scheme = "https://" -def collect_presencelike_data(distributor, user, content): - return distributor.fire("collect_presencelike_data", user, content) - - def user_left_room(distributor, user, room_id): return preserve_context_over_fn( distributor.fire, From 1a2197d7bf62437208643f750ee757b8b85e2db6 Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Mon, 15 Feb 2016 18:13:10 +0000 Subject: [PATCH 08/16] Simplify room creation code --- synapse/handlers/room.py | 62 ++++++++++++++-------------------- synapse/rest/client/v1/room.py | 18 ++-------- 2 files changed, 28 insertions(+), 52 deletions(-) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 505fb383e..bdaa05e0b 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -76,13 +76,11 @@ class RoomCreationHandler(BaseHandler): } @defer.inlineCallbacks - def create_room(self, user_id, room_id, config): + def create_room(self, requester, config): """ Creates a new room. Args: user_id (str): The ID of the user creating the new room. - room_id (str): The proposed ID for the new room. Can be None, in - which case one will be created for you. config (dict) : A dict of configuration options. Returns: The new room ID. @@ -90,6 +88,8 @@ class RoomCreationHandler(BaseHandler): SynapseError if the room ID was taken, couldn't be stored, or something went horribly wrong. """ + user_id = requester.user.to_string() + self.ratelimit(user_id) if "room_alias_name" in config: @@ -121,40 +121,28 @@ class RoomCreationHandler(BaseHandler): is_public = config.get("visibility", None) == "public" - if room_id: - # Ensure room_id is the correct type - room_id_obj = RoomID.from_string(room_id) - if not self.hs.is_mine(room_id_obj): - raise SynapseError(400, "Room id must be local") - - yield self.store.store_room( - room_id=room_id, - room_creator_user_id=user_id, - is_public=is_public - ) - else: - # autogen room IDs and try to create it. We may clash, so just - # try a few times till one goes through, giving up eventually. - attempts = 0 - room_id = None - while attempts < 5: - try: - random_string = stringutils.random_string(18) - gen_room_id = RoomID.create( - random_string, - self.hs.hostname, - ) - yield self.store.store_room( - room_id=gen_room_id.to_string(), - room_creator_user_id=user_id, - is_public=is_public - ) - room_id = gen_room_id.to_string() - break - except StoreError: - attempts += 1 - if not room_id: - raise StoreError(500, "Couldn't generate a room ID.") + # autogen room IDs and try to create it. We may clash, so just + # try a few times till one goes through, giving up eventually. + attempts = 0 + room_id = None + while attempts < 5: + try: + random_string = stringutils.random_string(18) + gen_room_id = RoomID.create( + random_string, + self.hs.hostname, + ) + yield self.store.store_room( + room_id=gen_room_id.to_string(), + room_creator_user_id=user_id, + is_public=is_public + ) + room_id = gen_room_id.to_string() + break + except StoreError: + attempts += 1 + if not room_id: + raise StoreError(500, "Couldn't generate a room ID.") if room_alias: directory_handler = self.hs.get_handlers().directory_handler diff --git a/synapse/rest/client/v1/room.py b/synapse/rest/client/v1/room.py index f8cd746a8..5f5c26a91 100644 --- a/synapse/rest/client/v1/room.py +++ b/synapse/rest/client/v1/room.py @@ -63,24 +63,12 @@ class RoomCreateRestServlet(ClientV1RestServlet): def on_POST(self, request): requester = yield self.auth.get_user_by_req(request) - room_config = self.get_room_config(request) - info = yield self.make_room( - room_config, - requester.user, - None, - ) - room_config.update(info) - defer.returnValue((200, info)) - - @defer.inlineCallbacks - def make_room(self, room_config, auth_user, room_id): handler = self.handlers.room_creation_handler info = yield handler.create_room( - user_id=auth_user.to_string(), - room_id=room_id, - config=room_config + requester, self.get_room_config(request) ) - defer.returnValue(info) + + defer.returnValue((200, info)) def get_room_config(self, request): try: From 4bfb32f685cff919141a3fc0cd9179447febc765 Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Mon, 15 Feb 2016 18:21:30 +0000 Subject: [PATCH 09/16] Branch off member and non member sends Unclean, needs tidy-up, but works --- synapse/handlers/directory.py | 2 +- synapse/handlers/federation.py | 4 +- synapse/handlers/message.py | 66 ++++++++++++++-------------- synapse/handlers/room.py | 80 +++++++++++++++++++--------------- synapse/rest/client/v1/room.py | 21 ++++++--- 5 files changed, 99 insertions(+), 74 deletions(-) diff --git a/synapse/handlers/directory.py b/synapse/handlers/directory.py index 4efecb1ff..e0a778e7f 100644 --- a/synapse/handlers/directory.py +++ b/synapse/handlers/directory.py @@ -216,7 +216,7 @@ class DirectoryHandler(BaseHandler): aliases = yield self.store.get_aliases_for_room(room_id) msg_handler = self.hs.get_handlers().message_handler - yield msg_handler.create_and_send_event({ + yield msg_handler.create_and_send_nonmember_event({ "type": EventTypes.Aliases, "state_key": self.hs.hostname, "room_id": room_id, diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index da55d4354..ac15f9e5d 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -1658,7 +1658,7 @@ class FederationHandler(BaseHandler): self.auth.check(event, context.current_state) yield self._validate_keyserver(event, auth_events=context.current_state) member_handler = self.hs.get_handlers().room_member_handler - yield member_handler.send_membership_event(event, context) + yield member_handler.send_membership_event(event, context, from_client=False) else: destinations = set([x.split(":", 1)[-1] for x in (sender, room_id)]) yield self.replication_layer.forward_third_party_invite( @@ -1687,7 +1687,7 @@ class FederationHandler(BaseHandler): # TODO: Make sure the signatures actually are correct. event.signatures.update(returned_invite.signatures) member_handler = self.hs.get_handlers().room_member_handler - yield member_handler.send_membership_event(event, context) + yield member_handler.send_membership_event(event, context, from_client=False) @defer.inlineCallbacks def add_display_name_to_third_party_invite(self, event_dict, event, context): diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index a94fad173..05dab172b 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -16,7 +16,7 @@ from twisted.internet import defer from synapse.api.constants import EventTypes, Membership -from synapse.api.errors import AuthError, Codes +from synapse.api.errors import AuthError, Codes, SynapseError from synapse.streams.config import PaginationConfig from synapse.events.utils import serialize_event from synapse.events.validator import EventValidator @@ -216,7 +216,7 @@ class MessageHandler(BaseHandler): defer.returnValue((event, context)) @defer.inlineCallbacks - def send_event(self, event, context, ratelimit=True, is_guest=False, room_hosts=None): + def send_nonmember_event(self, event, context, ratelimit=True): """ Persists and notifies local clients and federation of an event. @@ -226,61 +226,63 @@ class MessageHandler(BaseHandler): ratelimit (bool): Whether to rate limit this send. is_guest (bool): Whether the sender is a guest. """ + if event.type == EventTypes.Member: + raise SynapseError( + 500, + "Tried to send member even through non-member codepath" + ) + user = UserID.from_string(event.sender) assert self.hs.is_mine(user), "User must be our own: %s" % (user,) if event.is_state(): - prev_state = context.current_state.get((event.type, event.state_key)) - if prev_state and event.user_id == prev_state.user_id: - prev_content = encode_canonical_json(prev_state.content) - next_content = encode_canonical_json(event.content) - if prev_content == next_content: - # Duplicate suppression for state updates with same sender - # and content. - defer.returnValue(prev_state) + prev_state = self.deduplicate_state_event(event, context) + if prev_state is not None: + defer.returnValue(prev_state) - if event.type == EventTypes.Member: - member_handler = self.hs.get_handlers().room_member_handler - yield member_handler.send_membership_event( - event, - context, - is_guest=is_guest, - ratelimit=ratelimit, - room_hosts=room_hosts - ) - else: - yield self.handle_new_client_event( - event=event, - context=context, - ratelimit=ratelimit, - ) + yield self.handle_new_client_event( + event=event, + context=context, + ratelimit=ratelimit, + ) if event.type == EventTypes.Message: presence = self.hs.get_handlers().presence_handler with PreserveLoggingContext(): presence.bump_presence_active_time(user) + def deduplicate_state_event(self, event, context): + prev_state = context.current_state.get((event.type, event.state_key)) + if prev_state and event.user_id == prev_state.user_id: + prev_content = encode_canonical_json(prev_state.content) + next_content = encode_canonical_json(event.content) + if prev_content == next_content: + return prev_state + return None + @defer.inlineCallbacks - def create_and_send_event(self, event_dict, ratelimit=True, - token_id=None, txn_id=None, is_guest=False, - room_hosts=None): + def create_and_send_nonmember_event( + self, + event_dict, + ratelimit=True, + token_id=None, + txn_id=None + ): """ Creates an event, then sends it. - See self.create_event and self.send_event. + See self.create_event and self.send_nonmember_event. """ event, context = yield self.create_event( event_dict, token_id=token_id, txn_id=txn_id ) - yield self.send_event( + yield self.send_nonmember_event( event, context, ratelimit=ratelimit, - is_guest=is_guest, - room_hosts=room_hosts, ) defer.returnValue(event) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index bdaa05e0b..5d4e87b3b 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -179,13 +179,24 @@ class RoomCreationHandler(BaseHandler): ) msg_handler = self.hs.get_handlers().message_handler + room_member_handler = self.hs.get_handlers().room_member_handler for event in creation_events: - yield msg_handler.create_and_send_event(event, ratelimit=False) + if event["type"] == EventTypes.Member: + # TODO(danielwh): This is hideous + yield room_member_handler.update_membership( + requester, + user, + room_id, + "join", + ratelimit=False, + ) + else: + yield msg_handler.create_and_send_nonmember_event(event, ratelimit=False) if "name" in config: name = config["name"] - yield msg_handler.create_and_send_event({ + yield msg_handler.create_and_send_nonmember_event({ "type": EventTypes.Name, "room_id": room_id, "sender": user_id, @@ -195,7 +206,7 @@ class RoomCreationHandler(BaseHandler): if "topic" in config: topic = config["topic"] - yield msg_handler.create_and_send_event({ + yield msg_handler.create_and_send_nonmember_event({ "type": EventTypes.Topic, "room_id": room_id, "sender": user_id, @@ -204,13 +215,13 @@ class RoomCreationHandler(BaseHandler): }, ratelimit=False) for invitee in invite_list: - yield msg_handler.create_and_send_event({ - "type": EventTypes.Member, - "state_key": invitee, - "room_id": room_id, - "sender": user_id, - "content": {"membership": Membership.INVITE}, - }, ratelimit=False) + room_member_handler.update_membership( + requester, + UserID.from_string(invitee), + room_id, + "invite", + ratelimit=False, + ) for invite_3pid in invite_3pid_list: id_server = invite_3pid["id_server"] @@ -222,7 +233,7 @@ class RoomCreationHandler(BaseHandler): medium, address, id_server, - token_id=None, + requester, txn_id=None, ) @@ -439,12 +450,14 @@ class RoomMemberHandler(BaseHandler): errcode=Codes.BAD_STATE ) - yield msg_handler.send_event( + member_handler = self.hs.get_handlers().room_member_handler + yield member_handler.send_membership_event( event, context, - ratelimit=ratelimit, is_guest=requester.is_guest, + ratelimit=ratelimit, room_hosts=room_hosts, + from_client=True, ) if action == "forget": @@ -452,7 +465,7 @@ class RoomMemberHandler(BaseHandler): @defer.inlineCallbacks def send_membership_event( - self, event, context, is_guest=False, room_hosts=None, ratelimit=True + self, event, context, is_guest=False, room_hosts=None, ratelimit=True, from_client=True, ): """ Change the membership status of a user in a room. @@ -461,6 +474,16 @@ class RoomMemberHandler(BaseHandler): Raises: SynapseError if there was a problem changing the membership. """ + if from_client: + user = UserID.from_string(event.sender) + + assert self.hs.is_mine(user), "User must be our own: %s" % (user,) + + if event.is_state(): + prev_state = self.hs.get_handlers().message_handler.deduplicate_state_event(event, context) + if prev_state is not None: + return + target_user_id = event.state_key target_user = UserID.from_string(event.state_key) @@ -549,13 +572,11 @@ class RoomMemberHandler(BaseHandler): room_id, event.user_id ) - defer.returnValue({"room_id": room_id}) return # FIXME: This isn't idempotency. if prev_state and prev_state.membership == event.membership: # double same action, treat this event as a NOOP. - defer.returnValue({}) return yield self.handle_new_client_event( @@ -569,8 +590,6 @@ class RoomMemberHandler(BaseHandler): user = UserID.from_string(event.user_id) user_left_room(self.distributor, user, event.room_id) - defer.returnValue({"room_id": room_id}) - @defer.inlineCallbacks def lookup_room_alias(self, room_alias): """ @@ -657,7 +676,7 @@ class RoomMemberHandler(BaseHandler): medium, address, id_server, - token_id, + requester, txn_id ): invitee = yield self._lookup_3pid( @@ -665,19 +684,12 @@ class RoomMemberHandler(BaseHandler): ) if invitee: - # make sure it looks like a user ID; it'll throw if it's invalid. - UserID.from_string(invitee) - yield self.hs.get_handlers().message_handler.create_and_send_event( - { - "type": EventTypes.Member, - "content": { - "membership": unicode("invite") - }, - "room_id": room_id, - "sender": inviter.to_string(), - "state_key": invitee, - }, - token_id=token_id, + handler = self.hs.get_handlers().room_member_handler + yield handler.update_membership( + requester, + UserID.from_string(invitee), + room_id, + "invite", txn_id=txn_id, ) else: @@ -687,7 +699,7 @@ class RoomMemberHandler(BaseHandler): address, room_id, inviter, - token_id, + requester.access_token_id, txn_id=txn_id ) @@ -798,7 +810,7 @@ class RoomMemberHandler(BaseHandler): ) ) msg_handler = self.hs.get_handlers().message_handler - yield msg_handler.create_and_send_event( + yield msg_handler.create_and_send_nonmember_event( { "type": EventTypes.ThirdPartyInvite, "content": { diff --git a/synapse/rest/client/v1/room.py b/synapse/rest/client/v1/room.py index 5f5c26a91..179fe9a01 100644 --- a/synapse/rest/client/v1/room.py +++ b/synapse/rest/client/v1/room.py @@ -150,10 +150,21 @@ class RoomStateEventRestServlet(ClientV1RestServlet): event_dict["state_key"] = state_key msg_handler = self.handlers.message_handler - yield msg_handler.create_and_send_event( - event_dict, token_id=requester.access_token_id, txn_id=txn_id, + event, context = yield msg_handler.create_event( + event_dict, + token_id=requester.access_token_id, + txn_id=txn_id, ) + if event_type == EventTypes.Member: + yield self.handlers.room_member_handler.send_membership_event( + event, + context, + is_guest=requester.is_guest, + ) + else: + yield msg_handler.send_nonmember_event(event, context) + defer.returnValue((200, {})) @@ -171,7 +182,7 @@ class RoomSendEventRestServlet(ClientV1RestServlet): content = _parse_json(request) msg_handler = self.handlers.message_handler - event = yield msg_handler.create_and_send_event( + event = yield msg_handler.create_and_send_nonmember_event( { "type": event_type, "content": content, @@ -434,7 +445,7 @@ class RoomMembershipRestServlet(ClientV1RestServlet): content["medium"], content["address"], content["id_server"], - requester.access_token_id, + requester, txn_id ) defer.returnValue((200, {})) @@ -490,7 +501,7 @@ class RoomRedactEventRestServlet(ClientV1RestServlet): content = _parse_json(request) msg_handler = self.handlers.message_handler - event = yield msg_handler.create_and_send_event( + event = yield msg_handler.create_and_send_nonmember_event( { "type": EventTypes.Redaction, "content": content, From 04686df17ae26f86484965365d12039161d8ee2d Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Tue, 16 Feb 2016 11:52:46 +0000 Subject: [PATCH 10/16] Add comment --- synapse/handlers/profile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py index 2850db4a1..f3e73d926 100644 --- a/synapse/handlers/profile.py +++ b/synapse/handlers/profile.py @@ -217,7 +217,7 @@ class ProfileHandler(BaseHandler): user, j.room_id, "join", # We treat a profile update like a join. - ratelimit=False, + ratelimit=False, # Try to hide that these events aren't atomic. ) except Exception as e: logger.warn( From 1f403325acfe355125eeb4e7d7f9f04bf9fb807e Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Tue, 16 Feb 2016 12:00:50 +0000 Subject: [PATCH 11/16] Tidy? up room creation event sending --- synapse/handlers/room.py | 135 +++++++++++++++++++++------------------ 1 file changed, 72 insertions(+), 63 deletions(-) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 722dadde7..e384370d7 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -168,9 +168,14 @@ class RoomCreationHandler(BaseHandler): creation_content = config.get("creation_content", {}) - user = UserID.from_string(user_id) - creation_events = self._create_events_for_new_room( - user, room_id, + msg_handler = self.hs.get_handlers().message_handler + room_member_handler = self.hs.get_handlers().room_member_handler + + yield self._send_events_for_new_room( + requester, + room_id, + msg_handler, + room_member_handler, preset_config=preset_config, invite_list=invite_list, initial_state=initial_state, @@ -178,22 +183,6 @@ class RoomCreationHandler(BaseHandler): room_alias=room_alias, ) - msg_handler = self.hs.get_handlers().message_handler - room_member_handler = self.hs.get_handlers().room_member_handler - - for event in creation_events: - if event["type"] == EventTypes.Member: - # TODO(danielwh): This is hideous - yield room_member_handler.update_membership( - requester, - user, - room_id, - "join", - ratelimit=False, - ) - else: - yield msg_handler.create_and_send_nonmember_event(event, ratelimit=False) - if "name" in config: name = config["name"] yield msg_handler.create_and_send_nonmember_event({ @@ -229,7 +218,7 @@ class RoomCreationHandler(BaseHandler): medium = invite_3pid["medium"] yield self.hs.get_handlers().room_member_handler.do_3pid_invite( room_id, - user, + requester.user, medium, address, id_server, @@ -247,19 +236,19 @@ class RoomCreationHandler(BaseHandler): defer.returnValue(result) - def _create_events_for_new_room(self, creator, room_id, preset_config, - invite_list, initial_state, creation_content, - room_alias): - config = RoomCreationHandler.PRESETS_DICT[preset_config] - - creator_id = creator.to_string() - - event_keys = { - "room_id": room_id, - "sender": creator_id, - "state_key": "", - } - + @defer.inlineCallbacks + def _send_events_for_new_room( + self, + creator, # A Requester object. + room_id, + msg_handler, + room_member_handler, + preset_config, + invite_list, + initial_state, + creation_content, + room_alias + ): def create(etype, content, **kwargs): e = { "type": etype, @@ -271,26 +260,39 @@ class RoomCreationHandler(BaseHandler): return e - creation_content.update({"creator": creator.to_string()}) - creation_event = create( + @defer.inlineCallbacks + def send(etype, content, **kwargs): + event = create(etype, content, **kwargs) + yield msg_handler.create_and_send_nonmember_event(event, ratelimit=False) + + config = RoomCreationHandler.PRESETS_DICT[preset_config] + + creator_id = creator.user.to_string() + + event_keys = { + "room_id": room_id, + "sender": creator_id, + "state_key": "", + } + + creation_content.update({"creator": creator_id}) + yield send( etype=EventTypes.Create, content=creation_content, ) - join_event = create( - etype=EventTypes.Member, - state_key=creator_id, - content={ - "membership": Membership.JOIN, - }, + yield room_member_handler.update_membership( + creator, + creator.user, + room_id, + "join", + ratelimit=False, ) - returned_events = [creation_event, join_event] - if (EventTypes.PowerLevels, '') not in initial_state: power_level_content = { "users": { - creator.to_string(): 100, + creator_id: 100, }, "users_default": 0, "events": { @@ -312,45 +314,35 @@ class RoomCreationHandler(BaseHandler): for invitee in invite_list: power_level_content["users"][invitee] = 100 - power_levels_event = create( + yield send( etype=EventTypes.PowerLevels, content=power_level_content, ) - returned_events.append(power_levels_event) - if room_alias and (EventTypes.CanonicalAlias, '') not in initial_state: - room_alias_event = create( + yield send( etype=EventTypes.CanonicalAlias, content={"alias": room_alias.to_string()}, ) - returned_events.append(room_alias_event) - if (EventTypes.JoinRules, '') not in initial_state: - join_rules_event = create( + yield send( etype=EventTypes.JoinRules, content={"join_rule": config["join_rules"]}, ) - returned_events.append(join_rules_event) - if (EventTypes.RoomHistoryVisibility, '') not in initial_state: - history_event = create( + yield send( etype=EventTypes.RoomHistoryVisibility, content={"history_visibility": config["history_visibility"]} ) - returned_events.append(history_event) - for (etype, state_key), content in initial_state.items(): - returned_events.append(create( + yield send( etype=etype, state_key=state_key, content=content, - )) - - return returned_events + ) class RoomMemberHandler(BaseHandler): @@ -465,12 +457,28 @@ class RoomMemberHandler(BaseHandler): @defer.inlineCallbacks def send_membership_event( - self, event, context, is_guest=False, room_hosts=None, ratelimit=True, from_client=True, + self, + event, + context, + is_guest=False, + room_hosts=None, + ratelimit=True, + from_client=True, ): """ Change the membership status of a user in a room. Args: - event (SynapseEvent): The membership event + event (SynapseEvent): The membership event. + context: The context of the event. + is_guest (bool): Whether the sender is a guest. + room_hosts ([str]): Homeservers which are likely to already be in + the room, and could be danced with in order to join this + homeserver for the first time. + ratelimit (bool): Whether to rate limit this request. + from_client (bool): Whether this request is the result of a local + client request (rather than over federation). If so, we will + perform extra checks, like that this homeserver can act as this + client. Raises: SynapseError if there was a problem changing the membership. """ @@ -480,7 +488,8 @@ class RoomMemberHandler(BaseHandler): assert self.hs.is_mine(user), "User must be our own: %s" % (user,) if event.is_state(): - prev_state = self.hs.get_handlers().message_handler.deduplicate_state_event(event, context) + message_handler = self.hs.get_handlers().message_handler + prev_state = message_handler.deduplicate_state_event(event, context) if prev_state is not None: return From d1fb790818e0b93019850ca5db3ffba2baddc745 Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Tue, 16 Feb 2016 14:25:23 +0000 Subject: [PATCH 12/16] Some cleanup --- synapse/handlers/message.py | 14 +++++++++---- synapse/handlers/room.py | 39 +++++++++++++++++++------------------ 2 files changed, 30 insertions(+), 23 deletions(-) diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 05dab172b..9c3471f2e 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -253,12 +253,18 @@ class MessageHandler(BaseHandler): presence.bump_presence_active_time(user) def deduplicate_state_event(self, event, context): - prev_state = context.current_state.get((event.type, event.state_key)) - if prev_state and event.user_id == prev_state.user_id: - prev_content = encode_canonical_json(prev_state.content) + """ + Checks whether event is in the latest resolved state in context. + + If so, returns the version of the event in context. + Otherwise, returns None. + """ + prev_event = context.current_state.get((event.type, event.state_key)) + if prev_event and event.user_id == prev_event.user_id: + prev_content = encode_canonical_json(prev_event.content) next_content = encode_canonical_json(event.content) if prev_content == next_content: - return prev_state + return prev_event return None @defer.inlineCallbacks diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index e384370d7..b7ea321a9 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -493,35 +493,24 @@ class RoomMemberHandler(BaseHandler): if prev_state is not None: return - target_user_id = event.state_key target_user = UserID.from_string(event.state_key) prev_state = context.current_state.get( - (EventTypes.Member, target_user_id), + (EventTypes.Member, target_user.to_string()), None ) room_id = event.room_id - # If we're trying to join a room then we have to do this differently - # if this HS is not currently in the room, i.e. we have to do the - # invite/join dance. if event.membership == Membership.JOIN: - if is_guest: - guest_access = context.current_state.get( - (EventTypes.GuestAccess, ""), - None - ) - is_guest_access_allowed = ( - guest_access - and guest_access.content - and "guest_access" in guest_access.content - and guest_access.content["guest_access"] == "can_join" - ) - if not is_guest_access_allowed: - raise AuthError(403, "Guest access not allowed") + if is_guest and not self._can_guest_join(context.current_state): + # This should be an auth check, but guests are a local concept, + # so don't really fit into the general auth process. + raise AuthError(403, "Guest access not allowed") - room_id = event.room_id + # If we're trying to join a room then we have to do this differently + # if this HS is not currently in the room, i.e. we have to do the + # invite/join dance. # XXX: We don't do an auth check if we are doing an invite # join dance for now, since we're kinda implicitly checking @@ -599,6 +588,18 @@ class RoomMemberHandler(BaseHandler): user = UserID.from_string(event.user_id) user_left_room(self.distributor, user, event.room_id) + def _can_guest_join(self, current_state): + """ + Returns whether a guest can join a room based on its current state. + """ + guest_access = current_state.get((EventTypes.GuestAccess, ""), None) + return ( + guest_access + and guest_access.content + and "guest_access" in guest_access.content + and guest_access.content["guest_access"] == "can_join" + ) + @defer.inlineCallbacks def lookup_room_alias(self, room_alias): """ From 6605adf6699f6c00d219de763d9d14e5b38ddea2 Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Tue, 16 Feb 2016 19:05:02 +0000 Subject: [PATCH 13/16] Some cleanup, some TODOs, more to do --- synapse/handlers/room.py | 132 ++++++++++++++++++--------------------- 1 file changed, 62 insertions(+), 70 deletions(-) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index b7ea321a9..d4bb21e69 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -482,9 +482,9 @@ class RoomMemberHandler(BaseHandler): Raises: SynapseError if there was a problem changing the membership. """ - if from_client: - user = UserID.from_string(event.sender) + user = UserID.from_string(event.sender) + if from_client: assert self.hs.is_mine(user), "User must be our own: %s" % (user,) if event.is_state(): @@ -502,81 +502,59 @@ class RoomMemberHandler(BaseHandler): room_id = event.room_id + handled = False + if event.membership == Membership.JOIN: if is_guest and not self._can_guest_join(context.current_state): # This should be an auth check, but guests are a local concept, # so don't really fit into the general auth process. raise AuthError(403, "Guest access not allowed") - # If we're trying to join a room then we have to do this differently - # if this HS is not currently in the room, i.e. we have to do the - # invite/join dance. - - # XXX: We don't do an auth check if we are doing an invite - # join dance for now, since we're kinda implicitly checking - # that we are allowed to join when we decide whether or not we - # need to do the invite/join dance. - - is_host_in_room = yield self.is_host_in_room(room_id, context) - if is_host_in_room: - should_do_dance = False - elif room_hosts: # TODO: Shouldn't this be remote_room_host? - should_do_dance = True - else: - inviter = yield self.get_inviter(event) - if not inviter: - # return the same error as join_room_alias does - raise SynapseError(404, "No known servers") - should_do_dance = not self.hs.is_mine(inviter) - room_hosts = [inviter.domain] + should_do_dance, room_hosts = yield self._should_do_dance( + room_id, + context, + (yield self.get_inviter(target_user.to_string(), room_id)), + room_hosts, + ) if should_do_dance: - handler = self.hs.get_handlers().federation_handler - yield handler.do_invite_join( + if len(room_hosts) == 0: + # return the same error as join_room_alias does + raise SynapseError(404, "No known servers") + + # We don't do an auth check if we are doing an invite + # join dance for now, since we're kinda implicitly checking + # that we are allowed to join when we decide whether or not we + # need to do the invite/join dance. + yield self.hs.get_handlers().federation_handler.do_invite_join( room_hosts, room_id, event.user_id, event.content, ) - else: - logger.debug("Doing normal join") - - yield self.handle_new_client_event( - event, - context, - extra_users=[target_user], - ratelimit=ratelimit, + handled = True + if event.membership == Membership.LEAVE: + is_host_in_room = yield self.is_host_in_room(room_id, context) + if not is_host_in_room: + # Rejecting an invite, rather than leaving a joined room + handler = self.hs.get_handlers().federation_handler + inviter = yield self.get_inviter(target_user.to_string(), room_id) + if not inviter: + # return the same error as join_room_alias does + raise SynapseError(404, "No known servers") + yield handler.do_remotely_reject_invite( + [inviter.domain], + room_id, + event.user_id ) - - prev_state = context.current_state.get((event.type, event.state_key)) - if not prev_state or prev_state.membership != Membership.JOIN: - # Only fire user_joined_room if the user has acutally joined the - # room. Don't bother if the user is just changing their profile - # info. - user = UserID.from_string(event.user_id) - yield user_joined_room(self.distributor, user, room_id) - else: - if event.membership == Membership.LEAVE: - is_host_in_room = yield self.is_host_in_room(room_id, context) - if not is_host_in_room: - # Rejecting an invite, rather than leaving a joined room - handler = self.hs.get_handlers().federation_handler - inviter = yield self.get_inviter(event) - if not inviter: - # return the same error as join_room_alias does - raise SynapseError(404, "No known servers") - yield handler.do_remotely_reject_invite( - [inviter.domain], - room_id, - event.user_id - ) - return + handled = True # FIXME: This isn't idempotency. if prev_state and prev_state.membership == event.membership: # double same action, treat this event as a NOOP. return + if not handled: yield self.handle_new_client_event( event, context, @@ -584,9 +562,15 @@ class RoomMemberHandler(BaseHandler): ratelimit=ratelimit, ) + if event.membership == Membership.JOIN: + if not prev_state or prev_state.membership != Membership.JOIN: + # Only fire user_joined_room if the user has acutally joined the + # room. Don't bother if the user is just changing their profile + # info. + yield user_joined_room(self.distributor, target_user, room_id) + elif event.membership == Membership.LEAVE: if prev_state and prev_state.membership == Membership.JOIN: - user = UserID.from_string(event.user_id) - user_left_room(self.distributor, user, event.room_id) + user_left_room(self.distributor, target_user, room_id) def _can_guest_join(self, current_state): """ @@ -600,6 +584,21 @@ class RoomMemberHandler(BaseHandler): and guest_access.content["guest_access"] == "can_join" ) + @defer.inlineCallbacks + def _should_do_dance(self, room_id, context, inviter, room_hosts=None): + # TODO: Shouldn't this be remote_room_host? + room_hosts = room_hosts or [] + + # TODO(danielwh): This shouldn't need to yield for this check, we have a context. + is_host_in_room = yield self.is_host_in_room(room_id, context) + if is_host_in_room: + defer.returnValue((False, room_hosts)) + + if inviter and not self.hs.is_mine(inviter): + room_hosts.append(inviter.domain) + + defer.returnValue((True, room_hosts)) + @defer.inlineCallbacks def lookup_room_alias(self, room_alias): """ @@ -625,24 +624,17 @@ class RoomMemberHandler(BaseHandler): defer.returnValue((RoomID.from_string(room_id), hosts)) + # TODO(danielwh): This should use the context, rather than looking up the store. @defer.inlineCallbacks - def get_inviter(self, event): + def get_inviter(self, user_id, room_id): # TODO(markjh): get prev_state from snapshot prev_state = yield self.store.get_room_member( - event.user_id, event.room_id + user_id, room_id ) - if prev_state and prev_state.membership == Membership.INVITE: defer.returnValue(UserID.from_string(prev_state.user_id)) - return - elif "third_party_invite" in event.content: - if "sender" in event.content["third_party_invite"]: - inviter = UserID.from_string( - event.content["third_party_invite"]["sender"] - ) - defer.returnValue(inviter) - defer.returnValue(None) + # TODO(danielwh): This looks insane. Please make it not insane. @defer.inlineCallbacks def is_host_in_room(self, room_id, context): is_host_in_room = yield self.auth.check_host_in_room( From a4e278bfe7972b367e7782102461881c65720c08 Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Wed, 17 Feb 2016 15:25:12 +0000 Subject: [PATCH 14/16] Respond to federated invite with non-empty context Currently, we magically perform an extra database hit to find the inviter, and use this to guess where we should send the event. Instead, fill in a valid context, so that other callers relying on the context actually have one. --- synapse/handlers/_base.py | 51 ++++++++++++++++++++++++++-- synapse/handlers/room.py | 52 +++++++---------------------- synapse/storage/event_federation.py | 8 ++--- 3 files changed, 65 insertions(+), 46 deletions(-) diff --git a/synapse/handlers/_base.py b/synapse/handlers/_base.py index cad37f50e..41e153c93 100644 --- a/synapse/handlers/_base.py +++ b/synapse/handlers/_base.py @@ -147,7 +147,7 @@ class BaseHandler(object): @defer.inlineCallbacks def _create_new_client_event(self, builder): - latest_ret = yield self.store.get_latest_events_in_room( + latest_ret = yield self.store.get_latest_event_ids_and_hashes_in_room( builder.room_id, ) @@ -156,7 +156,10 @@ class BaseHandler(object): else: depth = 1 - prev_events = [(e, h) for e, h, _ in latest_ret] + prev_events = [ + (event_id, prev_hashes) + for event_id, prev_hashes, _ in latest_ret + ] builder.prev_events = prev_events builder.depth = depth @@ -165,6 +168,31 @@ class BaseHandler(object): context = yield state_handler.compute_event_context(builder) + # If we've received an invite over federation, there are no latest + # events in the room, because we don't know enough about the graph + # fragment we received to treat it like a graph, so the above returned + # no relevant events. It may have returned some events (if we have + # joined and left the room), but not useful ones, like the invite. So we + # forcibly set our context to the invite we received over federation. + if ( + not self.is_host_in_room(context.current_state) and + builder.type == EventTypes.Member + ): + prev_member_event = yield self.store.get_room_member( + builder.sender, builder.room_id + ) + if prev_member_event: + builder.prev_events = ( + prev_member_event.event_id, + prev_member_event.prev_events + ) + + context = yield state_handler.compute_event_context( + builder, + old_state=(prev_member_event,), + outlier=True + ) + if builder.is_state(): builder.prev_state = yield self.store.add_event_hashes( context.prev_state_events @@ -187,6 +215,25 @@ class BaseHandler(object): (event, context,) ) + def is_host_in_room(self, current_state): + room_members = [ + (state_key, event.membership) + for ((event_type, state_key), event) in current_state.items() + if event_type == EventTypes.Member + ] + if len(room_members) == 0: + # has the room been created so we can join it? + create_event = current_state.get(("m.room.create", "")) + if create_event: + return True + for (state_key, membership) in room_members: + if ( + UserID.from_string(state_key).domain == self.hs.hostname + and membership == Membership.JOIN + ): + return True + return False + @defer.inlineCallbacks def handle_new_client_event(self, event, context, ratelimit=True, extra_users=[]): # We now need to go and hit out to wherever we need to hit out to. diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index d4bb21e69..f85a5f267 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -510,10 +510,9 @@ class RoomMemberHandler(BaseHandler): # so don't really fit into the general auth process. raise AuthError(403, "Guest access not allowed") - should_do_dance, room_hosts = yield self._should_do_dance( - room_id, + should_do_dance, room_hosts = self._should_do_dance( context, - (yield self.get_inviter(target_user.to_string(), room_id)), + (self.get_inviter(target_user.to_string(), context.current_state)), room_hosts, ) @@ -534,11 +533,11 @@ class RoomMemberHandler(BaseHandler): ) handled = True if event.membership == Membership.LEAVE: - is_host_in_room = yield self.is_host_in_room(room_id, context) + is_host_in_room = self.is_host_in_room(context.current_state) if not is_host_in_room: # Rejecting an invite, rather than leaving a joined room handler = self.hs.get_handlers().federation_handler - inviter = yield self.get_inviter(target_user.to_string(), room_id) + inviter = self.get_inviter(target_user.to_string(), context.current_state) if not inviter: # return the same error as join_room_alias does raise SynapseError(404, "No known servers") @@ -584,20 +583,18 @@ class RoomMemberHandler(BaseHandler): and guest_access.content["guest_access"] == "can_join" ) - @defer.inlineCallbacks - def _should_do_dance(self, room_id, context, inviter, room_hosts=None): + def _should_do_dance(self, context, inviter, room_hosts=None): # TODO: Shouldn't this be remote_room_host? room_hosts = room_hosts or [] - # TODO(danielwh): This shouldn't need to yield for this check, we have a context. - is_host_in_room = yield self.is_host_in_room(room_id, context) + is_host_in_room = self.is_host_in_room(context.current_state) if is_host_in_room: - defer.returnValue((False, room_hosts)) + return False, room_hosts if inviter and not self.hs.is_mine(inviter): room_hosts.append(inviter.domain) - defer.returnValue((True, room_hosts)) + return True, room_hosts @defer.inlineCallbacks def lookup_room_alias(self, room_alias): @@ -624,36 +621,11 @@ class RoomMemberHandler(BaseHandler): defer.returnValue((RoomID.from_string(room_id), hosts)) - # TODO(danielwh): This should use the context, rather than looking up the store. - @defer.inlineCallbacks - def get_inviter(self, user_id, room_id): - # TODO(markjh): get prev_state from snapshot - prev_state = yield self.store.get_room_member( - user_id, room_id - ) + def get_inviter(self, user_id, current_state): + prev_state = current_state.get((EventTypes.Member, user_id)) if prev_state and prev_state.membership == Membership.INVITE: - defer.returnValue(UserID.from_string(prev_state.user_id)) - - # TODO(danielwh): This looks insane. Please make it not insane. - @defer.inlineCallbacks - def is_host_in_room(self, room_id, context): - is_host_in_room = yield self.auth.check_host_in_room( - room_id, - self.hs.hostname - ) - if not is_host_in_room: - # is *anyone* in the room? - room_member_keys = [ - v for (k, v) in context.current_state.keys() if ( - k == "m.room.member" - ) - ] - if len(room_member_keys) == 0: - # has the room been created so we can join it? - create_event = context.current_state.get(("m.room.create", "")) - if create_event: - is_host_in_room = True - defer.returnValue(is_host_in_room) + return UserID.from_string(prev_state.user_id) + return None @defer.inlineCallbacks def get_joined_rooms_for_user(self, user): diff --git a/synapse/storage/event_federation.py b/synapse/storage/event_federation.py index ce2c79402..3489315e0 100644 --- a/synapse/storage/event_federation.py +++ b/synapse/storage/event_federation.py @@ -114,10 +114,10 @@ class EventFederationStore(SQLBaseStore): retcol="event_id", ) - def get_latest_events_in_room(self, room_id): + def get_latest_event_ids_and_hashes_in_room(self, room_id): return self.runInteraction( - "get_latest_events_in_room", - self._get_latest_events_in_room, + "get_latest_event_ids_and_hashes_in_room", + self._get_latest_event_ids_and_hashes_in_room, room_id, ) @@ -132,7 +132,7 @@ class EventFederationStore(SQLBaseStore): desc="get_latest_event_ids_in_room", ) - def _get_latest_events_in_room(self, txn, room_id): + def _get_latest_event_ids_and_hashes_in_room(self, txn, room_id): sql = ( "SELECT e.event_id, e.depth FROM events as e " "INNER JOIN event_forward_extremities as f " From 591af2d074044a70a48b033c4dfc322f58189d3e Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Wed, 17 Feb 2016 15:50:13 +0000 Subject: [PATCH 15/16] Some cleanup I'm not particularly happy with the "action" switching, but there's no convenient way to defer the work that needs to happen after it, so... :( --- synapse/handlers/room.py | 120 +++++++++++++++------------------ synapse/rest/client/v1/room.py | 6 +- 2 files changed, 59 insertions(+), 67 deletions(-) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index f85a5f267..cd04ac09f 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -397,7 +397,7 @@ class RoomMemberHandler(BaseHandler): room_id, action, txn_id=None, - room_hosts=None, + remote_room_hosts=None, ratelimit=True, ): effective_membership_state = action @@ -448,7 +448,7 @@ class RoomMemberHandler(BaseHandler): context, is_guest=requester.is_guest, ratelimit=ratelimit, - room_hosts=room_hosts, + remote_room_hosts=remote_room_hosts, from_client=True, ) @@ -461,11 +461,12 @@ class RoomMemberHandler(BaseHandler): event, context, is_guest=False, - room_hosts=None, + remote_room_hosts=None, ratelimit=True, from_client=True, ): - """ Change the membership status of a user in a room. + """ + Change the membership status of a user in a room. Args: event (SynapseEvent): The membership event. @@ -482,78 +483,64 @@ class RoomMemberHandler(BaseHandler): Raises: SynapseError if there was a problem changing the membership. """ - user = UserID.from_string(event.sender) + target_user = UserID.from_string(event.state_key) + room_id = event.room_id if from_client: - assert self.hs.is_mine(user), "User must be our own: %s" % (user,) + sender = UserID.from_string(event.sender) + assert self.hs.is_mine(sender), "Sender must be our own: %s" % (sender,) if event.is_state(): message_handler = self.hs.get_handlers().message_handler - prev_state = message_handler.deduplicate_state_event(event, context) - if prev_state is not None: + prev_event = message_handler.deduplicate_state_event(event, context) + if prev_event is not None: return - target_user = UserID.from_string(event.state_key) - - prev_state = context.current_state.get( - (EventTypes.Member, target_user.to_string()), - None - ) - - room_id = event.room_id - - handled = False + action = "send" if event.membership == Membership.JOIN: if is_guest and not self._can_guest_join(context.current_state): # This should be an auth check, but guests are a local concept, # so don't really fit into the general auth process. raise AuthError(403, "Guest access not allowed") - - should_do_dance, room_hosts = self._should_do_dance( + do_remote_join_dance, remote_room_hosts = self._should_do_dance( context, - (self.get_inviter(target_user.to_string(), context.current_state)), - room_hosts, + (self.get_inviter(event.state_key, context.current_state)), + remote_room_hosts, ) - - if should_do_dance: - if len(room_hosts) == 0: - # return the same error as join_room_alias does - raise SynapseError(404, "No known servers") - - # We don't do an auth check if we are doing an invite - # join dance for now, since we're kinda implicitly checking - # that we are allowed to join when we decide whether or not we - # need to do the invite/join dance. - yield self.hs.get_handlers().federation_handler.do_invite_join( - room_hosts, - room_id, - event.user_id, - event.content, - ) - handled = True - if event.membership == Membership.LEAVE: + if do_remote_join_dance: + action = "remote_join" + elif event.membership == Membership.LEAVE: is_host_in_room = self.is_host_in_room(context.current_state) if not is_host_in_room: - # Rejecting an invite, rather than leaving a joined room - handler = self.hs.get_handlers().federation_handler - inviter = self.get_inviter(target_user.to_string(), context.current_state) - if not inviter: - # return the same error as join_room_alias does - raise SynapseError(404, "No known servers") - yield handler.do_remotely_reject_invite( - [inviter.domain], - room_id, - event.user_id - ) - handled = True + action = "remote_reject" - # FIXME: This isn't idempotency. - if prev_state and prev_state.membership == event.membership: - # double same action, treat this event as a NOOP. - return + federation_handler = self.hs.get_handlers().federation_handler - if not handled: + if action == "remote_join": + if len(remote_room_hosts) == 0: + raise SynapseError(404, "No known servers") + + # We don't do an auth check if we are doing an invite + # join dance for now, since we're kinda implicitly checking + # that we are allowed to join when we decide whether or not we + # need to do the invite/join dance. + yield federation_handler.do_invite_join( + remote_room_hosts, + event.room_id, + event.user_id, + event.content, + ) + elif action == "remote_reject": + inviter = self.get_inviter(target_user.to_string(), context.current_state) + if not inviter: + raise SynapseError(404, "No known servers") + yield federation_handler.do_remotely_reject_invite( + [inviter.domain], + room_id, + event.user_id + ) + else: yield self.handle_new_client_event( event, context, @@ -561,14 +548,19 @@ class RoomMemberHandler(BaseHandler): ratelimit=ratelimit, ) + prev_member_event = context.current_state.get( + (EventTypes.Member, target_user.to_string()), + None + ) + if event.membership == Membership.JOIN: - if not prev_state or prev_state.membership != Membership.JOIN: + if not prev_member_event or prev_member_event.membership != Membership.JOIN: # Only fire user_joined_room if the user has acutally joined the # room. Don't bother if the user is just changing their profile # info. yield user_joined_room(self.distributor, target_user, room_id) elif event.membership == Membership.LEAVE: - if prev_state and prev_state.membership == Membership.JOIN: + if prev_member_event and prev_member_event.membership == Membership.JOIN: user_left_room(self.distributor, target_user, room_id) def _can_guest_join(self, current_state): @@ -604,7 +596,9 @@ class RoomMemberHandler(BaseHandler): Args: room_alias (RoomAlias): The alias to look up. Returns: - The room ID as a RoomID object. + A tuple of: + The room ID as a RoomID object. + Hosts likely to be participating in the room ([str]). Raises: SynapseError if room alias could not be found. """ @@ -615,11 +609,9 @@ class RoomMemberHandler(BaseHandler): raise SynapseError(404, "No such room alias") room_id = mapping["room_id"] - hosts = mapping["servers"] - if not hosts: - raise SynapseError(404, "No known servers") + servers = mapping["servers"] - defer.returnValue((RoomID.from_string(room_id), hosts)) + defer.returnValue((RoomID.from_string(room_id), servers)) def get_inviter(self, user_id, current_state): prev_state = current_state.get((EventTypes.Member, user_id)) diff --git a/synapse/rest/client/v1/room.py b/synapse/rest/client/v1/room.py index 179fe9a01..1f5ee09dc 100644 --- a/synapse/rest/client/v1/room.py +++ b/synapse/rest/client/v1/room.py @@ -230,11 +230,11 @@ class JoinRoomAliasServlet(ClientV1RestServlet): if RoomID.is_valid(room_identifier): room_id = room_identifier - room_hosts = None + remote_room_hosts = None elif RoomAlias.is_valid(room_identifier): handler = self.handlers.room_member_handler room_alias = RoomAlias.from_string(room_identifier) - room_id, room_hosts = yield handler.lookup_room_alias(room_alias) + room_id, remote_room_hosts = yield handler.lookup_room_alias(room_alias) room_id = room_id.to_string() else: raise SynapseError(400, "%s was not legal room ID or room alias" % ( @@ -247,7 +247,7 @@ class JoinRoomAliasServlet(ClientV1RestServlet): room_id=room_id, action="join", txn_id=txn_id, - room_hosts=room_hosts, + remote_room_hosts=remote_room_hosts, ) defer.returnValue((200, {"room_id": room_id})) From f8d21e1431ce08b267f7b63a0a0772beb2588f25 Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Thu, 18 Feb 2016 11:02:14 +0000 Subject: [PATCH 16/16] Review comments --- synapse/handlers/_base.py | 3 ++- synapse/handlers/message.py | 2 +- synapse/handlers/room.py | 9 ++++----- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/synapse/handlers/_base.py b/synapse/handlers/_base.py index 41e153c93..a516a84a6 100644 --- a/synapse/handlers/_base.py +++ b/synapse/handlers/_base.py @@ -222,7 +222,8 @@ class BaseHandler(object): if event_type == EventTypes.Member ] if len(room_members) == 0: - # has the room been created so we can join it? + # Have we just created the room, and is this about to be the very + # first member event? create_event = current_state.get(("m.room.create", "")) if create_event: return True diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 9c3471f2e..723bc0e34 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -229,7 +229,7 @@ class MessageHandler(BaseHandler): if event.type == EventTypes.Member: raise SynapseError( 500, - "Tried to send member even through non-member codepath" + "Tried to send member event through non-member codepath" ) user = UserID.from_string(event.sender) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index cd04ac09f..b00cac4bd 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -490,11 +490,10 @@ class RoomMemberHandler(BaseHandler): sender = UserID.from_string(event.sender) assert self.hs.is_mine(sender), "Sender must be our own: %s" % (sender,) - if event.is_state(): - message_handler = self.hs.get_handlers().message_handler - prev_event = message_handler.deduplicate_state_event(event, context) - if prev_event is not None: - return + message_handler = self.hs.get_handlers().message_handler + prev_event = message_handler.deduplicate_state_event(event, context) + if prev_event is not None: + return action = "send"