From e075665c57b4a3c43b83954e30223722d3ba3640 Mon Sep 17 00:00:00 2001 From: anonymoose2 Date: Sat, 12 Oct 2019 13:32:53 -0400 Subject: [PATCH 1/3] fix(auth): fix reauthentication implementation Fixes #846 --- intranet/apps/auth/decorators.py | 7 ++++++- intranet/apps/auth/views.py | 3 ++- intranet/settings/__init__.py | 2 ++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/intranet/apps/auth/decorators.py b/intranet/apps/auth/decorators.py index 7d0679f9e68..d85da9559ca 100644 --- a/intranet/apps/auth/decorators.py +++ b/intranet/apps/auth/decorators.py @@ -1,5 +1,7 @@ """Decorators that restrict views to certain types of users.""" +import time +from django.conf import settings from django.contrib import messages from django.contrib.auth.decorators import user_passes_test from django.shortcuts import redirect @@ -51,7 +53,10 @@ def inner(*args, **kwargs): def reauthentication_required(wrapped): def inner(*args, **kwargs): request = args[0] # request is the first argument in a view - if request.session.get("reauthenticated", False): + if ( + "reauthenticated_at" in request.session + and 0 <= (time.time() - request.session["reauthenticated_at"]) <= settings.REAUTHENTICATION_EXPIRE_TIMEOUT + ): return wrapped(*args, **kwargs) else: return redirect("{}?next={}".format(reverse("reauth"), request.path)) diff --git a/intranet/apps/auth/views.py b/intranet/apps/auth/views.py index ddb8558f3a8..07e3a10c833 100644 --- a/intranet/apps/auth/views.py +++ b/intranet/apps/auth/views.py @@ -1,5 +1,6 @@ import logging import random +import time from datetime import timedelta from typing import Container, Tuple @@ -266,7 +267,7 @@ def reauthentication_view(request): context = {"login_failed": False} if request.method == "POST": if authenticate(username=request.user.username, password=request.POST.get("password", "")): - request.session["reauthenticated"] = True + request.session["reauthenticated_at"] = time.time() return redirect(request.POST.get("next", request.GET.get("next", "/"))) else: context["login_failed"] = True diff --git a/intranet/settings/__init__.py b/intranet/settings/__init__.py index 96387e73e84..564bbec059c 100644 --- a/intranet/settings/__init__.py +++ b/intranet/settings/__init__.py @@ -766,6 +766,8 @@ def get_log(name): # pylint: disable=redefined-outer-name; 'name' is used as th # The Referrer-policy header REFERRER_POLICY = "strict-origin-when-cross-origin" +REAUTHENTICATION_EXPIRE_TIMEOUT = 2 * 60 * 60 # seconds + # Shows a warning message with yellow background on the login page # LOGIN_WARNING = "This is a message to display on the login page." From 284e047ca4c32a2568b7fa006dbb3e0658da11c1 Mon Sep 17 00:00:00 2001 From: anonymoose2 Date: Sat, 12 Oct 2019 19:28:46 -0400 Subject: [PATCH 2/3] refactor(sessionmgmt): delete and block duplicate TrustedSession objects Enforced on a per-user, per-session basis --- Ion.egg-info/SOURCES.txt | 2 ++ .../migrations/0002_auto_20191012_1942.py | 24 +++++++++++++++++++ .../migrations/0003_auto_20191012_1947.py | 21 ++++++++++++++++ intranet/apps/sessionmgmt/models.py | 3 +++ 4 files changed, 50 insertions(+) create mode 100644 intranet/apps/sessionmgmt/migrations/0002_auto_20191012_1942.py create mode 100644 intranet/apps/sessionmgmt/migrations/0003_auto_20191012_1947.py diff --git a/Ion.egg-info/SOURCES.txt b/Ion.egg-info/SOURCES.txt index c114177d26f..5d4ac928a0a 100644 --- a/Ion.egg-info/SOURCES.txt +++ b/Ion.egg-info/SOURCES.txt @@ -572,6 +572,8 @@ intranet/apps/sessionmgmt/tests.py intranet/apps/sessionmgmt/urls.py intranet/apps/sessionmgmt/views.py intranet/apps/sessionmgmt/migrations/0001_initial.py +intranet/apps/sessionmgmt/migrations/0002_auto_20191012_1942.py +intranet/apps/sessionmgmt/migrations/0003_auto_20191012_1947.py intranet/apps/sessionmgmt/migrations/__init__.py intranet/apps/signage/__init__.py intranet/apps/signage/admin.py diff --git a/intranet/apps/sessionmgmt/migrations/0002_auto_20191012_1942.py b/intranet/apps/sessionmgmt/migrations/0002_auto_20191012_1942.py new file mode 100644 index 00000000000..6425c4de3ed --- /dev/null +++ b/intranet/apps/sessionmgmt/migrations/0002_auto_20191012_1942.py @@ -0,0 +1,24 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.23 on 2019-10-12 23:42 +from __future__ import unicode_literals + +from django.db import migrations + + +def remove_duplicate_trustedsessions(apps, schema_editor): + TrustedSession = apps.get_model("sessionmgmt", "TrustedSession") + for tsession in TrustedSession.objects.all(): + if TrustedSession.objects.filter(user=tsession.user, session_key=tsession.session_key).exclude(pk=tsession.pk).exists(): + tsession.delete() + + +class Migration(migrations.Migration): + + dependencies = [ + ('users', '0001_initial'), + ('sessionmgmt', '0001_initial'), + ] + + operations = [ + migrations.RunPython(remove_duplicate_trustedsessions, lambda *args, **kwargs: None), + ] diff --git a/intranet/apps/sessionmgmt/migrations/0003_auto_20191012_1947.py b/intranet/apps/sessionmgmt/migrations/0003_auto_20191012_1947.py new file mode 100644 index 00000000000..e2f57e807d7 --- /dev/null +++ b/intranet/apps/sessionmgmt/migrations/0003_auto_20191012_1947.py @@ -0,0 +1,21 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.23 on 2019-10-12 23:47 +from __future__ import unicode_literals + +from django.conf import settings +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('sessionmgmt', '0002_auto_20191012_1942'), + ] + + operations = [ + migrations.AlterUniqueTogether( + name='trustedsession', + unique_together=set([('user', 'session_key')]), + ), + ] diff --git a/intranet/apps/sessionmgmt/models.py b/intranet/apps/sessionmgmt/models.py index 3590b67880b..1c1d671136c 100644 --- a/intranet/apps/sessionmgmt/models.py +++ b/intranet/apps/sessionmgmt/models.py @@ -35,3 +35,6 @@ def delete_expired_sessions(cls, *, user=None) -> None: for trusted_session in trusted_sessions: if not SessionStore(session_key=trusted_session.session_key).exists(trusted_session.session_key): trusted_session.delete() + + class Meta: + unique_together = (("user", "session_key"),) From a72dd56fab7cfe3f1d44ccd16d36492922c6abef Mon Sep 17 00:00:00 2001 From: anonymoose2 Date: Sat, 12 Oct 2019 19:29:35 -0400 Subject: [PATCH 3/3] fix(sessionmgmt): don't attempt to create duplicate TrustedSessions --- intranet/apps/sessionmgmt/views.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/intranet/apps/sessionmgmt/views.py b/intranet/apps/sessionmgmt/views.py index 5b9bef7b865..cb135ef6258 100644 --- a/intranet/apps/sessionmgmt/views.py +++ b/intranet/apps/sessionmgmt/views.py @@ -57,12 +57,13 @@ def trust_session_view(request): description += request.user_agent.os.family - TrustedSession.objects.create( - user=request.user, - session_key=request.session.session_key, - description=description, - device_type=device_type, - ) + if not TrustedSession.objects.filter(user=request.user, session_key=request.session.session_key).exists(): + TrustedSession.objects.create( + user=request.user, + session_key=request.session.session_key, + description=description, + device_type=device_type, + ) request.session.set_expiry(7 * 24 * 60 * 60) # Trusted sessions expire after a week