Skip to content

Commit

Permalink
feat(security): Block abusive email patterns
Browse files Browse the repository at this point in the history
We're seeing a lot of junk signups from \[email protected] so this is a simply quick fix to make a configurable pattern match on blocking email addresses.

It's not the be-all-end-all, but it prevents this simplistic attack. We'll likely also want to restore captcha in some cases in the future, but thats going to be a much more complex feature.
  • Loading branch information
dcramer committed Oct 16, 2018
1 parent bca953f commit 6ad37e7
Show file tree
Hide file tree
Showing 11 changed files with 101 additions and 46 deletions.
3 changes: 2 additions & 1 deletion src/sentry/api/endpoints/organization_member_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from sentry.api.paginator import OffsetPaginator
from sentry.api.serializers import serialize
from sentry.api.serializers.rest_framework import ListField
from sentry.api.validators import AllowedEmailField
from sentry.models import AuditLogEntryEvent, OrganizationMember, OrganizationMemberTeam, Team, TeamStatus
from sentry.search.utils import tokenize_query
from sentry.signals import member_invited
Expand All @@ -31,7 +32,7 @@ class MemberPermission(OrganizationPermission):


class OrganizationMemberSerializer(serializers.Serializer):
email = serializers.EmailField(max_length=75, required=True)
email = AllowedEmailField(max_length=75, required=True)
role = serializers.ChoiceField(choices=roles.get_choices(), required=True)
teams = ListField(required=False, allow_null=False)

Expand Down
3 changes: 2 additions & 1 deletion src/sentry/api/endpoints/user_emails.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from sentry.api.bases.user import UserEndpoint
from sentry.api.decorators import sudo_required
from sentry.api.serializers import serialize
from sentry.api.validators import AllowedEmailField
from sentry.models import User, UserEmail, UserOption

logger = logging.getLogger('sentry.accounts')
Expand All @@ -23,7 +24,7 @@ class DuplicateEmailError(Exception):


class EmailValidator(serializers.Serializer):
email = serializers.EmailField(required=True)
email = AllowedEmailField(required=True)


def add_email(email, user):
Expand Down
3 changes: 2 additions & 1 deletion src/sentry/api/endpoints/user_emails_confirm.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

from sentry.api.bases.user import UserEndpoint
from sentry.api.decorators import sudo_required
from sentry.api.validators import AllowedEmailField
from sentry.models import UserEmail

logger = logging.getLogger('sentry.accounts')
Expand All @@ -29,7 +30,7 @@ class DuplicateEmailError(Exception):


class EmailSerializer(serializers.Serializer):
email = serializers.EmailField(required=True)
email = AllowedEmailField(required=True)


class UserEmailsConfirmEndpoint(UserEndpoint):
Expand Down
17 changes: 17 additions & 0 deletions src/sentry/api/validators/email.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
from __future__ import absolute_import

from django.utils.translation import ugettext_lazy as _
from rest_framework import serializers

from sentry.web.forms import fields


class AllowedEmailField(serializers.EmailField):
type_name = 'AllowedEmailField'
type_label = 'email'
form_field_class = fields.AllowedEmailField

default_error_messages = {
'invalid': _('Enter a valid email address.'),
}
default_validators = fields.AllowedEmailField.default_validators
5 changes: 5 additions & 0 deletions src/sentry/conf/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

import os
import os.path
import re
import socket
import sys
import tempfile
Expand Down Expand Up @@ -1424,3 +1425,7 @@ def get_raven_config():
JS_SDK_LOADER_SDK_VERSION = ''
# This should be the url pointing to the JS SDK
JS_SDK_LOADER_DEFAULT_SDK_URL = ''

# block domains which are generally used by spammers -- keep this configurable in case an onpremise
# install wants to allow it
INVALID_EMAIL_ADDRESS_PATTERN = re.compile(r'\@qq\.com$', re.I)
37 changes: 1 addition & 36 deletions src/sentry/security/__init__.py
Original file line number Diff line number Diff line change
@@ -1,38 +1,3 @@
from __future__ import absolute_import, print_function

import logging

from django.utils import timezone

from .emails import generate_security_email

logger = logging.getLogger('sentry.security')


def capture_security_activity(
account, type, actor, ip_address, context=None, send_email=True, current_datetime=None
):
if current_datetime is None:
current_datetime = timezone.now()

logger_context = {
'ip_address': ip_address,
'user_id': account.id,
'actor_id': actor.id,
}

if type == 'mfa-removed' or type == 'mfa-added':
logger_context['authenticator_id'] = context['authenticator'].id

logger.info(u'user.{}'.format(type), extra=logger_context)

