From a45caee57bf72103a2a7c4cbca5313c43af2e5f8 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Thu, 11 Dec 2025 13:45:39 -0600 Subject: [PATCH 1/7] Prevent access to media if the attached event id has been redacted --- synapse/media/media_repository.py | 7 ++ tests/rest/client/test_media_download.py | 106 +++++++++++++++++++++++ 2 files changed, 113 insertions(+) diff --git a/synapse/media/media_repository.py b/synapse/media/media_repository.py index 24c2b86935..2a325166e7 100644 --- a/synapse/media/media_repository.py +++ b/synapse/media/media_repository.py @@ -349,6 +349,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 + # 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(): # The standard event visibility utility, filter_events_for_client(), # does not seem to meet the needs of a good UX when restricting and diff --git a/tests/rest/client/test_media_download.py b/tests/rest/client/test_media_download.py index e71b4c8acc..8f07a4c164 100644 --- a/tests/rest/client/test_media_download.py +++ b/tests/rest/client/test_media_download.py @@ -338,3 +338,109 @@ def test_local_media_download_attached_to_state_event_failure(self) -> None: # This user has joined the room and can now see this image. Can't see the # related membership event, but :man-shrug: self.fetch_media(mxc_uri, access_token=self.other_user_tok) + + def _redact_event( + self, + access_token: str, + room_id: str, + event_id: str, + expect_code: int = 200, + with_relations: Optional[list[str]] = None, + content: Optional[JsonDict] = None, + ) -> JsonDict: + """Helper function to send a redaction event. + + Returns the json body. + """ + path = "/_matrix/client/r0/rooms/%s/redact/%s" % (room_id, event_id) + + request_content = content or {} + if with_relations: + request_content["org.matrix.msc3912.with_relations"] = with_relations + + channel = self.make_request( + "POST", path, request_content, access_token=access_token + ) + self.assertEqual(channel.code, expect_code) + return channel.json_body + + def test_local_media_download_attached_to_redacted_event(self) -> None: + """Test that can local media attached to image event can be restricted if redacted""" + mxc_uri = self._create_restricted_media(self.creator) + room_id = self.helper.create_room_as(self.creator, tok=self.creator_tok) + + # set room history_visibility to joined, otherwise it will be 'shared' + self.helper.send_state( + room_id=room_id, + event_type=EventTypes.RoomHistoryVisibility, + body={"history_visibility": HistoryVisibility.JOINED}, + tok=self.creator_tok, + ) + + _ = self.helper.join(room_id, self.other_user, tok=self.other_user_tok) + + image = { + "body": "test_png_upload", + "info": {"h": 1, "mimetype": "image/png", "size": 67, "w": 1}, + "msgtype": "m.image", + "url": str(mxc_uri), + } + json_body = self.helper.send_event( + room_id, + "m.room.message", + content=image, + tok=self.creator_tok, + expect_code=200, + attach_media_mxc=str(mxc_uri), + ) + assert "event_id" in json_body + + # 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) + + # 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) + + def test_local_media_download_attached_to_redacted_state_event(self) -> None: + """Test that a simple membership avatar is viewable when appropriate""" + mxc_uri = self._create_restricted_media(self.creator) + room_id = self.helper.create_room_as(self.creator, tok=self.creator_tok) + + # set room history_visibility to joined + self.helper.send_state( + room_id=room_id, + event_type=EventTypes.RoomHistoryVisibility, + body={"history_visibility": HistoryVisibility.JOINED}, + tok=self.creator_tok, + ) + + _ = self.helper.join(room_id, self.other_user, tok=self.other_user_tok) + + membership_content = { + EventContentFields.MEMBERSHIP: Membership.JOIN, + "avatar_url": str(mxc_uri), + } + json_body = self.helper.send_state( + room_id, + EventTypes.Member, + body=membership_content, + tok=self.creator_tok, + expect_code=200, + state_key=self.creator, + attach_media_mxc=str(mxc_uri), + ) + assert "event_id" in json_body + + # 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) + + # 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) From 24beb29e6e448a729428013a80ff04049dedcd98 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Mon, 15 Dec 2025 10:11:47 -0600 Subject: [PATCH 2/7] wire up endpoint to accept and pass through redacted media access request --- synapse/media/media_repository.py | 54 ++++++++++++++++++------ synapse/rest/client/media.py | 14 +++++- tests/rest/client/test_media_download.py | 27 ++++++++++-- 3 files changed, 78 insertions(+), 17 deletions(-) diff --git a/synapse/media/media_repository.py b/synapse/media/media_repository.py index 2a325166e7..25f5b84881 100644 --- a/synapse/media/media_repository.py +++ b/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_redacted_media: 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_redacted_media: bool = False, ) -> None: """ Verify that media requested for download should be visible to the user making @@ -349,11 +353,10 @@ 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_redacted_media: + # 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 +954,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_redacted_media: 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 +970,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_redacted_media: Returns: Either the info dictionary for the given local media ID or @@ -986,7 +994,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_redacted_media, + ) return media_info # Check if the media ID has expired and still hasn't been uploaded to. @@ -1015,6 +1027,7 @@ async def get_local_media( requester: Optional[Requester] = None, allow_authenticated: bool = True, federation: bool = False, + allow_redacted_media: bool = False, ) -> None: """Responds to requests for local media, if exists, or returns 404. @@ -1026,6 +1039,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_redacted_media: 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 +1048,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_redacted_media + ) if not media_info: return @@ -1049,7 +1065,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_redacted_media + ) restrictions = await self.validate_media_restriction( request, media_info, None, federation ) @@ -1103,6 +1121,7 @@ async def get_remote_media( use_federation_endpoint: bool, requester: Optional[Requester] = None, allow_authenticated: bool = True, + allow_redacted_media: bool = False, ) -> None: """Respond to requests for remote media. @@ -1121,6 +1140,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_redacted_media: Returns: Resolves once a response has successfully been written to request @@ -1153,7 +1173,8 @@ async def get_remote_media( ip_address, use_federation_endpoint, allow_authenticated, - requester, + allow_redacted_media=allow_redacted_media, + requester=requester, ) # Check if the media is cached on the client, if so return 304. We need @@ -1189,6 +1210,7 @@ async def get_remote_media_info( use_federation: bool, allow_authenticated: bool, requester: Optional[Requester] = None, + allow_redacted_media: bool = False, ) -> RemoteMedia: """Gets the media info associated with the remote file, downloading if necessary. @@ -1205,6 +1227,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_redacted_media: Returns: The media info of the file @@ -1227,7 +1250,8 @@ async def get_remote_media_info( ip_address, use_federation, allow_authenticated, - requester, + allow_redacted_media=allow_redacted_media, + requester=requester, ) # Ensure we actually use the responder so that it releases resources @@ -1246,6 +1270,7 @@ async def _get_remote_media_impl( ip_address: str, use_federation_endpoint: bool, allow_authenticated: bool, + allow_redacted_media: bool = False, requester: Optional[Requester] = None, ) -> Tuple[Optional[Responder], RemoteMedia]: """Looks for media in local cache, if not there then attempt to @@ -1263,6 +1288,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_redacted_media: requester: The user making the request, to verify restricted media. Only used for local users, not over federation @@ -1284,7 +1310,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_redacted_media + ) # 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 +1372,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_redacted_media + ) file_id = media_info.filesystem_id diff --git a/synapse/rest/client/media.py b/synapse/rest/client/media.py index d2c491bcd9..8abe506e17 100644 --- a/synapse/rest/client/media.py +++ b/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,19 @@ async def on_GET( ) max_timeout_ms = min(max_timeout_ms, MAXIMUM_ALLOWED_MAX_TIMEOUT_MS) + # TODO: determine if the parameter needs an unstable identifier + allow_redacted_media = parse_boolean( + request, "allow_redacted_media", 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_redacted_media=allow_redacted_media, + requester=requester, ) else: ip_address = request.getClientAddress().host @@ -292,6 +303,7 @@ async def on_GET( ip_address, True, requester, + allow_redacted_media=allow_redacted_media, ) diff --git a/tests/rest/client/test_media_download.py b/tests/rest/client/test_media_download.py index 8f07a4c164..6fc2302f21 100644 --- a/tests/rest/client/test_media_download.py +++ b/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, + attempt_bypass: 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 attempt_bypass: + path += "?allow_redacted_media=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, attempt_bypass=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, attempt_bypass=True) From 8e719e43234671c4f1d2286e31cb6b1e2884cda2 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Mon, 15 Dec 2025 11:48:25 -0600 Subject: [PATCH 3/7] turns out this whole block is duplicated inside get_local_media_info() and therefore can be removed --- synapse/media/media_repository.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/synapse/media/media_repository.py b/synapse/media/media_repository.py index 25f5b84881..ec710f7bb8 100644 --- a/synapse/media/media_repository.py +++ b/synapse/media/media_repository.py @@ -1062,12 +1062,6 @@ async def get_local_media( # if MSC3911 is enabled, check visibility of the media for the user and retrieve # any restrictions if self.msc3911_config.enabled: - 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, allow_redacted_media - ) restrictions = await self.validate_media_restriction( request, media_info, None, federation ) From 268f7cd80ebf3d4b6c4375077611fe2eb40e4fb4 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Mon, 15 Dec 2025 11:13:34 -0600 Subject: [PATCH 4/7] Create a new namedtuple object to store configuration for the is_media_visible() redaction bypass --- synapse/media/media_repository.py | 38 +++++++++++++++++++++++-------- synapse/types/__init__.py | 6 +++++ 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/synapse/media/media_repository.py b/synapse/media/media_repository.py index ec710f7bb8..3bbe54cf5b 100644 --- a/synapse/media/media_repository.py +++ b/synapse/media/media_repository.py @@ -76,7 +76,7 @@ MediaRestrictions, RemoteMedia, ) -from synapse.types import JsonDict, Requester, UserID +from synapse.types import JsonDict, RedactedMediaBypass, Requester, UserID from synapse.types.state import StateFilter from synapse.util import json_decoder from synapse.util.async_helpers import Linearizer @@ -316,7 +316,7 @@ async def is_media_visible( self, requesting_user: UserID, media_info_object: Union[LocalMedia, RemoteMedia], - allow_redacted_media: bool = False, + redacted_media_bypass_config: Optional[RedactedMediaBypass] = None, ) -> None: """ Verify that media requested for download should be visible to the user making @@ -353,11 +353,15 @@ 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() and not allow_redacted_media: - # 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 - raise NotFoundError() + if event_base.internal_metadata.is_redacted(): + if ( + not redacted_media_bypass_config + or not redacted_media_bypass_config.requesting_bypass + ): + # 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 + raise NotFoundError() if event_base.is_state(): # The standard event visibility utility, filter_events_for_client(), @@ -994,10 +998,16 @@ 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): + # Only check media visibility if this is for a local request + is_admin = await self.auth.is_server_admin(request.requester) + redacted_media_bypass_config = RedactedMediaBypass( + allow_redacted_media, is_admin + ) + await self.is_media_visible( request.requester.user, media_info, - allow_redacted_media, + redacted_media_bypass_config, ) return media_info @@ -1303,9 +1313,13 @@ async def _get_remote_media_impl( # exists in the local database and again further down for after it was # retrieved from the remote. if self.msc3911_config.enabled and requester is not None: + is_admin = await self.auth.is_server_admin(requester) + redacted_media_bypass_config = RedactedMediaBypass( + allow_redacted_media, is_admin + ) # This will raise directly back to the client if not visible await self.is_media_visible( - requester.user, media_info, allow_redacted_media + requester.user, media_info, redacted_media_bypass_config ) # file_id is the ID we use to track the file locally. If we've already @@ -1365,9 +1379,13 @@ async def _get_remote_media_impl( and self.msc3911_config.enabled and requester is not None ): + is_admin = await self.auth.is_server_admin(requester) + redacted_media_bypass_config = RedactedMediaBypass( + allow_redacted_media, is_admin + ) # This will raise directly back to the client if not visible await self.is_media_visible( - requester.user, media_info, allow_redacted_media + requester.user, media_info, redacted_media_bypass_config ) file_id = media_info.filesystem_id diff --git a/synapse/types/__init__.py b/synapse/types/__init__.py index 914bb6cb23..d6518e14ac 100644 --- a/synapse/types/__init__.py +++ b/synapse/types/__init__.py @@ -23,6 +23,7 @@ import logging import re import string +from collections import namedtuple from enum import Enum from typing import ( TYPE_CHECKING, @@ -1530,3 +1531,8 @@ class ScheduledTask: result: Optional[JsonMapping] # Optional error that should be assigned a value when the status is FAILED error: Optional[str] + + +RedactedMediaBypass = namedtuple( + "RedactedMediaBypass", ["requesting_bypass", "is_admin"] +) From dc8758bb64c804e502907d9619111f930fddc1c9 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Mon, 15 Dec 2025 11:49:21 -0600 Subject: [PATCH 5/7] Can we bypass if we are allowed to redact events? --- synapse/media/media_repository.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/synapse/media/media_repository.py b/synapse/media/media_repository.py index 3bbe54cf5b..7b6e3e6059 100644 --- a/synapse/media/media_repository.py +++ b/synapse/media/media_repository.py @@ -33,6 +33,7 @@ import twisted.web.http from twisted.internet.defer import Deferred +from synapse import event_auth from synapse.api.constants import EventTypes, HistoryVisibility, Membership from synapse.api.errors import ( Codes, @@ -71,6 +72,7 @@ from synapse.media.url_previewer import UrlPreviewer from synapse.metrics.background_process_metrics import run_as_background_process from synapse.replication.http.media import ReplicationCopyMediaServlet +from synapse.state import CREATE_KEY, POWER_KEY from synapse.storage.databases.main.media_repository import ( LocalMedia, MediaRestrictions, @@ -111,6 +113,7 @@ def __init__(self, hs: "HomeServer"): self.clock = hs.get_clock() self.server_name = hs.hostname self.store = hs.get_datastores().main + self._storage_controllers = hs.get_storage_controllers() self._is_mine_server_name = hs.is_mine_server_name self.msc3911_config = hs.config.experimental.msc3911 @@ -363,6 +366,32 @@ async def is_media_visible( # from this restriction raise NotFoundError() + # Which means a bypass was requested + if not redacted_media_bypass_config.is_admin: + # Lifted this directly from RoomEventServlet for msc2815 + auth_events = ( + await self._storage_controllers.state.get_current_state( + event_base.room_id, + StateFilter.from_types( + [ + POWER_KEY, + CREATE_KEY, + ] + ), + ) + ) + + redact_level = event_auth.get_named_level(auth_events, "redact", 50) + user_level = event_auth.get_user_power_level( + requesting_user.to_string(), auth_events + ) + if user_level < redact_level: + raise SynapseError( + 403, + "You don't have permission to view redacted events in this room.", + errcode=Codes.FORBIDDEN, + ) + if event_base.is_state(): # The standard event visibility utility, filter_events_for_client(), # does not seem to meet the needs of a good UX when restricting and From fa8d5edcdf8548641347d3f7f84a7364a7c94a57 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Mon, 15 Dec 2025 13:07:04 -0600 Subject: [PATCH 6/7] Adjust tests to separate out concerns into separate tests, and add one for the room moderator case --- tests/rest/client/test_media_download.py | 221 ++++++++++++++++++++++- 1 file changed, 218 insertions(+), 3 deletions(-) diff --git a/tests/rest/client/test_media_download.py b/tests/rest/client/test_media_download.py index 6fc2302f21..cd9c0915b5 100644 --- a/tests/rest/client/test_media_download.py +++ b/tests/rest/client/test_media_download.py @@ -371,8 +371,54 @@ def _redact_event( self.assertEqual(channel.code, expect_code) return channel.json_body - def test_local_media_download_attached_to_redacted_event(self) -> None: - """Test that can local media attached to image event can be restricted if redacted""" + def test_local_media_download_attached_to_redacted_event_normal(self) -> None: + """ + Test that can local media attached to image event can be restricted if redacted + """ + mxc_uri = self._create_restricted_media(self.creator) + room_id = self.helper.create_room_as(self.creator, tok=self.creator_tok) + + # set room history_visibility to joined, otherwise it will be 'shared' + self.helper.send_state( + room_id=room_id, + event_type=EventTypes.RoomHistoryVisibility, + body={"history_visibility": HistoryVisibility.JOINED}, + tok=self.creator_tok, + ) + + self.helper.join(room_id, self.other_user, tok=self.other_user_tok) + + image = { + "body": "test_png_upload", + "info": {"h": 1, "mimetype": "image/png", "size": 67, "w": 1}, + "msgtype": "m.image", + "url": str(mxc_uri), + } + json_body = self.helper.send_event( + room_id, + "m.room.message", + content=image, + tok=self.creator_tok, + expect_code=200, + attach_media_mxc=str(mxc_uri), + ) + assert "event_id" in json_body + + # 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) + + # 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) + + def test_local_media_download_attached_to_redacted_event_admin(self) -> None: + """ + Test that can local media attached to image event can be restricted if redacted. + Specifically, test that a system administrator can bypass that if requested + """ mxc_uri = self._create_restricted_media(self.creator) room_id = self.helper.create_room_as(self.creator, tok=self.creator_tok) @@ -418,7 +464,70 @@ def test_local_media_download_attached_to_redacted_event(self) -> None: # Let's see if the bypass works self.fetch_media(mxc_uri, access_token=self.admin_tok, attempt_bypass=True) - def test_local_media_download_attached_to_redacted_state_event(self) -> None: + def test_local_media_download_attached_to_redacted_event_room_moderator( + self, + ) -> None: + """ + Test that can local media attached to image event can be restricted if redacted. + Specifically, test that a room moderator can bypass that if requested and + empowered to + """ + mxc_uri = self._create_restricted_media(self.creator) + room_id = self.helper.create_room_as(self.creator, tok=self.creator_tok) + + # set room history_visibility to joined, otherwise it will be 'shared' + self.helper.send_state( + room_id=room_id, + event_type=EventTypes.RoomHistoryVisibility, + body={"history_visibility": HistoryVisibility.JOINED}, + tok=self.creator_tok, + ) + + # Adjust power levels in the room. Redacting is defaulted to 50, so let's bump + # the other user. "user_default" dictates this was at "0" + pl = self.helper.get_state( + room_id, EventTypes.PowerLevels, tok=self.creator_tok + ) + pl["users"][self.other_user] = 50 + self.helper.send_state( + room_id, EventTypes.PowerLevels, body=pl, tok=self.creator_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", + "info": {"h": 1, "mimetype": "image/png", "size": 67, "w": 1}, + "msgtype": "m.image", + "url": str(mxc_uri), + } + json_body = self.helper.send_event( + room_id, + "m.room.message", + content=image, + tok=self.creator_tok, + expect_code=200, + attach_media_mxc=str(mxc_uri), + ) + assert "event_id" in json_body + + # 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.other_user_tok, attempt_bypass=True) + + def test_local_media_download_attached_to_redacted_state_event_normal(self) -> None: """Test that a simple membership avatar is viewable when appropriate""" mxc_uri = self._create_restricted_media(self.creator) room_id = self.helper.create_room_as(self.creator, tok=self.creator_tok) @@ -431,6 +540,50 @@ 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) + + membership_content = { + EventContentFields.MEMBERSHIP: Membership.JOIN, + "avatar_url": str(mxc_uri), + } + json_body = self.helper.send_state( + room_id, + EventTypes.Member, + body=membership_content, + tok=self.creator_tok, + expect_code=200, + state_key=self.creator, + attach_media_mxc=str(mxc_uri), + ) + assert "event_id" in json_body + + # 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) + + # 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) + + def test_local_media_download_attached_to_redacted_state_event_admin(self) -> None: + """ + Test that a simple membership avatar is viewable when appropriate. Specifically, + test that a system administrator can bypass that if requested + + """ + mxc_uri = self._create_restricted_media(self.creator) + room_id = self.helper.create_room_as(self.creator, tok=self.creator_tok) + + # set room history_visibility to joined + self.helper.send_state( + room_id=room_id, + event_type=EventTypes.RoomHistoryVisibility, + body={"history_visibility": HistoryVisibility.JOINED}, + tok=self.creator_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) @@ -463,3 +616,65 @@ def test_local_media_download_attached_to_redacted_state_event(self) -> None: # Let's see if the bypass works self.fetch_media(mxc_uri, access_token=self.admin_tok, attempt_bypass=True) + + def test_local_media_download_attached_to_redacted_state_event_room_moderator( + self, + ) -> None: + """ + Test that a simple membership avatar is viewable when appropriate. + Specifically, test that a room moderator can bypass that if requested and + empowered to + """ + mxc_uri = self._create_restricted_media(self.creator) + room_id = self.helper.create_room_as(self.creator, tok=self.creator_tok) + + # set room history_visibility to joined + self.helper.send_state( + room_id=room_id, + event_type=EventTypes.RoomHistoryVisibility, + body={"history_visibility": HistoryVisibility.JOINED}, + tok=self.creator_tok, + ) + + # Adjust power levels in the room. Redacting is defaulted to 50, so let's bump + # the other user. "user_default" dictates this was at "0" + pl = self.helper.get_state( + room_id, EventTypes.PowerLevels, tok=self.creator_tok + ) + pl["users"][self.other_user] = 50 + self.helper.send_state( + room_id, EventTypes.PowerLevels, body=pl, tok=self.creator_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, + "avatar_url": str(mxc_uri), + } + json_body = self.helper.send_state( + room_id, + EventTypes.Member, + body=membership_content, + tok=self.creator_tok, + expect_code=200, + state_key=self.creator, + attach_media_mxc=str(mxc_uri), + ) + assert "event_id" in json_body + + # 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.other_user_tok, attempt_bypass=True) From 538f4636f9aafcc4381b5d83e704998e28b4355e Mon Sep 17 00:00:00 2001 From: Jason Little Date: Mon, 15 Dec 2025 13:14:18 -0600 Subject: [PATCH 7/7] [maybe] Allow system admins to bypass all other checks --- synapse/media/media_repository.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/synapse/media/media_repository.py b/synapse/media/media_repository.py index 7b6e3e6059..1f87ed406f 100644 --- a/synapse/media/media_repository.py +++ b/synapse/media/media_repository.py @@ -367,7 +367,11 @@ async def is_media_visible( raise NotFoundError() # Which means a bypass was requested - if not redacted_media_bypass_config.is_admin: + if redacted_media_bypass_config.is_admin: + # System admins get to bypass the rest of the checks + return + else: + # Not an admin, let's check they have a high enough power level. # Lifted this directly from RoomEventServlet for msc2815 auth_events = ( await self._storage_controllers.state.get_current_state(