Make soft_limit optional

Otherwise it won't appear on the documentation website's sidebar.

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/SUMMARY.md

@@ -60,6 +60,7 @@
     - [Admin API](usage/administration/admin_api/README.md)
       - [Account Validity](admin_api/account_validity.md)
       - [Background Updates](usage/administration/admin_api/background_updates.md)
+      - [Fetch Event](admin_api/fetch_event.md)
       - [Event Reports](admin_api/event_reports.md)
       - [Experimental Features](admin_api/experimental_features.md)
       - [Media](admin_api/media_admin_api.md)

docs/usage/configuration/config_documentation.md

@@ -2174,6 +2174,18 @@ 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_uri` (string): Experimental MSC4335 URI to where the user can find information about the upload limit. Optional.
+
+* `msc4335_soft_limit` (boolean): Experimental MSC4335 value to say if the limit can be increased. Optional.
+
+* `msc4335_increase_uri` (string): Experimental MSC4335 URI to where the user can increase the upload limit. Required if msc4335_soft_limit is true.
+
 Example configuration:
 ```yaml
 media_upload_limits:
@@ -2181,6 +2193,9 @@ media_upload_limits:
   max_size: 100M
 - time_period: 1w
   max_size: 500M
+  msc4335_info_uri: https://example.com/quota
+  msc4335_soft_limit: true
+  msc4335_increase_uri: https://example.com/increase-quota
 ```
 ---
 ### `max_image_pixels`

schema/synapse-config.schema.yaml

@@ -2424,20 +2424,40 @@ 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_uri:
+          type: string
+          description: >-
+            Experimental MSC4335 URI to where the user can find information about the upload limit. Optional.
+        msc4335_soft_limit:
+          type: boolean
+          description: >-
+            Experimental MSC4335 value to say if the limit can be increased. Optional.
+        msc4335_increase_uri:
+          type: string
+          description: >-
+            Experimental MSC4335 URI to where the user can increase the upload limit. Required if msc4335_soft_limit is true.
     examples:
       - - time_period: 1h
           max_size: 100M
         - time_period: 1w
           max_size: 500M
+          msc4335_info_uri: https://example.com/quota
+          msc4335_soft_limit: true
+          msc4335_increase_uri: https://example.com/increase-quota
   max_image_pixels:
     $ref: "#/$defs/bytes"
     description: Maximum number of pixels that will be thumbnailed.

synapse/api/errors.py

@@ -152,6 +152,8 @@ class Codes(str, Enum):
     # Part of MSC4326
     UNKNOWN_DEVICE = "ORG.MATRIX.MSC4326.M_UNKNOWN_DEVICE"
 
+    MSC4335_USER_LIMIT_EXCEEDED = "ORG.MATRIX.MSC4335_USER_LIMIT_EXCEEDED"
+
 
 class CodeMessageException(RuntimeError):
     """An exception with integer code, a message string attributes and optional headers.
@@ -513,6 +515,40 @@ class ResourceLimitError(SynapseError):
         )
 
 
+class MSC4335UserLimitExceededError(SynapseError):
+    """
+    Experimental implementation of MSC4335 M_USER_LIMIT_EXCEEDED error
+    """
+
+    def __init__(
+        self,
+        code: int,
+        msg: str,
+        info_uri: str,
+        soft_limit: bool = False,
+        increase_uri: Optional[str] = None,
+    ):
+        if soft_limit and increase_uri is None:
+            raise ValueError("increase_uri must be provided if soft_limit is True")
+
+        additional_fields: dict[str, Union[str, bool]] = {
+            "org.matrix.msc4335.info_uri": info_uri,
+        }
+
+        if soft_limit:
+            additional_fields["org.matrix.msc4335.soft_limit"] = soft_limit
+
+        if soft_limit and increase_uri is not None:
+            additional_fields["org.matrix.msc4335.increase_uri"] = increase_uri
+
+        super().__init__(
+            code,
+            msg,
+            Codes.MSC4335_USER_LIMIT_EXCEEDED,
+            additional_fields=additional_fields,
+        )
+
+
 class EventSizeError(SynapseError):
     """An error raised when an event is too big."""
 

synapse/config/experimental.py

@@ -598,3 +598,6 @@ class ExperimentalConfig(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,15 @@ class MediaUploadLimit:
     time_period_ms: int
     """The time period in milliseconds."""
 
+    msc4335_info_uri: Optional[str] = None
+    """Used for experimental MSC4335 error code feature"""
+
+    msc4335_soft_limit: Optional[bool] = None
+    """Used for experimental MSC4335 error code feature"""
+
+    msc4335_increase_uri: Optional[str] = None
+    """Used for experimental MSC4335 error code feature"""
+
 
 class ContentRepositoryConfig(Config):
     section = "media"
@@ -302,8 +311,34 @@ class ContentRepositoryConfig(Config):
         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_uri = limit_config.get("msc4335_info_uri", None)
+            msc4335_soft_limit = limit_config.get("msc4335_soft_limit", None)
+            msc4335_increase_uri = limit_config.get("msc4335_increase_uri", None)
 
-            self.media_upload_limits.append(MediaUploadLimit(max_bytes, time_period_ms))
+            if (
+                msc4335_info_uri is not None
+                or msc4335_soft_limit is not None
+                or msc4335_increase_uri is not None
+            ) and (not (msc4335_info_uri and msc4335_soft_limit is not None)):
+                raise ConfigError(
+                    "If any of msc4335_info_uri, msc4335_soft_limit or "
+                    "msc4335_increase_uri are set, then both msc4335_info_uri and "
+                    "msc4335_soft_limit must be set."
+                )
+            if msc4335_soft_limit and not msc4335_increase_uri:
+                raise ConfigError(
+                    "msc4335_increase_uri must be set if msc4335_soft_limit is true."
+                )
+
+            self.media_upload_limits.append(
+                MediaUploadLimit(
+                    max_bytes,
+                    time_period_ms,
+                    msc4335_info_uri,
+                    msc4335_soft_limit,
+                    msc4335_increase_uri,
+                )
+            )
 
     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 @@ from synapse.api.errors import (
     Codes,
     FederationDeniedError,
     HttpResponseException,
+    MSC4335UserLimitExceededError,
     NotFoundError,
     RequestSendFailed,
     SynapseError,
@@ -67,6 +68,7 @@ from synapse.media.media_storage import (
 from synapse.media.storage_provider import StorageProviderWrapper
 from synapse.media.thumbnailer import Thumbnailer, ThumbnailError
 from synapse.media.url_previewer import UrlPreviewer
+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
@@ -379,6 +381,25 @@ class MediaRepository:
                     sent_bytes=uploaded_media_size,
                     attempted_bytes=content_length,
                 )
+                # If the MSC4335 experimental feature is enabled and the media limit
+                # has the info_uri 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_uri
+                    and limit.msc4335_soft_limit is not None
+                ):
+                    raise MSC4335UserLimitExceededError(
+                        403,
+                        "Media upload limit exceeded",
+                        limit.msc4335_info_uri,
+                        limit.msc4335_soft_limit,
+                        limit.msc4335_increase_uri,
+                    )
+                # 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 @@ class ExperimentalFeature(str, Enum):
             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,9 +44,11 @@ from twisted.web.http_headers import Headers
 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 import ConfigError
 from synapse.config._base import Config
+from synapse.config.homeserver import HomeServerConfig
 from synapse.config.oembed import OEmbedEndpointConfig
 from synapse.http.client import MultipartResponse
 from synapse.http.types import QueryParams
@@ -75,6 +77,7 @@ from tests.media.test_media_storage import (
 from tests.server import FakeChannel, FakeTransport, ThreadedMemoryReactorClock
 from tests.test_utils import SMALL_PNG
 from tests.unittest import override_config
+from tests.utils import default_config
 
 try:
     import lxml
@@ -2880,11 +2883,12 @@ class MediaUploadLimits(unittest.HomeserverTestCase):
 
         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)
 
@@ -2970,6 +2974,173 @@ class MediaUploadLimits(unittest.HomeserverTestCase):
         channel = self.upload_media(900)
         self.assertEqual(channel.code, 200)
 
+    def test_msc4335_requires_config(self) -> None:
+        config_dict = default_config("test")
+
+        # msc4335_info_uri and msc4335_soft_limit are required
+        # msc4335_increase_uri is required if msc4335_soft_limit is true
+
+        with self.assertRaises(ConfigError):
+            HomeServerConfig().parse_config_dict(
+                {
+                    "media_upload_limits": [
+                        {
+                            "time_period": "1d",
+                            "max_size": "1K",
+                            "msc4335_info_uri": "https://example.com",
+                        }
+                    ],
+                    **config_dict,
+                },
+                "",
+                "",
+            )
+
+        with self.assertRaises(ConfigError):
+            HomeServerConfig().parse_config_dict(
+                {
+                    "media_upload_limits": [
+                        {
+                            "time_period": "1d",
+                            "max_size": "1K",
+                            "msc4335_info_uri": "https://example.com",
+                            "msc4335_soft_limit": True,
+                        }
+                    ],
+                    **config_dict,
+                },
+                "",
+                "",
+            )
+
+        with self.assertRaises(ConfigError):
+            HomeServerConfig().parse_config_dict(
+                {
+                    "media_upload_limits": [
+                        {
+                            "time_period": "1d",
+                            "max_size": "1K",
+                            "msc4335_soft_limit": False,
+                        }
+                    ],
+                    **config_dict,
+                },
+                "",
+                "",
+            )
+
+        with self.assertRaises(ConfigError):
+            HomeServerConfig().parse_config_dict(
+                {
+                    "media_upload_limits": [
+                        {
+                            "time_period": "1d",
+                            "max_size": "1K",
+                            "msc4335_soft_limit": True,
+                        }
+                    ],
+                    **config_dict,
+                },
+                "",
+                "",
+            )
+
+        with self.assertRaises(ConfigError):
+            HomeServerConfig().parse_config_dict(
+                {
+                    "media_upload_limits": [
+                        {
+                            "time_period": "1d",
+                            "max_size": "1K",
+                            "msc4335_increase_uri": "https://example.com/increase",
+                        }
+                    ],
+                    **config_dict,
+                },
+                "",
+                "",
+            )
+
+    @override_config(
+        {
+            "experimental_features": {"msc4335_enabled": True},
+            "media_upload_limits": [
+                {
+                    "time_period": "1d",
+                    "max_size": "1K",
+                    "msc4335_info_uri": "https://example.com",
+                    "msc4335_soft_limit": False,
+                }
+            ],
+        }
+    )
+    def test_msc4335_returns_hard_user_limit_exceeded(self) -> None:
+        """Test that the MSC4335 error is returned with soft_limit False when experimental feature is enabled."""
+        channel = self.upload_media(500)
+        self.assertEqual(channel.code, 200)
+
+        channel = self.upload_media(800)
+        self.assertEqual(channel.code, 403)
+        self.assertEqual(
+            channel.json_body["errcode"], "ORG.MATRIX.MSC4335_USER_LIMIT_EXCEEDED"
+        )
+        self.assertEqual(
+            channel.json_body["org.matrix.msc4335.info_uri"], "https://example.com"
+        )
+        self.assertEquals(hasattr(channel.json_body, "org.matrix.msc4335.increase_uri"), False)
+
+    @override_config(
+        {
+            "experimental_features": {"msc4335_enabled": True},
+            "media_upload_limits": [
+                {
+                    "time_period": "1d",
+                    "max_size": "1K",
+                    "msc4335_info_uri": "https://example.com",
+                    "msc4335_soft_limit": True,
+                    "msc4335_increase_uri": "https://example.com/increase",
+                }
+            ],
+        }
+    )
+    def test_msc4335_returns_soft_user_limit_exceeded(self) -> None:
+        """Test that the MSC4335 error is returned with soft_limit True when experimental feature is enabled."""
+        channel = self.upload_media(500)
+        self.assertEqual(channel.code, 200)
+
+        channel = self.upload_media(800)
+        self.assertEqual(channel.code, 403)
+        self.assertEqual(
+            channel.json_body["errcode"], "ORG.MATRIX.MSC4335_USER_LIMIT_EXCEEDED"
+        )
+        self.assertEqual(
+            channel.json_body["org.matrix.msc4335.info_uri"], "https://example.com"
+        )
+        self.assertEqual(channel.json_body["org.matrix.msc4335.soft_limit"], True)
+        self.assertEqual(
+            channel.json_body["org.matrix.msc4335.increase_uri"],
+            "https://example.com/increase",
+        )
+
+    @override_config(
+        {
+            "experimental_features": {"msc4335_enabled": True},
+            "media_upload_limits": [
+                {
+                    "time_period": "1d",
+                    "max_size": "1K",
+                }
+            ],
+        }
+    )
+    def test_msc4335_requires_info_uri(self) -> None:
+        """Test that the MSC4335 error is not used if info_uri is not provided."""
+        channel = self.upload_media(500)
+        self.assertEqual(channel.code, 200)
+
+        channel = self.upload_media(800)
+        self.assertEqual(channel.code, 400)
+
 
 class MediaUploadLimitsModuleOverrides(unittest.HomeserverTestCase):
     """
@@ -3002,10 +3173,11 @@ class MediaUploadLimitsModuleOverrides(unittest.HomeserverTestCase):
         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 +3330,25 @@ class MediaUploadLimitsModuleOverrides(unittest.HomeserverTestCase):
         )
         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_uri": "https://example.com",
+                    "msc4335_soft_limit": False,
+                },
+            ]
+        }
+    )
+    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)