Fix wrapping of legacy check_registration_for_spam (#10238)

Fixes #10234
This commit is contained in:
Brendan Abolivier 2021-06-23 17:22:08 +02:00 committed by GitHub
parent 9ec45aca1f
commit c955e37868
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 84 additions and 6 deletions

View File

@ -0,0 +1 @@
The current spam checker interface is deprecated in favour of a new generic modules system. See the [upgrade notes](https://github.com/matrix-org/synapse/blob/master/UPGRADE.rst#deprecation-of-the-current-spam-checker-interface) for more information on how to update to the new system.

View File

@ -109,6 +109,8 @@ def load_legacy_spam_checkers(hs: "synapse.server.HomeServer"):
if f is None: if f is None:
return None return None
wrapped_func = f
if f.__name__ == "check_registration_for_spam": if f.__name__ == "check_registration_for_spam":
checker_args = inspect.signature(f) checker_args = inspect.signature(f)
if len(checker_args.parameters) == 3: if len(checker_args.parameters) == 3:
@ -133,19 +135,18 @@ def load_legacy_spam_checkers(hs: "synapse.server.HomeServer"):
request_info, request_info,
) )
f = wrapper wrapped_func = wrapper
elif len(checker_args.parameters) != 4: elif len(checker_args.parameters) != 4:
raise RuntimeError( raise RuntimeError(
"Bad signature for callback check_registration_for_spam", "Bad signature for callback check_registration_for_spam",
) )
def run(*args, **kwargs): def run(*args, **kwargs):
# We've already made sure f is not None above, but mypy doesn't do well # mypy doesn't do well across function boundaries so we need to tell it
# across function boundaries so we need to tell it f is definitely not # wrapped_func is definitely not None.
# None. assert wrapped_func is not None
assert f is not None
return maybe_awaitable(f(*args, **kwargs)) return maybe_awaitable(wrapped_func(*args, **kwargs))
return run return run

View File

@ -17,6 +17,7 @@ from unittest.mock import Mock
from synapse.api.auth import Auth from synapse.api.auth import Auth
from synapse.api.constants import UserTypes from synapse.api.constants import UserTypes
from synapse.api.errors import Codes, ResourceLimitError, SynapseError from synapse.api.errors import Codes, ResourceLimitError, SynapseError
from synapse.events.spamcheck import load_legacy_spam_checkers
from synapse.spam_checker_api import RegistrationBehaviour from synapse.spam_checker_api import RegistrationBehaviour
from synapse.types import RoomAlias, UserID, create_requester from synapse.types import RoomAlias, UserID, create_requester
@ -79,6 +80,39 @@ class BanBadIdPUser(TestSpamChecker):
return RegistrationBehaviour.ALLOW return RegistrationBehaviour.ALLOW
class TestLegacyRegistrationSpamChecker:
def __init__(self, config, api):
pass
async def check_registration_for_spam(
self,
email_threepid,
username,
request_info,
):
pass
class LegacyAllowAll(TestLegacyRegistrationSpamChecker):
async def check_registration_for_spam(
self,
email_threepid,
username,
request_info,
):
return RegistrationBehaviour.ALLOW
class LegacyDenyAll(TestLegacyRegistrationSpamChecker):
async def check_registration_for_spam(
self,
email_threepid,
username,
request_info,
):
return RegistrationBehaviour.DENY
class RegistrationTestCase(unittest.HomeserverTestCase): class RegistrationTestCase(unittest.HomeserverTestCase):
"""Tests the RegistrationHandler.""" """Tests the RegistrationHandler."""
@ -95,6 +129,8 @@ class RegistrationTestCase(unittest.HomeserverTestCase):
hs = self.setup_test_homeserver(config=hs_config) hs = self.setup_test_homeserver(config=hs_config)
load_legacy_spam_checkers(hs)
module_api = hs.get_module_api() module_api = hs.get_module_api()
for module, config in hs.config.modules.loaded_modules: for module, config in hs.config.modules.loaded_modules:
module(config=config, api=module_api) module(config=config, api=module_api)
@ -535,6 +571,46 @@ class RegistrationTestCase(unittest.HomeserverTestCase):
"""A spam checker can deny registration, which results in an error.""" """A spam checker can deny registration, which results in an error."""
self.get_failure(self.handler.register_user(localpart="user"), SynapseError) self.get_failure(self.handler.register_user(localpart="user"), SynapseError)
@override_config(
{
"spam_checker": [
{
"module": TestSpamChecker.__module__ + ".LegacyAllowAll",
}
]
}
)
def test_spam_checker_legacy_allow(self):
"""Tests that a legacy spam checker implementing the legacy 3-arg version of the
check_registration_for_spam callback is correctly called.
In this test and the following one we test both success and failure to make sure
any failure comes from the spam checker (and not something else failing in the
call stack) and any success comes from the spam checker (and not because a
misconfiguration prevented it from being loaded).
"""
self.get_success(self.handler.register_user(localpart="user"))
@override_config(
{
"spam_checker": [
{
"module": TestSpamChecker.__module__ + ".LegacyDenyAll",
}
]
}
)
def test_spam_checker_legacy_deny(self):
"""Tests that a legacy spam checker implementing the legacy 3-arg version of the
check_registration_for_spam callback is correctly called.
In this test and the previous one we test both success and failure to make sure
any failure comes from the spam checker (and not something else failing in the
call stack) and any success comes from the spam checker (and not because a
misconfiguration prevented it from being loaded).
"""
self.get_failure(self.handler.register_user(localpart="user"), SynapseError)
@override_config( @override_config(
{ {
"modules": [ "modules": [