From 09957ce0e4dcfd84c2de4039653059faae03065b Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Mon, 4 Nov 2019 17:09:22 +0000
Subject: Implement per-room message retention policies

---
 tests/rest/client/test_retention.py | 320 ++++++++++++++++++++++++++++++++++++
 1 file changed, 320 insertions(+)
 create mode 100644 tests/rest/client/test_retention.py

(limited to 'tests/rest/client')

diff --git a/tests/rest/client/test_retention.py b/tests/rest/client/test_retention.py
new file mode 100644
index 0000000000..41ea9db689
--- /dev/null
+++ b/tests/rest/client/test_retention.py
@@ -0,0 +1,320 @@
+# -*- coding: utf-8 -*-
+# Copyright 2019 New Vector Ltd
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+from mock import Mock
+
+from synapse.api.constants import EventTypes
+from synapse.rest import admin
+from synapse.rest.client.v1 import login, room
+from synapse.visibility import filter_events_for_client
+
+from tests import unittest
+
+one_hour_ms = 3600000
+one_day_ms = one_hour_ms * 24
+
+
+class RetentionTestCase(unittest.HomeserverTestCase):
+    servlets = [
+        admin.register_servlets,
+        login.register_servlets,
+        room.register_servlets,
+    ]
+
+    def make_homeserver(self, reactor, clock):
+        config = self.default_config()
+        config["default_room_version"] = "1"
+        config["retention"] = {
+            "enabled": True,
+            "default_policy": {
+                "min_lifetime": one_day_ms,
+                "max_lifetime": one_day_ms * 3,
+            },
+            "allowed_lifetime_min": one_day_ms,
+            "allowed_lifetime_max": one_day_ms * 3,
+        }
+
+        self.hs = self.setup_test_homeserver(config=config)
+        return self.hs
+
+    def prepare(self, reactor, clock, homeserver):
+        self.user_id = self.register_user("user", "password")
+        self.token = self.login("user", "password")
+
+    def test_retention_state_event(self):
+        """Tests that the server configuration can limit the values a user can set to the
+        room's retention policy.
+        """
+        room_id = self.helper.create_room_as(self.user_id, tok=self.token)
+
+        self.helper.send_state(
+            room_id=room_id,
+            event_type=EventTypes.Retention,
+            body={
+                "max_lifetime": one_day_ms * 4,
+            },
+            tok=self.token,
+            expect_code=400,
+        )
+
+        self.helper.send_state(
+            room_id=room_id,
+            event_type=EventTypes.Retention,
+            body={
+                "max_lifetime": one_hour_ms,
+            },
+            tok=self.token,
+            expect_code=400,
+        )
+
+    def test_retention_event_purged_with_state_event(self):
+        """Tests that expired events are correctly purged when the room's retention policy
+        is defined by a state event.
+        """
+        room_id = self.helper.create_room_as(self.user_id, tok=self.token)
+
+        # Set the room's retention period to 2 days.
+        lifetime = one_day_ms * 2
+        self.helper.send_state(
+            room_id=room_id,
+            event_type=EventTypes.Retention,
+            body={
+                "max_lifetime": lifetime,
+            },
+            tok=self.token,
+        )
+
+        self._test_retention_event_purged(room_id, one_day_ms * 1.5)
+
+    def test_retention_event_purged_without_state_event(self):
+        """Tests that expired events are correctly purged when the room's retention policy
+        is defined by the server's configuration's default retention policy.
+        """
+        room_id = self.helper.create_room_as(self.user_id, tok=self.token)
+
+        self._test_retention_event_purged(room_id, one_day_ms * 2)
+
+    def test_visibility(self):
+        """Tests that synapse.visibility.filter_events_for_client correctly filters out
+        outdated events
+        """
+        store = self.hs.get_datastore()
+        storage = self.hs.get_storage()
+        room_id = self.helper.create_room_as(self.user_id, tok=self.token)
+        events = []
+
+        # Send a first event, which should be filtered out at the end of the test.
+        resp = self.helper.send(
+            room_id=room_id,
+            body="1",
+            tok=self.token,
+        )
+
+        # Get the event from the store so that we end up with a FrozenEvent that we can
+        # give to filter_events_for_client. We need to do this now because the event won't
+        # be in the database anymore after it has expired.
+        events.append(self.get_success(
+            store.get_event(
+                resp.get("event_id")
+            )
+        ))
+
+        # Advance the time by 2 days. We're using the default retention policy, therefore
+        # after this the first event will still be valid.
+        self.reactor.advance(one_day_ms * 2 / 1000)
+
+        # Send another event, which shouldn't get filtered out.
+        resp = self.helper.send(
+            room_id=room_id,
+            body="2",
+            tok=self.token,
+        )
+
+        valid_event_id = resp.get("event_id")
+
+        events.append(self.get_success(
+            store.get_event(
+                valid_event_id
+            )
+        ))
+
+        # Advance the time by anothe 2 days. After this, the first event should be
+        # outdated but not the second one.
+        self.reactor.advance(one_day_ms * 2 / 1000)
+
+        # Run filter_events_for_client with our list of FrozenEvents.
+        filtered_events = self.get_success(filter_events_for_client(
+            storage, self.user_id, events
+        ))
+
+        # We should only get one event back.
+        self.assertEqual(len(filtered_events), 1, filtered_events)
+        # That event should be the second, not outdated event.
+        self.assertEqual(filtered_events[0].event_id, valid_event_id, filtered_events)
+
+    def _test_retention_event_purged(self, room_id, increment):
+        # Send a first event to the room. This is the event we'll want to be purged at the
+        # end of the test.
+        resp = self.helper.send(
+            room_id=room_id,
+            body="1",
+            tok=self.token,
+        )
+
+        expired_event_id = resp.get("event_id")
+
+        # Check that we can retrieve the event.
+        expired_event = self.get_event(room_id, expired_event_id)
+        self.assertEqual(expired_event.get("content", {}).get("body"), "1", expired_event)
+
+        # Advance the time.
+        self.reactor.advance(increment / 1000)
+
+        # Send another event. We need this because the purge job won't purge the most
+        # recent event in the room.
+        resp = self.helper.send(
+            room_id=room_id,
+            body="2",
+            tok=self.token,
+        )
+
+        valid_event_id = resp.get("event_id")
+
+        # Advance the time again. Now our first event should have expired but our second
+        # one should still be kept.
+        self.reactor.advance(increment / 1000)
+
+        # Check that the event has been purged from the database.
+        self.get_event(room_id, expired_event_id, expected_code=404)
+
+        # Check that the event that hasn't been purged can still be retrieved.
+        valid_event = self.get_event(room_id, valid_event_id)
+        self.assertEqual(valid_event.get("content", {}).get("body"), "2", valid_event)
+
+    def get_event(self, room_id, event_id, expected_code=200):
+        url = "/_matrix/client/r0/rooms/%s/event/%s" % (room_id, event_id)
+
+        request, channel = self.make_request("GET", url, access_token=self.token)
+        self.render(request)
+
+        self.assertEqual(channel.code, expected_code, channel.result)
+
+        return channel.json_body
+
+
+class RetentionNoDefaultPolicyTestCase(unittest.HomeserverTestCase):
+    servlets = [
+        admin.register_servlets,
+        login.register_servlets,
+        room.register_servlets,
+    ]
+
+    def make_homeserver(self, reactor, clock):
+        config = self.default_config()
+        config["default_room_version"] = "1"
+        config["retention"] = {
+            "enabled": True,
+        }
+
+        mock_federation_client = Mock(spec=["backfill"])
+
+        self.hs = self.setup_test_homeserver(
+            config=config,
+            federation_client=mock_federation_client,
+        )
+        return self.hs
+
+    def prepare(self, reactor, clock, homeserver):
+        self.user_id = self.register_user("user", "password")
+        self.token = self.login("user", "password")
+
+    def test_no_default_policy(self):
+        """Tests that an event doesn't get expired if there is neither a default retention
+        policy nor a policy specific to the room.
+        """
+        room_id = self.helper.create_room_as(self.user_id, tok=self.token)
+
+        self._test_retention(room_id)
+
+    def test_state_policy(self):
+        """Tests that an event gets correctly expired if there is no default retention
+        policy but there's a policy specific to the room.
+        """
+        room_id = self.helper.create_room_as(self.user_id, tok=self.token)
+
+        # Set the maximum lifetime to 35 days so that the first event gets expired but not
+        # the second one.
+        self.helper.send_state(
+            room_id=room_id,
+            event_type=EventTypes.Retention,
+            body={
+                "max_lifetime": one_day_ms * 35,
+            },
+            tok=self.token,
+        )
+
+        self._test_retention(room_id, expected_code_for_first_event=404)
+
+    def _test_retention(self, room_id, expected_code_for_first_event=200):
+        # Send a first event to the room. This is the event we'll want to be purged at the
+        # end of the test.
+        resp = self.helper.send(
+            room_id=room_id,
+            body="1",
+            tok=self.token,
+        )
+
+        first_event_id = resp.get("event_id")
+
+        # Check that we can retrieve the event.
+        expired_event = self.get_event(room_id, first_event_id)
+        self.assertEqual(expired_event.get("content", {}).get("body"), "1", expired_event)
+
+        # Advance the time by a month.
+        self.reactor.advance(one_day_ms * 30 / 1000)
+
+        # Send another event. We need this because the purge job won't purge the most
+        # recent event in the room.
+        resp = self.helper.send(
+            room_id=room_id,
+            body="2",
+            tok=self.token,
+        )
+
+        second_event_id = resp.get("event_id")
+
+        # Advance the time by another month.
+        self.reactor.advance(one_day_ms * 30 / 1000)
+
+        # Check if the event has been purged from the database.
+        first_event = self.get_event(
+            room_id, first_event_id, expected_code=expected_code_for_first_event
+        )
+
+        if expected_code_for_first_event == 200:
+            self.assertEqual(first_event.get("content", {}).get("body"), "1", first_event)
+
+        # Check that the event that hasn't been purged can still be retrieved.
+        second_event = self.get_event(room_id, second_event_id)
+        self.assertEqual(second_event.get("content", {}).get("body"), "2", second_event)
+
+    def get_event(self, room_id, event_id, expected_code=200):
+        url = "/_matrix/client/r0/rooms/%s/event/%s" % (room_id, event_id)
+
+        request, channel = self.make_request("GET", url, access_token=self.token)
+        self.render(request)
+
+        self.assertEqual(channel.code, expected_code, channel.result)
+
+        return channel.json_body
-- 
cgit 1.4.1


