Skip to content

Commit 0b46fe5

Browse files
nijelamCap1712
andcommitted
fix(auth): shorten session expiry while in 2fa
The 2fa should have a short session expiry as this is expected to happen interactively. Previously 2fa input got the session expiry for authenticated users what was wrong. Co-authored-by: Kartik Ohri <[email protected]>
1 parent 5018009 commit 0b46fe5

File tree

9 files changed

+54
-14
lines changed

9 files changed

+54
-14
lines changed

docs/admin/config.rst

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1812,6 +1812,16 @@ Configure sampling rate for profiling monitoring. Set to 1 to trace all events,
18121812

18131813
`Sentry Profiling <https://docs.sentry.io/product/explore/profiling/>`_
18141814

1815+
.. setting:: SESSION_COOKIE_AGE_2FA
1816+
1817+
SESSION_COOKIE_AGE_2FA
1818+
----------------------
1819+
1820+
.. versionadded:: 5.13.1
1821+
1822+
Set session expiry while in :ref:`2fa`. This complements
1823+
:setting:`django:SESSION_COOKIE_AGE` which is used for unauthenticated users.
1824+
18151825
.. setting:: SESSION_COOKIE_AGE_AUTHENTICATED
18161826

18171827
SESSION_COOKIE_AGE_AUTHENTICATED

docs/changes.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ Weblate 5.13.1
1313
* Access control for :http:get:`/api/users/(str:username)/`.
1414
* :ref:`file_format_params` were not properly applied in some situations.
1515
* :ref:`mt-libretranslate` compatibility with LibreTranslate 1.7.0.
16+
* Shorten session expiry while in :ref:`2fa`.
1617

1718
.. rubric:: Compatibility
1819

@@ -22,6 +23,8 @@ Weblate 5.13.1
2223

2324
Please follow :ref:`generic-upgrade-instructions` in order to perform update.
2425

26+
* There is a change in :file:`settings_example.py`, ``django_otp.middleware.OTPMiddleware`` was removed from ``MIDDLEWARE``; please adjust your settings accordingly.
27+
2528
.. rubric:: Contributors
2629

2730
.. include:: changes/contributors/5.13.1.rst

