Do checks for memberships before creating events

synapse/handlers/room_member.py

@@ -95,6 +95,80 @@ class RoomMemberHandler(BaseHandler):
                 if remotedomains is not None:
                     remotedomains.add(member.domain)
 
+    @defer.inlineCallbacks
+    def _local_membership_update(
+        self, requester, target, room_id, membership,
+        txn_id=None,
+        ratelimit=True,
+    ):
+        msg_handler = self.hs.get_handlers().message_handler
+
+        content = {"membership": membership}
+        if requester.is_guest:
+            content["kind"] = "guest"
+
+        event, context = yield msg_handler.create_event(
+            {
+                "type": EventTypes.Member,
+                "content": content,
+                "room_id": room_id,
+                "sender": requester.user.to_string(),
+                "state_key": target.to_string(),
+
+                # For backwards compatibility:
+                "membership": membership,
+            },
+            token_id=requester.access_token_id,
+            txn_id=txn_id,
+        )
+
+        yield self.handle_new_client_event(
+            requester,
+            event,
+            context,
+            extra_users=[target],
+            ratelimit=ratelimit,
+        )
+
+        prev_member_event = context.current_state.get(
+            (EventTypes.Member, target.to_string()),
+            None
+        )
+
+        if event.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, room_id)
+        elif event.membership == Membership.LEAVE:
+            if prev_member_event and prev_member_event.membership == Membership.JOIN:
+                user_left_room(self.distributor, target, room_id)
+
+    @defer.inlineCallbacks
+    def remote_join(self, remote_room_hosts, room_id, user, content):
+        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 self.hs.get_handlers().federation_handler.do_invite_join(
+            remote_room_hosts,
+            room_id,
+            user.to_string(),
+            content,
+        )
+        yield user_joined_room(self.distributor, user, room_id)
+
+    def reject_remote_invite(self, user_id, room_id, remote_room_hosts):
+        return self.hs.get_handlers().federation_handler.do_remotely_reject_invite(
+            remote_room_hosts,
+            room_id,
+            user_id
+        )
+
     @defer.inlineCallbacks
     def update_membership(
             self,
@@ -120,28 +194,15 @@ class RoomMemberHandler(BaseHandler):
                 third_party_signed,
             )
 
-        msg_handler = self.hs.get_handlers().message_handler
+        if not remote_room_hosts:
+            remote_room_hosts = []
 