From f03c9d34442368c8f2c26d2ac16b770bc451c76d Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Wed, 6 Nov 2019 15:47:40 +0000
Subject: Don't apply retention policy based filtering on state events

As per MSC1763, 'Retention is only considered for non-state events.', so don't filter out state events based on the room's retention policy.
---
 synapse/visibility.py               | 15 +++++++++------
 tests/rest/client/test_retention.py | 10 ++++++++++
 2 files changed, 19 insertions(+), 6 deletions(-)

(limited to 'tests/rest/client')

diff --git a/synapse/visibility.py b/synapse/visibility.py
index 4498c156bc..4d4141dacc 100644
--- a/synapse/visibility.py
+++ b/synapse/visibility.py
@@ -111,14 +111,17 @@ def filter_events_for_client(
         if not event.is_state() and event.sender in ignore_list:
             return None
 
-        retention_policy = retention_policies[event.room_id]
-        max_lifetime = retention_policy.get("max_lifetime")
+        # Don't try to apply the room's retention policy if the event is a state event, as
+        # MSC1763 states that retention is only considered for non-state events.
+        if not event.is_state():
+            retention_policy = retention_policies[event.room_id]
+            max_lifetime = retention_policy.get("max_lifetime")
 
-        if max_lifetime is not None:
-            oldest_allowed_ts = storage.main.clock.time_msec() - max_lifetime
+            if max_lifetime is not None:
+                oldest_allowed_ts = storage.main.clock.time_msec() - max_lifetime
 
-            if event.origin_server_ts < oldest_allowed_ts:
-                return None
+                if event.origin_server_ts < oldest_allowed_ts:
+                    return None
 
         if event.event_id in always_include_ids:
             return event
diff --git a/tests/rest/client/test_retention.py b/tests/rest/client/test_retention.py
index 41ea9db689..7b6f25a838 100644
--- a/tests/rest/client/test_retention.py
+++ b/tests/rest/client/test_retention.py
@@ -164,6 +164,12 @@ class RetentionTestCase(unittest.HomeserverTestCase):
         self.assertEqual(filtered_events[0].event_id, valid_event_id, filtered_events)
 
     def _test_retention_event_purged(self, room_id, increment):
+        # Get the create event to, later, check that we can still access it.
+        message_handler = self.hs.get_message_handler()
+        create_event = self.get_success(
+            message_handler.get_room_data(self.user_id, room_id, EventTypes.Create)
+        )
+
         # Send a first event to the room. This is the event we'll want to be purged at the
         # end of the test.
         resp = self.helper.send(
@@ -202,6 +208,10 @@ class RetentionTestCase(unittest.HomeserverTestCase):
         valid_event = self.get_event(room_id, valid_event_id)
         self.assertEqual(valid_event.get("content", {}).get("body"), "2", valid_event)
 
+        # Check that we can still access state events that were sent before the event that
+        # has been purged.
+        self.get_event(room_id, create_event.event_id)
+
     def get_event(self, room_id, event_id, expected_code=200):
         url = "/_matrix/client/r0/rooms/%s/event/%s" % (room_id, event_id)
 
-- 
cgit 1.4.1


From 7c24d0f443724082376c89f9f75954d81f524a8e Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Tue, 19 Nov 2019 13:22:37 +0000
Subject: Lint

---
 synapse/config/server.py                 | 39 ++++++++++-------
 synapse/handlers/pagination.py           | 17 +++-----
 synapse/storage/data_stores/main/room.py | 49 +++++++++++----------
 tests/rest/client/test_retention.py      | 73 ++++++++++----------------------
 4 files changed, 77 insertions(+), 101 deletions(-)

(limited to 'tests/rest/client')

diff --git a/synapse/config/server.py b/synapse/config/server.py
index aa93a416f1..8a55ffac4f 100644
--- a/synapse/config/server.py
+++ b/synapse/config/server.py
@@ -19,7 +19,7 @@ import logging
 import os.path
 import re
 from textwrap import indent
-from typing import List
+from typing import List, Dict, Optional
 
 import attr
 import yaml
@@ -287,13 +287,17 @@ class ServerConfig(Config):
             self.retention_default_min_lifetime = None
             self.retention_default_max_lifetime = None
 
-        self.retention_allowed_lifetime_min = retention_config.get("allowed_lifetime_min")
+        self.retention_allowed_lifetime_min = retention_config.get(
+            "allowed_lifetime_min"
+        )
         if self.retention_allowed_lifetime_min is not None:
             self.retention_allowed_lifetime_min = self.parse_duration(
                 self.retention_allowed_lifetime_min
             )
 
-        self.retention_allowed_lifetime_max = retention_config.get("allowed_lifetime_max")
+        self.retention_allowed_lifetime_max = retention_config.get(
+            "allowed_lifetime_max"
+        )
         if self.retention_allowed_lifetime_max is not None:
             self.retention_allowed_lifetime_max = self.parse_duration(
                 self.retention_allowed_lifetime_max
@@ -302,14 +306,15 @@ class ServerConfig(Config):
         if (
             self.retention_allowed_lifetime_min is not None
             and self.retention_allowed_lifetime_max is not None
-            and self.retention_allowed_lifetime_min > self.retention_allowed_lifetime_max
+            and self.retention_allowed_lifetime_min
+            > self.retention_allowed_lifetime_max
         ):
             raise ConfigError(
                 "Invalid retention policy limits: 'allowed_lifetime_min' can not be"
                 " greater than 'allowed_lifetime_max'"
             )
 
-        self.retention_purge_jobs = []
+        self.retention_purge_jobs = []  # type: List[Dict[str, Optional[int]]]
         for purge_job_config in retention_config.get("purge_jobs", []):
             interval_config = purge_job_config.get("interval")
 
@@ -342,18 +347,22 @@ class ServerConfig(Config):
                     " 'longest_max_lifetime' value."
                 )
 
-            self.retention_purge_jobs.append({
-                "interval": interval,
-                "shortest_max_lifetime": shortest_max_lifetime,
-                "longest_max_lifetime": longest_max_lifetime,
-            })
+            self.retention_purge_jobs.append(
+                {
+                    "interval": interval,
+                    "shortest_max_lifetime": shortest_max_lifetime,
+                    "longest_max_lifetime": longest_max_lifetime,
+                }
+            )
 
         if not self.retention_purge_jobs:
-            self.retention_purge_jobs = [{
-                "interval": self.parse_duration("1d"),
-                "shortest_max_lifetime": None,
-                "longest_max_lifetime": None,
-            }]
+            self.retention_purge_jobs = [
+                {
+                    "interval": self.parse_duration("1d"),
+                    "shortest_max_lifetime": None,
+                    "longest_max_lifetime": None,
+                }
+            ]
 
         self.listeners = []  # type: List[dict]
         for listener in config.get("listeners", []):
diff --git a/synapse/handlers/pagination.py b/synapse/handlers/pagination.py
index e1800177fa..d122c11a4d 100644
--- a/synapse/handlers/pagination.py
+++ b/synapse/handlers/pagination.py
@@ -154,20 +154,17 @@ class PaginationHandler(object):
             # Figure out what token we should start purging at.
             ts = self.clock.time_msec() - max_lifetime
 
-            stream_ordering = (
-                yield self.store.find_first_stream_ordering_after_ts(ts)
-            )
+            stream_ordering = yield self.store.find_first_stream_ordering_after_ts(ts)
 
-            r = (
-                yield self.store.get_room_event_after_stream_ordering(
-                    room_id, stream_ordering,
-                )
+            r = yield self.store.get_room_event_after_stream_ordering(
+                room_id, stream_ordering,
             )
             if not r:
                 logger.warning(
                     "[purge] purging events not possible: No event found "
                     "(ts %i => stream_ordering %i)",
-                    ts, stream_ordering,
+                    ts,
+                    stream_ordering,
                 )
                 continue
 
@@ -186,9 +183,7 @@ class PaginationHandler(object):
             # the background so that it's not blocking any other operation apart from
             # other purges in the same room.
             run_as_background_process(
-                "_purge_history",
-                self._purge_history,
-                purge_id, room_id, token, True,
+                "_purge_history", self._purge_history, purge_id, room_id, token, True,
             )
 
     def start_purge_history(self, room_id, token, delete_local_events=False):
diff --git a/synapse/storage/data_stores/main/room.py b/synapse/storage/data_stores/main/room.py
index 54a7d24c73..7fceae59ca 100644
--- a/synapse/storage/data_stores/main/room.py
+++ b/synapse/storage/data_stores/main/room.py
@@ -334,8 +334,9 @@ class RoomStore(RoomWorkerStore, SearchStore):
                 WHERE state.room_id > ? AND state.type = '%s'
                 ORDER BY state.room_id ASC
                 LIMIT ?;
-                """ % EventTypes.Retention,
-                (last_room, batch_size)
+                """
+                % EventTypes.Retention,
+                (last_room, batch_size),
             )
 
             rows = self.cursor_to_dict(txn)
@@ -358,15 +359,13 @@ class RoomStore(RoomWorkerStore, SearchStore):
                         "event_id": row["event_id"],
                         "min_lifetime": retention_policy.get("min_lifetime"),
                         "max_lifetime": retention_policy.get("max_lifetime"),
-                    }
+                    },
                 )
 
             logger.info("Inserted %d rows into room_retention", len(rows))
 
             self._background_update_progress_txn(
-                txn, "insert_room_retention", {
-                    "room_id": rows[-1]["room_id"],
-                }
+                txn, "insert_room_retention", {"room_id": rows[-1]["room_id"]}
             )
 
             if batch_size > len(rows):