weblate/accounts/forms.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -785,14 +785,14 @@ def clean(self):
785785
)
786786
% lockout_period
787787
)
788-
self.user_cache = cast(
788+
user = self.user_cache = cast(
789789
"User | None",
790790
authenticate(self.request, username=username, password=password),
791791
)
792-
if self.user_cache is None:
793-
for user in try_get_user(username, True):
792+
if user is None:
793+
for failed_user in try_get_user(username, True):
794794
audit = AuditLog.objects.create(
795-
user,
795+
failed_user,
796796
self.request,
797797
"failed-auth",
798798
method="password",
@@ -803,14 +803,14 @@ def clean(self):
803803
raise forms.ValidationError(
804804
self.error_messages["invalid_login"], code="invalid_login"
805805
)
806-
if not self.user_cache.is_active or self.user_cache.is_bot:
806+
if not user.is_active or user.is_bot:
807807
raise forms.ValidationError(
808808
self.error_messages["inactive"], code="inactive"
809809
)
810810
AuditLog.objects.create(
811-
self.user_cache, self.request, "login", method="password", name=username
811+
user, self.request, "login", method="password", name=username
812812
)
813-
adjust_session_expiry(self.request)
813+
adjust_session_expiry(request=self.request, user=user)
814814
reset_rate_limit("login", self.request)
815815
return self.cleaned_data
816816

weblate/accounts/middleware.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from django.contrib.auth.models import AnonymousUser
1212
from django.utils.functional import SimpleLazyObject
1313
from django.utils.translation import activate, get_language, get_language_from_request
14+
from django_otp.middleware import OTPMiddleware
1415

1516
from weblate.accounts.models import set_lang_cookie
1617
from weblate.accounts.utils import adjust_session_expiry
@@ -35,8 +36,12 @@ def get_user(request: AuthenticatedHttpRequest):
3536
return request.weblate_cached_user
3637

3738

38-
class AuthenticationMiddleware:
39-
"""Copy of django.contrib.auth.middleware.AuthenticationMiddleware."""
39+
class AuthenticationMiddleware(OTPMiddleware):
40+
"""
41+
Copy of django.contrib.auth.middleware.AuthenticationMiddleware.
42+
43+
It subclasses OTPMiddleware to get access to _verify_user.
44+
"""
4045

4146
def __init__(self, get_response=None) -> None:
4247
self.get_response = get_response
@@ -47,6 +52,7 @@ def __call__(self, request: AuthenticatedHttpRequest):
4752
# Django uses lazy object here, but we need the user in pretty
4853
# much every request, so there is no reason to delay this
4954
request.user = user = get_user(request)
55+
self._verify_user(request, user)
5056

5157
# Get language to use in this request
5258
if user.is_authenticated and user.profile.language:
@@ -60,7 +66,7 @@ def __call__(self, request: AuthenticatedHttpRequest):
6066

6167
# Extend session expiry for authenticated users
6268
if user.is_authenticated:
63-
adjust_session_expiry(request, is_login=False)
69+
adjust_session_expiry(request=request, user=user, is_login=False)
6470

6571
# Based on django.middleware.locale.LocaleMiddleware
6672
activate(language)

weblate/accounts/pipeline.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -455,7 +455,7 @@ def notify_connect(
455455
action = "auth-connect"
456456
else:
457457
action = "login"
458-
adjust_session_expiry(strategy.request)
458+
adjust_session_expiry(request=strategy.request, user=user)
459459
AuditLog.objects.create(
460460
user,
461461
strategy.request,

weblate/accounts/utils.py

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,14 +151,22 @@ def cycle_session_keys(request: AuthenticatedHttpRequest, user: User) -> None:
151151

152152

153153
def adjust_session_expiry(
154-
request: AuthenticatedHttpRequest, *, is_login: bool = True
154+
*,
155+
request: AuthenticatedHttpRequest,
156+
user: User,
157+
is_login: bool = True,
155158
) -> None:
156159
"""
157160
Adjust session expiry based on scope.
158161
159162
- Set longer expiry for authenticated users.
160163
- Set short lived session for SAML authentication flow.
161164
"""
165+
is_2fa = False
166+
if user.profile.has_2fa and not user.is_verified():
167+
# Still in second factor view
168+
is_2fa = True
169+
162170
if "saml_only" not in request.session:
163171
if is_login:
164172
next_url = request.POST.get("next", request.GET.get("next"))
@@ -169,6 +177,11 @@ def adjust_session_expiry(
169177
if request.session["saml_only"]:
170178
# Short lived session for SAML authentication only
171179
request.session.set_expiry(60)
180+
elif is_2fa:
181+
request.session.set_expiry(settings.SESSION_COOKIE_AGE_2FA)
182+
elif is_login:
183+
# Using default expiry for login flow
184+
request.session.set_expiry(settings.SESSION_COOKIE_AGE)
172185
else:
173186
request.session.set_expiry(settings.SESSION_COOKIE_AGE_AUTHENTICATED)
174187

weblate/auth/models.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262
if TYPE_CHECKING:
6363
from collections.abc import Iterable, Mapping
6464

65+
from django_otp.models import Device
6566
from social_core.backends.base import BaseAuth
6667
from social_django.models import DjangoStorage
6768
from social_django.strategy import DjangoStrategy
@@ -506,6 +507,9 @@ class User(AbstractBaseUser):
506507
# social_auth integration
507508
social_auth: DjangoStorage
508509

510+
# django_otp integration (via OTPMiddleware)
511+
otp_device: Device
512+
509513
EMAIL_FIELD = "email"
510514
USERNAME_FIELD = "username"
511515
REQUIRED_FIELDS = ["email", "full_name"]
@@ -594,6 +598,10 @@ def has_usable_password(self):
594598
def is_anonymous(self):
595599
return self.username == settings.ANONYMOUS_USER_NAME
596600

601+
def is_verified(self) -> bool:
602+
# django_otp overrides this method in OTPMiddleware
603+
return False
604+
597605
@cached_property
598606
def is_authenticated(self) -> bool: # type: ignore[override]
599607
return not self.is_anonymous
@@ -1253,7 +1261,9 @@ class WeblateAuthConf(AppConf):
12531261

12541262
# Anonymous user name
12551263
ANONYMOUS_USER_NAME = "anonymous"
1264+
12561265
SESSION_COOKIE_AGE_AUTHENTICATED = 1209600
1266+
SESSION_COOKIE_AGE_2FA = 180
12571267

12581268
class Meta:
12591269
prefix = ""

weblate/settings_docker.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -707,7 +707,6 @@
707707
"django.contrib.sessions.middleware.SessionMiddleware",
708708
"django.middleware.csrf.CsrfViewMiddleware",
709709
"weblate.accounts.middleware.AuthenticationMiddleware",
710-
"django_otp.middleware.OTPMiddleware",
711710
"django.contrib.messages.middleware.MessageMiddleware",
712711
"django.middleware.clickjacking.XFrameOptionsMiddleware",
713712
"social_django.middleware.SocialAuthExceptionMiddleware",

weblate/settings_example.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,6 @@
395395
"django.contrib.sessions.middleware.SessionMiddleware",
396396
"django.middleware.csrf.CsrfViewMiddleware",
397397
"weblate.accounts.middleware.AuthenticationMiddleware",
398-
"django_otp.middleware.OTPMiddleware",
399398
"django.contrib.messages.middleware.MessageMiddleware",
400399
"django.middleware.clickjacking.XFrameOptionsMiddleware",
401400
"social_django.middleware.SocialAuthExceptionMiddleware",

0 commit comments

Comments
 (0)