if send_email:
msg = generate_security_email(
account=account,
type=type,
actor=actor,
ip_address=ip_address,
context=context,
current_datetime=current_datetime,
)
msg.send_async([account.email])
from .utils import * # NOQA
43 changes: 43 additions & 0 deletions src/sentry/security/utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
from __future__ import absolute_import, print_function

import logging

from django.conf import settings
from django.utils import timezone

from .emails import generate_security_email

logger = logging.getLogger('sentry.security')


def capture_security_activity(
account, type, actor, ip_address, context=None, send_email=True, current_datetime=None
):
if current_datetime is None:
current_datetime = timezone.now()

logger_context = {
'ip_address': ip_address,
'user_id': account.id,
'actor_id': actor.id,
}

if type == 'mfa-removed' or type == 'mfa-added':
logger_context['authenticator_id'] = context['authenticator'].id

logger.info(u'user.{}'.format(type), extra=logger_context)

if send_email:
msg = generate_security_email(
account=account,
type=type,
actor=actor,
ip_address=ip_address,
context=context,
current_datetime=current_datetime,
)
msg.send_async([account.email])


def is_valid_email_address(value):
return not settings.INVALID_EMAIL_ADDRESS_PATTERN.search(value)
11 changes: 5 additions & 6 deletions src/sentry/web/forms/accounts.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
from sentry.models import (Organization, OrganizationStatus, User, UserOption, UserOptionValue)
from sentry.security import capture_security_activity
from sentry.utils.auth import find_users, logger
from sentry.web.forms.fields import CustomTypedChoiceField, ReadOnlyTextField
from sentry.web.forms.fields import CustomTypedChoiceField, ReadOnlyTextField, AllowedEmailField
from six.moves import range


Expand Down Expand Up @@ -185,7 +185,7 @@ class PasswordlessRegistrationForm(forms.ModelForm):
widget=forms.TextInput(attrs={'placeholder': 'Jane Doe'}),
required=True
)
username = forms.EmailField(
username = AllowedEmailField(
label=_('Email'),
max_length=128,
widget=forms.TextInput(attrs={'placeholder': '[email protected]'}),
Expand Down Expand Up @@ -308,8 +308,7 @@ def clean_password(self):


class EmailForm(forms.Form):

alt_email = forms.EmailField(
alt_email = AllowedEmailField(
label=_('New Email'),
required=False,
help_text='Designate an alternative email for this account',
Expand Down Expand Up @@ -345,7 +344,7 @@ def clean_password(self):
class AccountSettingsForm(forms.Form):
name = forms.CharField(required=True, label=_('Name'), max_length=30)
username = forms.CharField(label=_('Username'), max_length=128)
email = forms.EmailField(label=_('Email'))
email = AllowedEmailField(label=_('Email'))
new_password = forms.CharField(
label=_('New password'),
widget=forms.PasswordInput(),
Expand Down Expand Up @@ -630,7 +629,7 @@ def save(self):


class NotificationSettingsForm(forms.Form):
alert_email = forms.EmailField(
alert_email = AllowedEmailField(
label=_('Email'),
help_text=_('Designate an alternative email address to send email notifications to.'),
required=False
Expand Down
13 changes: 12 additions & 1 deletion src/sentry/web/forms/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,15 @@
from django.forms.widgets import RadioFieldRenderer, TextInput, Widget
from django.forms.util import flatatt
from django.forms import (
Field, CharField, TypedChoiceField, ValidationError
Field, CharField, EmailField, TypedChoiceField, ValidationError
)
from django.utils.encoding import force_text
from django.utils.html import format_html
from django.utils.safestring import mark_safe
from django.utils.translation import ugettext_lazy as _

from sentry.models import User
from sentry.security import is_valid_email_address


class CustomTypedChoiceField(TypedChoiceField):
Expand Down Expand Up @@ -98,3 +99,13 @@ def bound_data(self, data, initial):
# Always return initial because the widget doesn't
# render an input field.
return initial


def email_address_validator(value):
if not is_valid_email_address(value):
raise ValidationError(_('Enter a valid email address.'), code='invalid')
return value


class AllowedEmailField(EmailField):
default_validators = EmailField.default_validators + [is_valid_email_address]
1 change: 1 addition & 0 deletions tests/sentry/security/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
from __future__ import absolute_import
11 changes: 11 additions & 0 deletions tests/sentry/security/test_utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
from __future__ import absolute_import

from sentry.security.utils import is_valid_email_address


def test_is_valid_email_address_number_at_qqcom():
assert is_valid_email_address('[email protected]') is False


def test_is_valid_email_address_normal_human_email_address():
assert is_valid_email_address('[email protected]') is True

0 comments on commit 6ad37e7

Please sign in to comment.