@@ -375,8 +374,7 @@ class RoomStore(RoomWorkerStore, SearchStore):
                 return False
 
         end = yield self.runInteraction(
-            "insert_room_retention",
-            _background_insert_retention_txn,
+            "insert_room_retention", _background_insert_retention_txn,
         )
 
         if end:
@@ -585,17 +583,15 @@ class RoomStore(RoomWorkerStore, SearchStore):
             )
 
     def _store_retention_policy_for_room_txn(self, txn, event):
-        if (
-            hasattr(event, "content")
-            and ("min_lifetime" in event.content or "max_lifetime" in event.content)
+        if hasattr(event, "content") and (
+            "min_lifetime" in event.content or "max_lifetime" in event.content
         ):
             if (
-                ("min_lifetime" in event.content and not isinstance(
-                    event.content.get("min_lifetime"), integer_types
-                ))
-                or ("max_lifetime" in event.content and not isinstance(
-                    event.content.get("max_lifetime"), integer_types
-                ))
+                "min_lifetime" in event.content
+                and not isinstance(event.content.get("min_lifetime"), integer_types)
+            ) or (
+                "max_lifetime" in event.content
+                and not isinstance(event.content.get("max_lifetime"), integer_types)
             ):
                 # Ignore the event if one of the value isn't an integer.
                 return
@@ -798,7 +794,9 @@ class RoomStore(RoomWorkerStore, SearchStore):
         return local_media_mxcs, remote_media_mxcs
 
     @defer.inlineCallbacks
-    def get_rooms_for_retention_period_in_range(self, min_ms, max_ms, include_null=False):
+    def get_rooms_for_retention_period_in_range(
+        self, min_ms, max_ms, include_null=False
+    ):
         """Retrieves all of the rooms within the given retention range.
 
         Optionally includes the rooms which don't have a retention policy.
@@ -904,23 +902,24 @@ class RoomStore(RoomWorkerStore, SearchStore):
                 INNER JOIN current_state_events USING (event_id, room_id)
                 WHERE room_id = ?;
                 """,
-                (room_id,)
+                (room_id,),
             )
 
             return self.cursor_to_dict(txn)
 
         ret = yield self.runInteraction(
-            "get_retention_policy_for_room",
-            get_retention_policy_for_room_txn,
+            "get_retention_policy_for_room", get_retention_policy_for_room_txn,
         )
 
         # If we don't know this room ID, ret will be None, in this case return the default
         # policy.
         if not ret:
