diff --git a/changelog.d/18876.feature b/changelog.d/18876.feature new file mode 100644 index 0000000000..a976987a56 --- /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 3c401d569b..4571141ab3 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -2176,6 +2176,14 @@ 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: @@ -2183,6 +2191,7 @@ media_upload_limits: max_size: 100M - time_period: 1w max_size: 500M + msc4335_info_url: https://example.com/quota ``` --- ### `max_image_pixels` diff --git a/schema/synapse-config.schema.yaml b/schema/synapse-config.schema.yaml index 2a7f94a700..0a658979d9 100644 --- a/schema/synapse-config.schema.yaml +++ b/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. diff --git a/synapse/api/errors.py b/synapse/api/errors.py index fb6721c0ee..ec477ec880 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -513,6 +513,24 @@ class ResourceLimitError(SynapseError): ) +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.""" diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index d7a3d67558..f5369017a5 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -595,3 +595,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) diff --git a/synapse/config/repository.py b/synapse/config/repository.py index e7d23740f9..a667733ed7 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,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 @@ 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_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 diff --git a/synapse/media/media_repository.py b/synapse/media/media_repository.py index a3c0b3036e..6a0f276672 100644 --- a/synapse/media/media_repository.py +++ b/synapse/media/media_repository.py @@ -37,6 +37,7 @@ from synapse.api.errors import ( Codes, FederationDeniedError, HttpResponseException, + MSC4335UserLimitExceededError, NotFoundError, RequestSendFailed, SynapseError, @@ -68,6 +69,7 @@ from synapse.media.storage_provider import StorageProviderWrapper 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 @@ 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_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 ) diff --git a/synapse/rest/admin/experimental_features.py b/synapse/rest/admin/experimental_features.py index 3d3015cef7..e6063e9701 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 @@ 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) diff --git a/tests/rest/client/test_media.py b/tests/rest/client/test_media.py index 91bf94b672..22e7bcdfb3 100644 --- a/tests/rest/client/test_media.py +++ b/tests/rest/client/test_media.py @@ -44,7 +44,7 @@ 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._base import Config from synapse.config.oembed import OEmbedEndpointConfig @@ -2880,11 +2880,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) @@ -3002,10 +3003,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 +3160,70 @@ 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_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)