Security hardening for 2FA configuration (#5685)

* reduce default RecentAuthenticationRequiredMixin timeout to 15 min
* never cache pages with RecentAuthenticationRequiredMixin
* show emergency codes only once after generating
This commit is contained in:
luelista
2026-02-19 12:43:23 +01:00
committed by GitHub
parent fd9ed15065
commit 7e45837295
3 changed files with 31 additions and 17 deletions

View File

@@ -144,14 +144,23 @@
</div> </div>
<div class="panel-body"> <div class="panel-body">
<p> <p>
{% trans "If you lose access to your devices, you can use one of the following keys to log in. We recommend to store them in a safe place, e.g. printed out or in a password manager. Every token can be used at most once." %} {% blocktrans trimmed %}
If you lose access to your devices, you can use one of your emergency tokens to log in.
We recommend to store them in a safe place, e.g. printed out or in a password manager.
Every token can be used at most once.
{% endblocktrans %}
</p> </p>
<p>{% trans "Unused tokens:" %}</p> {% if static_tokens_device %}
<ul> <p>
{% for t in static_tokens %} {% blocktrans trimmed with generation_date_time=static_tokens_device.created_at %}
<li><code>{{ t.token }}</code></li> You generated your emergency tokens on {{ generation_date_time }}.
{% endfor %} {% endblocktrans %}
</ul> </p>
{% else %}
<p>
{% trans "You don't have any emergency tokens yet." %}
</p>
{% endif %}
<a href="{% url "control:user.settings.2fa.regenemergency" %}" class="btn btn-default"> <a href="{% url "control:user.settings.2fa.regenemergency" %}" class="btn btn-default">
<span class="fa fa-refresh"></span> <span class="fa fa-refresh"></span>
{% trans "Generate new emergency tokens" %} {% trans "Generate new emergency tokens" %}

View File

@@ -49,12 +49,14 @@ from django.db import transaction
from django.shortcuts import get_object_or_404, redirect from django.shortcuts import get_object_or_404, redirect
from django.urls import reverse from django.urls import reverse
from django.utils.crypto import get_random_string from django.utils.crypto import get_random_string
from django.utils.decorators import method_decorator
from django.utils.functional import cached_property from django.utils.functional import cached_property
from django.utils.html import format_html from django.utils.html import format_html
from django.utils.http import url_has_allowed_host_and_scheme from django.utils.http import url_has_allowed_host_and_scheme
from django.utils.timezone import now from django.utils.timezone import now
from django.utils.translation import gettext_lazy as _ from django.utils.translation import gettext_lazy as _
from django.views import View from django.views import View
from django.views.decorators.cache import never_cache
from django.views.generic import FormView, ListView, TemplateView, UpdateView from django.views.generic import FormView, ListView, TemplateView, UpdateView
from django_otp.plugins.otp_static.models import StaticDevice from django_otp.plugins.otp_static.models import StaticDevice
from django_otp.plugins.otp_totp.models import TOTPDevice from django_otp.plugins.otp_totp.models import TOTPDevice
@@ -85,8 +87,9 @@ logger = logging.getLogger(__name__)
class RecentAuthenticationRequiredMixin: class RecentAuthenticationRequiredMixin:
max_time = 3600 max_time = 900
@method_decorator(never_cache)
def dispatch(self, request, *args, **kwargs): def dispatch(self, request, *args, **kwargs):
tdelta = time.time() - request.session.get('pretix_auth_login_time', 0) tdelta = time.time() - request.session.get('pretix_auth_login_time', 0)
if tdelta > self.max_time: if tdelta > self.max_time:
@@ -289,16 +292,13 @@ class User2FAMainView(RecentAuthenticationRequiredMixin, TemplateView):
ctx = super().get_context_data() ctx = super().get_context_data()
try: try:
ctx['static_tokens'] = StaticDevice.objects.get(user=self.request.user, name='emergency').token_set.all() ctx['static_tokens_device'] = StaticDevice.objects.get(user=self.request.user, name='emergency')
except StaticDevice.MultipleObjectsReturned: except StaticDevice.MultipleObjectsReturned:
ctx['static_tokens'] = StaticDevice.objects.filter( ctx['static_tokens_device'] = StaticDevice.objects.filter(
user=self.request.user, name='emergency' user=self.request.user, name='emergency'
).first().token_set.all() ).first()
except StaticDevice.DoesNotExist: except StaticDevice.DoesNotExist:
d = StaticDevice.objects.create(user=self.request.user, name='emergency') ctx['static_tokens_device'] = None
for i in range(10):
d.token_set.create(token=get_random_string(length=12, allowed_chars='1234567890'))
ctx['static_tokens'] = d.token_set.all()
ctx['devices'] = [] ctx['devices'] = []
for dt in REAL_DEVICE_TYPES: for dt in REAL_DEVICE_TYPES:
@@ -631,7 +631,8 @@ class User2FARegenerateEmergencyView(RecentAuthenticationRequiredMixin, Template
self.request.user.update_session_token() self.request.user.update_session_token()
update_session_auth_hash(self.request, self.request.user) update_session_auth_hash(self.request, self.request.user)
messages.success(request, _('Your emergency codes have been newly generated. Remember to store them in a safe ' messages.success(request, _('Your emergency codes have been newly generated. Remember to store them in a safe '
'place in case you lose access to your devices.')) 'place in case you lose access to your devices. You will not be able to view them '
'again here.\n\nYour emergency codes:\n- ' + '\n- '.join(t.token for t in d.token_set.all())))
return redirect(reverse('control:user.settings.2fa')) return redirect(reverse('control:user.settings.2fa'))

View File

@@ -339,13 +339,17 @@ class UserSettings2FATest(SoupTest):
def test_gen_emergency(self): def test_gen_emergency(self):
self.client.get('/control/settings/2fa/') self.client.get('/control/settings/2fa/')
assert not StaticDevice.objects.filter(user=self.user, name='emergency').exists()
self.client.post('/control/settings/2fa/regenemergency')
d = StaticDevice.objects.get(user=self.user, name='emergency') d = StaticDevice.objects.get(user=self.user, name='emergency')
assert d.token_set.count() == 10 assert d.token_set.count() == 10
old_tokens = set(t.token for t in d.token_set.all()) old_tokens = set(t.token for t in d.token_set.all())
self.client.post('/control/settings/2fa/regenemergency') self.client.post('/control/settings/2fa/regenemergency')
new_tokens = set(t.token for t in d.token_set.all())
d = StaticDevice.objects.get(user=self.user, name='emergency') d = StaticDevice.objects.get(user=self.user, name='emergency')
assert d.token_set.count() == 10 assert d.token_set.count() == 10
new_tokens = set(t.token for t in d.token_set.all())
assert old_tokens != new_tokens assert old_tokens != new_tokens
def test_delete_u2f(self): def test_delete_u2f(self):