-            defer.returnValue({
-                "min_lifetime": self.config.retention_default_min_lifetime,
-                "max_lifetime": self.config.retention_default_max_lifetime,
-            })
+            defer.returnValue(
+                {
+                    "min_lifetime": self.config.retention_default_min_lifetime,
+                    "max_lifetime": self.config.retention_default_max_lifetime,
+                }
+            )
 
         row = ret[0]
 
diff --git a/tests/rest/client/test_retention.py b/tests/rest/client/test_retention.py
index 7b6f25a838..6bf485c239 100644
--- a/tests/rest/client/test_retention.py
+++ b/tests/rest/client/test_retention.py
@@ -61,9 +61,7 @@ class RetentionTestCase(unittest.HomeserverTestCase):
         self.helper.send_state(
             room_id=room_id,
             event_type=EventTypes.Retention,
-            body={
-                "max_lifetime": one_day_ms * 4,
-            },
+            body={"max_lifetime": one_day_ms * 4},
             tok=self.token,
             expect_code=400,
         )
@@ -71,9 +69,7 @@ class RetentionTestCase(unittest.HomeserverTestCase):
         self.helper.send_state(
             room_id=room_id,
             event_type=EventTypes.Retention,
-            body={
-                "max_lifetime": one_hour_ms,
-            },
+            body={"max_lifetime": one_hour_ms},
             tok=self.token,
             expect_code=400,
         )