-        content = {"membership": effective_membership_state}
+        latest_event_ids = yield self.store.get_latest_event_ids_in_room(room_id)
-        if requester.is_guest:
+        current_state = yield self.state_handler.get_current_state(
-            content["kind"] = "guest"
+            room_id, latest_event_ids=latest_event_ids,
-
-        event, context = yield msg_handler.create_event(
-            {
-                "type": EventTypes.Member,
-                "content": content,
-                "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,
         )
 
-        old_state = context.current_state.get((EventTypes.Member, event.state_key))
+        old_state = current_state.get((EventTypes.Member, target.to_string()))
         old_membership = old_state.content.get("membership") if old_state else None
         if action == "unban" and old_membership != "ban":
             raise SynapseError(
@@ -156,13 +217,57 @@ class RoomMemberHandler(BaseHandler):
                 errcode=Codes.BAD_STATE
             )
 
-        member_handler = self.hs.get_handlers().room_member_handler
+        is_host_in_room = self.is_host_in_room(current_state)
-        yield member_handler.send_membership_event(
+
-            requester,
+        if effective_membership_state == Membership.JOIN:
-            event,
+            if requester.is_guest and not self._can_guest_join(current_state):
-            context,
+                # 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 not is_host_in_room:
+                inviter = yield self.get_inviter(target.to_string(), room_id)
+                if inviter and not self.hs.is_mine(inviter):
+                    remote_room_hosts.append(inviter.domain)
+
+                content = {"membership": Membership.JOIN}
+                if requester.is_guest:
+                    content["kind"] = "guest"
+
+                ret = yield self.remote_join(
+                    remote_room_hosts, room_id, target, content
+                )
+                defer.returnValue(ret)
+
+        elif effective_membership_state == Membership.LEAVE:
+            if not is_host_in_room:
+                # perhaps we've been invited
+                inviter = yield self.get_inviter(target.to_string(), room_id)
+                if not inviter:
+                    raise SynapseError(404, "Not a known room")
+
+                if self.hs.is_mine(inviter):
+                    # the inviter was on our server, but has now left. Carry on
+                    # with the normal rejection codepath.
+                    #
+                    # This is a bit of a hack, because the room might still be
+                    # active on other servers.
+                    pass
+                else:
+                    # send the rejection to the inviter's HS.
+                    remote_room_hosts = remote_room_hosts + [inviter.domain]
+                    ret = yield self.reject_remote_invite(
+                        target.to_string(), room_id, remote_room_hosts
+                    )
+                    defer.returnValue(ret)
+
+        yield self._local_membership_update(
+            requester=requester,
+            target=target,
+            room_id=room_id,
+            membership=effective_membership_state,
+            txn_id=txn_id,
             ratelimit=ratelimit,
-            remote_room_hosts=remote_room_hosts,
         )
 
     @defer.inlineCallbacks
@@ -211,73 +316,19 @@ class RoomMemberHandler(BaseHandler):
         if prev_event is not None:
             return
 
-        action = "send"
-
         if event.membership == Membership.JOIN:
             if requester.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")
-            do_remote_join_dance, remote_room_hosts = self._should_do_dance(
-                context,
-                (self.get_inviter(event.state_key, context.current_state)),
-                remote_room_hosts,
-            )
-            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:
+        yield self.handle_new_client_event(
-                # perhaps we've been invited
+            requester,
-                inviter = self.get_inviter(
+            event,
-                    target_user.to_string(), context.current_state
+            context,
-                )
+            extra_users=[target_user],
-                if not inviter:
+            ratelimit=ratelimit,
-                    raise SynapseError(404, "Not a known room")
+        )
-
-                if self.hs.is_mine(inviter):
-                    # the inviter was on our server, but has now left. Carry on
-                    # with the normal rejection codepath.
-                    #
-                    # This is a bit of a hack, because the room might still be
-                    # active on other servers.
-                    pass
-                else:
-                    # send the rejection to the inviter's HS.
-                    remote_room_hosts = remote_room_hosts + [inviter.domain]
-                    action = "remote_reject"
-
-        federation_handler = self.hs.get_handlers().federation_handler
-
-        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":
-            yield federation_handler.do_remotely_reject_invite(
-                remote_room_hosts,
-                room_id,
-                event.user_id
-            )
-        else:
-            yield self.handle_new_client_event(
-                requester,
-                event,
-                context,
-                extra_users=[target_user],
-                ratelimit=ratelimit,
-            )
 
         prev_member_event = context.current_state.get(
             (EventTypes.Member, target_user.to_string()),
@@ -306,11 +357,11 @@ class RoomMemberHandler(BaseHandler):
             and guest_access.content["guest_access"] == "can_join"
         )
 
-    def _should_do_dance(self, context, inviter, room_hosts=None):
+    def _should_do_dance(self, current_state, inviter, room_hosts=None):
         # TODO: Shouldn't this be remote_room_host?
         room_hosts = room_hosts or []
 
-        is_host_in_room = self.is_host_in_room(context.current_state)
+        is_host_in_room = self.is_host_in_room(current_state)
         if is_host_in_room:
             return False, room_hosts
 
@@ -344,11 +395,11 @@ class RoomMemberHandler(BaseHandler):
 
         defer.returnValue((RoomID.from_string(room_id), servers))
 
-    def get_inviter(self, user_id, current_state):
+    @defer.inlineCallbacks
-        prev_state = current_state.get((EventTypes.Member, user_id))
+    def get_inviter(self, user_id, room_id):
-        if prev_state and prev_state.membership == Membership.INVITE:
+        invite = yield self.store.get_room_member(user_id=user_id, room_id=room_id)
-            return UserID.from_string(prev_state.user_id)
+        if invite:
-        return None
+            defer.returnValue(UserID.from_string(invite.sender))
 
     @defer.inlineCallbacks
     def get_joined_rooms_for_user(self, user):

synapse/state.py

@@ -75,7 +75,8 @@ class StateHandler(object):
         self._state_cache.start()
 
     @defer.inlineCallbacks
-    def get_current_state(self, room_id, event_type=None, state_key=""):
+    def get_current_state(self, room_id, event_type=None, state_key="",
+                          latest_event_ids=None):
         """ Retrieves the current state for the room. This is done by
         calling `get_latest_events_in_room` to get the leading edges of the
         event graph and then resolving any of the state conflicts.
@@ -88,9 +89,10 @@ class StateHandler(object):
 
         :returns map from (type, state_key) to event
         """
-        event_ids = yield self.store.get_latest_event_ids_in_room(room_id)
+        if not latest_event_ids:
+            latest_event_ids = yield self.store.get_latest_event_ids_in_room(room_id)
 
-        res = yield self.resolve_state_groups(room_id, event_ids)
+        res = yield self.resolve_state_groups(room_id, latest_event_ids)
         state = res[1]
 
         if event_type:

tests/rest/client/v1/test_rooms.py

@@ -259,8 +259,8 @@ class RoomPermissionsTestCase(RestTestCase):
         # set [invite/join/left] of self, set [invite/join/left] of other,
         # expect all 404s because room doesn't exist on any server
         for usr in [self.user_id, self.rmcreator_id]:
-            yield self.join(room=room, user=usr, expect_code=404)
+            yield self.join(room=room, user=usr, expect_code=403)
-            yield self.leave(room=room, user=usr, expect_code=404)
+            yield self.leave(room=room, user=usr, expect_code=403)
 
     @defer.inlineCallbacks
     def test_membership_private_room_perms(self):