Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

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

Merged
merged 26 commits into from
Jul 11, 2022
Merged
Changes from 1 commit
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
WIP: Applied feedback
Yoric committed Jun 16, 2022
commit 05773190583ecaebe365568a3cd8e82dfaea5a72
2 changes: 1 addition & 1 deletion changelog.d/13044.misc
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Experimental: expand Spam-Checker API callbacks of `user_may_join_room`, `user_may_invite`, `user_may_send_3pid_invite`, `user_may_create_room`, `user_may_create_room_alias`, `user_may_publish_room`, `check_media_file_for_spam` with ability to return additional fields. This enables spam-checker implementations to experiment with mechanisms to give users more information about why they are blocked and whether any action is needed from them to be unblocked.
Support temporary experimental return values for spam checker module callbacks.
23 changes: 11 additions & 12 deletions synapse/events/spamcheck.py
Original file line number Diff line number Diff line change
@@ -21,7 +21,6 @@
Awaitable,
Callable,
Collection,
Dict,
List,
Optional,
Tuple,
@@ -35,7 +34,7 @@
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

@@ -55,7 +54,7 @@
# 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["synapse.api.errors.Codes", JsonDict],
# Deprecated
bool,
]
@@ -75,7 +74,7 @@
# 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["synapse.api.errors.Codes", JsonDict],
# Deprecated
bool,
]
@@ -91,7 +90,7 @@
# 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["synapse.api.errors.Codes", JsonDict],
# Deprecated
bool,
]
@@ -107,7 +106,7 @@
# 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["synapse.api.errors.Codes", JsonDict],
# Deprecated
bool,
]
@@ -123,7 +122,7 @@
# 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["synapse.api.errors.Codes", JsonDict],
# Deprecated
bool,
]
@@ -139,7 +138,7 @@
# 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["synapse.api.errors.Codes", JsonDict],
# Deprecated
bool,
]
@@ -155,7 +154,7 @@
# 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["synapse.api.errors.Codes", JsonDict],
# Deprecated
bool,
]
@@ -189,7 +188,7 @@
# 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["synapse.api.errors.Codes", JsonDict],
# Deprecated
bool,
]
@@ -380,7 +379,7 @@ def register_callbacks(

async def check_event_for_spam(
self, event: "synapse.events.EventBase"
) -> Union[Tuple["synapse.api.errors.Codes", Dict], str]:
) -> Union[Tuple["synapse.api.errors.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
@@ -464,7 +463,7 @@ async def should_drop_federated_event(

async def user_may_join_room(
self, user_id: str, room_id: str, is_invited: bool
) -> Union[Tuple["synapse.api.errors.Codes", Dict], Literal["NOT_SPAM"]]:
) -> Union[Tuple["synapse.api.errors.Codes", JsonDict], Literal["NOT_SPAM"]]:
"""Checks if a given users is allowed to join a room.
Not called when a user creates a room.
8 changes: 5 additions & 3 deletions tests/rest/client/test_rooms.py
Original file line number Diff line number Diff line change
@@ -839,8 +839,7 @@ async def user_may_join_room_tuple(
) -> Tuple[Codes, dict]:
return (Codes.CONSENT_NOT_GIVEN, {})

join_mock = Mock(side_effect=user_may_join_room_codes)
self.hs.get_spam_checker()._user_may_join_room_callbacks = [join_mock]
join_mock.side_effect = user_may_join_room_tuple

channel = self.make_request(
"POST",
@@ -3182,12 +3181,13 @@ def test_threepid_invite_spamcheck(self) -> None:
access_token=self.tok,
)
self.assertEqual(channel.code, 403)
self.assertEqual(channel.json_body.errcode, Codes.CONSENT_NOT_GIVEN)

# Also check that it stopped before calling _make_and_store_3pid_invite.
make_invite_mock.assert_called_once()

# Run variant with `Tuple[Codes, dict]`.
mock.return_value = make_awaitable((Codes.CONSENT_NOT_GIVEN, {}))
mock.return_value = make_awaitable((Codes.EXPIRED_ACCOUNT, {"field": "value"}))
channel = self.make_request(
method="POST",
path="/rooms/" + self.room_id + "/invite",
@@ -3200,6 +3200,8 @@ def test_threepid_invite_spamcheck(self) -> None:
access_token=self.tok,
)
self.assertEqual(channel.code, 403)
self.assertEqual(channel.json_body.errcode, Codes.EXPIRED_ACCOUNT)
self.assertEqual(channel.json_body.field, "value")
Yoric marked this conversation as resolved.
Show resolved Hide resolved

# Also check that it stopped before calling _make_and_store_3pid_invite.
make_invite_mock.assert_called_once()
12 changes: 8 additions & 4 deletions tests/rest/media/v1/test_media_storage.py
Original file line number Diff line number Diff line change
@@ -649,6 +649,10 @@ def test_upload_ban(self) -> None:
)


EVIL_DATA = b"Some evil data"
EVIL_DATA_EXPERIMENT = b"Some evil data to trigger the experimental tuple API"


class SpamCheckerTestCase(unittest.HomeserverTestCase):
servlets = [
login.register_servlets,
@@ -674,9 +678,9 @@ async def check_media_file_for_spam(
buf = BytesIO()
await file_wrapper.write_chunks_to(buf.write)

if b"evil" in buf.getvalue():
if buf.getvalue() == EVIL_DATA:
return Codes.FORBIDDEN
elif b"experimental" in buf.getvalue():
elif buf.getvalue() == EVIL_DATA_EXPERIMENT:
return (Codes.FORBIDDEN, {})
else:
return "NOT_SPAM"
@@ -693,12 +697,12 @@ def test_upload_ban(self) -> None:
"""

self.helper.upload_media(
self.upload_resource, b"Some evil data", tok=self.tok, expect_code=400
self.upload_resource, EVIL_DATA, tok=self.tok, expect_code=400
)

self.helper.upload_media(
self.upload_resource,
b"Let's try the experimental API",
EVIL_DATA_EXPERIMENT,
tok=self.tok,
expect_code=400,
)