diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index 5aa3b3a75c..d7454bbf33 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -248,6 +248,19 @@ class ExperimentalConfig(Config): ("experimental", "msc4388_enabled"), ) + # How long a rendezvous session lasts before it expires. MSC4388 + # recommends at least 2 minutes for usability, to allow the user time to + # complete the sign in including any extra steps such as MFA, and at + # most 5 minutes for security. + self.msc4388_session_ttl_ms = self.parse_duration( + experimental.get("msc4388_session_ttl", "2m") + ) + if not 2 * 60 * 1000 <= self.msc4388_session_ttl_ms <= 5 * 60 * 1000: + raise ConfigError( + "msc4388_session_ttl must be between 2 and 5 minutes", + ("experimental", "msc4388_session_ttl"), + ) + # MSC4388 requests are ratelimited by client IP address, as most are # unauthenticated: only discovery and creating a session may carry an # access token, and even then it is not used for ratelimiting. @@ -255,8 +268,8 @@ class ExperimentalConfig(Config): # Creating a session is limited more strictly than other requests, as # there is a cap on the number of concurrent sessions, and creating new # sessions beyond it evicts the oldest ones. A client using these - # defaults can hold at most around 20 live sessions at once, as each - # session expires after 2 minutes. + # defaults can hold at most around 20 live sessions at once with the + # default session TTL of 2 minutes, or 40 with the maximum of 5 minutes. self.msc4388_rc_create = RatelimitSettings.parse( experimental, "msc4388_rc_create", diff --git a/synapse/server.py b/synapse/server.py index 266440534c..a6a4ffac67 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -1209,7 +1209,9 @@ class HomeServer(metaclass=abc.ABCMeta): @cache_in_self def get_msc4388_rendezvous_handler(self) -> MSC4388RendezvousHandler: - return MSC4388RendezvousHandler(self) + return MSC4388RendezvousHandler( + self, ttl=self.config.experimental.msc4388_session_ttl_ms + ) @cache_in_self def get_outbound_redis_connection(self) -> "ConnectionHandler": diff --git a/tests/config/test_experimental.py b/tests/config/test_experimental.py index 0fede98ee9..a18c03f2be 100644 --- a/tests/config/test_experimental.py +++ b/tests/config/test_experimental.py @@ -15,6 +15,7 @@ import yaml from parameterized import parameterized +from synapse.config import ConfigError from synapse.config._base import RootConfig from synapse.config.experimental import ExperimentalConfig from synapse.config.homeserver import HomeServerConfig @@ -56,9 +57,39 @@ class ExperimentalConfigTestCase(unittest.TestCase): _read_config(config_values) + @parameterized.expand( + [ + ["default", None, 2 * 60 * 1000], + ["minimum", "2m", 2 * 60 * 1000], + ["maximum", "5m", 5 * 60 * 1000], + ["milliseconds", 200000, 200000], + ] + ) + def test_msc4388_session_ttl( + self, test_description: str, ttl: str | int | None, expected_ms: int + ) -> None: + """ + Test that the MSC4388 session TTL is parsed as a duration. + """ + experimental_features = {} if ttl is None else {"msc4388_session_ttl": ttl} + config = _read_config({"experimental_features": experimental_features}) + self.assertEqual(config.msc4388_session_ttl_ms, expected_ms) -def _read_config(config_values: JsonDict) -> None: - ExperimentalConfig(RootConfig()).read_config( + @parameterized.expand([["too_short", "119s"], ["too_long", "301s"]]) + def test_msc4388_session_ttl_out_of_range( + self, test_description: str, ttl: str + ) -> None: + """ + Test that an MSC4388 session TTL outside of the range recommended by the + MSC is rejected. + """ + with self.assertRaises(ConfigError): + _read_config({"experimental_features": {"msc4388_session_ttl": ttl}}) + + +def _read_config(config_values: JsonDict) -> ExperimentalConfig: + config = ExperimentalConfig(RootConfig()) + config.read_config( yaml.safe_load( HomeServerConfig().generate_config( config_dir_path="CONFDIR", @@ -69,3 +100,4 @@ def _read_config(config_values: JsonDict) -> None: | config_values, allow_secrets_in_config=False, ) + return config diff --git a/tests/rest/client/test_msc4388_rendezvous.py b/tests/rest/client/test_msc4388_rendezvous.py index ec326202cd..eb09be8e5f 100644 --- a/tests/rest/client/test_msc4388_rendezvous.py +++ b/tests/rest/client/test_msc4388_rendezvous.py @@ -622,6 +622,34 @@ class RendezvousServletTestCase(unittest.HomeserverTestCase): ) self.assertEqual(channel.code, 404) + @override_config({"experimental_features": {"msc4388_session_ttl": "5m"}}) + def test_configured_ttl(self) -> None: + """ + Test that sessions expire after the configured TTL. + """ + channel = self.make_request( + "POST", + rz_endpoint, + {"data": "foo=bar"}, + access_token=None, + ) + self.assertEqual(channel.code, 200) + self.assertEqual(channel.json_body["expires_in_ms"], 5 * 60 * 1000) + session_endpoint = rz_endpoint + "/" + channel.json_body["id"] + + # The session is still there after the default TTL of 2 minutes... + self.reactor.advance(120) + channel = self.make_request("GET", session_endpoint, access_token=None) + self.assertEqual(channel.code, 200) + # (allowing for the time taken by the requests) + self.assertLessEqual(channel.json_body["expires_in_ms"], 3 * 60 * 1000) + self.assertGreater(channel.json_body["expires_in_ms"], 3 * 60 * 1000 - 1000) + + # ...but is gone after the configured TTL of 5 minutes + self.reactor.advance(180) + channel = self.make_request("GET", session_endpoint, access_token=None) + self.assertEqual(channel.code, 404) + def test_delete_expired(self) -> None: """ Test that deleting a session which has expired, but has not yet been