Support MSC3916 by adding _matrix/client/v1/media/download endpoint

changelog.d/17365.feature

@@ -0,0 +1 @@
+Support [MSC3916](https://github.com/matrix-org/matrix-spec-proposals/blob/rav/authentication-for-media/proposals/3916-authentication-for-media.md) by adding _matrix/client/v1/media/download endpoint.

docker/configure_workers_and_start.py

@@ -117,14 +117,15 @@
  },
  "media_repository": {
  "app": "synapse.app.generic_worker",
- "listener_resources": ["media"],
+ "listener_resources": ["media", "client"],
  "endpoint_patterns": [
  "^/_matrix/media/",
  "^/_synapse/admin/v1/purge_media_cache$",
  "^/_synapse/admin/v1/room/.*/media.*$",
  "^/_synapse/admin/v1/user/.*/media.*$",
  "^/_synapse/admin/v1/media/.*$",
  "^/_synapse/admin/v1/quarantine_media/.*$",
+ "^/_matrix/client/v1/media/.*$",
  ],
  # The first configured media worker will run the media background jobs
  "shared_extra_conf": {

docs/upgrade.md

@@ -117,6 +117,19 @@ each upgrade are complete before moving on to the next upgrade, to avoid
 stacking them up. You can monitor the currently running background updates with
 [the Admin API](usage/administration/admin_api/background_updates.html#status).
 
+# Upgrading to v1.111.0
+
+## New worker endpoints for authenticated client media
+
+[Media repository workers](./workers.md#synapseappmedia_repository) handling
+Media APIs can now handle the following endpoint pattern:
+
+```
+^/_matrix/client/v1/media/.*$
+```
+
+Please update your reverse proxy configuration.
+
 # Upgrading to v1.106.0
 
 ## Minimum supported Rust version

docs/workers.md

@@ -739,6 +739,7 @@ An example for a federation sender instance:
 Handles the media repository. It can handle all endpoints starting with:
 
  /_matrix/media/
+ /_matrix/client/v1/media/
 
 ... and the following regular expressions matching media-specific administration APIs:
 

mypy.ini

@@ -96,3 +96,6 @@ ignore_missing_imports = True
 # https://github.com/twisted/treq/pull/366
 [mypy-treq.*]
 ignore_missing_imports = True
+
+[mypy-multipart.*]
+ignore_missing_imports = True

pyproject.toml

@@ -224,6 +224,8 @@ pydantic = ">=1.7.4, <3"
 # needed.
 setuptools_rust = ">=1.3"
 
+# This is used for parsing multipart responses
+python-multipart = ">=0.0.9"
 
 # Optional Dependencies
 # ---------------------

synapse/api/ratelimiting.py

@@ -130,7 +130,8 @@ async def can_do_action(
  Overrides the value set during instantiation if set.
  burst_count: How many actions that can be performed before being limited.
  Overrides the value set during instantiation if set.
- update: Whether to count this check as performing the action
+ update: Whether to count this check as performing the action. If the action
+ cannot be performed, the user's action count is not incremented at all.
  n_actions: The number of times the user wants to do this action. If the user
  cannot do all of the actions, the user's action count is not incremented
  at all.

synapse/federation/federation_client.py

@@ -1871,6 +1871,52 @@ def filter_user_id(user_id: str) -> bool:
 
  return filtered_statuses, filtered_failures
 
+ async def federation_download_media(
+ self,
+ destination: str,
+ media_id: str,
+ output_stream: BinaryIO,
+ max_size: int,
+ max_timeout_ms: int,
+ download_ratelimiter: Ratelimiter,
+ ip_address: str,
+ ) -> Union[
+ Tuple[int, Dict[bytes, List[bytes]], bytes],
+ Tuple[int, Dict[bytes, List[bytes]]],
+ ]:
+ try:
+ return await self.transport_layer.federation_download_media(
+ destination,
+ media_id,
+ output_stream=output_stream,
+ max_size=max_size,
+ max_timeout_ms=max_timeout_ms,
+ download_ratelimiter=download_ratelimiter,
+ ip_address=ip_address,
+ )
+ except HttpResponseException as e:
+ # If an error is received that is due to an unrecognised endpoint,
+ # fallback to the _matrix/media/v3/download endpoint. Otherwise, consider it a legitimate error
+ # and raise.
+ if not is_unknown_endpoint(e):
+ raise
+
+ logger.debug(
+ "Couldn't download media %s/%s over _matrix/federation/v1/media/download, falling back to _matrix/media/v3/download path",
+ destination,
+ media_id,
+ )
+
+ return await self.transport_layer.download_media_v3(
+ destination,
+ media_id,
+ output_stream=output_stream,
+ max_size=max_size,
+ max_timeout_ms=max_timeout_ms,
+ download_ratelimiter=download_ratelimiter,
+ ip_address=ip_address,
+ )
+
  async def download_media(
  self,
  destination: str,

synapse/federation/transport/client.py

@@ -824,7 +824,6 @@ async def download_media_r0(
  ip_address: str,
  ) -> Tuple[int, Dict[bytes, List[bytes]]]:
  path = f"/_matrix/media/r0/download/{destination}/{media_id}"
-
  return await self.client.get_file(
  destination,
  path,
@@ -852,7 +851,6 @@ async def download_media_v3(
  ip_address: str,
  ) -> Tuple[int, Dict[bytes, List[bytes]]]:
  path = f"/_matrix/media/v3/download/{destination}/{media_id}"
-
  return await self.client.get_file(
  destination,
  path,
@@ -873,6 +871,29 @@ async def download_media_v3(
  ip_address=ip_address,
  )
 
+ async def federation_download_media(
+ self,
+ destination: str,
+ media_id: str,
+ output_stream: BinaryIO,
+ max_size: int,
+ max_timeout_ms: int,
+ download_ratelimiter: Ratelimiter,
+ ip_address: str,
+ ) -> Tuple[int, Dict[bytes, List[bytes]], bytes]:
+ path = f"/_matrix/federation/v1/media/download/{media_id}"
+ return await self.client.federation_get_file(
+ destination,
+ path,
+ output_stream=output_stream,
+ max_size=max_size,
+ args={
+ "timeout_ms": str(max_timeout_ms),
+ },
+ download_ratelimiter=download_ratelimiter,
+ ip_address=ip_address,
+ )
+
 
 def _create_path(federation_prefix: str, path: str, *args: str) -> str:
  """

synapse/federation/transport/server/__init__.py

@@ -32,8 +32,8 @@
 from synapse.federation.transport.server.federation import (
  FEDERATION_SERVLET_CLASSES,
  FederationAccountStatusServlet,
+ FederationMediaDownloadServlet,
  FederationUnstableClientKeysClaimServlet,
- FederationUnstableMediaDownloadServlet,
 )
 from synapse.http.server import HttpServer, JsonResource
 from synapse.http.servlet import (
@@ -316,11 +316,8 @@ def register_servlets(
  ):
  continue
 
- if servletclass == FederationUnstableMediaDownloadServlet:
- if (
- not hs.config.server.enable_media_repo
- or not hs.config.experimental.msc3916_authenticated_media_enabled
- ):
+ if servletclass == FederationMediaDownloadServlet:
+ if not hs.config.server.enable_media_repo:
  continue
 
  servletclass(

synapse/federation/transport/server/_base.py

@@ -362,7 +362,7 @@ async def new_func(
  return None
  if (
  func.__self__.__class__.__name__ # type: ignore
- == "FederationUnstableMediaDownloadServlet"
+ == "FederationMediaDownloadServlet"
  ):
  response = await func(
  origin, content, request, *args, **kwargs
@@ -374,7 +374,7 @@ async def new_func(
  else:
  if (
  func.__self__.__class__.__name__ # type: ignore
- == "FederationUnstableMediaDownloadServlet"
+ == "FederationMediaDownloadServlet"
  ):
  response = await func(
  origin, content, request, *args, **kwargs

synapse/federation/transport/server/federation.py

@@ -790,15 +790,14 @@ async def on_POST(
  return 200, {"account_statuses": statuses, "failures": failures}
 
 
-class FederationUnstableMediaDownloadServlet(BaseFederationServerServlet):
+class FederationMediaDownloadServlet(BaseFederationServerServlet):
  """
  Implementation of new federation media `/download` endpoint outlined in MSC3916. Returns
  a multipart/mixed response consisting of a JSON object and the requested media
  item. This endpoint only returns local media.
  """
 
  PATH = "/media/download/(?P<media_id>[^/]*)"
- PREFIX = FEDERATION_UNSTABLE_PREFIX + "/org.matrix.msc3916"
  RATELIMIT = True
 
  def __init__(
@@ -858,5 +857,5 @@ async def on_GET(
  FederationV1SendKnockServlet,
  FederationMakeKnockServlet,
  FederationAccountStatusServlet,
- FederationUnstableMediaDownloadServlet,
+ FederationMediaDownloadServlet,
 )