forked from CGM_Public/pretix_original
Prevent email enumeration (#1000)
Here is my attempt to prevent user enumeration. I've made the following changes: **Application:** - replaces success and failure messages in the form with two (with/without redis) information messages - adds logging for attempted password resets of unknown users - adds logging for failing emails **Tests:** - test_unknown asserts a redirect instead of a ok - adds test_email_reset_twice_redis to assert the correct logging of a twice reset email - adds a FakeRedis class similiar to the one implemented in test_metrics.py. I could refactor them into the testutils folder if prefered. Please excuse the commit mess. I am currently fighting with my tooling.
This commit is contained in:
committed by
Raphael Michel
parent
099b08f009
commit
a643abe293
@@ -180,12 +180,4 @@ class PasswordForgotForm(forms.Form):
|
|||||||
super().__init__(*args, **kwargs)
|
super().__init__(*args, **kwargs)
|
||||||
|
|
||||||
def clean_email(self):
|
def clean_email(self):
|
||||||
email = self.cleaned_data['email']
|
return self.cleaned_data['email']
|
||||||
try:
|
|
||||||
self.cleaned_data['user'] = User.objects.get(email=email)
|
|
||||||
return email
|
|
||||||
except User.DoesNotExist:
|
|
||||||
raise forms.ValidationError(
|
|
||||||
_("We are unable to find a user matching the data you provided."),
|
|
||||||
code='unknown_user'
|
|
||||||
)
|
|
||||||
|
|||||||
@@ -174,6 +174,10 @@ def invite(request, token):
|
|||||||
return render(request, 'pretixcontrol/auth/invite.html', ctx)
|
return render(request, 'pretixcontrol/auth/invite.html', ctx)
|
||||||
|
|
||||||
|
|
||||||
|
class RepeatedResetDenied(Exception):
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
class Forgot(TemplateView):
|
class Forgot(TemplateView):
|
||||||
template_name = 'pretixcontrol/auth/forgot.html'
|
template_name = 'pretixcontrol/auth/forgot.html'
|
||||||
|
|
||||||
@@ -189,27 +193,43 @@ class Forgot(TemplateView):
|
|||||||
|
|
||||||
def post(self, request, *args, **kwargs):
|
def post(self, request, *args, **kwargs):
|
||||||
if self.form.is_valid():
|
if self.form.is_valid():
|
||||||
user = self.form.cleaned_data['user']
|
email = self.form.cleaned_data['email']
|
||||||
|
|
||||||
if settings.HAS_REDIS:
|
has_redis = settings.HAS_REDIS
|
||||||
from django_redis import get_redis_connection
|
|
||||||
rc = get_redis_connection("redis")
|
|
||||||
if rc.exists('pretix_pwreset_%s' % (user.id)):
|
|
||||||
user.log_action('pretix.control.auth.user.forgot_password.denied.repeated')
|
|
||||||
messages.error(request, _('We already sent you an email in the last 24 hours.'))
|
|
||||||
return redirect('control:auth.forgot')
|
|
||||||
else:
|
|
||||||
rc.setex('pretix_pwreset_%s' % (user.id), 3600 * 24, '1')
|
|
||||||
|
|
||||||
try:
|
try:
|
||||||
user.send_password_reset()
|
user = User.objects.get(email=email)
|
||||||
except SendMailException:
|
|
||||||
messages.error(request, _('There was an error sending the mail. Please try again later.'))
|
|
||||||
return self.get(request, *args, **kwargs)
|
|
||||||
|
|
||||||
user.log_action('pretix.control.auth.user.forgot_password.mail_sent')
|
if has_redis:
|
||||||
messages.success(request, _('We sent you an e-mail containing further instructions.'))
|
from django_redis import get_redis_connection
|
||||||
return redirect('control:auth.forgot')
|
rc = get_redis_connection("redis")
|
||||||
|
if rc.exists('pretix_pwreset_%s' % (user.id)):
|
||||||
|
user.log_action('pretix.control.auth.user.forgot_password.denied.repeated')
|
||||||
|
raise RepeatedResetDenied()
|
||||||
|
else:
|
||||||
|
rc.setex('pretix_pwreset_%s' % (user.id), 3600 * 24, '1')
|
||||||
|
|
||||||
|
except User.DoesNotExist:
|
||||||
|
logger.warning('Password reset for unregistered e-mail \"' + email + '\"requested.')
|
||||||
|
|
||||||
|
except SendMailException:
|
||||||
|
logger.exception('Sending password reset e-mail to \"' + email + '\" failed.')
|
||||||
|
|
||||||
|
except RepeatedResetDenied:
|
||||||
|
pass
|
||||||
|
|
||||||
|
else:
|
||||||
|
user.send_password_reset()
|
||||||
|
user.log_action('pretix.control.auth.user.forgot_password.mail_sent')
|
||||||
|
|
||||||
|
finally:
|
||||||
|
if has_redis:
|
||||||
|
messages.info(request, _('If the adress is registred to valid account, then we have sent you an e-mail containing further instructions. '
|
||||||
|
'Please note that we will send at most one email every 24 hours.'))
|
||||||
|
else:
|
||||||
|
messages.info(request, _('If the adress is registred to valid account, then we have sent you an e-mail containing further instructions.'))
|
||||||
|
|
||||||
|
return redirect('control:auth.forgot')
|
||||||
else:
|
else:
|
||||||
return self.get(request, *args, **kwargs)
|
return self.get(request, *args, **kwargs)
|
||||||
|
|
||||||
|
|||||||
@@ -301,16 +301,38 @@ class Login2FAFormTest(TestCase):
|
|||||||
m.undo()
|
m.undo()
|
||||||
|
|
||||||
|
|
||||||
|
class FakeRedis(object):
|
||||||
|
def get_redis_connection(self, connection_string):
|
||||||
|
return self
|
||||||
|
|
||||||
|
def __init__(self):
|
||||||
|
self.storage = {}
|
||||||
|
|
||||||
|
def exists(self, rkey):
|
||||||
|
print(rkey in self.storage)
|
||||||
|
return rkey in self.storage
|
||||||
|
|
||||||
|
def setex(self, rkey, value, expiration):
|
||||||
|
self.storage[rkey] = value
|
||||||
|
|
||||||
|
def execute(self):
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.usefixtures("class_monkeypatch")
|
||||||
class PasswordRecoveryFormTest(TestCase):
|
class PasswordRecoveryFormTest(TestCase):
|
||||||
def setUp(self):
|
def setUp(self):
|
||||||
super().setUp()
|
super().setUp()
|
||||||
self.user = User.objects.create_user('demo@demo.dummy', 'demo')
|
self.user = User.objects.create_user('demo@demo.dummy', 'demo')
|
||||||
|
|
||||||
def test_unknown(self):
|
def test_unknown(self):
|
||||||
|
djmail.outbox = []
|
||||||
|
|
||||||
response = self.client.post('/control/forgot', {
|
response = self.client.post('/control/forgot', {
|
||||||
'email': 'dummy@dummy.dummy',
|
'email': 'dummy@dummy.dummy',
|
||||||
})
|
})
|
||||||
self.assertEqual(response.status_code, 200)
|
self.assertEqual(response.status_code, 302)
|
||||||
|
assert len(djmail.outbox) == 0
|
||||||
|
|
||||||
def test_email_sent(self):
|
def test_email_sent(self):
|
||||||
djmail.outbox = []
|
djmail.outbox = []
|
||||||
@@ -323,6 +345,33 @@ class PasswordRecoveryFormTest(TestCase):
|
|||||||
assert len(djmail.outbox) == 1
|
assert len(djmail.outbox) == 1
|
||||||
assert djmail.outbox[0].to == [self.user.email]
|
assert djmail.outbox[0].to == [self.user.email]
|
||||||
assert "recover?id=%d&token=" % self.user.id in djmail.outbox[0].body
|
assert "recover?id=%d&token=" % self.user.id in djmail.outbox[0].body
|
||||||
|
assert self.user.all_logentries[0].action_type == 'pretix.control.auth.user.forgot_password.mail_sent'
|
||||||
|
|
||||||
|
@override_settings(HAS_REDIS=True)
|
||||||
|
def test_email_reset_twice_redis(self):
|
||||||
|
fake_redis = FakeRedis()
|
||||||
|
m = self.monkeypatch
|
||||||
|
m.setattr('django_redis.get_redis_connection', fake_redis.get_redis_connection, raising=False)
|
||||||
|
|
||||||
|
djmail.outbox = []
|
||||||
|
|
||||||
|
response = self.client.post('/control/forgot', {
|
||||||
|
'email': 'demo@demo.dummy',
|
||||||
|
})
|
||||||
|
self.assertEqual(response.status_code, 302)
|
||||||
|
|
||||||
|
assert len(djmail.outbox) == 1
|
||||||
|
assert djmail.outbox[0].to == [self.user.email]
|
||||||
|
assert "recover?id=%d&token=" % self.user.id in djmail.outbox[0].body
|
||||||
|
assert self.user.all_logentries[0].action_type == 'pretix.control.auth.user.forgot_password.mail_sent'
|
||||||
|
|
||||||
|
response = self.client.post('/control/forgot', {
|
||||||
|
'email': 'demo@demo.dummy',
|
||||||
|
})
|
||||||
|
self.assertEqual(response.status_code, 302)
|
||||||
|
|
||||||
|
assert len(djmail.outbox) == 1
|
||||||
|
assert self.user.all_logentries[0].action_type == 'pretix.control.auth.user.forgot_password.denied.repeated'
|
||||||
|
|
||||||
def test_recovery_unknown_user(self):
|
def test_recovery_unknown_user(self):
|
||||||
response = self.client.get('/control/forgot/recover?id=0&token=foo')
|
response = self.client.get('/control/forgot/recover?id=0&token=foo')
|
||||||
|
|||||||
Reference in New Issue
Block a user