@@ -89,9 +85,7 @@ class RetentionTestCase(unittest.HomeserverTestCase):
         self.helper.send_state(
             room_id=room_id,
             event_type=EventTypes.Retention,
-            body={
-                "max_lifetime": lifetime,
-            },
+            body={"max_lifetime": lifetime},
             tok=self.token,
         )
 
@@ -115,20 +109,12 @@ class RetentionTestCase(unittest.HomeserverTestCase):
         events = []
 
         # Send a first event, which should be filtered out at the end of the test.
-        resp = self.helper.send(
-            room_id=room_id,
-            body="1",
-            tok=self.token,
-        )
+        resp = self.helper.send(room_id=room_id, body="1", tok=self.token)
 
         # Get the event from the store so that we end up with a FrozenEvent that we can
         # give to filter_events_for_client. We need to do this now because the event won't
         # be in the database anymore after it has expired.
-        events.append(self.get_success(
-            store.get_event(
-                resp.get("event_id")
-            )
-        ))
+        events.append(self.get_success(store.get_event(resp.get("event_id"))))
 
         # Advance the time by 2 days. We're using the default retention policy, therefore
         # after this the first event will still be valid.
@@ -143,20 +129,16 @@ class RetentionTestCase(unittest.HomeserverTestCase):
 
         valid_event_id = resp.get("event_id")
 
-        events.append(self.get_success(
-            store.get_event(
-                valid_event_id
-            )
-        ))
+        events.append(self.get_success(store.get_event(valid_event_id)))
 
         # Advance the time by anothe 2 days. After this, the first event should be
         # outdated but not the second one.
         self.reactor.advance(one_day_ms * 2 / 1000)
 
         # Run filter_events_for_client with our list of FrozenEvents.
-        filtered_events = self.get_success(filter_events_for_client(
-            storage, self.user_id, events
-        ))
+        filtered_events = self.get_success(
+            filter_events_for_client(storage, self.user_id, events)
+        )
 
         # We should only get one event back.
         self.assertEqual(len(filtered_events), 1, filtered_events)
@@ -172,28 +154,22 @@ class RetentionTestCase(unittest.HomeserverTestCase):
 
         # Send a first event to the room. This is the event we'll want to be purged at the
         # end of the test.
-        resp = self.helper.send(
-            room_id=room_id,
-            body="1",
-            tok=self.token,
-        )
+        resp = self.helper.send(room_id=room_id, body="1", tok=self.token)
 
         expired_event_id = resp.get("event_id")
 
         # Check that we can retrieve the event.
         expired_event = self.get_event(room_id, expired_event_id)
-        self.assertEqual(expired_event.get("content", {}).get("body"), "1", expired_event)
+        self.assertEqual(
+            expired_event.get("content", {}).get("body"), "1", expired_event
+        )
 
         # Advance the time.
         self.reactor.advance(increment / 1000)
 
         # Send another event. We need this because the purge job won't purge the most
         # recent event in the room.
-        resp = self.helper.send(
-            room_id=room_id,
-            body="2",
-            tok=self.token,
-        )
+        resp = self.helper.send(room_id=room_id, body="2", tok=self.token)
 
         valid_event_id = resp.get("event_id")
 
@@ -240,8 +216,7 @@ class RetentionNoDefaultPolicyTestCase(unittest.HomeserverTestCase):
         mock_federation_client = Mock(spec=["backfill"])
 
         self.hs = self.setup_test_homeserver(
-            config=config,
-            federation_client=mock_federation_client,
+            config=config, federation_client=mock_federation_client,
         )
         return self.hs
 
@@ -268,9 +243,7 @@ class RetentionNoDefaultPolicyTestCase(unittest.HomeserverTestCase):
         self.helper.send_state(
             room_id=room_id,
             event_type=EventTypes.Retention,
-            body={
-                "max_lifetime": one_day_ms * 35,
-            },
+            body={"max_lifetime": one_day_ms * 35},
             tok=self.token,
         )
 
@@ -289,18 +262,16 @@ class RetentionNoDefaultPolicyTestCase(unittest.HomeserverTestCase):
 
         # Check that we can retrieve the event.
         expired_event = self.get_event(room_id, first_event_id)
-        self.assertEqual(expired_event.get("content", {}).get("body"), "1", expired_event)
+        self.assertEqual(
+            expired_event.get("content", {}).get("body"), "1", expired_event
+        )
 
         # Advance the time by a month.
         self.reactor.advance(one_day_ms * 30 / 1000)
 
         # Send another event. We need this because the purge job won't purge the most
         # recent event in the room.
-        resp = self.helper.send(
-            room_id=room_id,
-            body="2",
-            tok=self.token,
-        )
+        resp = self.helper.send(room_id=room_id, body="2", tok=self.token)
 
         second_event_id = resp.get("event_id")
 
