diff --git a/changelog.d/19573.feature b/changelog.d/19573.feature new file mode 100644 index 00000000000..69eca83681f --- /dev/null +++ b/changelog.d/19573.feature @@ -0,0 +1 @@ +Updated experimental support for [MSC4388: Secure out-of-band channel for sign in with QR](https://github.com/matrix-org/matrix-spec-proposals/pull/4388). diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index a8c9305704e..9077c0f5e21 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -544,9 +544,9 @@ def read_config( # See: https://github.com/element-hq/synapse/issues/19433 msc4388_mode = experimental.get("msc4388_mode", "off") - if msc4388_mode not in ["off", "public", "authenticated"]: + if msc4388_mode not in ["off", "open", "authenticated"]: raise ConfigError( - "msc4388_mode must be one of 'off', 'public' or 'authenticated'", + "msc4388_mode must be one of 'off', 'open' or 'authenticated'", ("experimental", "msc4388_mode"), ) self.msc4388_enabled: bool = msc4388_mode != "off" diff --git a/synapse/rest/client/rendezvous.py b/synapse/rest/client/rendezvous.py index bd9205fc5f0..246788609e2 100644 --- a/synapse/rest/client/rendezvous.py +++ b/synapse/rest/client/rendezvous.py @@ -20,6 +20,7 @@ # import logging +from http import HTTPStatus from http.client import TEMPORARY_REDIRECT from typing import TYPE_CHECKING, Any @@ -81,6 +82,12 @@ def __init__(self, hs: "HomeServer") -> None: hs.config.experimental.msc4388_requires_authentication ) + async def on_GET(self, request: SynapseRequest) -> tuple[int, Any]: + if self.require_authentication: + # This will raise if the user is not authenticated + await self.auth.get_user_by_req(request) + return HTTPStatus.OK, {"create_available": True} + async def on_POST(self, request: SynapseRequest) -> tuple[int, Any]: if self.require_authentication: # This will raise if the user is not authenticated diff --git a/synapse/rest/client/versions.py b/synapse/rest/client/versions.py index f8d7a1a4d9d..c9f0667f8d6 100644 --- a/synapse/rest/client/versions.py +++ b/synapse/rest/client/versions.py @@ -189,8 +189,6 @@ async def on_GET(self, request: SynapseRequest) -> tuple[int, JsonDict]: is not None ) ), - # MSC4388: Secure out-of-band channel for sign in with QR - "io.element.msc4388": (self.config.experimental.msc4388_enabled), # MSC4140: Delayed events "org.matrix.msc4140": bool(self.config.server.max_event_delay_ms), # Simplified sliding sync diff --git a/tests/rest/client/test_msc4388_rendezvous.py b/tests/rest/client/test_msc4388_rendezvous.py index f16bb3f344a..913a41dedb2 100644 --- a/tests/rest/client/test_msc4388_rendezvous.py +++ b/tests/rest/client/test_msc4388_rendezvous.py @@ -131,6 +131,10 @@ def test_disabled(self) -> None: } ) def test_off(self) -> None: + # Discovery endpoint should return 404 + channel = self.make_request("GET", rz_endpoint, {}, access_token=None) + self.assertEqual(channel.code, 404) + # Create session should also fail channel = self.make_request("POST", rz_endpoint, {}, access_token=None) self.assertEqual(channel.code, 404) @@ -143,7 +147,7 @@ def test_off(self) -> None: "endpoint": "https://issuer", }, "experimental_features": { - "msc4388_mode": "public", + "msc4388_mode": "open", }, } ) @@ -156,6 +160,11 @@ def test_rendezvous_public(self) -> None: - Deleting the data - Sequence token handling """ + # Discovery should return 200 + channel = self.make_request("GET", rz_endpoint, {}, access_token=None) + self.assertEqual(channel.code, 200) + self.assertTrue(channel.json_body.get("create_available")) + # We can post arbitrary data to the endpoint channel = self.make_request( "POST", @@ -268,7 +277,11 @@ def test_rendezvous_requires_authentication(self) -> None: self.setup_mock_oauth() alice_token = self.register_oauth_user("alice", "device1") - # This should fail without authentication: + # Discovery should fail due to lack of authentication + channel = self.make_request("GET", rz_endpoint, {}, access_token=None) + self.assertEqual(channel.code, 401) + + # Creating a session should fail without authentication: channel = self.make_request( "POST", rz_endpoint, @@ -277,6 +290,11 @@ def test_rendezvous_requires_authentication(self) -> None: ) self.assertEqual(channel.code, 401) + # Discovery should succeed with authentication + channel = self.make_request("GET", rz_endpoint, {}, access_token=alice_token) + self.assertEqual(channel.code, 200) + self.assertTrue(channel.json_body.get("create_available")) + # This should work as we are now authenticated channel = self.make_request( "POST", @@ -288,7 +306,7 @@ def test_rendezvous_requires_authentication(self) -> None: rendezvous_id = channel.json_body["id"] sequence_token = channel.json_body["sequence_token"] expires_in_ms = channel.json_body["expires_in_ms"] - self.assertGreater(expires_in_ms, 0) + self.assertEqual(expires_in_ms, 120000) session_endpoint = rz_endpoint + f"/{rendezvous_id}" @@ -302,7 +320,7 @@ def test_rendezvous_requires_authentication(self) -> None: self.assertEqual(channel.code, 200) self.assertEqual(channel.json_body["data"], "foo=bar") self.assertEqual(channel.json_body["sequence_token"], sequence_token) - self.assertEqual(channel.json_body["expires_in_ms"], expires_in_ms) + self.assertEqual(channel.json_body["expires_in_ms"], expires_in_ms - 100) # We can update the data without authentication channel = self.make_request( @@ -325,7 +343,7 @@ def test_rendezvous_requires_authentication(self) -> None: self.assertEqual(channel.code, 200) self.assertEqual(channel.json_body["data"], "foo=baz") self.assertEqual(channel.json_body["sequence_token"], new_sequence_token) - self.assertEqual(channel.json_body["expires_in_ms"], expires_in_ms - 200) + self.assertEqual(channel.json_body["expires_in_ms"], expires_in_ms - 300) # We can delete the data without authentication channel = self.make_request( @@ -355,7 +373,7 @@ def test_rendezvous_requires_authentication(self) -> None: "endpoint": "https://issuer", }, "experimental_features": { - "msc4388_mode": "public", + "msc4388_mode": "open", }, } ) @@ -402,7 +420,7 @@ def test_expiration(self) -> None: "endpoint": "https://issuer", }, "experimental_features": { - "msc4388_mode": "public", + "msc4388_mode": "open", }, } ) @@ -473,7 +491,7 @@ def test_capacity(self) -> None: "endpoint": "https://issuer", }, "experimental_features": { - "msc4388_mode": "public", + "msc4388_mode": "open", }, } ) @@ -531,7 +549,7 @@ def test_hard_capacity(self) -> None: "endpoint": "https://issuer", }, "experimental_features": { - "msc4388_mode": "public", + "msc4388_mode": "open", }, } ) @@ -585,7 +603,7 @@ def test_data_type(self) -> None: "endpoint": "https://issuer", }, "experimental_features": { - "msc4388_mode": "public", + "msc4388_mode": "open", }, } ) @@ -648,7 +666,7 @@ def test_max_length(self) -> None: "endpoint": "https://issuer", }, "experimental_features": { - "msc4388_mode": "public", + "msc4388_mode": "open", }, } ) @@ -698,7 +716,7 @@ def test_rendezvous_rejects_unsafe_get_requests( "endpoint": "https://issuer", }, "experimental_features": { - "msc4388_mode": "public", + "msc4388_mode": "open", }, } )