wire up endpoint to accept and pass through admin bypass for redacted media

Author: jason-famedly

synapse/media/media_repository.py

@@ -241,6 +241,7 @@ async def get_local_media(
         requester: Optional[Requester] = None,
         allow_authenticated: bool = True,
         federation: bool = False,
+        allow_media_linked_to_redacted_event: bool = False,
     ) -> None:
         raise NotImplementedError(
             "Sorry Mario, your MediaRepository related function is in another castle"
@@ -312,7 +313,10 @@ async def validate_media_restriction(
         return attachments
 
     async def is_media_visible(
-        self, requesting_user: UserID, media_info_object: Union[LocalMedia, RemoteMedia]
+        self,
+        requesting_user: UserID,
+        media_info_object: Union[LocalMedia, RemoteMedia],
+        allow_media_linked_to_redacted_event: bool = False,
     ) -> None:
         """
         Verify that media requested for download should be visible to the user making
@@ -349,11 +353,13 @@ async def is_media_visible(
 
         if attached_event_id:
             event_base = await self.store.get_event(attached_event_id)
-            if event_base.internal_metadata.is_redacted():
-                # If the event the media is attached to is redacted, don't server that
+            if (
+                event_base.internal_metadata.is_redacted()
+                and not allow_media_linked_to_redacted_event
+            ):
+                # If the event the media is attached to is redacted, don't serve that
                 # media to the user. Moderators and admins should probably be excluded
                 # from this restriction
-                # XXX: better error code?
                 raise NotFoundError()
 
             if event_base.is_state():
@@ -951,7 +957,11 @@ def respond_not_yet_uploaded(self, request: SynapseRequest) -> None:
         )
 
     async def get_local_media_info(
-        self, request: SynapseRequest, media_id: str, max_timeout_ms: int
+        self,
+        request: SynapseRequest,
+        media_id: str,
+        max_timeout_ms: int,
+        allow_media_linked_to_redacted_event: bool = False,
     ) -> Optional[LocalMedia]:
         """Gets the info dictionary for given local media ID. If the media has
         not been uploaded yet, this function will wait up to ``max_timeout_ms``
@@ -963,6 +973,7 @@ async def get_local_media_info(
                 the file_id for local content.)
             max_timeout_ms: the maximum number of milliseconds to wait for the
                 media to be uploaded.
+            allow_media_linked_to_redacted_event:
 
         Returns:
             Either the info dictionary for the given local media ID or
@@ -986,7 +997,11 @@ async def get_local_media_info(
             # The file has been uploaded, so stop looping
             if media_info.media_length is not None:
                 if isinstance(request.requester, Requester):
-                    await self.is_media_visible(request.requester.user, media_info)
+                    await self.is_media_visible(
+                        request.requester.user,
+                        media_info,
+                        allow_media_linked_to_redacted_event,
+                    )
                 return media_info
 
             # Check if the media ID has expired and still hasn't been uploaded to.
@@ -1015,6 +1030,7 @@ async def get_local_media(
         requester: Optional[Requester] = None,
         allow_authenticated: bool = True,
         federation: bool = False,
+        allow_media_linked_to_redacted_event: bool = False,
     ) -> None:
         """Responds to requests for local media, if exists, or returns 404.
 
@@ -1026,6 +1042,7 @@ async def get_local_media(
                 the filename in the Content-Disposition header of the response.
             max_timeout_ms: the maximum number of milliseconds to wait for the
                 media to be uploaded.
+            allow_media_linked_to_redacted_event:
             requester: The user making the request, to verify restricted media. Only
                 used for local users, not over federation
             allow_authenticated: whether media marked as authenticated may be served to this request
@@ -1034,7 +1051,9 @@ async def get_local_media(
         Returns:
             Resolves once a response has successfully been written to request
         """
-        media_info = await self.get_local_media_info(request, media_id, max_timeout_ms)
+        media_info = await self.get_local_media_info(
+            request, media_id, max_timeout_ms, allow_media_linked_to_redacted_event
+        )
         if not media_info:
             return
 
@@ -1049,7 +1068,9 @@ async def get_local_media(
             if requester is not None:
                 # Only check media visibility if this is for a local request. This will
                 # raise directly back to the client if not visible
-                await self.is_media_visible(requester.user, media_info)
+                await self.is_media_visible(
+                    requester.user, media_info, allow_media_linked_to_redacted_event
+                )
             restrictions = await self.validate_media_restriction(
                 request, media_info, None, federation
             )
@@ -1103,6 +1124,7 @@ async def get_remote_media(
         use_federation_endpoint: bool,
         requester: Optional[Requester] = None,
         allow_authenticated: bool = True,
+        allow_media_linked_to_redacted_event: bool = False,
     ) -> None:
         """Respond to requests for remote media.
 
@@ -1121,6 +1143,7 @@ async def get_remote_media(
                 used for local users, not over federation
             allow_authenticated: whether media marked as authenticated may be served to this
                 request
+            allow_media_linked_to_redacted_event:
 
         Returns:
             Resolves once a response has successfully been written to request
@@ -1153,7 +1176,8 @@ async def get_remote_media(
                 ip_address,
                 use_federation_endpoint,
                 allow_authenticated,
-                requester,
+                allow_media_linked_to_redacted_event=allow_media_linked_to_redacted_event,
+                requester=requester,
             )
 
         # Check if the media is cached on the client, if so return 304. We need
@@ -1189,6 +1213,7 @@ async def get_remote_media_info(
         use_federation: bool,
         allow_authenticated: bool,
         requester: Optional[Requester] = None,
+        allow_media_linked_to_redacted_event: bool = False,
     ) -> RemoteMedia:
         """Gets the media info associated with the remote file, downloading
         if necessary.
@@ -1205,6 +1230,7 @@ async def get_remote_media_info(
                 request
             requester: The user making the request, to verify restricted media. Only
                 used for local users, not over federation
+            allow_media_linked_to_redacted_event:
 
         Returns:
             The media info of the file
@@ -1227,7 +1253,8 @@ async def get_remote_media_info(
                 ip_address,
                 use_federation,
                 allow_authenticated,
-                requester,
+                allow_media_linked_to_redacted_event=allow_media_linked_to_redacted_event,
+                requester=requester,
             )
 
         # Ensure we actually use the responder so that it releases resources
@@ -1246,6 +1273,7 @@ async def _get_remote_media_impl(
         ip_address: str,
         use_federation_endpoint: bool,
         allow_authenticated: bool,
+        allow_media_linked_to_redacted_event: bool = False,
         requester: Optional[Requester] = None,
     ) -> Tuple[Optional[Responder], RemoteMedia]:
         """Looks for media in local cache, if not there then attempt to
@@ -1263,6 +1291,7 @@ async def _get_remote_media_impl(
             use_federation_endpoint: whether to request the remote media over the new federation
             /download endpoint
             allow_authenticated:
+            allow_media_linked_to_redacted_event:
             requester: The user making the request, to verify restricted media. Only
                 used for local users, not over federation
 
@@ -1284,7 +1313,9 @@ async def _get_remote_media_impl(
             # retrieved from the remote.
             if self.msc3911_config.enabled and requester is not None:
                 # This will raise directly back to the client if not visible
-                await self.is_media_visible(requester.user, media_info)
+                await self.is_media_visible(
+                    requester.user, media_info, allow_media_linked_to_redacted_event
+                )
 
             # file_id is the ID we use to track the file locally. If we've already
             # seen the file then reuse the existing ID, otherwise generate a new
@@ -1344,7 +1375,9 @@ async def _get_remote_media_impl(
             and requester is not None
         ):
             # This will raise directly back to the client if not visible
-            await self.is_media_visible(requester.user, media_info)
+            await self.is_media_visible(
+                requester.user, media_info, allow_media_linked_to_redacted_event
+            )
 
         file_id = media_info.filesystem_id
 

synapse/rest/client/media.py

@@ -40,6 +40,7 @@
 )
 from synapse.http.servlet import (
     RestServlet,
+    parse_boolean,
     parse_integer,
     parse_json_object_from_request,
     parse_string,
@@ -277,9 +278,18 @@ async def on_GET(
         )
         max_timeout_ms = min(max_timeout_ms, MAXIMUM_ALLOWED_MAX_TIMEOUT_MS)
 
+        allow_media_linked_to_redacted_event = parse_boolean(
+            request, "admin_bypass_redacted", default=False
+        )
+
         if self._is_mine_server_name(server_name):
             await self.media_repo.get_local_media(
-                request, media_id, file_name, max_timeout_ms, requester
+                request,
+                media_id,
+                file_name,
+                max_timeout_ms,
+                allow_media_linked_to_redacted_event=allow_media_linked_to_redacted_event,
+                requester=requester,
             )
         else:
             ip_address = request.getClientAddress().host
@@ -292,6 +302,7 @@ async def on_GET(
                 ip_address,
                 True,
                 requester,
+                allow_media_linked_to_redacted_event=allow_media_linked_to_redacted_event,
             )
 
 

tests/rest/client/test_media_download.py

@@ -72,6 +72,8 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
             "profile_test_user", "testpass"
         )
         self.other_profile_test_user_tok = self.login("profile_test_user", "testpass")
+        self.admin_user = self.register_user("bossman", "testpass", admin=True)
+        self.admin_tok = self.login("bossman", "testpass")
 
     def _create_restricted_media(self, user: str) -> MXCUri:
         mxc_uri = self.get_success(
@@ -91,17 +93,22 @@ def fetch_media(
         mxc_uri: MXCUri,
         access_token: Optional[str] = None,
         expected_code: int = 200,
+        admin_override: bool = False,
     ) -> None:
         """
         Test retrieving the media. We do not care about the content of the media, just
         that the response is correct
         """
+        path = f"/_matrix/client/v1/media/download/{mxc_uri.server_name}/{mxc_uri.media_id}"
+        if admin_override:
+            path += "?admin_bypass_redacted=true"
         channel = self.make_request(
             "GET",
-            f"/_matrix/client/v1/media/download/{mxc_uri.server_name}/{mxc_uri.media_id}",
+            path,
             access_token=access_token or self.creator_tok,
+            shorthand=False,
         )
-        assert channel.code == expected_code, channel.code
+        assert channel.code == expected_code, channel.json_body
 
     def test_local_media_download_unrestricted(self) -> None:
         """Test that unrestricted media is not affected"""
@@ -377,7 +384,8 @@ def test_local_media_download_attached_to_redacted_event(self) -> None:
             tok=self.creator_tok,
         )
 
-        _ = self.helper.join(room_id, self.other_user, tok=self.other_user_tok)
+        self.helper.join(room_id, self.other_user, tok=self.other_user_tok)
+        self.helper.join(room_id, self.admin_user, tok=self.admin_tok)
 
         image = {
             "body": "test_png_upload",
@@ -398,12 +406,17 @@ def test_local_media_download_attached_to_redacted_event(self) -> None:
         # Both users should be able to see the event
         self.fetch_media(mxc_uri)
         self.fetch_media(mxc_uri, access_token=self.other_user_tok)
+        self.fetch_media(mxc_uri, access_token=self.admin_tok)
 
         # now, redact that event, and try and retrieve the media again
         self._redact_event(self.creator_tok, room_id, json_body["event_id"])
 
         self.fetch_media(mxc_uri, expected_code=404)
         self.fetch_media(mxc_uri, access_token=self.other_user_tok, expected_code=404)
+        self.fetch_media(mxc_uri, access_token=self.admin_tok, expected_code=404)
+
+        # Let's see if the bypass works
+        self.fetch_media(mxc_uri, access_token=self.admin_tok, admin_override=True)
 
     def test_local_media_download_attached_to_redacted_state_event(self) -> None:
         """Test that a simple membership avatar is viewable when appropriate"""
@@ -418,7 +431,8 @@ def test_local_media_download_attached_to_redacted_state_event(self) -> None:
             tok=self.creator_tok,
         )
 
-        _ = self.helper.join(room_id, self.other_user, tok=self.other_user_tok)
+        self.helper.join(room_id, self.other_user, tok=self.other_user_tok)
+        self.helper.join(room_id, self.admin_user, tok=self.admin_tok)
 
         membership_content = {
             EventContentFields.MEMBERSHIP: Membership.JOIN,
@@ -438,9 +452,14 @@ def test_local_media_download_attached_to_redacted_state_event(self) -> None:
         # Both users should be able to see the media
         self.fetch_media(mxc_uri)
         self.fetch_media(mxc_uri, access_token=self.other_user_tok)
+        self.fetch_media(mxc_uri, access_token=self.admin_tok)
 
         # now, redact that event, and try and retrieve the media again
         self._redact_event(self.creator_tok, room_id, json_body["event_id"])
 
         self.fetch_media(mxc_uri, expected_code=404)
         self.fetch_media(mxc_uri, access_token=self.other_user_tok, expected_code=404)
+        self.fetch_media(mxc_uri, access_token=self.admin_tok, expected_code=404)
+
+        # Let's see if the bypass works
+        self.fetch_media(mxc_uri, access_token=self.admin_tok, admin_override=True)