Update MSC4190 support (#18946)

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.

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"

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:

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

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")
 

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")

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",
         )

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(

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)

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)

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,
         )