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

Pass SSO IdP information to spam checker's registration function #9626

Merged
merged 6 commits into from
Mar 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions changelog.d/9626.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Tell spam checker modules about the SSO IdP a user registered through if one was used.
8 changes: 7 additions & 1 deletion docs/spam_checker.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,13 @@ class ExampleSpamChecker:
async def check_username_for_spam(self, user_profile):
return False # allow all usernames

async def check_registration_for_spam(self, email_threepid, username, request_info):
async def check_registration_for_spam(
self,
email_threepid,
username,
request_info,
auth_provider_id,
):
return RegistrationBehaviour.ALLOW # allow all registrations

async def check_media_file_for_spam(self, file_wrapper, file_info):
Expand Down
29 changes: 26 additions & 3 deletions synapse/events/spamcheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
# limitations under the License.

import inspect
import logging
from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, Union

from synapse.rest.media.v1._base import FileInfo
Expand All @@ -27,6 +28,8 @@
import synapse.events
import synapse.server

logger = logging.getLogger(__name__)


class SpamChecker:
def __init__(self, hs: "synapse.server.HomeServer"):
Expand Down Expand Up @@ -190,6 +193,7 @@ async def check_registration_for_spam(
email_threepid: Optional[dict],
username: Optional[str],
request_info: Collection[Tuple[str, str]],
auth_provider_id: Optional[str] = None,
) -> RegistrationBehaviour:
"""Checks if we should allow the given registration request.

Expand All @@ -198,6 +202,9 @@ async def check_registration_for_spam(
username: The request user name, if any
request_info: List of tuples of user agent and IP that
were used during the registration process.
auth_provider_id: The SSO IdP the user used, e.g "oidc", "saml",
"cas". If any. Note this does not include users registered
via a password provider.

Returns:
Enum for how the request should be handled
Expand All @@ -208,9 +215,25 @@ async def check_registration_for_spam(
# spam checker
checker = getattr(spam_checker, "check_registration_for_spam", None)
if checker:
behaviour = await maybe_awaitable(
checker(email_threepid, username, request_info)
)
# Provide auth_provider_id if the function supports it
checker_args = inspect.signature(checker)
if len(checker_args.parameters) == 4:
d = checker(
email_threepid,
username,
request_info,
auth_provider_id,
)
elif len(checker_args.parameters) == 3:
d = checker(email_threepid, username, request_info)
else:
logger.error(
"Invalid signature for %s.check_registration_for_spam. Denying registration",
spam_checker.__module__,
)
return RegistrationBehaviour.DENY

behaviour = await maybe_awaitable(d)
assert isinstance(behaviour, RegistrationBehaviour)
if behaviour != RegistrationBehaviour.ALLOW:
return behaviour
Expand Down
4 changes: 2 additions & 2 deletions synapse/handlers/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,7 @@ async def register_user(
admin api, otherwise False.
user_agent_ips: Tuples of IP addresses and user-agents used
during the registration process.
auth_provider_id: The SSO IdP the user used, if any (just used for the
prometheus metrics).
auth_provider_id: The SSO IdP the user used, if any.
Returns:
The registered user_id.
Raises:
Expand All @@ -211,6 +210,7 @@ async def register_user(
threepid,
localpart,
user_agent_ips or [],
auth_provider_id=auth_provider_id,
)

if result == RegistrationBehaviour.DENY:
Expand Down
31 changes: 31 additions & 0 deletions tests/handlers/test_register.py
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,37 @@ def check_registration_for_spam(

self.assertTrue(requester.shadow_banned)

def test_spam_checker_receives_sso_type(self):
"""Test rejecting registration based on SSO type"""

class BanBadIdPUser:
def check_registration_for_spam(
self, email_threepid, username, request_info, auth_provider_id=None
):
# Reject any user coming from CAS and whose username contains profanity
if auth_provider_id == "cas" and "flimflob" in username:
return RegistrationBehaviour.DENY
return RegistrationBehaviour.ALLOW

# Configure a spam checker that denies a certain user on a specific IdP
spam_checker = self.hs.get_spam_checker()
spam_checker.spam_checkers = [BanBadIdPUser()]

f = self.get_failure(
self.handler.register_user(localpart="bobflimflob", auth_provider_id="cas"),
SynapseError,
)
exception = f.value

# We return 429 from the spam checker for denied registrations
self.assertIsInstance(exception, SynapseError)
self.assertEqual(exception.code, 429)

# Check the same username can register using SAML
self.get_success(
self.handler.register_user(localpart="bobflimflob", auth_provider_id="saml")
)

async def get_or_create_user(
self, requester, localpart, displayname, password_hash=None
):
Expand Down