prevent leakage on /aggregations and fix tests

synapse/rest/client/v1/room.py

@@ -748,7 +748,7 @@ def on_POST(self, request, room_id, event_id, txn_id=None):
         relation_ids = relation_chunk.to_dict()
 
         for relation_id in relation_ids:
-            self.event_creation_handler.create_and_send_nonmember_event(
+            yield self.event_creation_handler.create_and_send_nonmember_event(
                 requester,
                 {
                     "type": EventTypes.Redaction,

synapse/rest/client/v2_alpha/relations.py

@@ -147,12 +147,20 @@ def on_GET(self, request, room_id, parent_id, relation_type=None, event_type=Non
 
         # This checks that a) the event exists and b) the user is allowed to
         # view it.
-        yield self.event_handler.get_event(requester.user, room_id, parent_id)
+        event = yield self.event_handler.get_event(requester.user, room_id, parent_id)
 
         limit = parse_integer(request, "limit", default=5)
         from_token = parse_string(request, "from")
         to_token = parse_string(request, "to")
 
+        # Check if the event is redacted, and if so return an empty chunk
+        # list and zero tokens
+        if "redacted_because" in event.unsigned:
+            res = {
+                "chunk": [],
+            }
+            defer.returnValue((200, res))
+
         if from_token:
             from_token = RelationPaginationToken.from_string(from_token)
 
@@ -222,7 +230,7 @@ def on_GET(self, request, room_id, parent_id, relation_type=None, event_type=Non
 
         # This checks that a) the event exists and b) the user is allowed to
         # view it.
-        yield self.event_handler.get_event(requester.user, room_id, parent_id)
+        event = yield self.event_handler.get_event(requester.user, room_id, parent_id)
 
         if relation_type not in (RelationTypes.ANNOTATION, None):
             raise SynapseError(400, "Relation type must be 'annotation'")
@@ -231,6 +239,14 @@ def on_GET(self, request, room_id, parent_id, relation_type=None, event_type=Non
         from_token = parse_string(request, "from")
         to_token = parse_string(request, "to")
 
+        # Check if the event is redacted, and if so return an empty chunk
+        # list and zero tokens
+        if "redacted_because" in event.unsigned:
+            res = {
+                "chunk": [],
+            }
+            defer.returnValue((200, res))
+
         if from_token:
             from_token = AggregationPaginationToken.from_string(from_token)
 

tests/rest/client/v2_alpha/test_relations.py

@@ -93,7 +93,7 @@ def test_deny_membership(self):
     def test_deny_double_react(self):
         """Test that we deny relations on membership events
         """
-        channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", "a")
+        channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", key="a")
         self.assertEquals(200, channel.code, channel.json_body)
 
         channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", "a")
@@ -523,44 +523,127 @@ def test_multi_edit(self):
             {RelationTypes.REPLACE: {"event_id": edit_event_id}},
         )
 
-    def test_aggregation_redaction_redacts_edits(self):
+    def test_relations_redaction_redacts_edits(self):
         """Test that edits of an event are redacted when the original event
         is redacted.
-
-        NOTE: This will delete the event with id self.parent_id and all
-        associated message edits. Be sure not to put any tests after this
-        that require those.
         """
+        # Send a new event
+        res = self.helper.send(self.room, body="Heyo!", tok=self.user_token)
+        original_event_id = res["event_id"]
 
-        # Redact the original
+        # Add a relation
+        channel = self._send_relation(
+            RelationTypes.REPLACE,
+            "m.room.message",
+            parent_id=original_event_id,
+            content={
+                "msgtype": "m.text",
+                "body": "Wibble",
+                "m.new_content": {"msgtype": "m.text", "body": "First edit"},
+            },
+        )
+        self.assertEquals(200, channel.code, channel.json_body)
+
+        # Check the relation is returned
+        request, channel = self.make_request(
+            "GET",
+            "/_matrix/client/unstable/rooms/%s/relations/%s/m.replace/m.room.message"
+            % (self.room, original_event_id),
+            access_token=self.user_token,
+        )
+        self.render(request)
+        self.assertEquals(200, channel.code, channel.json_body)
+
+        self.assertIn("chunk", channel.json_body)
+        self.assertEquals(len(channel.json_body["chunk"]), 1)
+
+        # Redact the original event
         request, channel = self.make_request(
             "PUT",
             "/rooms/%s/redact/%s/%s"
-            % (self.room, self.parent_id, "test_aggregation_redaction_redacts_edits"),
+            % (self.room, original_event_id, "test_relations_redaction_redacts_edits"),
             access_token=self.user_token,
+            content="{}",
         )
         self.render(request)
         self.assertEquals(200, channel.code, channel.json_body)
 
-        # TODO: Make this request 403 (will need to stick it in the new agg. MSC most likely)
         # Try to check for remaining m.replace relations
         request, channel = self.make_request(
             "GET",
             "/_matrix/client/unstable/rooms/%s/relations/%s/m.replace/m.room.message"
-            % (self.room, self.parent_id),
+            % (self.room, original_event_id),
             access_token=self.user_token,
         )
         self.render(request)
-        self.assertEquals(403, channel.code, channel.json_body)
+        self.assertEquals(200, channel.code, channel.json_body)
+
+        # Check that no relations are returned
+        self.assertIn("chunk", channel.json_body)
+        self.assertEquals(
+            channel.json_body["chunk"], [],
+        )
+
+    def test_aggregations_redaction_prevents_access_to_aggregations(self):
+        """Test that annotations of an event are redacted when the original event
+        is redacted.
+        """
+        # Send a new event
+        res = self.helper.send(self.room, body="Hello!", tok=self.user_token)
+        original_event_id = res["event_id"]
+
+        # Add a relation
+        channel = self._send_relation(
+            RelationTypes.ANNOTATION, "m.reaction", key="👍", parent_id=original_event_id,
+        )
+        self.assertEquals(200, channel.code, channel.json_body)
+
+        # Redact the original
+        request, channel = self.make_request(
+            "PUT",
+            "/rooms/%s/redact/%s/%s"
+            % (
+                self.room,
+                original_event_id,
+                "test_aggregations_redaction_prevents_access_to_aggregations",
+            ),
+            access_token=self.user_token,
+            content="{}",
+        )
+        self.render(request)
+        self.assertEquals(200, channel.code, channel.json_body)
+
+        # Check that aggregations returns zero
+        request, channel = self.make_request(
+            "GET",
+            "/_matrix/client/unstable/rooms/%s/aggregations/%s/m.annotation/m.reaction"
+            % (self.room, original_event_id),
+            access_token=self.user_token,
+        )
+        self.render(request)
+        self.assertEquals(200, channel.code, channel.json_body)
+
+        self.assertIn("chunk", channel.json_body)
+        self.assertEquals(
+            channel.json_body["chunk"], [],
+        )
+
 
     def _send_relation(
-        self, relation_type, event_type, key=None, content={}, access_token=None
+        self,
+        relation_type,
+        event_type, 
+        key=None,
+        content={},
+        access_token=None,
+        parent_id=None,
     ):
         """Helper function to send a relation pointing at `self.parent_id`
 
         Args:
             relation_type (str): One of `RelationTypes`
             event_type (str): The type of the event to create
+            parent_id (str): The event_id this relation relates to. If None, then self.parent_id
             key (str|None): The aggregation key used for m.annotation relation
                 type.
             content(dict|None): The content of the created event.
@@ -577,10 +660,12 @@ def _send_relation(
         if key:
             query = "?key=" + six.moves.urllib.parse.quote_plus(key.encode("utf-8"))
 
+        original_id = parent_id if parent_id else self.parent_id
+
         request, channel = self.make_request(
             "POST",
             "/_matrix/client/unstable/rooms/%s/send_relation/%s/%s/%s%s"
-            % (self.room, self.parent_id, relation_type, event_type, query),
+            % (self.room, original_id, relation_type, event_type, query),
             json.dumps(content).encode("utf-8"),
             access_token=access_token,
         )