Show error when timestamp in seconds is provided to the /purge_media_cache API (#11101)

4 files changed, 133 insertions, 13 deletions

diff --git a/changelog.d/11101.bugfix b/changelog.d/11101.bugfix
new file mode 100644
index 0000000000..0de507848f
--- /dev/null
+++ b/changelog.d/11101.bugfix
@@ -0,0 +1 @@
+Show an error when timestamp in seconds is provided to the `/purge_media_cache` Admin API.
\ No newline at end of file
diff --git a/docs/admin_api/media_admin_api.md b/docs/admin_api/media_admin_api.md
index ea05bd6e44..60b8bc7379 100644
--- a/docs/admin_api/media_admin_api.md
+++ b/docs/admin_api/media_admin_api.md
@@ -257,9 +257,9 @@ POST /_synapse/admin/v1/media/<server_name>/delete?before_ts=<before_ts>
 URL Parameters
 
 * `server_name`: string - The name of your local server (e.g `matrix.org`).
-* `before_ts`: string representing a positive integer - Unix timestamp in ms.
+* `before_ts`: string representing a positive integer - Unix timestamp in milliseconds.
 Files that were last used before this timestamp will be deleted. It is the timestamp of
-last access and not the timestamp creation.
+last access, not the timestamp when the file was created.
 * `size_gt`: Optional - string representing a positive integer - Size of the media in bytes.
 Files that are larger will be deleted. Defaults to `0`.
 * `keep_profiles`: Optional - string representing a boolean - Switch to also delete files
@@ -302,7 +302,7 @@ POST /_synapse/admin/v1/purge_media_cache?before_ts=<unix_timestamp_in_ms>
 
 URL Parameters
 
-* `unix_timestamp_in_ms`: string representing a positive integer - Unix timestamp in ms.
+* `unix_timestamp_in_ms`: string representing a positive integer - Unix timestamp in milliseconds.
 All cached media that was last accessed before this timestamp will be removed.
 
 Response:
diff --git a/synapse/rest/admin/media.py b/synapse/rest/admin/media.py
index 8ce443049e..30a687d234 100644
--- a/synapse/rest/admin/media.py
+++ b/synapse/rest/admin/media.py
@@ -40,7 +40,7 @@ class QuarantineMediaInRoom(RestServlet):
     """
 
     PATTERNS = [
-        *admin_patterns("/room/(?P<room_id>[^/]+)/media/quarantine"),
+        *admin_patterns("/room/(?P<room_id>[^/]+)/media/quarantine$"),
         # This path kept around for legacy reasons
         *admin_patterns("/quarantine_media/(?P<room_id>[^/]+)"),
     ]
@@ -70,7 +70,7 @@ class QuarantineMediaByUser(RestServlet):
     this server.
     """
 
-    PATTERNS = admin_patterns("/user/(?P<user_id>[^/]+)/media/quarantine")
+    PATTERNS = admin_patterns("/user/(?P<user_id>[^/]+)/media/quarantine$")
 
     def __init__(self, hs: "HomeServer"):
         self.store = hs.get_datastore()
@@ -199,7 +199,7 @@ class UnprotectMediaByID(RestServlet):
 class ListMediaInRoom(RestServlet):
     """Lists all of the media in a given room."""
 
-    PATTERNS = admin_patterns("/room/(?P<room_id>[^/]+)/media")
+    PATTERNS = admin_patterns("/room/(?P<room_id>[^/]+)/media$")
 
     def __init__(self, hs: "HomeServer"):
         self.store = hs.get_datastore()
@@ -219,7 +219,7 @@ class ListMediaInRoom(RestServlet):
 
 
 class PurgeMediaCacheRestServlet(RestServlet):
-    PATTERNS = admin_patterns("/purge_media_cache")
+    PATTERNS = admin_patterns("/purge_media_cache$")
 
     def __init__(self, hs: "HomeServer"):
         self.media_repository = hs.get_media_repository()
@@ -231,6 +231,20 @@ class PurgeMediaCacheRestServlet(RestServlet):
         before_ts = parse_integer(request, "before_ts", required=True)
         logger.info("before_ts: %r", before_ts)
 
+        if before_ts < 0:
+            raise SynapseError(
+                400,
+                "Query parameter before_ts must be a positive integer.",
+                errcode=Codes.INVALID_PARAM,
+            )
+        elif before_ts < 30000000000:  # Dec 1970 in milliseconds, Aug 2920 in seconds
+            raise SynapseError(
+                400,
+                "Query parameter before_ts you provided is from the year 1970. "
+                + "Double check that you are providing a timestamp in milliseconds.",
+                errcode=Codes.INVALID_PARAM,
+            )
+
         ret = await self.media_repository.delete_old_remote_media(before_ts)
 
         return 200, ret
@@ -271,7 +285,7 @@ class DeleteMediaByDateSize(RestServlet):
     timestamp and size.
     """
 
-    PATTERNS = admin_patterns("/media/(?P<server_name>[^/]+)/delete")
+    PATTERNS = admin_patterns("/media/(?P<server_name>[^/]+)/delete$")
 
     def __init__(self, hs: "HomeServer"):
         self.store = hs.get_datastore()
@@ -291,7 +305,14 @@ class DeleteMediaByDateSize(RestServlet):
         if before_ts < 0:
             raise SynapseError(
                 400,
-                "Query parameter before_ts must be a string representing a positive integer.",
+                "Query parameter before_ts must be a positive integer.",
+                errcode=Codes.INVALID_PARAM,
+            )
+        elif before_ts < 30000000000:  # Dec 1970 in milliseconds, Aug 2920 in seconds
+            raise SynapseError(
+                400,
+                "Query parameter before_ts you provided is from the year 1970. "
+                + "Double check that you are providing a timestamp in milliseconds.",
                 errcode=Codes.INVALID_PARAM,
             )
         if size_gt < 0:
diff --git a/tests/rest/admin/test_media.py b/tests/rest/admin/test_media.py
index ce30a19213..db0e78c039 100644
--- a/tests/rest/admin/test_media.py
+++ b/tests/rest/admin/test_media.py
@@ -27,6 +27,9 @@ from tests import unittest
 from tests.server import FakeSite, make_request
 from tests.test_utils import SMALL_PNG
 
+VALID_TIMESTAMP = 1609459200000  # 2021-01-01 in milliseconds
+INVALID_TIMESTAMP_IN_S = 1893456000  # 2030-01-01 in seconds
+
 
 class DeleteMediaByIDTestCase(unittest.HomeserverTestCase):
 
@@ -203,6 +206,9 @@ class DeleteMediaByDateSizeTestCase(unittest.HomeserverTestCase):
         self.filepaths = MediaFilePaths(hs.config.media.media_store_path)
         self.url = "/_synapse/admin/v1/media/%s/delete" % self.server_name
 
+        # Move clock up to somewhat realistic time
+        self.reactor.advance(1000000000)
+
     def test_no_auth(self):
         """
         Try to delete media without authentication.
@@ -237,7 +243,7 @@ class DeleteMediaByDateSizeTestCase(unittest.HomeserverTestCase):
 
         channel = self.make_request(
             "POST",
-            url + "?before_ts=1234",
+            url + f"?before_ts={VALID_TIMESTAMP}",
             access_token=self.admin_user_tok,
         )
 
@@ -273,13 +279,27 @@ class DeleteMediaByDateSizeTestCase(unittest.HomeserverTestCase):
         self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"])
         self.assertEqual(Codes.INVALID_PARAM, channel.json_body["errcode"])
         self.assertEqual(
-            "Query parameter before_ts must be a string representing a positive integer.",
+            "Query parameter before_ts must be a positive integer.",
             channel.json_body["error"],
         )
 
         channel = self.make_request(
             "POST",
-            self.url + "?before_ts=1234&size_gt=-1234",
+            self.url + f"?before_ts={INVALID_TIMESTAMP_IN_S}",
+            access_token=self.admin_user_tok,
+        )
+
+        self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"])
+        self.assertEqual(Codes.INVALID_PARAM, channel.json_body["errcode"])
+        self.assertEqual(
+            "Query parameter before_ts you provided is from the year 1970. "
+            + "Double check that you are providing a timestamp in milliseconds.",
+            channel.json_body["error"],
+        )
+
+        channel = self.make_request(
+            "POST",
+            self.url + f"?before_ts={VALID_TIMESTAMP}&size_gt=-1234",
             access_token=self.admin_user_tok,
         )
 
@@ -292,7 +312,7 @@ class DeleteMediaByDateSizeTestCase(unittest.HomeserverTestCase):
 
         channel = self.make_request(
             "POST",
-            self.url + "?before_ts=1234&keep_profiles=not_bool",
+            self.url + f"?before_ts={VALID_TIMESTAMP}&keep_profiles=not_bool",
             access_token=self.admin_user_tok,
         )
 
@@ -767,3 +787,81 @@ class ProtectMediaByIDTestCase(unittest.HomeserverTestCase):
 
         media_info = self.get_success(self.store.get_local_media(self.media_id))
         self.assertFalse(media_info["safe_from_quarantine"])
+
+
+class PurgeMediaCacheTestCase(unittest.HomeserverTestCase):
+
+    servlets = [
+        synapse.rest.admin.register_servlets,
+        synapse.rest.admin.register_servlets_for_media_repo,
+        login.register_servlets,
+        profile.register_servlets,
+        room.register_servlets,
+    ]
+
+    def prepare(self, reactor, clock, hs):
+        self.media_repo = hs.get_media_repository_resource()
+        self.server_name = hs.hostname
+
+        self.admin_user = self.register_user("admin", "pass", admin=True)
+        self.admin_user_tok = self.login("admin", "pass")
+
+        self.filepaths = MediaFilePaths(hs.config.media.media_store_path)
+        self.url = "/_synapse/admin/v1/purge_media_cache"
+
+    def test_no_auth(self):
+        """
+        Try to delete media without authentication.
+        """
+
+        channel = self.make_request("POST", self.url, b"{}")
+
+        self.assertEqual(401, int(channel.result["code"]), msg=channel.result["body"])
+        self.assertEqual(Codes.MISSING_TOKEN, channel.json_body["errcode"])
+
+    def test_requester_is_not_admin(self):
+        """
+        If the user is not a server admin, an error is returned.
+        """
+        self.other_user = self.register_user("user", "pass")
+        self.other_user_token = self.login("user", "pass")
+
+        channel = self.make_request(
+            "POST",
+            self.url,
+            access_token=self.other_user_token,
+        )
+
+        self.assertEqual(403, int(channel.result["code"]), msg=channel.result["body"])
+        self.assertEqual(Codes.FORBIDDEN, channel.json_body["errcode"])
+
+    def test_invalid_parameter(self):
+        """
+        If parameters are invalid, an error is returned.
+        """
+        channel = self.make_request(
+            "POST",
+            self.url + "?before_ts=-1234",
+            access_token=self.admin_user_tok,
+        )
+
+        self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"])
+        self.assertEqual(Codes.INVALID_PARAM, channel.json_body["errcode"])
+        self.assertEqual(
+            "Query parameter before_ts must be a positive integer.",
+            channel.json_body["error"],
+        )
+
+        channel = self.make_request(
+            "POST",
+            self.url + f"?before_ts={INVALID_TIMESTAMP_IN_S}",
+            access_token=self.admin_user_tok,
+        )
+
+        self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"])
+        self.assertEqual(Codes.INVALID_PARAM, channel.json_body["errcode"])
+        self.assertEqual(
+            "Query parameter before_ts you provided is from the year 1970. "
+            + "Double check that you are providing a timestamp in milliseconds.",
+            channel.json_body["error"],
+        )