Uniformize spam-checker API, part 5: expand other spam-checker callbacks to return `Tuple[Codes, dict]` (#13044)

Signed-off-by: David Teller <davidt@element.io>
Co-authored-by: Brendan Abolivier <babolivier@matrix.org>

1 files changed, 128 insertions, 35 deletions

diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py
index 32712d2042..4a3bfb38f1 100644
--- a/synapse/events/spamcheck.py
+++ b/synapse/events/spamcheck.py
@@ -21,7 +21,6 @@ from typing import (
     Awaitable,
     Callable,
     Collection,
-    Dict,
     List,
     Optional,
     Tuple,
@@ -32,10 +31,11 @@ from typing import (
 from typing_extensions import Literal
 
 import synapse
+from synapse.api.errors import Codes
 from synapse.rest.media.v1._base import FileInfo
 from synapse.rest.media.v1.media_storage import ReadableFileWrapper
 from synapse.spam_checker_api import RegistrationBehaviour
-from synapse.types import RoomAlias, UserProfile
+from synapse.types import JsonDict, RoomAlias, UserProfile
 from synapse.util.async_helpers import delay_cancellation, maybe_awaitable
 from synapse.util.metrics import Measure
 
@@ -50,12 +50,12 @@ CHECK_EVENT_FOR_SPAM_CALLBACK = Callable[
     Awaitable[
         Union[
             str,
-            "synapse.api.errors.Codes",
+            Codes,
             # Highly experimental, not officially part of the spamchecker API, may
             # disappear without warning depending on the results of ongoing
             # experiments.
             # Use this to return additional information as part of an error.
-            Tuple["synapse.api.errors.Codes", Dict],
+            Tuple[Codes, JsonDict],
             # Deprecated
             bool,
         ]
@@ -70,7 +70,12 @@ USER_MAY_JOIN_ROOM_CALLBACK = Callable[
     Awaitable[
         Union[
             Literal["NOT_SPAM"],
-            "synapse.api.errors.Codes",
+            Codes,
+            # Highly experimental, not officially part of the spamchecker API, may
+            # disappear without warning depending on the results of ongoing
+            # experiments.
+            # Use this to return additional information as part of an error.
+            Tuple[Codes, JsonDict],
             # Deprecated
             bool,
         ]
@@ -81,7 +86,12 @@ USER_MAY_INVITE_CALLBACK = Callable[
     Awaitable[
         Union[
             Literal["NOT_SPAM"],
-            "synapse.api.errors.Codes",
+            Codes,
+            # Highly experimental, not officially part of the spamchecker API, may
+            # disappear without warning depending on the results of ongoing
+            # experiments.
+            # Use this to return additional information as part of an error.
+            Tuple[Codes, JsonDict],
             # Deprecated
             bool,
         ]
@@ -92,7 +102,12 @@ USER_MAY_SEND_3PID_INVITE_CALLBACK = Callable[
     Awaitable[
         Union[
             Literal["NOT_SPAM"],
-            "synapse.api.errors.Codes",
+            Codes,
+            # Highly experimental, not officially part of the spamchecker API, may
+            # disappear without warning depending on the results of ongoing
+            # experiments.
+            # Use this to return additional information as part of an error.
+            Tuple[Codes, JsonDict],
             # Deprecated
             bool,
         ]
@@ -103,7 +118,12 @@ USER_MAY_CREATE_ROOM_CALLBACK = Callable[
     Awaitable[
         Union[
             Literal["NOT_SPAM"],
-            "synapse.api.errors.Codes",
+            Codes,
+            # Highly experimental, not officially part of the spamchecker API, may
+            # disappear without warning depending on the results of ongoing
+            # experiments.
+            # Use this to return additional information as part of an error.
+            Tuple[Codes, JsonDict],
             # Deprecated
             bool,
         ]
@@ -114,7 +134,12 @@ USER_MAY_CREATE_ROOM_ALIAS_CALLBACK = Callable[
     Awaitable[
         Union[
             Literal["NOT_SPAM"],
-            "synapse.api.errors.Codes",
+            Codes,
+            # Highly experimental, not officially part of the spamchecker API, may
+            # disappear without warning depending on the results of ongoing
+            # experiments.
+            # Use this to return additional information as part of an error.
+            Tuple[Codes, JsonDict],
             # Deprecated
             bool,
         ]
@@ -125,7 +150,12 @@ USER_MAY_PUBLISH_ROOM_CALLBACK = Callable[
     Awaitable[
         Union[
             Literal["NOT_SPAM"],
-            "synapse.api.errors.Codes",
+            Codes,
+            # Highly experimental, not officially part of the spamchecker API, may
+            # disappear without warning depending on the results of ongoing
+            # experiments.
+            # Use this to return additional information as part of an error.
+            Tuple[Codes, JsonDict],
             # Deprecated
             bool,
         ]
@@ -154,7 +184,12 @@ CHECK_MEDIA_FILE_FOR_SPAM_CALLBACK = Callable[
     Awaitable[
         Union[
             Literal["NOT_SPAM"],
-            "synapse.api.errors.Codes",
+            Codes,
+            # Highly experimental, not officially part of the spamchecker API, may
+            # disappear without warning depending on the results of ongoing
+            # experiments.
+            # Use this to return additional information as part of an error.
+            Tuple[Codes, JsonDict],
             # Deprecated
             bool,
         ]
@@ -345,7 +380,7 @@ class SpamChecker:
 
     async def check_event_for_spam(
         self, event: "synapse.events.EventBase"
-    ) -> Union[Tuple["synapse.api.errors.Codes", Dict], str]:
+    ) -> Union[Tuple[Codes, JsonDict], str]:
         """Checks if a given event is considered "spammy" by this server.
 
         If the server considers an event spammy, then it will be rejected if
@@ -376,7 +411,16 @@ class SpamChecker:
                 elif res is True:
                     # This spam-checker rejects the event with deprecated
                     # return value `True`
-                    return (synapse.api.errors.Codes.FORBIDDEN, {})
+                    return synapse.api.errors.Codes.FORBIDDEN, {}
+                elif (
+                    isinstance(res, tuple)
+                    and len(res) == 2
+                    and isinstance(res[0], synapse.api.errors.Codes)
+                    and isinstance(res[1], dict)
+                ):
+                    return res
+                elif isinstance(res, synapse.api.errors.Codes):
+                    return res, {}
                 elif not isinstance(res, str):
                     # mypy complains that we can't reach this code because of the
                     # return type in CHECK_EVENT_FOR_SPAM_CALLBACK, but we don't know
@@ -422,7 +466,7 @@ class SpamChecker:
 
     async def user_may_join_room(
         self, user_id: str, room_id: str, is_invited: bool
-    ) -> Union["synapse.api.errors.Codes", Literal["NOT_SPAM"]]:
+    ) -> Union[Tuple[Codes, JsonDict], Literal["NOT_SPAM"]]:
         """Checks if a given users is allowed to join a room.
         Not called when a user creates a room.
 
@@ -432,7 +476,7 @@ class SpamChecker:
             is_invited: Whether the user is invited into the room
 
         Returns:
-            NOT_SPAM if the operation is permitted, Codes otherwise.
+            NOT_SPAM if the operation is permitted, [Codes, Dict] otherwise.
         """
         for callback in self._user_may_join_room_callbacks:
             with Measure(
@@ -443,21 +487,28 @@ class SpamChecker:
                 if res is True or res is self.NOT_SPAM:
                     continue
                 elif res is False:
-                    return synapse.api.errors.Codes.FORBIDDEN
+                    return synapse.api.errors.Codes.FORBIDDEN, {}
                 elif isinstance(res, synapse.api.errors.Codes):
+                    return res, {}
+                elif (
+                    isinstance(res, tuple)
+                    and len(res) == 2
+                    and isinstance(res[0], synapse.api.errors.Codes)
+                    and isinstance(res[1], dict)
+                ):
                     return res
                 else:
                     logger.warning(
                         "Module returned invalid value, rejecting join as spam"
                     )
-                    return synapse.api.errors.Codes.FORBIDDEN
+                    return synapse.api.errors.Codes.FORBIDDEN, {}
 
         # No spam-checker has rejected the request, let it pass.
         return self.NOT_SPAM
 
     async def user_may_invite(
         self, inviter_userid: str, invitee_userid: str, room_id: str
-    ) -> Union["synapse.api.errors.Codes", Literal["NOT_SPAM"]]:
+    ) -> Union[Tuple[Codes, dict], Literal["NOT_SPAM"]]:
         """Checks if a given user may send an invite
 
         Args:
@@ -479,21 +530,28 @@ class SpamChecker:
                 if res is True or res is self.NOT_SPAM:
                     continue
                 elif res is False:
-                    return synapse.api.errors.Codes.FORBIDDEN
+                    return synapse.api.errors.Codes.FORBIDDEN, {}
                 elif isinstance(res, synapse.api.errors.Codes):
+                    return res, {}
+                elif (
+                    isinstance(res, tuple)
+                    and len(res) == 2
+                    and isinstance(res[0], synapse.api.errors.Codes)
+                    and isinstance(res[1], dict)
+                ):
                     return res
                 else:
                     logger.warning(
                         "Module returned invalid value, rejecting invite as spam"
                     )
-                    return synapse.api.errors.Codes.FORBIDDEN
+                    return synapse.api.errors.Codes.FORBIDDEN, {}
 
         # No spam-checker has rejected the request, let it pass.
         return self.NOT_SPAM
 
     async def user_may_send_3pid_invite(
         self, inviter_userid: str, medium: str, address: str, room_id: str
-    ) -> Union["synapse.api.errors.Codes", Literal["NOT_SPAM"]]:
+    ) -> Union[Tuple[Codes, dict], Literal["NOT_SPAM"]]:
         """Checks if a given user may invite a given threepid into the room
 
         Note that if the threepid is already associated with a Matrix user ID, Synapse
@@ -519,20 +577,27 @@ class SpamChecker:
                 if res is True or res is self.NOT_SPAM:
                     continue
                 elif res is False:
-                    return synapse.api.errors.Codes.FORBIDDEN
+                    return synapse.api.errors.Codes.FORBIDDEN, {}
                 elif isinstance(res, synapse.api.errors.Codes):
+                    return res, {}
+                elif (
+                    isinstance(res, tuple)
+                    and len(res) == 2
+                    and isinstance(res[0], synapse.api.errors.Codes)
+                    and isinstance(res[1], dict)
+                ):
                     return res
                 else:
                     logger.warning(
                         "Module returned invalid value, rejecting 3pid invite as spam"
                     )
-                    return synapse.api.errors.Codes.FORBIDDEN
+                    return synapse.api.errors.Codes.FORBIDDEN, {}
 
         return self.NOT_SPAM
 
     async def user_may_create_room(
         self, userid: str
-    ) -> Union["synapse.api.errors.Codes", Literal["NOT_SPAM"]]:
+    ) -> Union[Tuple[Codes, dict], Literal["NOT_SPAM"]]:
         """Checks if a given user may create a room
 
         Args:
@@ -546,20 +611,27 @@ class SpamChecker:
                 if res is True or res is self.NOT_SPAM:
                     continue
                 elif res is False:
-                    return synapse.api.errors.Codes.FORBIDDEN
+                    return synapse.api.errors.Codes.FORBIDDEN, {}
                 elif isinstance(res, synapse.api.errors.Codes):
+                    return res, {}
+                elif (
+                    isinstance(res, tuple)
+                    and len(res) == 2
+                    and isinstance(res[0], synapse.api.errors.Codes)
+                    and isinstance(res[1], dict)
+                ):
                     return res
                 else:
                     logger.warning(
                         "Module returned invalid value, rejecting room creation as spam"
                     )
-                    return synapse.api.errors.Codes.FORBIDDEN
+                    return synapse.api.errors.Codes.FORBIDDEN, {}
 
         return self.NOT_SPAM
 
     async def user_may_create_room_alias(
         self, userid: str, room_alias: RoomAlias
-    ) -> Union["synapse.api.errors.Codes", Literal["NOT_SPAM"]]:
+    ) -> Union[Tuple[Codes, dict], Literal["NOT_SPAM"]]:
         """Checks if a given user may create a room alias
 
         Args:
@@ -575,20 +647,27 @@ class SpamChecker:
                 if res is True or res is self.NOT_SPAM:
                     continue
                 elif res is False:
-                    return synapse.api.errors.Codes.FORBIDDEN
+                    return synapse.api.errors.Codes.FORBIDDEN, {}
                 elif isinstance(res, synapse.api.errors.Codes):
+                    return res, {}
+                elif (
+                    isinstance(res, tuple)
+                    and len(res) == 2
+                    and isinstance(res[0], synapse.api.errors.Codes)
+                    and isinstance(res[1], dict)
+                ):
                     return res
                 else:
                     logger.warning(
                         "Module returned invalid value, rejecting room create as spam"
                     )
-                    return synapse.api.errors.Codes.FORBIDDEN
+                    return synapse.api.errors.Codes.FORBIDDEN, {}
 
         return self.NOT_SPAM
 
     async def user_may_publish_room(
         self, userid: str, room_id: str
-    ) -> Union["synapse.api.errors.Codes", Literal["NOT_SPAM"]]:
+    ) -> Union[Tuple[Codes, dict], Literal["NOT_SPAM"]]:
         """Checks if a given user may publish a room to the directory
 
         Args:
@@ -603,14 +682,21 @@ class SpamChecker:
                 if res is True or res is self.NOT_SPAM:
                     continue
                 elif res is False:
-                    return synapse.api.errors.Codes.FORBIDDEN
+                    return synapse.api.errors.Codes.FORBIDDEN, {}
                 elif isinstance(res, synapse.api.errors.Codes):
+                    return res, {}
+                elif (
+                    isinstance(res, tuple)
+                    and len(res) == 2
+                    and isinstance(res[0], synapse.api.errors.Codes)
+                    and isinstance(res[1], dict)
+                ):
                     return res
                 else:
                     logger.warning(
                         "Module returned invalid value, rejecting room publication as spam"
                     )
-                    return synapse.api.errors.Codes.FORBIDDEN
+                    return synapse.api.errors.Codes.FORBIDDEN, {}
 
         return self.NOT_SPAM
 
@@ -678,7 +764,7 @@ class SpamChecker:
 
     async def check_media_file_for_spam(
         self, file_wrapper: ReadableFileWrapper, file_info: FileInfo
-    ) -> Union["synapse.api.errors.Codes", Literal["NOT_SPAM"]]:
+    ) -> Union[Tuple[Codes, dict], Literal["NOT_SPAM"]]:
         """Checks if a piece of newly uploaded media should be blocked.
 
         This will be called for local uploads, downloads of remote media, each
@@ -715,13 +801,20 @@ class SpamChecker:
                 if res is False or res is self.NOT_SPAM:
                     continue
                 elif res is True:
-                    return synapse.api.errors.Codes.FORBIDDEN
+                    return synapse.api.errors.Codes.FORBIDDEN, {}
                 elif isinstance(res, synapse.api.errors.Codes):
+                    return res, {}
+                elif (
+                    isinstance(res, tuple)
+                    and len(res) == 2
+                    and isinstance(res[0], synapse.api.errors.Codes)
+                    and isinstance(res[1], dict)
+                ):
                     return res
                 else:
                     logger.warning(
                         "Module returned invalid value, rejecting media file as spam"
                     )
-                    return synapse.api.errors.Codes.FORBIDDEN
+                    return synapse.api.errors.Codes.FORBIDDEN, {}
 
         return self.NOT_SPAM