Support for MSC4190: device management for application services (#17705)

This is an implementation of MSC4190, which allows appservices to manage
their user's devices without /login & /logout.

---------

Co-authored-by: Andrew Morgan <[email protected]>

Author: sandhose

changelog.d/17705.feature

@@ -0,0 +1 @@
+Support for [MSC4190](https://github.com/matrix-org/matrix-spec-proposals/pull/4190): device management for Application Services.

synapse/appservice/__init__.py

@@ -87,6 +87,7 @@ def __init__(
         ip_range_whitelist: Optional[IPSet] = None,
         supports_ephemeral: bool = False,
         msc3202_transaction_extensions: bool = False,
+        msc4190_device_management: bool = False,
     ):
         self.token = token
         self.url = (
@@ -100,6 +101,7 @@ def __init__(
         self.ip_range_whitelist = ip_range_whitelist
         self.supports_ephemeral = supports_ephemeral
         self.msc3202_transaction_extensions = msc3202_transaction_extensions
+        self.msc4190_device_management = msc4190_device_management
 
         if "|" in self.id:
             raise Exception("application service ID cannot contain '|' character")

synapse/config/appservice.py

@@ -183,6 +183,18 @@ def _load_appservice(
             "The `org.matrix.msc3202` option should be true or false if specified."
         )
 
+    # Opt-in flag for the MSC4190 behaviours.
+    # When enabled, the following C-S API endpoints change for appservices:
+    # - POST /register does not return an access token
+    # - PUT /devices/{device_id} creates a new device if one does not exist
+    # - DELETE /devices/{device_id} no longer requires UIA
+    # - POST /delete_devices/{device_id} no longer requires UIA
+    msc4190_enabled = as_info.get("io.element.msc4190", False)
+    if not isinstance(msc4190_enabled, bool):
+        raise ValueError(
+            "The `io.element.msc4190` option should be true or false if specified."
+        )
+
     return ApplicationService(
         token=as_info["as_token"],
         url=as_info["url"],
@@ -195,4 +207,5 @@ def _load_appservice(
         ip_range_whitelist=ip_range_whitelist,
         supports_ephemeral=supports_ephemeral,
         msc3202_transaction_extensions=msc3202_transaction_extensions,
+        msc4190_device_management=msc4190_enabled,
     )

synapse/handlers/device.py

@@ -729,6 +729,40 @@ async def delete_devices(self, user_id: str, device_ids: List[str]) -> None:
 
         await self.notify_device_update(user_id, device_ids)
 
+    async def upsert_device(
+        self, user_id: str, device_id: str, display_name: Optional[str] = None
+    ) -> bool:
+        """Create or update a device
+
+        Args:
+            user_id: The user to update devices of.
+            device_id: The device to update.
+            display_name: The new display name for this device.
+
+        Returns:
+            True if the device was created, False if it was updated.
+
+        """
+
+        # Reject a new displayname which is too long.
+        self._check_device_name_length(display_name)
+
+        created = await self.store.store_device(
+            user_id,
+            device_id,
+            initial_device_display_name=display_name,
+        )
+
+        if not created:
+            await self.store.update_device(
+                user_id,
+                device_id,
+                new_display_name=display_name,
+            )
+
+        await self.notify_device_update(user_id, [device_id])
+        return created
+
     async def update_device(self, user_id: str, device_id: str, content: dict) -> None:
         """Update the given device
 

synapse/handlers/register.py

@@ -630,7 +630,9 @@ async def post_consent_actions(self, user_id: str) -> None:
         """
         await self._auto_join_rooms(user_id)
 
-    async def appservice_register(self, user_localpart: str, as_token: str) -> str:
+    async def appservice_register(
+        self, user_localpart: str, as_token: str
+    ) -> Tuple[str, ApplicationService]:
         user = UserID(user_localpart, self.hs.hostname)
         user_id = user.to_string()
         service = self.store.get_app_service_by_token(as_token)
@@ -653,7 +655,7 @@ async def appservice_register(self, user_localpart: str, as_token: str) -> str:
             appservice_id=service_id,
             create_profile_with_displayname=user.localpart,
         )
-        return user_id
+        return (user_id, service)
 
     def check_user_id_not_appservice_exclusive(
         self, user_id: str, allowed_appservice: Optional[ApplicationService] = None

synapse/rest/client/devices.py

@@ -114,15 +114,19 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
             else:
                 raise e
 
-        await self.auth_handler.validate_user_via_ui_auth(
-            requester,
-            request,
-            body.dict(exclude_unset=True),
-            "remove device(s) from your account",
-            # Users might call this multiple times in a row while cleaning up
-            # devices, allow a single UI auth session to be re-used.
-            can_skip_ui_auth=True,
-        )
+        if requester.app_service and requester.app_service.msc4190_device_management:
+            # MSC4190 can skip UIA for this endpoint
+            pass
+        else:
+            await self.auth_handler.validate_user_via_ui_auth(
+                requester,
+                request,
+                body.dict(exclude_unset=True),
+                "remove device(s) from your account",
+                # Users might call this multiple times in a row while cleaning up
+                # devices, allow a single UI auth session to be re-used.
+                can_skip_ui_auth=True,
+            )
 
         await self.device_handler.delete_devices(
             requester.user.to_string(), body.devices
@@ -175,9 +179,6 @@ class DeleteBody(RequestBodyModel):
     async def on_DELETE(
         self, request: SynapseRequest, device_id: str
     ) -> Tuple[int, JsonDict]:
-        if self._msc3861_oauth_delegation_enabled:
-            raise UnrecognizedRequestError(code=404)
-
         requester = await self.auth.get_user_by_req(request)
 
         try:
@@ -192,15 +193,24 @@ async def on_DELETE(
             else:
                 raise
 
-        await self.auth_handler.validate_user_via_ui_auth(
-            requester,
-            request,
-            body.dict(exclude_unset=True),
-            "remove a device from your account",
-            # Users might call this multiple times in a row while cleaning up
-            # devices, allow a single UI auth session to be re-used.
-            can_skip_ui_auth=True,
-        )
+        if requester.app_service and requester.app_service.msc4190_device_management:
+            # MSC4190 allows appservices to delete devices through this endpoint without UIA
+            # It's also allowed with MSC3861 enabled
+            pass
+
+        else:
+            if self._msc3861_oauth_delegation_enabled:
+                raise UnrecognizedRequestError(code=404)
+
+            await self.auth_handler.validate_user_via_ui_auth(
+                requester,
+                request,
+                body.dict(exclude_unset=True),
+                "remove a device from your account",
+                # Users might call this multiple times in a row while cleaning up
+                # devices, allow a single UI auth session to be re-used.
+                can_skip_ui_auth=True,
+            )
 
         await self.device_handler.delete_devices(
             requester.user.to_string(), [device_id]
@@ -216,6 +226,16 @@ async def on_PUT(
         requester = await self.auth.get_user_by_req(request, allow_guest=True)
 
         body = parse_and_validate_json_object_from_request(request, self.PutBody)
+
+        # MSC4190 allows appservices to create devices through this endpoint
+        if requester.app_service and requester.app_service.msc4190_device_management:
+            created = await self.device_handler.upsert_device(
+                user_id=requester.user.to_string(),
+                device_id=device_id,
+                display_name=body.display_name,
+            )
+            return 201 if created else 200, {}
+
         await self.device_handler.update_device(
             requester.user.to_string(), device_id, body.dict()
         )

synapse/rest/client/register.py

@@ -771,9 +771,12 @@ async def _do_appservice_registration(
         body: JsonDict,
         should_issue_refresh_token: bool = False,
     ) -> JsonDict:
-        user_id = await self.registration_handler.appservice_register(
+        user_id, appservice = await self.registration_handler.appservice_register(
             username, as_token
         )
+        if appservice.msc4190_device_management:
+            body["inhibit_login"] = True
+
         return await self._create_registration_details(
             user_id,
             body,
@@ -937,7 +940,7 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
 
         as_token = self.auth.get_access_token_from_request(request)
 
-        user_id = await self.registration_handler.appservice_register(
+        user_id, _ = await self.registration_handler.appservice_register(
             desired_username, as_token
         )
         return 200, {"user_id": user_id}

tests/handlers/test_appservice.py

@@ -1165,12 +1165,23 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
         self.hs.get_datastores().main.services_cache = [self._service]
 
         # Register some appservice users
-        self._sender_user, self._sender_device = self.register_appservice_user(
+        user_id, device_id = self.register_appservice_user(
             "as.sender", self._service_token
         )
-        self._namespaced_user, self._namespaced_device = self.register_appservice_user(
+        # With MSC4190 enabled, there will not be a device created
+        # during AS registration. However MSC4190 is not enabled
+        # in this test. It may become the default behaviour in the
+        # future, in which case this test will need to be updated.
+        assert device_id is not None
+        self._sender_user = user_id
+        self._sender_device = device_id
+
+        user_id, device_id = self.register_appservice_user(
             "_as_user1", self._service_token
         )
+        assert device_id is not None
+        self._namespaced_user = user_id
+        self._namespaced_device = device_id
 
         # Register a real user as well.
         self._real_user = self.register_user("real.user", "meow")

tests/handlers/test_oauth_delegation.py

@@ -560,9 +560,15 @@ def expect_unauthorized(
         self.assertEqual(channel.code, 401, channel.json_body)
 
     def expect_unrecognized(
-        self, method: str, path: str, content: Union[bytes, str, JsonDict] = ""
+        self,
+        method: str,
+        path: str,
+        content: Union[bytes, str, JsonDict] = "",
+        auth: bool = False,
     ) -> None:
-        channel = self.make_request(method, path, content)
+        channel = self.make_request(
+            method, path, content, access_token="token" if auth else None
+        )
 
         self.assertEqual(channel.code, 404, channel.json_body)
         self.assertEqual(
@@ -648,8 +654,25 @@ def test_session_management_endpoints_removed(self) -> None:
 
     def test_device_management_endpoints_removed(self) -> None:
         """Test that device management endpoints that were removed in MSC2964 are no longer available."""
-        self.expect_unrecognized("POST", "/_matrix/client/v3/delete_devices")
-        self.expect_unrecognized("DELETE", "/_matrix/client/v3/devices/{DEVICE}")
+
+        # Because we still support those endpoints with ASes, it checks the
+        # access token before returning 404
+        self.http_client.request = AsyncMock(
+            return_value=FakeResponse.json(
+                code=200,
+                payload={
+                    "active": True,
+                    "sub": SUBJECT,
+                    "scope": " ".join([MATRIX_USER_SCOPE, MATRIX_DEVICE_SCOPE]),
+                    "username": USERNAME,
+                },
+            )
+        )
+
+        self.expect_unrecognized("POST", "/_matrix/client/v3/delete_devices", auth=True)
+        self.expect_unrecognized(
+            "DELETE", "/_matrix/client/v3/devices/{DEVICE}", auth=True
+        )
 
     def test_openid_endpoints_removed(self) -> None:
         """Test that OpenID id_token endpoints that were removed in MSC2964 are no longer available."""