diff --git a/changelog.d/18876.feature b/changelog.d/18876.feature new file mode 100644 index 00000000000..a976987a56b --- /dev/null +++ b/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. diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index fec8d468a86..db6dd85daf6 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/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` diff --git a/schema/synapse-config.schema.yaml b/schema/synapse-config.schema.yaml index 285df53afeb..e3599b057f6 100644 --- a/schema/synapse-config.schema.yaml +++ b/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. diff --git a/synapse/api/errors.py b/synapse/api/errors.py index fb6721c0eea..caab6c4d6d1 100644 --- a/synapse/api/errors.py +++ b/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 @@ 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_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.""" diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index 04ca6e3c517..64735909c3c 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -598,3 +598,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) diff --git a/synapse/config/repository.py b/synapse/config/repository.py index e7d23740f9e..e3591a44cfa 100644 --- a/synapse/config/repository.py +++ b/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 @@ 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_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 diff --git a/synapse/media/media_repository.py b/synapse/media/media_repository.py index 238dc6cb2f3..04072505abd 100644 --- a/synapse/media/media_repository.py +++ b/synapse/media/media_repository.py @@ -37,6 +37,7 @@ Codes, FederationDeniedError, HttpResponseException, + MSC4335UserLimitExceededError, NotFoundError, RequestSendFailed, SynapseError, @@ -67,6 +68,7 @@ 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 @@ 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_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 ) diff --git a/synapse/rest/admin/experimental_features.py b/synapse/rest/admin/experimental_features.py index 3d3015cef77..e6063e9701c 100644 --- a/synapse/rest/admin/experimental_features.py +++ b/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) diff --git a/tests/rest/client/test_media.py b/tests/rest/client/test_media.py index 91bf94b672d..51d03daeff9 100644 --- a/tests/rest/client/test_media.py +++ b/tests/rest/client/test_media.py @@ -44,9 +44,11 @@ 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.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 @@ 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) @@ -2970,6 +2974,173 @@ def test_over_weekly_limit(self) -> None: 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 @@ 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 +3330,25 @@ 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_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)