Support for experimental MSC4335

- Make it available behind experimental feature flag
- return it for media upload limits

Author: hughns

changelog.d/18876.feature

@@ -0,0 +1 @@
+Add support for experimental [MSC4335](https://github.com/matrix-org/matrix-spec-proposals/pull/4335) M_USER_LIMIT_EXCEEDED error code for media upload limits.

docs/usage/configuration/config_documentation.md

@@ -2176,13 +2176,22 @@ These settings can be overridden using the `get_media_upload_limits_for_user` mo
 
 Defaults to `[]`.
 
+Options for each entry include:
+
+* `time_period` (duration): The time period over which the limit applies. Required.
+
+* `max_size` (byte size): Amount of data that can be uploaded in the time period by the user. Required.
+
+* `msc4335_info_url` (string): Experimental MSC4335 URL to a page with more information about the upload limit. Optional.
+
 Example configuration:
 ```yaml
 media_upload_limits:
 - time_period: 1h
   max_size: 100M
 - time_period: 1w
   max_size: 500M
+  msc4335_info_url: https://example.com/quota
 ```
 ---
 ### `max_image_pixels`

schema/synapse-config.schema.yaml

@@ -2426,20 +2426,30 @@ properties:
       module API [callback](../../modules/media_repository_callbacks.md#get_media_upload_limits_for_user).
     default: []
     items:
-      time_period:
-        type: "#/$defs/duration"
-        description: >-
-          The time period over which the limit applies. Required.
-      max_size:
-        type: "#/$defs/bytes"
-        description: >-
-          Amount of data that can be uploaded in the time period by the user.
-          Required.
+      type: object
+      required:
+        - time_period
+        - max_size
+      properties:
+        time_period:
+          $ref: "#/$defs/duration"
+          description: >-
+            The time period over which the limit applies. Required.
+        max_size:
+          $ref: "#/$defs/bytes"
+          description: >-
+            Amount of data that can be uploaded in the time period by the user.
+            Required.
+        msc4335_info_url:
+          type: string
+          description: >-
+            Experimental MSC4335 URL to a page with more information about the upload limit. Optional.
     examples:
       - - time_period: 1h
           max_size: 100M
         - time_period: 1w
           max_size: 500M
+          msc4335_info_url: https://example.com/quota
   max_image_pixels:
     $ref: "#/$defs/bytes"
     description: Maximum number of pixels that will be thumbnailed.

synapse/api/errors.py

@@ -513,6 +513,24 @@ def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict":
         )
 
 
+class MSC4335UserLimitExceededError(SynapseError):
+    """
+    Experimental implementation of MSC4335 M_USER_LIMIT_EXCEEDED error
+    """
+
+    def __init__(
+        self,
+        code: int,
+        msg: str,
+        info_url: str,
+    ):
+        additional_fields = {
+            "org.matrix.msc4335.info_url": info_url,
+            "org.matrix.msc4335.errcode": "M_USER_LIMIT_EXCEEDED",
+        }
+        super().__init__(code, msg, Codes.UNKNOWN, additional_fields=additional_fields)
+
+
 class EventSizeError(SynapseError):
     """An error raised when an event is too big."""
 

synapse/config/experimental.py

@@ -595,3 +595,6 @@ def read_config(
         # MSC4306: Thread Subscriptions
         # (and MSC4308: Thread Subscriptions extension to Sliding Sync)
         self.msc4306_enabled: bool = experimental.get("msc4306_enabled", False)
+
+        # MSC4335: M_USER_LIMIT_EXCEEDED error
+        self.msc4335_enabled: bool = experimental.get("msc4335_enabled", False)

synapse/config/repository.py

@@ -21,7 +21,7 @@
 
 import logging
 import os
-from typing import Any, Dict, List, Tuple
+from typing import Any, Dict, List, Optional, Tuple
 
 import attr
 
@@ -134,6 +134,9 @@ class MediaUploadLimit:
     time_period_ms: int
     """The time period in milliseconds."""
 
+    msc4335_info_url: Optional[str] = None
+    """Used for experimental MSC4335 error code feature"""
+
 
 class ContentRepositoryConfig(Config):
     section = "media"
@@ -302,8 +305,11 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
         for limit_config in config.get("media_upload_limits", []):
             time_period_ms = self.parse_duration(limit_config["time_period"])
             max_bytes = self.parse_size(limit_config["max_size"])
+            msc4335_info_url = limit_config.get("msc4335_info_url", None)
 
-            self.media_upload_limits.append(MediaUploadLimit(max_bytes, time_period_ms))
+            self.media_upload_limits.append(
+                MediaUploadLimit(max_bytes, time_period_ms, msc4335_info_url)
+            )
 
     def generate_config_section(self, data_dir_path: str, **kwargs: Any) -> str:
         assert data_dir_path is not None

synapse/media/media_repository.py

@@ -37,6 +37,7 @@
     Codes,
     FederationDeniedError,
     HttpResponseException,
+    MSC4335UserLimitExceededError,
     NotFoundError,
     RequestSendFailed,
     SynapseError,
@@ -68,6 +69,7 @@
 from synapse.media.thumbnailer import Thumbnailer, ThumbnailError
 from synapse.media.url_previewer import UrlPreviewer
 from synapse.metrics.background_process_metrics import run_as_background_process
+from synapse.rest.admin.experimental_features import ExperimentalFeature
 from synapse.storage.databases.main.media_repository import LocalMedia, RemoteMedia
 from synapse.types import UserID
 from synapse.util.async_helpers import Linearizer
@@ -382,6 +384,17 @@ async def create_or_update_content(
                     sent_bytes=uploaded_media_size,
                     attempted_bytes=content_length,
                 )
+                # If the MSC4335 experimental feature is enabled and the media limit
+                # has the info_url configured then we raise the MSC4335 error
+                msc4335_enabled = await self.store.is_feature_enabled(
+                    auth_user.to_string(), ExperimentalFeature.MSC4335
+                )
+                if msc4335_enabled and limit.msc4335_info_url:
+                    raise MSC4335UserLimitExceededError(
+                        403, "Media upload limit exceeded", limit.msc4335_info_url
+                    )
+                # Otherwise we use the current behaviour albeit not spec compliant
+                # See: https://github.com/element-hq/synapse/issues/18749
                 raise SynapseError(
                     400, "Media upload limit exceeded", Codes.RESOURCE_LIMIT_EXCEEDED
                 )

synapse/rest/admin/experimental_features.py

@@ -44,6 +44,7 @@ class ExperimentalFeature(str, Enum):
     MSC3881 = "msc3881"
     MSC3575 = "msc3575"
     MSC4222 = "msc4222"
+    MSC4335 = "msc4335"
 
     def is_globally_enabled(self, config: "HomeServerConfig") -> bool:
         if self is ExperimentalFeature.MSC3881:
@@ -52,6 +53,8 @@ def is_globally_enabled(self, config: "HomeServerConfig") -> bool:
             return config.experimental.msc3575_enabled
         if self is ExperimentalFeature.MSC4222:
             return config.experimental.msc4222_enabled
+        if self is ExperimentalFeature.MSC4335:
+            return config.experimental.msc4335_enabled
 
         assert_never(self)
 

tests/rest/client/test_media.py

@@ -44,7 +44,7 @@
 from twisted.web.iweb import UNKNOWN_LENGTH, IResponse
 from twisted.web.resource import Resource
 
-from synapse.api.errors import HttpResponseException
+from synapse.api.errors import Codes, HttpResponseException
 from synapse.api.ratelimiting import Ratelimiter
 from synapse.config._base import Config
 from synapse.config.oembed import OEmbedEndpointConfig
@@ -2880,11 +2880,12 @@ def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer:
 
         config["media_storage_providers"] = [provider_config]
 
-        # These are the limits that we are testing
-        config["media_upload_limits"] = [
-            {"time_period": "1d", "max_size": "1K"},
-            {"time_period": "1w", "max_size": "3K"},
-        ]
+        # These are the limits that we are testing unless overridden
+        if config.get("media_upload_limits") is None:
+            config["media_upload_limits"] = [
+                {"time_period": "1d", "max_size": "1K"},
+                {"time_period": "1w", "max_size": "3K"},
+            ]
 
         return self.setup_test_homeserver(config=config)
 
@@ -3002,10 +3003,11 @@ def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer:
         config["media_storage_providers"] = [provider_config]
 
         # default limits to use
-        config["media_upload_limits"] = [
-            {"time_period": "1d", "max_size": "1K"},
-            {"time_period": "1w", "max_size": "3K"},
-        ]
+        if config.get("media_upload_limits") is None:
+            config["media_upload_limits"] = [
+                {"time_period": "1d", "max_size": "1K"},
+                {"time_period": "1w", "max_size": "3K"},
+            ]
 
         return self.setup_test_homeserver(config=config)
 
@@ -3158,3 +3160,70 @@ def test_uses_defaults(self) -> None:
         )
         self.assertEqual(self.last_media_upload_limit_exceeded["sent_bytes"], 500)
         self.assertEqual(self.last_media_upload_limit_exceeded["attempted_bytes"], 800)
+
+    @override_config(
+        {
+            "media_upload_limits": [
+                {
+                    "time_period": "1d",
+                    "max_size": "1K",
+                    "msc4335_info_url": "https://example.com",
+                },
+            ]
+        }
+    )
+    def test_msc4335_defaults_disabled(self) -> None:
+        """Test that the MSC4335 is not used unless experimental feature is enabled."""
+        channel = self.upload_media(500, self.tok3)
+        self.assertEqual(channel.code, 200)
+
+        channel = self.upload_media(800, self.tok3)
+        # n.b. this response is not spec compliant as described at: https://github.com/element-hq/synapse/issues/18749
+        self.assertEqual(channel.code, 400)
+        self.assertEqual(channel.json_body["errcode"], Codes.RESOURCE_LIMIT_EXCEEDED)
+
+    @override_config(
+        {
+            "experimental_features": {"msc4335_enabled": True},
+            "media_upload_limits": [
+                {
+                    "time_period": "1d",
+                    "max_size": "1K",
+                    "msc4335_info_url": "https://example.com",
+                }
+            ],
+        }
+    )
+    def test_msc4335_returns_user_limit_exceeded(self) -> None:
+        """Test that the MSC4335 error is returned when experimental feature is enabled."""
+        channel = self.upload_media(500, self.tok3)
+        self.assertEqual(channel.code, 200)
+
+        channel = self.upload_media(800, self.tok3)
+        self.assertEqual(channel.code, 403)
+        self.assertEqual(channel.json_body["errcode"], Codes.UNKNOWN)
+        self.assertEqual(
+            channel.json_body["org.matrix.msc4335.errcode"], "M_USER_LIMIT_EXCEEDED"
+        )
+        self.assertEqual(
+            channel.json_body["org.matrix.msc4335.info_url"], "https://example.com"
+        )
+
+    @override_config(
+        {
+            "experimental_features": {"msc4335_enabled": True},
+            "media_upload_limits": [
+                {
+                    "time_period": "1d",
+                    "max_size": "1K",
+                }
+            ],
+        }
+    )
+    def test_msc4335_requires_info_url(self) -> None:
+        """Test that the MSC4335 error is not used if info_url is not provided."""
+        channel = self.upload_media(500, self.tok3)
+        self.assertEqual(channel.code, 200)
+
+        channel = self.upload_media(800, self.tok3)
+        self.assertEqual(channel.code, 400)