@@ -313,7 +284,9 @@ class RetentionNoDefaultPolicyTestCase(unittest.HomeserverTestCase):
         )
 
         if expected_code_for_first_event == 200:
-            self.assertEqual(first_event.get("content", {}).get("body"), "1", first_event)
+            self.assertEqual(
+                first_event.get("content", {}).get("body"), "1", first_event
+            )
 
         # Check that the event that hasn't been purged can still be retrieved.
         second_event = self.get_event(room_id, second_event_id)
-- 
cgit 1.4.1


From bf9a11c54d3c9ca1e4fa9420b567efceb1e46d5b Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Tue, 19 Nov 2019 13:30:04 +0000
Subject: Lint again

---
 synapse/config/server.py            |  2 +-
 tests/rest/client/test_retention.py | 12 ++----------
 2 files changed, 3 insertions(+), 11 deletions(-)

(limited to 'tests/rest/client')

diff --git a/synapse/config/server.py b/synapse/config/server.py
index 8a55ffac4f..ed916e1400 100644
--- a/synapse/config/server.py
+++ b/synapse/config/server.py
@@ -19,7 +19,7 @@ import logging
 import os.path
 import re
 from textwrap import indent
-from typing import List, Dict, Optional
+from typing import Dict, List, Optional
 
 import attr
 import yaml
diff --git a/tests/rest/client/test_retention.py b/tests/rest/client/test_retention.py
index 6bf485c239..9e549d8a91 100644
--- a/tests/rest/client/test_retention.py
+++ b/tests/rest/client/test_retention.py
@@ -121,11 +121,7 @@ class RetentionTestCase(unittest.HomeserverTestCase):
         self.reactor.advance(one_day_ms * 2 / 1000)
 
         # Send another event, which shouldn't get filtered out.
-        resp = self.helper.send(
-            room_id=room_id,
-            body="2",
-            tok=self.token,
-        )
+        resp = self.helper.send(room_id=room_id, body="2", tok=self.token)
 
         valid_event_id = resp.get("event_id")
 
@@ -252,11 +248,7 @@ class RetentionNoDefaultPolicyTestCase(unittest.HomeserverTestCase):
     def _test_retention(self, room_id, expected_code_for_first_event=200):
         # Send a first event to the room. This is the event we'll want to be purged at the
         # end of the test.
-        resp = self.helper.send(
-            room_id=room_id,
-            body="1",
-            tok=self.token,
-        )
+        resp = self.helper.send(room_id=room_id, body="1", tok=self.token)
 
         first_event_id = resp.get("event_id")
 
-- 
cgit 1.4.1


From 6356f2088f0adb681fe24a8435955b19883fa3b4 Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Wed, 20 Nov 2019 12:09:06 +0000
Subject: Test if a purge can make /messages return 500 responses

---
 tests/rest/client/v1/test_rooms.py | 72 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

(limited to 'tests/rest/client')

diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py
index 5e38fd6ced..ebaa67e899 100644
--- a/tests/rest/client/v1/test_rooms.py
+++ b/tests/rest/client/v1/test_rooms.py
@@ -25,7 +25,9 @@ from twisted.internet import defer
 
 import synapse.rest.admin
 from synapse.api.constants import EventContentFields, EventTypes, Membership
+from synapse.handlers.pagination import PurgeStatus
 from synapse.rest.client.v1 import login, profile, room
+from synapse.util.stringutils import random_string
 
 from tests import unittest
 
@@ -910,6 +912,76 @@ class RoomMessageListTestCase(RoomBase):
 
         return channel.json_body["chunk"]
 
