mirror of
https://github.com/retspen/webvirtcloud
synced 2026-11-08 19:26:48 +00:00
fix(accounts): the "Lost OTP?" page cannot be used to flood or break
/accounts/email_otp/ mails a user their OTP QR code and needs no login. Measured with the default settings (OTP disabled): - It was reachable although OTP was off, and created OTP devices. - Every request sent a mail: 20 requests for one address, 20 mails, so anyone who knew an address could fill that mailbox. - An address shared by two users, or a mail server that could not be reached, ended in an error page (500). - Inactive users were mailed too. The page now exists only while OTP_ENABLED is set. It mails a single active user whose address matches (case-insensitively), at most once every 5 minutes per user (Django's cache: with several workers the bound is per worker), and logs a failed delivery. The answer is the same in every case, so it does not reveal whether an address belongs to a user. The QR codes come from django-qr-code (segno); both the mail and the account page render them without the qrcode package removed earlier. The new tests fail on the previous code.
This commit is contained in:
parent
5bc8b1915e
commit
da63e570e5
2 changed files with 82 additions and 9 deletions
61
accounts/test_email_otp.py
Normal file
61
accounts/test_email_otp.py
Normal file
|
|
@ -0,0 +1,61 @@
|
|||
"""The "Lost OTP?" page mails a user their OTP QR code without a login. It
|
||||
exists only while OTP is enabled, mails one exact match at most once per
|
||||
interval, and never fails with an error page."""
|
||||
|
||||
from django.contrib.auth import get_user_model
|
||||
from django.core import mail
|
||||
from django.core.cache import cache
|
||||
from django.test import TestCase, override_settings
|
||||
from django.urls import reverse
|
||||
|
||||
URL = reverse("accounts:email_otp")
|
||||
|
||||
|
||||
@override_settings(
|
||||
OTP_ENABLED=True,
|
||||
EMAIL_BACKEND="django.core.mail.backends.locmem.EmailBackend",
|
||||
CACHES={"default": {"BACKEND": "django.core.cache.backends.locmem.LocMemCache", "LOCATION": "email-otp-tests"}},
|
||||
)
|
||||
class EmailOTPTests(TestCase):
|
||||
def setUp(self):
|
||||
cache.clear()
|
||||
self.user = get_user_model().objects.create_user("otp-user", email="otp.user@example.com", password="x")
|
||||
|
||||
def post(self, email):
|
||||
response = self.client.post(URL, {"email": email})
|
||||
self.assertRedirects(response, reverse("accounts:login"), fetch_redirect_response=False)
|
||||
return response
|
||||
|
||||
def test_one_mail_to_the_user(self):
|
||||
self.post("OTP.User@Example.com") # addresses match case-insensitively
|
||||
self.assertEqual([m.to for m in mail.outbox], [["otp.user@example.com"]])
|
||||
|
||||
def test_repeated_requests_send_one_mail_per_interval(self):
|
||||
for _ in range(20):
|
||||
self.post("otp.user@example.com")
|
||||
self.assertEqual(len(mail.outbox), 1)
|
||||
|
||||
def test_an_address_of_two_users_sends_nothing(self):
|
||||
get_user_model().objects.create_user("otp-twin", email="otp.user@example.com", password="x")
|
||||
self.post("otp.user@example.com")
|
||||
self.assertEqual(mail.outbox, [])
|
||||
|
||||
def test_unknown_and_inactive_addresses_send_nothing(self):
|
||||
get_user_model().objects.create_user("otp-gone", email="gone@example.com", password="x", is_active=False)
|
||||
self.post("nobody@example.com")
|
||||
self.post("gone@example.com")
|
||||
self.assertEqual(mail.outbox, [])
|
||||
|
||||
@override_settings(EMAIL_BACKEND="django.core.mail.backends.smtp.EmailBackend", EMAIL_HOST="127.0.0.1", EMAIL_PORT=1)
|
||||
def test_a_mail_server_that_is_down_is_no_error_page(self):
|
||||
with self.assertLogs("accounts.views", level="ERROR"):
|
||||
self.post("otp.user@example.com")
|
||||
|
||||
|
||||
@override_settings(OTP_ENABLED=False)
|
||||
class EmailOTPDisabledTests(TestCase):
|
||||
def test_the_page_does_not_exist_without_otp(self):
|
||||
get_user_model().objects.create_user("otp-off", email="off@example.com", password="x")
|
||||
self.assertEqual(self.client.get(URL).status_code, 404)
|
||||
self.assertEqual(self.client.post(URL, {"email": "off@example.com"}).status_code, 404)
|
||||
self.assertEqual(mail.outbox, [])
|
||||
|
|
@ -11,7 +11,8 @@ from django.core.exceptions import PermissionDenied
|
|||
from django.db import transaction
|
||||
from django.shortcuts import get_object_or_404, redirect, render
|
||||
from django.urls import reverse
|
||||
from django.http import HttpResponseRedirect
|
||||
from django.core.cache import cache
|
||||
from django.http import Http404, HttpResponseRedirect
|
||||
from django.utils.http import url_has_allowed_host_and_scheme
|
||||
from django.utils.translation import gettext_lazy as _
|
||||
from django.views.decorators.http import require_POST
|
||||
|
|
@ -201,18 +202,29 @@ def user_instance_delete(request, pk):
|
|||
)
|
||||
|
||||
|
||||
# a user gets at most one OTP mail per this many seconds
|
||||
EMAIL_OTP_INTERVAL = 300
|
||||
|
||||
|
||||
@login_not_required
|
||||
def email_otp(request):
|
||||
"""
|
||||
"Lost OTP?": mail the OTP QR code to the user with this address. Open to
|
||||
anyone, so it mails only one exact, active match, at most once per
|
||||
EMAIL_OTP_INTERVAL, and answers the same in every case.
|
||||
"""
|
||||
if not settings.OTP_ENABLED:
|
||||
raise Http404
|
||||
form = EmailOTPForm(request.POST or None)
|
||||
if form.is_valid():
|
||||
UserModel = get_user_model()
|
||||
try:
|
||||
user = UserModel.objects.get(email=form.cleaned_data["email"])
|
||||
except UserModel.DoesNotExist:
|
||||
pass
|
||||
else:
|
||||
device = get_user_totp_device(user)
|
||||
send_email_with_otp(user, device)
|
||||
users = list(get_user_model().objects.filter(email__iexact=form.cleaned_data["email"], is_active=True)[:2])
|
||||
# the cache is per process: with several workers the bound is per worker
|
||||
if len(users) == 1 and cache.add(f"email_otp:{users[0].pk}", True, EMAIL_OTP_INTERVAL):
|
||||
user = users[0]
|
||||
try:
|
||||
send_email_with_otp(user, get_user_totp_device(user))
|
||||
except Exception:
|
||||
logging.getLogger(__name__).exception("Could not mail the OTP QR code to user %s", user.pk)
|
||||
|
||||
messages.success(
|
||||
request, _("OTP Sent to %(email)s") % {"email": form.cleaned_data["email"]}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue