From d80f515622a678289aad7068751a26c6bb256dcd Mon Sep 17 00:00:00 2001 From: Tulir Asokan Date: Mon, 22 Sep 2025 15:45:05 +0200 Subject: [PATCH] Update MSC4190 support (#18946) --- changelog.d/18946.misc | 1 + synapse/api/errors.py | 3 ++ synapse/rest/client/keys.py | 7 +++- synapse/rest/client/login.py | 7 ++++ synapse/rest/client/register.py | 14 ++++++-- synapse/storage/databases/main/appservice.py | 4 +++ tests/handlers/test_oauth_delegation.py | 6 +++- tests/rest/client/test_devices.py | 12 +++++-- tests/rest/client/test_login.py | 35 ++++++++++++++++++++ tests/rest/client/test_register.py | 29 ++++++++++++++++ tests/unittest.py | 2 ++ 11 files changed, 113 insertions(+), 7 deletions(-) create mode 100644 changelog.d/18946.misc diff --git a/changelog.d/18946.misc b/changelog.d/18946.misc new file mode 100644 index 0000000000..53c246a638 --- /dev/null +++ b/changelog.d/18946.misc @@ -0,0 +1 @@ +Update [MSC4190](https://github.com/matrix-org/matrix-spec-proposals/pull/4190) support to return correct errors and allow appservices to reset cross-signing keys without user-interactive authentication. Contributed by @tulir @ Beeper. diff --git a/synapse/api/errors.py b/synapse/api/errors.py index ec4d707b7b..b3e391cd96 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -140,6 +140,9 @@ class Codes(str, Enum): # Part of MSC4155 INVITE_BLOCKED = "ORG.MATRIX.MSC4155.M_INVITE_BLOCKED" + # Part of MSC4190 + APPSERVICE_LOGIN_UNSUPPORTED = "IO.ELEMENT.MSC4190.M_APPSERVICE_LOGIN_UNSUPPORTED" + # Part of MSC4306: Thread Subscriptions MSC4306_CONFLICTING_UNSUBSCRIPTION = ( "IO.ELEMENT.MSC4306.M_CONFLICTING_UNSUBSCRIPTION" diff --git a/synapse/rest/client/keys.py b/synapse/rest/client/keys.py index 9f39889c75..6cf480952e 100644 --- a/synapse/rest/client/keys.py +++ b/synapse/rest/client/keys.py @@ -399,10 +399,15 @@ class SigningKeyUploadServlet(RestServlet): if not keys_are_different: return 200, {} + # MSC4190 can skip UIA for replacing cross-signing keys as well. + is_appservice_with_msc4190 = ( + requester.app_service and requester.app_service.msc4190_device_management + ) + # The keys are different; is x-signing set up? If no, then this is first-time # setup, and that is allowed without UIA, per MSC3967. # If yes, then we need to authenticate the change. - if is_cross_signing_setup: + if is_cross_signing_setup and not is_appservice_with_msc4190: # With MSC3861, UIA is not possible. Instead, the auth service has to # explicitly mark the master key as replaceable. if self.hs.config.mas.enabled: diff --git a/synapse/rest/client/login.py b/synapse/rest/client/login.py index acb9111ad2..921232a3ea 100644 --- a/synapse/rest/client/login.py +++ b/synapse/rest/client/login.py @@ -216,6 +216,13 @@ class LoginRestServlet(RestServlet): "This login method is only valid for application services" ) + if appservice.msc4190_device_management: + raise SynapseError( + 400, + "This appservice has MSC4190 enabled, so appservice login cannot be used.", + errcode=Codes.APPSERVICE_LOGIN_UNSUPPORTED, + ) + if appservice.is_rate_limited(): await self._address_ratelimiter.ratelimit( None, request.getClientAddress().host diff --git a/synapse/rest/client/register.py b/synapse/rest/client/register.py index 102c04bb67..b42006e4ce 100644 --- a/synapse/rest/client/register.py +++ b/synapse/rest/client/register.py @@ -782,8 +782,12 @@ class RegisterRestServlet(RestServlet): user_id, appservice = await self.registration_handler.appservice_register( username, as_token ) - if appservice.msc4190_device_management: - body["inhibit_login"] = True + if appservice.msc4190_device_management and not body.get("inhibit_login"): + raise SynapseError( + 400, + "This appservice has MSC4190 enabled, so the inhibit_login parameter must be set to true.", + errcode=Codes.APPSERVICE_LOGIN_UNSUPPORTED, + ) return await self._create_registration_details( user_id, @@ -923,6 +927,12 @@ class RegisterAppServiceOnlyRestServlet(RestServlet): "Registration has been disabled. Only m.login.application_service registrations are allowed.", errcode=Codes.FORBIDDEN, ) + if not body.get("inhibit_login"): + raise SynapseError( + 400, + "This server uses OAuth2, so the inhibit_login parameter must be set to true for appservice registrations.", + errcode=Codes.APPSERVICE_LOGIN_UNSUPPORTED, + ) kind = parse_string(request, "kind", default="user") diff --git a/synapse/storage/databases/main/appservice.py b/synapse/storage/databases/main/appservice.py index 9862e574fd..90ff0f0f12 100644 --- a/synapse/storage/databases/main/appservice.py +++ b/synapse/storage/databases/main/appservice.py @@ -83,6 +83,10 @@ class ApplicationServiceWorkerStore(RoomMemberWorkerStore): hs.hostname, hs.config.appservice.app_service_config_files ) self.exclusive_user_regex = _make_exclusive_regex(self.services_cache) + # When OAuth is enabled, force all appservices to enable MSC4190 too. + if hs.config.mas.enabled or hs.config.experimental.msc3861.enabled: + for appservice in self.services_cache: + appservice.msc4190_device_management = True def get_max_as_txn_id(txn: Cursor) -> int: logger.warning("Falling back to slow query, you should port to postgres") diff --git a/tests/handlers/test_oauth_delegation.py b/tests/handlers/test_oauth_delegation.py index d24614f6a3..b93e366b01 100644 --- a/tests/handlers/test_oauth_delegation.py +++ b/tests/handlers/test_oauth_delegation.py @@ -1219,7 +1219,11 @@ class DisabledEndpointsTestCase(HomeserverTestCase): channel = self.make_request( "POST", "/_matrix/client/v3/register", - {"username": "alice", "type": "m.login.application_service"}, + { + "username": "alice", + "type": "m.login.application_service", + "inhibit_login": True, + }, shorthand=False, access_token="i_am_an_app_service", ) diff --git a/tests/rest/client/test_devices.py b/tests/rest/client/test_devices.py index 2c498e97e1..309e6ec686 100644 --- a/tests/rest/client/test_devices.py +++ b/tests/rest/client/test_devices.py @@ -494,7 +494,9 @@ class MSC4190AppserviceDevicesTestCase(unittest.HomeserverTestCase): return self.hs def test_PUT_device(self) -> None: - self.register_appservice_user("alice", self.msc4190_service.token) + self.register_appservice_user( + "alice", self.msc4190_service.token, inhibit_login=True + ) self.register_appservice_user("bob", self.pre_msc_service.token) channel = self.make_request( @@ -542,7 +544,9 @@ class MSC4190AppserviceDevicesTestCase(unittest.HomeserverTestCase): self.assertEqual(channel.code, 404, channel.json_body) def test_DELETE_device(self) -> None: - self.register_appservice_user("alice", self.msc4190_service.token) + self.register_appservice_user( + "alice", self.msc4190_service.token, inhibit_login=True + ) # There should be no device channel = self.make_request( @@ -589,7 +593,9 @@ class MSC4190AppserviceDevicesTestCase(unittest.HomeserverTestCase): self.assertEqual(channel.json_body, {"devices": []}) def test_POST_delete_devices(self) -> None: - self.register_appservice_user("alice", self.msc4190_service.token) + self.register_appservice_user( + "alice", self.msc4190_service.token, inhibit_login=True + ) # There should be no device channel = self.make_request( diff --git a/tests/rest/client/test_login.py b/tests/rest/client/test_login.py index 8f9856fa2e..2f70a7a87e 100644 --- a/tests/rest/client/test_login.py +++ b/tests/rest/client/test_login.py @@ -1498,9 +1498,23 @@ class AppserviceLoginRestServletTestCase(unittest.HomeserverTestCase): ApplicationService.NS_ALIASES: [], }, ) + self.msc4190_service = ApplicationService( + id="third__identifier", + token="third_token", + sender=UserID.from_string("@as3bot:example.com"), + namespaces={ + ApplicationService.NS_USERS: [ + {"regex": r"@as3_user.*", "exclusive": False} + ], + ApplicationService.NS_ROOMS: [], + ApplicationService.NS_ALIASES: [], + }, + msc4190_device_management=True, + ) self.hs.get_datastores().main.services_cache.append(self.service) self.hs.get_datastores().main.services_cache.append(self.another_service) + self.hs.get_datastores().main.services_cache.append(self.msc4190_service) return self.hs def test_login_appservice_user(self) -> None: @@ -1517,6 +1531,27 @@ class AppserviceLoginRestServletTestCase(unittest.HomeserverTestCase): self.assertEqual(channel.code, 200, msg=channel.result) + def test_login_appservice_msc4190_fail(self) -> None: + """Test that an appservice user can use /login""" + self.register_appservice_user( + "as3_user_alice", self.msc4190_service.token, inhibit_login=True + ) + + params = { + "type": login.LoginRestServlet.APPSERVICE_TYPE, + "identifier": {"type": "m.id.user", "user": "as3_user_alice"}, + } + channel = self.make_request( + b"POST", LOGIN_URL, params, access_token=self.msc4190_service.token + ) + + self.assertEqual(channel.code, 400, msg=channel.result) + self.assertEqual( + channel.json_body.get("errcode"), + Codes.APPSERVICE_LOGIN_UNSUPPORTED, + channel.json_body, + ) + def test_login_appservice_user_bot(self) -> None: """Test that the appservice bot can use /login""" self.register_appservice_user(AS_USER, self.service.token) diff --git a/tests/rest/client/test_register.py b/tests/rest/client/test_register.py index 70e005caf4..0ffc64dd1f 100644 --- a/tests/rest/client/test_register.py +++ b/tests/rest/client/test_register.py @@ -136,6 +136,7 @@ class RegisterRestServletTestCase(unittest.HomeserverTestCase): request_data = { "username": "as_user_kermit", "type": APP_SERVICE_REGISTRATION_TYPE, + "inhibit_login": True, } channel = self.make_request( @@ -147,6 +148,34 @@ class RegisterRestServletTestCase(unittest.HomeserverTestCase): self.assertLessEqual(det_data.items(), channel.json_body.items()) self.assertNotIn("access_token", channel.json_body) + def test_POST_appservice_msc4190_enabled_fail(self) -> None: + # With MSC4190 enabled, the registration should fail unless inhibit_login is set + as_token = "i_am_an_app_service" + + appservice = ApplicationService( + as_token, + id="1234", + namespaces={"users": [{"regex": r"@as_user.*", "exclusive": True}]}, + sender=UserID.from_string("@as:test"), + msc4190_device_management=True, + ) + + self.hs.get_datastores().main.services_cache.append(appservice) + request_data = { + "username": "as_user_kermit", + "type": APP_SERVICE_REGISTRATION_TYPE, + } + + channel = self.make_request( + b"POST", self.url + b"?access_token=i_am_an_app_service", request_data + ) + self.assertEqual(channel.code, 400, channel.json_body) + self.assertEqual( + channel.json_body.get("errcode"), + Codes.APPSERVICE_LOGIN_UNSUPPORTED, + channel.json_body, + ) + def test_POST_bad_password(self) -> None: request_data = {"username": "kermit", "password": 666} channel = self.make_request(b"POST", self.url, request_data) diff --git a/tests/unittest.py b/tests/unittest.py index 5e6957dc6d..c9f8c48665 100644 --- a/tests/unittest.py +++ b/tests/unittest.py @@ -782,6 +782,7 @@ class HomeserverTestCase(TestCase): self, username: str, appservice_token: str, + inhibit_login: bool = False, ) -> Tuple[str, Optional[str]]: """Register an appservice user as an application service. Requires the client-facing registration API be registered. @@ -802,6 +803,7 @@ class HomeserverTestCase(TestCase): { "username": username, "type": "m.login.application_service", + "inhibit_login": inhibit_login, }, access_token=appservice_token, )