From d1fb790818e0b93019850ca5db3ffba2baddc745 Mon Sep 17 00:00:00 2001
From: Daniel Wagner-Hall <daniel@matrix.org>
Date: Tue, 16 Feb 2016 14:25:23 +0000
Subject: [PATCH] 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 05dab172b8..9c3471f2e3 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 e384370d7e..b7ea321a9b 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):
         """