Add 3pid unbind callback to module API

docs/modules/third_party_rules_callbacks.md

@@ -265,6 +265,23 @@ server_.
 
 If multiple modules implement this callback, Synapse runs them all in order.
 
+### `on_threepid_unbind`
+
+_First introduced in Synapse v1.63.0_
+
+```python
+async def on_threepid_unbind(
+    user_id: str, medium: str, address: str, identity_server: str
+) -> (bool, bool):
+```
+
+Called before a threepid association is removed.
+
+Note that this callback is called before an association is deleted on the
+local homeserver. If CONTINUE_UNBIND is returned unbind will be called
+on the identity server that was used during registration, if STOP_UNBIND
+is returned it will not do so and stop calling other handlers too.
+
 ## Example
 
 The example below is a module that implements the third-party rules callback

synapse/events/third_party_rules.py

@@ -12,7 +12,10 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 import logging
-from typing import TYPE_CHECKING, Any, Awaitable, Callable, List, Optional, Tuple
+from typing import TYPE_CHECKING, Any, Awaitable, Callable, List, Optional, Tuple, Union
+
+# `Literal` appears with Python 3.8.
+from typing_extensions import Literal
 
 from twisted.internet.defer import CancelledError
 
@@ -45,6 +48,10 @@
 ON_PROFILE_UPDATE_CALLBACK = Callable[[str, ProfileInfo, bool, bool], Awaitable]
 ON_USER_DEACTIVATION_STATUS_CHANGED_CALLBACK = Callable[[str, bool, bool], Awaitable]
 ON_THREEPID_BIND_CALLBACK = Callable[[str, str, str], Awaitable]
+ON_THREEPID_UNBIND_CALLBACK = Callable[
+    [str, str, str, str],
+    Awaitable[Union[Literal["CONTINUE_UNBIND"], Literal["STOP_UNBIND"]]],
+]
 
 
 def load_legacy_third_party_event_rules(hs: "HomeServer") -> None:
@@ -174,6 +181,7 @@ def __init__(self, hs: "HomeServer"):
             ON_USER_DEACTIVATION_STATUS_CHANGED_CALLBACK
         ] = []
         self._on_threepid_bind_callbacks: List[ON_THREEPID_BIND_CALLBACK] = []
+        self._on_threepid_unbind_callbacks: List[ON_THREEPID_UNBIND_CALLBACK] = []
 
     def register_third_party_rules_callbacks(
         self,
@@ -193,6 +201,7 @@ def register_third_party_rules_callbacks(
             ON_USER_DEACTIVATION_STATUS_CHANGED_CALLBACK
         ] = None,
         on_threepid_bind: Optional[ON_THREEPID_BIND_CALLBACK] = None,
+        on_threepid_unbind: Optional[ON_THREEPID_UNBIND_CALLBACK] = None,
     ) -> None:
         """Register callbacks from modules for each hook."""
         if check_event_allowed is not None:
