Fix bug where people could join private rooms

3 files changed, 63 insertions, 45 deletions

diff --git a/synapse/api/auth.py b/synapse/api/auth.py
index 50ce7eb4cd..93a3533304 100644
--- a/synapse/api/auth.py
+++ b/synapse/api/auth.py
@@ -49,12 +49,12 @@ class Auth(object):
         """
         try:
             if hasattr(event, "room_id"):
-                if not event.old_state_events:
+                if event.old_state_events is None:
                     # Oh, we don't know what the state of the room was, so we
                     # are trusting that this is allowed (at least for now)
                     defer.returnValue(True)
 
-                if hasattr(event, "outlier") and event.outlier:
+                if hasattr(event, "outlier") and event.outlier is True:
                     # TODO (erikj): Auth for outliers is done differently.
                     defer.returnValue(True)
 
@@ -65,8 +65,12 @@ class Auth(object):
                     defer.returnValue(True)
 
                 if event.type == RoomMemberEvent.TYPE:
-                    yield self._can_replace_state(event)
-                    allowed = yield self.is_membership_change_allowed(event)
+                    self._can_replace_state(event)
+                    allowed = self.is_membership_change_allowed(event)
+                    if allowed:
+                        logger.debug("Allowing! %s", event)
+                    else:
+                        logger.debug("Denying! %s", event)
                     defer.returnValue(allowed)
                     return
 
@@ -77,24 +81,28 @@ class Auth(object):
                     # TODO (erikj): This really only should be called for *new*
                     # state
                     yield self._can_add_state(event)
-                    yield self._can_replace_state(event)
+                    self._can_replace_state(event)
                 else:
                     yield self._can_send_event(event)
 
                 if event.type == RoomPowerLevelsEvent.TYPE:
-                    yield self._check_power_levels(event)
+                    self._check_power_levels(event)
 
                 if event.type == RoomRedactionEvent.TYPE:
-                    yield self._check_redaction(event)
+                    self._check_redaction(event)
 
+
+                logger.debug("Allowing! %s", event)
                 defer.returnValue(True)
             else:
                 raise AuthError(500, "Unknown event: %s" % event)
         except AuthError as e:
             logger.info("Event auth check failed on event %s with msg: %s",
                         event, e.msg)
+            logger.info("Denying! %s", event)
             if raises:
                 raise e
+
         defer.returnValue(False)
 
     @defer.inlineCallbacks
@@ -126,7 +134,7 @@ class Auth(object):
                 user_id, room_id, repr(member)
             ))
 
-    @defer.inlineCallbacks
+    @log_function
     def is_membership_change_allowed(self, event):
         target_user_id = event.state_key
 
@@ -159,11 +167,23 @@ class Auth(object):
         )
 
         ban_level, kick_level, redact_level = (
-            yield self._get_ops_level_from_event_state(
+            self._get_ops_level_from_event_state(
                 event
             )
         )
 
+        logger.debug(
+            "is_membership_change_allowed: %s",
+            {
+                "caller_in_room": caller_in_room,
+                "target_in_room": target_in_room,
+                "membership": membership,
+                "join_rule": join_rule,
+                "target_user_id": target_user_id,
+                "event.user_id": event.user_id,
+            }
+        )
+
         if Membership.INVITE == membership:
             # TODO (erikj): We should probably handle this more intelligently
             # PRIVATE join rules.
@@ -183,10 +203,7 @@ class Auth(object):
             elif join_rule == JoinRules.PUBLIC:
                 pass
             elif join_rule == JoinRules.INVITE:
-                if (
-                    not caller or caller.membership not in
-                    [Membership.INVITE, Membership.JOIN]
-                ):
+                if not caller_in_room:
                     raise AuthError(403, "You are not invited to this room.")
             else:
                 # TODO (erikj): may_join list
@@ -218,7 +235,7 @@ class Auth(object):
         else:
             raise AuthError(500, "Unknown membership %s" % membership)
 
-        defer.returnValue(True)
+        return True
 
     def _get_power_level_from_event_state(self, event, user_id):
         key = (RoomPowerLevelsEvent.TYPE, "", )
@@ -359,17 +376,7 @@ class Auth(object):
 
         defer.returnValue(True)
 
-    @defer.inlineCallbacks
     def _can_replace_state(self, event):
-        current_state = yield self.store.get_current_state(
-            event.room_id,
-            event.type,
-            event.state_key,
-        )
-
-        if current_state:
-            current_state = current_state[0]
-
         user_level = self._get_power_level_from_event_state(
             event,
             event.user_id,
@@ -383,6 +390,10 @@ class Auth(object):
         logger.debug(
             "Checking power level for %s, %s", event.user_id, user_level
         )
+
+        key = (event.type, event.state_key, )
+        current_state = event.old_state_events.get(key)
+
         if current_state and hasattr(current_state, "required_power_level"):
             req = current_state.required_power_level
 
@@ -393,19 +404,20 @@ class Auth(object):
                     "You don't have permission to change that state"
                 )
 
-    @defer.inlineCallbacks
     def _check_redaction(self, event):
-        user_level = yield self.store.get_power_level(
-            event.room_id,
-            event.user_id,
-        )
-
         user_level = self._get_power_level_from_event_state(
             event,
             event.user_id,
         )
 
-        _, _, redact_level  = yield self.store.get_ops_levels(event.room_id)
+        if user_level:
+            user_level = int(user_level)
+        else:
+            user_level = 0
+
+        _, _, redact_level = self.store._get_ops_level_from_event_state(
+            event.room_id
+        )
 
         if not redact_level:
             redact_level = 50
@@ -416,7 +428,6 @@ class Auth(object):
                 "You don't have permission to redact events"
             )
 
-    @defer.inlineCallbacks
     def _check_power_levels(self, event):
         for k, v in event.content.items():
             if k == "default":
@@ -436,19 +447,16 @@ class Auth(object):
             except:
                 raise SynapseError(400, "Not a valid power level: %s" % (v,))
 
-        current_state = yield self.store.get_current_state(
-            event.room_id,
-            event.type,
-            event.state_key,
-        )
+        key = (event.type, event.state_key, )
+        current_state = event.old_state_events.get(key)
 
         if not current_state:
             return
         else:
             current_state = current_state[0]
 
-        user_level = yield self.store.get_power_level(
-            event.room_id,
+        user_level = self._get_power_level_from_event_state(
+            event,
             event.user_id,
         )
 
diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py
index 70790aaa72..8c80a37164 100644
--- a/synapse/handlers/federation.py
+++ b/synapse/handlers/federation.py
@@ -269,12 +269,12 @@ class FederationHandler(BaseHandler):
         del self.room_queues[room_id]
 
         for p in room_queue:
-            p.outlier = True
             yield self.on_receive_pdu(p, backfilled=False)
 
         defer.returnValue(True)
 
     @defer.inlineCallbacks
+    @log_function
     def on_make_join_request(self, context, user_id):
         event = self.event_factory.create_event(
             etype=RoomMemberEvent.TYPE,
@@ -289,15 +289,21 @@ class FederationHandler(BaseHandler):
         )
         snapshot.fill_out_prev_events(event)
 
+        yield self.state_handler.annotate_state_groups(event)
+        yield self.auth.check(event, None, raises=True)
+
         pdu = self.pdu_codec.pdu_from_event(event)
 
         defer.returnValue(pdu)
 
     @defer.inlineCallbacks
+    @log_function
     def on_send_join_request(self, origin, pdu):
         event = self.pdu_codec.event_from_pdu(pdu)
 
-        is_new_state= yield self.state_handler.annotate_state_groups(event)
+        event.outlier = False
+
+        is_new_state = yield self.state_handler.annotate_state_groups(event)
         yield self.auth.check(event, None, raises=True)
 
         # FIXME (erikj):  All this is duplicated above :(
diff --git a/synapse/state.py b/synapse/state.py
index 24685c6fb4..c062cef757 100644
--- a/synapse/state.py
+++ b/synapse/state.py
@@ -22,6 +22,7 @@ from synapse.federation.pdu_codec import encode_event_id
 
 from collections import namedtuple
 
+import copy
 import logging
 import hashlib
 
@@ -143,13 +144,13 @@ class StateHandler(object):
         if hasattr(event, "outlier") and event.outlier:
             event.state_group = None
             event.old_state_events = None
-            event.state_events = None
+            event.state_events = {}
             defer.returnValue(False)
             return
 
         new_state = yield self.resolve_state_groups(event.prev_events)
 
-        event.old_state_events = new_state
+        event.old_state_events = copy.deepcopy(new_state)
 
         if hasattr(event, "state_key"):
             new_state[(event.type, event.state_key)] = event
@@ -164,9 +165,12 @@ class StateHandler(object):
         # FIXME: HACK!
         pdus = yield self.store.get_latest_pdus_in_context(room_id)
 
-        event_ids = [encode_event_id(p.pdu_id, p.origin) for p in pdus]
+        event_ids = [
+            encode_event_id(pdu_id, origin)
+            for pdu_id, origin, _ in pdus
+        ]
 
-        res = self.resolve_state_groups(event_ids)
+        res = yield self.resolve_state_groups(event_ids)
 
         if event_type:
             defer.returnValue(res.get((event_type, state_key)))