+    def test_room_messages_purge(self):
+        store = self.hs.get_datastore()
+        pagination_handler = self.hs.get_pagination_handler()
+
+        # Send a first message in the room, which will be removed by the purge.
+        first_event_id = self.helper.send(self.room_id, "message 1")["event_id"]
+        first_token = self.get_success(
+            store.get_topological_token_for_event(first_event_id)
+        )
+
+        # Send a second message in the room, which won't be removed, and which we'll
+        # use as the marker to purge events before.
+        second_event_id = self.helper.send(self.room_id, "message 2")["event_id"]
+        second_token = self.get_success(
+            store.get_topological_token_for_event(second_event_id)
+        )
+
+        # Send a third event in the room to ensure we don't fall under any edge case
+        # due to our marker being the latest forward extremity in the room.
+        self.helper.send(self.room_id, "message 3")
+
+        # Check that we get the first and second message when querying /messages.
+        request, channel = self.make_request(
+            "GET",
+            "/rooms/%s/messages?access_token=x&from=%s&dir=b&filter=%s"
+            % (self.room_id, second_token, json.dumps({"types": [EventTypes.Message]})),
+        )
+        self.render(request)
+        self.assertEqual(channel.code, 200, channel.json_body)
+
+        chunk = channel.json_body["chunk"]
+        self.assertEqual(len(chunk), 2, [event["content"] for event in chunk])
+
+        # Purge every event before the second event.
+        purge_id = random_string(16)
+        pagination_handler._purges_by_id[purge_id] = PurgeStatus()
+        self.get_success(pagination_handler._purge_history(
+            purge_id=purge_id,
+            room_id=self.room_id,
+            token=second_token,
+            delete_local_events=True,
+        ))
+
+        # Check that we only get the second message through /message now that the first
+        # has been purged.
+        request, channel = self.make_request(
+            "GET",
+            "/rooms/%s/messages?access_token=x&from=%s&dir=b&filter=%s"
+            % (self.room_id, second_token, json.dumps({"types": [EventTypes.Message]})),
+        )
+        self.render(request)
+        self.assertEqual(channel.code, 200, channel.json_body)
+
+        chunk = channel.json_body["chunk"]
+        self.assertEqual(len(chunk), 1, [event["content"] for event in chunk])
+
+        # Check that we get no event, but also no error, when querying /messages with
+        # the token that was pointing at the first event, because we don't have it
+        # anymore.
+        request, channel = self.make_request(
+            "GET",
+            "/rooms/%s/messages?access_token=x&from=%s&dir=b&filter=%s"
+            % (self.room_id, first_token, json.dumps({"types": [EventTypes.Message]})),
+        )
+        self.render(request)
+        self.assertEqual(channel.code, 200, channel.json_body)
+
+        chunk = channel.json_body["chunk"]
+        self.assertEqual(len(chunk), 0, [event["content"] for event in chunk])
+
 
 class RoomSearchTestCase(unittest.HomeserverTestCase):
     servlets = [
-- 
cgit 1.4.1


From e2a20326e8141fdf9304434901da38c64b917a78 Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Wed, 20 Nov 2019 15:08:47 +0000
Subject: Lint

---
 tests/rest/client/v1/test_rooms.py | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

(limited to 'tests/rest/client')

diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py
index ebaa67e899..e84e578f99 100644
--- a/tests/rest/client/v1/test_rooms.py
+++ b/tests/rest/client/v1/test_rooms.py
@@ -948,12 +948,14 @@ class RoomMessageListTestCase(RoomBase):
         # Purge every event before the second event.
         purge_id = random_string(16)
         pagination_handler._purges_by_id[purge_id] = PurgeStatus()
-        self.get_success(pagination_handler._purge_history(
-            purge_id=purge_id,
-            room_id=self.room_id,
-            token=second_token,
-            delete_local_events=True,
-        ))
+        self.get_success(
+            pagination_handler._purge_history(
+                purge_id=purge_id,
+                room_id=self.room_id,
+                token=second_token,
+                delete_local_events=True,
+            )
+        )
 
         # Check that we only get the second message through /message now that the first
         # has been purged.
-- 
cgit 1.4.1


From f0ef9708241ec65fe6f32f1ad36f719ab4ab2b53 Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Tue, 26 Nov 2019 17:49:12 +0000
Subject: Don't restrict the tests to v1 rooms

---
 tests/rest/client/test_retention.py | 2 --
 1 file changed, 2 deletions(-)

(limited to 'tests/rest/client')

diff --git a/tests/rest/client/test_retention.py b/tests/rest/client/test_retention.py
index 9e549d8a91..95475bb651 100644
--- a/tests/rest/client/test_retention.py
+++ b/tests/rest/client/test_retention.py
@@ -34,7 +34,6 @@ class RetentionTestCase(unittest.HomeserverTestCase):
 
     def make_homeserver(self, reactor, clock):
         config = self.default_config()
-        config["default_room_version"] = "1"
         config["retention"] = {
             "enabled": True,
             "default_policy": {
@@ -204,7 +203,6 @@ class RetentionNoDefaultPolicyTestCase(unittest.HomeserverTestCase):
 
     def make_homeserver(self, reactor, clock):
         config = self.default_config()
-        config["default_room_version"] = "1"
         config["retention"] = {
             "enabled": True,
         }
-- 
cgit 1.4.1


From ce578031f4d0fe6f1eb26de4cb3d30a4175468db Mon Sep 17 00:00:00 2001
From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
Date: Tue, 26 Nov 2019 18:42:27 +0000
Subject: Remove assertion and provide a clear warning on startup for missing
 public_baseurl (#6379)

---
 changelog.d/6379.misc                       | 1 +
 synapse/config/emailconfig.py               | 2 ++
 synapse/config/registration.py              | 7 +++++++
 tests/rest/client/v2_alpha/test_register.py | 1 +
 4 files changed, 11 insertions(+)
 create mode 100644 changelog.d/6379.misc

(limited to 'tests/rest/client')

diff --git a/changelog.d/6379.misc b/changelog.d/6379.misc
new file mode 100644
index 0000000000..725c2e7d87
--- /dev/null
+++ b/changelog.d/6379.misc
@@ -0,0 +1 @@
+Complain on startup instead of 500'ing during runtime when `public_baseurl` isn't set when necessary.
\ No newline at end of file
diff --git a/synapse/config/emailconfig.py b/synapse/config/emailconfig.py
index 43fad0bf8b..ac1724045f 100644
--- a/synapse/config/emailconfig.py
+++ b/synapse/config/emailconfig.py
@@ -146,6 +146,8 @@ class EmailConfig(Config):
                 if k not in email_config:
                     missing.append("email." + k)
 
+            # public_baseurl is required to build password reset and validation links that
+            # will be emailed to users
             if config.get("public_baseurl") is None:
                 missing.append("public_baseurl")
 
diff --git a/synapse/config/registration.py b/synapse/config/registration.py
index 1f6dac69da..ee9614c5f7 100644
--- a/synapse/config/registration.py
+++ b/synapse/config/registration.py
@@ -106,6 +106,13 @@ class RegistrationConfig(Config):
         account_threepid_delegates = config.get("account_threepid_delegates") or {}
         self.account_threepid_delegate_email = account_threepid_delegates.get("email")
         self.account_threepid_delegate_msisdn = account_threepid_delegates.get("msisdn")
+        if self.account_threepid_delegate_msisdn and not self.public_baseurl:
+            raise ConfigError(
+                "The configuration option `public_baseurl` is required if "
+                "`account_threepid_delegate.msisdn` is set, such that "
+                "clients know where to submit validation tokens to. Please "
+                "configure `public_baseurl`."
+            )
 
         self.default_identity_server = config.get("default_identity_server")
         self.allow_guest_access = config.get("allow_guest_access", False)
diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py
index dab87e5edf..c0d0d2b44e 100644
--- a/tests/rest/client/v2_alpha/test_register.py
+++ b/tests/rest/client/v2_alpha/test_register.py
@@ -203,6 +203,7 @@ class RegisterRestServletTestCase(unittest.HomeserverTestCase):
 
     @unittest.override_config(
         {
+            "public_baseurl": "https://test_server",
             "enable_registration_captcha": True,
             "user_consent": {
                 "version": "1",
-- 
cgit 1.4.1


From 8c9a713f8db1d6fcc1f876ac6fbd0e54b5e5819c Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Thu, 28 Nov 2019 11:32:06 +0000
Subject: Add tests

---
 tests/rest/client/v1/test_rooms.py | 140 +++++++++++++++++++++++++++++++++++++
 1 file changed, 140 insertions(+)

(limited to 'tests/rest/client')

diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py
index e84e578f99..eda2fabc71 100644
--- a/tests/rest/client/v1/test_rooms.py
+++ b/tests/rest/client/v1/test_rooms.py
@@ -1180,3 +1180,143 @@ class PerRoomProfilesForbiddenTestCase(unittest.HomeserverTestCase):
 
         res_displayname = channel.json_body["content"]["displayname"]
         self.assertEqual(res_displayname, self.displayname, channel.result)
+
+
+class RoomMembershipReasonTestCase(unittest.HomeserverTestCase):
+    """Tests that clients can add a "reason" field to membership events and
+    that they get correctly added to the generated events and propagated.
+    """
+
+    servlets = [
+        synapse.rest.admin.register_servlets_for_client_rest_resource,
+        room.register_servlets,
+        login.register_servlets,
+    ]
+
+    def prepare(self, reactor, clock, homeserver):
+        self.creator = self.register_user("creator", "test")
+        self.creator_tok = self.login("creator", "test")
+
+        self.second_user_id = self.register_user("second", "test")
+        self.second_tok = self.login("second", "test")
+
+        self.room_id = self.helper.create_room_as(self.creator, tok=self.creator_tok)
+
+    def test_join_reason(self):
+        reason = "hello"
+        request, channel = self.make_request(
+            "POST",
+            "/_matrix/client/r0/rooms/{}/join".format(self.room_id),
+            content={"reason": reason},
+            access_token=self.second_tok,
+        )
+        self.render(request)
+        self.assertEqual(channel.code, 200, channel.result)
+
+        self._check_for_reason(reason)
+
+    def test_leave_reason(self):
+        self.helper.join(self.room_id, user=self.second_user_id, tok=self.second_tok)
+
+        reason = "hello"
+        request, channel = self.make_request(
+            "POST",
+            "/_matrix/client/r0/rooms/{}/leave".format(self.room_id),
+            content={"reason": reason},
+            access_token=self.second_tok,
+        )
+        self.render(request)
+        self.assertEqual(channel.code, 200, channel.result)
+
+        self._check_for_reason(reason)
+
+    def test_kick_reason(self):
+        self.helper.join(self.room_id, user=self.second_user_id, tok=self.second_tok)
+
+        reason = "hello"
+        request, channel = self.make_request(
+            "POST",
+            "/_matrix/client/r0/rooms/{}/kick".format(self.room_id),
+            content={"reason": reason, "user_id": self.second_user_id},
+            access_token=self.second_tok,
+        )
+        self.render(request)
+        self.assertEqual(channel.code, 200, channel.result)
+
+        self._check_for_reason(reason)
+
+    def test_ban_reason(self):
+        self.helper.join(self.room_id, user=self.second_user_id, tok=self.second_tok)
+
+        reason = "hello"
+        request, channel = self.make_request(
+            "POST",
+            "/_matrix/client/r0/rooms/{}/ban".format(self.room_id),
+            content={"reason": reason, "user_id": self.second_user_id},
+            access_token=self.creator_tok,
+        )
+        self.render(request)
+        self.assertEqual(channel.code, 200, channel.result)
+
+        self._check_for_reason(reason)
+
+    def test_unban_reason(self):
+        reason = "hello"
+        request, channel = self.make_request(
+            "POST",
+            "/_matrix/client/r0/rooms/{}/unban".format(self.room_id),
+            content={"reason": reason, "user_id": self.second_user_id},
+            access_token=self.creator_tok,
+        )
+        self.render(request)
+        self.assertEqual(channel.code, 200, channel.result)
+
+        self._check_for_reason(reason)
+
+    def test_invite_reason(self):
+        reason = "hello"
+        request, channel = self.make_request(
+            "POST",
+            "/_matrix/client/r0/rooms/{}/invite".format(self.room_id),
+            content={"reason": reason, "user_id": self.second_user_id},
+            access_token=self.creator_tok,
+        )
+        self.render(request)
+        self.assertEqual(channel.code, 200, channel.result)
+
+        self._check_for_reason(reason)
+
+    def test_reject_invite_reason(self):
+        self.helper.invite(
+            self.room_id,
+            src=self.creator,
+            targ=self.second_user_id,
+            tok=self.creator_tok,
+        )
+
+        reason = "hello"
+        request, channel = self.make_request(
+            "POST",
+            "/_matrix/client/r0/rooms/{}/leave".format(self.room_id),
+            content={"reason": reason},
+            access_token=self.second_tok,
+        )
+        self.render(request)
+        self.assertEqual(channel.code, 200, channel.result)
+
+        self._check_for_reason(reason)
+
+    def _check_for_reason(self, reason):
+        request, channel = self.make_request(
+            "GET",
+            "/_matrix/client/r0/rooms/{}/state/m.room.member/{}".format(
+                self.room_id, self.second_user_id
+            ),
+            access_token=self.creator_tok,
+        )
+        self.render(request)
+        self.assertEqual(channel.code, 200, channel.result)
+
+        event_content = channel.json_body
+
+        self.assertEqual(event_content.get("reason"), reason, channel.result)
-- 
cgit 1.4.1