@@ -230,6 +239,9 @@ def register_third_party_rules_callbacks(
         if on_threepid_bind is not None:
             self._on_threepid_bind_callbacks.append(on_threepid_bind)
 
+        if on_threepid_unbind is not None:
+            self._on_threepid_unbind_callbacks.append(on_threepid_unbind)
+
     async def check_event_allowed(
         self, event: EventBase, context: EventContext
     ) -> Tuple[bool, Optional[dict]]:
@@ -523,3 +535,36 @@ async def on_threepid_bind(self, user_id: str, medium: str, address: str) -> Non
                 logger.exception(
                     "Failed to run module API callback %s: %s", callback, e
                 )
+
+    async def on_threepid_unbind(
+        self, user_id: str, medium: str, address: str, identity_server: str
+    ) -> (bool, bool):
+        """Called before a threepid association is removed.
+
+        Note that this callback is called before an association is deleted on the
+        local homeserver. If CONTINUE_UNBIND is returned unbind will be called
+        on the identity server that was used during registration.
+
+        Args:
+            user_id: the user being associated with the threepid.
+            medium: the threepid's medium.
+            address: the threepid's address.
+            identity_server: the identity server where the threepid was successfully registered.
+        """
+
+        global_changed = True
+        for callback in self._on_threepid_unbind_callbacks:
+            try:
+                (changed, stop) = await callback(
+                    user_id, medium, address, identity_server
+                )
+                global_changed &= changed
+                if stop:
+                    return (global_changed, True)
+            except Exception as e:
+                logger.exception(
+                    "Failed to run module API callback %s: %s", callback, e
+                )
+                raise e
+
+        return (global_changed, False)

synapse/handlers/auth.py

@@ -1535,6 +1535,7 @@ async def delete_threepid(
             address = canonicalise_email(address)
 
         identity_handler = self.hs.get_identity_handler()
+
         result = await identity_handler.try_unbind_threepid(
             user_id, {"medium": medium, "address": address, "id_server": id_server}
         )

synapse/handlers/identity.py

@@ -288,55 +288,73 @@ async def try_unbind_threepid_with_id_server(
             SynapseError: On any of the following conditions
                 - the supplied id_server is not a valid identity server name
                 - we failed to contact the supplied identity server
-
+            return "STOP_UNBIND"
         Returns:
             True on success, otherwise False if the identity
             server doesn't support unbinding
         """
 
-        if not valid_id_server_location(id_server):
-            raise SynapseError(
-                400,
-                "id_server must be a valid hostname with optional port and path components",
-            )
+        medium = threepid["medium"]
+        address = threepid["address"]
 
-        url = "https://%s/_matrix/identity/api/v1/3pid/unbind" % (id_server,)
-        url_bytes = b"/_matrix/identity/api/v1/3pid/unbind"
+        (
+            changed,
+            stop,
+        ) = await self.hs.get_third_party_event_rules().on_threepid_unbind(
+            mxid, medium, address, id_server
+        )
 
-        content = {
-            "mxid": mxid,
-            "threepid": {"medium": threepid["medium"], "address": threepid["address"]},
-        }
+        # If a module wants to take over unbind it will return stop = True,
+        # in this case we should just purge the table from the 3pid record
+        if not stop:
+            if not valid_id_server_location(id_server):
+                raise SynapseError(
+                    400,
+                    "id_server must be a valid hostname with optional port and path components",
+                )
 
-        # we abuse the federation http client to sign the request, but we have to send it
-        # using the normal http client since we don't want the SRV lookup and want normal
-        # 'browser-like' HTTPS.
-        auth_headers = self.federation_http_client.build_auth_headers(
-            destination=None,
-            method=b"POST",
-            url_bytes=url_bytes,
-            content=content,
-            destination_is=id_server.encode("ascii"),
-        )
-        headers = {b"Authorization": auth_headers}
+            url = "https://%s/_matrix/identity/api/v1/3pid/unbind" % (id_server,)
+            url_bytes = b"/_matrix/identity/api/v1/3pid/unbind"
 
-        try:
-            # Use the blacklisting http client as this call is only to identity servers
-            # provided by a client
-            await self.blacklisting_http_client.post_json_get_json(
-                url, content, headers
+            content = {
+                "mxid": mxid,
+                "threepid": {
+                    "medium": threepid["medium"],
+                    "address": threepid["address"],
+                },
+            }
+
+            # we abuse the federation http client to sign the request, but we have to send it
+            # using the normal http client since we don't want the SRV lookup and want normal
+            # 'browser-like' HTTPS.
+            auth_headers = self.federation_http_client.build_auth_headers(
+                destination=None,
+                method=b"POST",
+                url_bytes=url_bytes,
+                content=content,
+                destination_is=id_server.encode("ascii"),
             )
-            changed = True
-        except HttpResponseException as e:
-            changed = False
-            if e.code in (400, 404, 501):
-                # The remote server probably doesn't support unbinding (yet)
-                logger.warning("Received %d response while unbinding threepid", e.code)
-            else:
-                logger.error("Failed to unbind threepid on identity server: %s", e)
-                raise SynapseError(500, "Failed to contact identity server")
-        except RequestTimedOutError:
-            raise SynapseError(500, "Timed out contacting identity server")
+            headers = {b"Authorization": auth_headers}
+
+            try:
+                # Use the blacklisting http client as this call is only to identity servers
+                # provided by a client
+                await self.blacklisting_http_client.post_json_get_json(
+                    url, content, headers
+                )
+                changed &= True
+            except HttpResponseException as e:
+                changed &= False
+                if e.code in (400, 404, 501):
+                    # The remote server probably doesn't support unbinding (yet)
+                    logger.warning(
+                        "Received %d response while unbinding threepid", e.code
+                    )
+                else:
+                    logger.error("Failed to unbind threepid on identity server: %s", e)
+                    raise SynapseError(500, "Failed to contact identity server")
+            except RequestTimedOutError:
+                raise SynapseError(500, "Timed out contacting identity server")
 
         await self.store.remove_user_bound_threepid(
             user_id=mxid,

synapse/module_api/__init__.py

@@ -67,6 +67,7 @@
     ON_NEW_EVENT_CALLBACK,
     ON_PROFILE_UPDATE_CALLBACK,
     ON_THREEPID_BIND_CALLBACK,
+    ON_THREEPID_UNBIND_CALLBACK,
     ON_USER_DEACTIVATION_STATUS_CHANGED_CALLBACK,
 )
 from synapse.handlers.account_data import ON_ACCOUNT_DATA_UPDATED_CALLBACK
@@ -317,6 +318,7 @@ def register_third_party_rules_callbacks(
             ON_USER_DEACTIVATION_STATUS_CHANGED_CALLBACK
         ] = None,
         on_threepid_bind: Optional[ON_THREEPID_BIND_CALLBACK] = None,
+        on_threepid_unbind: Optional[ON_THREEPID_UNBIND_CALLBACK] = None,
     ) -> None:
         """Registers callbacks for third party event rules capabilities.
 
@@ -333,6 +335,7 @@ def register_third_party_rules_callbacks(
             on_profile_update=on_profile_update,
             on_user_deactivation_status_changed=on_user_deactivation_status_changed,
             on_threepid_bind=on_threepid_bind,
+            on_threepid_unbind=on_threepid_unbind,
         )
 
     def register_presence_router_callbacks(

tests/rest/client/test_third_party_rules.py

@@ -937,3 +937,58 @@ def test_on_threepid_bind(self) -> None:
 
         # Check that the mock was called with the right parameters
         self.assertEqual(args, (user_id, "email", "[email protected]"))
+
+    def test_on_threepid_unbind(self) -> None:
+        """Tests that the on_threepid_unbind module callback is called correctly before
+        removing a 3PID mapping.
+        """
+        # Register a mocked callback.
+        threepid_unbind_mock = Mock(return_value=make_awaitable(None))
+        third_party_rules = self.hs.get_third_party_event_rules()
+        third_party_rules._on_threepid_unbind_callbacks.append(threepid_unbind_mock)
+
+        # Register an admin user.
+        self.register_user("admin", "password", admin=True)
+        admin_tok = self.login("admin", "password")
+
+        # Also register a normal user we can modify.
+        user_id = self.register_user("user", "password")
+
+        # Add a 3PID to the user.
+        channel = self.make_request(
+            "PUT",
+            "/_synapse/admin/v2/users/%s" % user_id,
+            {
+                "threepids": [
+                    {
+                        "medium": "email",
+                        "address": "[email protected]",
+                    },
+                ],
+            },
+            access_token=admin_tok,
+        )
+        self.assertEqual(channel.code, 200, channel.json_body)
+
+        # Remove the 3PID mapping.
+        channel = self.make_request(
+            "DELETE",
+            "/_synapse/admin/v2/users/%s" % user_id,
+            {
+                "threepids": [
+                    {
+                        "medium": "email",
+                        "address": "[email protected]",
+                    },
+                ],
+            },
+            access_token=admin_tok,
+        )
+        self.assertEqual(channel.code, 200, channel.json_body)
+
+        # Check that the mock was called once.
+        threepid_unbind_mock.assert_called_once()
+        args = threepid_unbind_mock.call_args[0]
+
+        # Check that the mock was called with the right parameters
+        self.assertEqual(args, (user_id, "email", "[email protected]"))