From 1a98730e3ac362d4ec3f54eeb6aef62f6703e387 Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Tue, 30 Jul 2024 15:55:59 +0200 Subject: [PATCH 01/17] UserSession model: keep track of user session activity --- bid_main/middleware.py | 13 ++++++++ bid_main/migrations/0046_usersession.py | 43 +++++++++++++++++++++++++ bid_main/models.py | 40 ++++++++++++++++++++++- bid_main/signals.py | 3 +- blenderid/settings.py | 3 ++ requirements.txt | 1 + 6 files changed, 101 insertions(+), 2 deletions(-) create mode 100644 bid_main/middleware.py create mode 100644 bid_main/migrations/0046_usersession.py diff --git a/bid_main/middleware.py b/bid_main/middleware.py new file mode 100644 index 0000000..59d3968 --- /dev/null +++ b/bid_main/middleware.py @@ -0,0 +1,13 @@ +from bid_main.models import UserSession + + +def user_session_middleware(get_response): + def middleware(request): + if ( + hasattr(request, 'session') + and hasattr(request, 'user') + and request.user.is_authenticated + ): + UserSession.update_or_create_from_request(request) + return get_response(request) + return middleware diff --git a/bid_main/migrations/0046_usersession.py b/bid_main/migrations/0046_usersession.py new file mode 100644 index 0000000..a1dd292 --- /dev/null +++ b/bid_main/migrations/0046_usersession.py @@ -0,0 +1,43 @@ +# Generated by Django 4.2.13 on 2024-07-30 13:57 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('sessions', '0001_initial'), + ('bid_main', '0045_alter_oauth2accesstoken_user_and_more'), + ] + + operations = [ + migrations.CreateModel( + name='UserSession', + fields=[ + ('id', models.BigAutoField(primary_key=True, serialize=False)), + ('agent_is_trusted', models.BooleanField(default=False)), + ('created_at', models.DateTimeField(auto_now_add=True)), + ('last_active_at', models.DateTimeField()), + ('ip', models.GenericIPAddressField(blank=True, null=True)), + ('user_agent', models.CharField(blank=True, max_length=255)), + ( + 'session', + models.OneToOneField( + editable=False, + on_delete=django.db.models.deletion.CASCADE, + to='sessions.session', + ), + ), + ( + 'user', + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name='sessions', + to=settings.AUTH_USER_MODEL, + ), + ), + ], + ), + ] diff --git a/bid_main/models.py b/bid_main/models.py index f8f6bdb..27d5c7d 100644 --- a/bid_main/models.py +++ b/bid_main/models.py @@ -8,6 +8,7 @@ from django import urls from django.conf import settings from django.contrib.auth.base_user import AbstractBaseUser, BaseUserManager from django.contrib.auth.models import PermissionsMixin +from django.contrib.sessions.models import Session from django.core import validators from django.core.exceptions import ValidationError from django.core.mail import send_mail @@ -17,11 +18,12 @@ from django.templatetags.static import static from django.utils import timezone from django.utils.deconstruct import deconstructible from django.utils.translation import gettext_lazy as _ - import oauth2_provider.models as oa2_models + from . import fields from . import hashers import bid_main.file_utils +import bid_main.utils log = logging.getLogger(__name__) nickname_illegal_chars = re.compile(r"[^\w.+-]") @@ -646,3 +648,39 @@ class UserNote(models.Model): def __str__(self): return "Note" + + +class UserSession(models.Model): + id = models.BigAutoField(primary_key=True) + agent_is_trusted = models.BooleanField(default=False, null=False) + created_at = models.DateTimeField(auto_now_add=True) + last_active_at = models.DateTimeField() + ip = models.GenericIPAddressField(null=True, blank=True) + session = models.OneToOneField(Session, on_delete=models.CASCADE, editable=False) + user = models.ForeignKey(User, related_name="sessions", on_delete=models.CASCADE) + user_agent = models.CharField( + max_length=255, + blank=True, + null=False, + ) + + @classmethod + @transaction.atomic + def update_or_create_from_request(cls, request, user=None): + if not user: + user = request.user + if not request.session.session_key: + request.session.save() + return cls.objects.update_or_create( + session_id=request.session.session_key, + defaults={ + 'agent_is_trusted': hasattr(request, 'agent') and request.agent.is_trusted, + 'ip': bid_main.utils.get_client_ip(request), + 'user': user, + 'last_active_at': timezone.now(), + 'user_agent': request.headers.get("User-Agent", "")[:255], + }, + ) + + def __str__(self): + return f'UserSession pk={self.pk} for {self.user}' diff --git a/bid_main/signals.py b/bid_main/signals.py index 8814566..c424acc 100644 --- a/bid_main/signals.py +++ b/bid_main/signals.py @@ -19,7 +19,7 @@ def log_exception(sender, **kwargs): @receiver(user_logged_in) -def update_user_for_login(sender, request, user, **kwargs): +def process_new_login(sender, request, user, **kwargs): """Updates user fields upon login. Only saves specific fields, so that the webhook trigger knows what changed. @@ -37,6 +37,7 @@ def update_user_for_login(sender, request, user, **kwargs): fields.update({"last_login_ip", "current_login_ip"}) user.save(update_fields=fields) + models.UserSession.update_or_create_from_request(request, user) @receiver(m2m_changed) diff --git a/blenderid/settings.py b/blenderid/settings.py index 611f0f6..49b502e 100644 --- a/blenderid/settings.py +++ b/blenderid/settings.py @@ -64,6 +64,7 @@ INSTALLED_APPS = [ "bid_api", "bid_addon_support", "background_task", + "django_agent_trust", ] MIDDLEWARE = [ @@ -72,6 +73,8 @@ MIDDLEWARE = [ "django.middleware.common.CommonMiddleware", "django.middleware.csrf.CsrfViewMiddleware", "django.contrib.auth.middleware.AuthenticationMiddleware", + "django_agent_trust.middleware.AgentMiddleware", + "bid_main.middleware.user_session_middleware", "django.contrib.messages.middleware.MessageMiddleware", "django.middleware.clickjacking.XFrameOptionsMiddleware", "oauth2_provider.middleware.OAuth2TokenMiddleware", diff --git a/requirements.txt b/requirements.txt index f8ccc8e..3eabd49 100644 --- a/requirements.txt +++ b/requirements.txt @@ -10,6 +10,7 @@ csscompressor==0.9.5 ; python_version >= "3.8" and python_version < "4" deprecated==1.2.14 ; python_version >= "3.8" and python_version < "4" dj-database-url==2.2.0 django-admin-select2==1.0.1 ; python_version >= "3.8" and python_version < "4" +django-agent-trust==1.1.0 django-background-tasks-updated @ git+https://projects.blender.org/infrastructure/django-background-tasks.git@1.2.10 django-compat==1.0.15 ; python_version >= "3.8" and python_version < "4" django-loginas==0.3.11 ; python_version >= "3.8" and python_version < "4" -- 2.30.2 From f6ecf235a5762d4d681e799bcd92575efab65fcb Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Tue, 30 Jul 2024 16:13:02 +0200 Subject: [PATCH 02/17] add test --- bid_main/tests/test_user_sessions.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 bid_main/tests/test_user_sessions.py diff --git a/bid_main/tests/test_user_sessions.py b/bid_main/tests/test_user_sessions.py new file mode 100644 index 0000000..a6beb8e --- /dev/null +++ b/bid_main/tests/test_user_sessions.py @@ -0,0 +1,16 @@ +from django.test import TestCase +from django.test.client import Client + +from bid_main.tests.factories import UserFactory + + +class TestUserSessions(TestCase): + def test_records_created_and_delete(self): + user = UserFactory() + client1 = Client() + client2 = Client() + client1.force_login(user) + client2.force_login(user) + self.assertEqual(user.sessions.count(), 2) + client2.logout() + self.assertEqual(user.sessions.count(), 1) -- 2.30.2 From 0662506ffe074c9b477d2cd4830cb34c4ea0a817 Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Tue, 30 Jul 2024 16:42:33 +0200 Subject: [PATCH 03/17] assume we always have a session --- bid_main/models.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/bid_main/models.py b/bid_main/models.py index 27d5c7d..26517ff 100644 --- a/bid_main/models.py +++ b/bid_main/models.py @@ -669,8 +669,6 @@ class UserSession(models.Model): def update_or_create_from_request(cls, request, user=None): if not user: user = request.user - if not request.session.session_key: - request.session.save() return cls.objects.update_or_create( session_id=request.session.session_key, defaults={ -- 2.30.2 From 6bc5d1eccdb9a1e80fa5d3294adb6ab51955773d Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Thu, 1 Aug 2024 10:49:41 +0200 Subject: [PATCH 04/17] send email about a login from a new IP --- bid_main/email.py | 39 +++++++++++++++++++ bid_main/models.py | 8 ++++ bid_main/signals.py | 9 +++-- .../bid_main/emails/new_user_session.txt | 15 +++++++ requirements.txt | 1 + 5 files changed, 69 insertions(+), 3 deletions(-) create mode 100644 bid_main/templates/bid_main/emails/new_user_session.txt diff --git a/bid_main/email.py b/bid_main/email.py index dc9afa4..4aa8646 100644 --- a/bid_main/email.py +++ b/bid_main/email.py @@ -257,3 +257,42 @@ def check_verification_payload( log.debug("verification OK") return VerificationResult.OK, payload + + +def send_new_user_session(session): + if not hasattr(session, 'user'): + log.error('programming error: called for a session without a user') + return False + user = session.user + + # sending only a text/plain email to reduce the room for look-alike phishing emails + email_body_txt, subject = construct_new_user_session(session) + + email = user.email + try: + send_mail( + subject, + message=email_body_txt, + from_email=None, # just use the configured default From-address. + recipient_list=[email], + fail_silently=False, + ) + except (smtplib.SMTPException, OSError): + log.exception("failed to send a new user session email for account %s", user.pk) + return False + log.info("sent a new user session email for account %s", user.pk) + return True + + +def construct_new_user_session(session): + context = { + "session": session, + "user": session.user, + "subject": "Blender ID new sign-in", + } + + email_body_txt = loader.render_to_string( + "bid_main/emails/new_user_session.txt", context + ) + + return email_body_txt, context["subject"] diff --git a/bid_main/models.py b/bid_main/models.py index 26517ff..4fc25cc 100644 --- a/bid_main/models.py +++ b/bid_main/models.py @@ -19,6 +19,7 @@ from django.utils import timezone from django.utils.deconstruct import deconstructible from django.utils.translation import gettext_lazy as _ import oauth2_provider.models as oa2_models +import user_agents from . import fields from . import hashers @@ -682,3 +683,10 @@ class UserSession(models.Model): def __str__(self): return f'UserSession pk={self.pk} for {self.user}' + + @property + def device(self): + if self.user_agent: + return user_agents.parse(self.user_agent) + else: + return None diff --git a/bid_main/signals.py b/bid_main/signals.py index c424acc..61b40c9 100644 --- a/bid_main/signals.py +++ b/bid_main/signals.py @@ -6,7 +6,7 @@ from django.db.models import F from django.db.models.signals import m2m_changed, post_delete from django.dispatch import receiver -from . import models +from . import email, models import bid_main.utils as utils import bid_main.file_utils @@ -20,7 +20,7 @@ def log_exception(sender, **kwargs): @receiver(user_logged_in) def process_new_login(sender, request, user, **kwargs): - """Updates user fields upon login. + """Updates user fields and creates a UserSession upon login. Sends and email if IP is new. Only saves specific fields, so that the webhook trigger knows what changed. """ @@ -33,8 +33,11 @@ def process_new_login(sender, request, user, **kwargs): if request_ip and user.current_login_ip != request_ip: user.last_login_ip = F("current_login_ip") user.current_login_ip = request_ip - fields.update({"last_login_ip", "current_login_ip"}) + try: + email.send_new_user_session(request.session.create_model_instance({})) + except Exception: + log.exception('failed to send a new user session email') user.save(update_fields=fields) models.UserSession.update_or_create_from_request(request, user) diff --git a/bid_main/templates/bid_main/emails/new_user_session.txt b/bid_main/templates/bid_main/emails/new_user_session.txt new file mode 100644 index 0000000..a3cdb06 --- /dev/null +++ b/bid_main/templates/bid_main/emails/new_user_session.txt @@ -0,0 +1,15 @@ +{% autoescape off %} +Dear {{ user.full_name|default:user.email }}! + +A new sign-in for your Blender ID account {{ user.email }} + +IP address: {{ session.ip }} +Device: {{ session.device }} + +If this was you, you can ignore this message. +If this wasn't you, please change or reset your password. + +-- +Kind regards, +The Blender Web Team +{% endautoescape %} diff --git a/requirements.txt b/requirements.txt index 3eabd49..6820b25 100644 --- a/requirements.txt +++ b/requirements.txt @@ -51,6 +51,7 @@ sorl-thumbnail==12.7.0 ; python_version >= "3.8" and python_version < "4" sqlparse==0.5.0 ; python_version >= "3.8" and python_version < "4" tornado==6.0.3 ; python_version >= "3.8" and python_version < "4" urllib3==1.25.11 ; python_version >= "3.8" and python_version < "4" +user-agents==2.2.0 uwsgi==2.0.23 wrapt==1.15.0 ; python_version >= "3.8" and python_version < "4" zipp==0.6.0 ; python_version >= "3.8" and python_version < "4" -- 2.30.2 From 1045f83da38287f7f4f316eb67c696c9dd0ca56a Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Thu, 1 Aug 2024 11:11:28 +0200 Subject: [PATCH 05/17] reorder columns in migration --- bid_main/migrations/0046_usersession.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/bid_main/migrations/0046_usersession.py b/bid_main/migrations/0046_usersession.py index a1dd292..dcd3a46 100644 --- a/bid_main/migrations/0046_usersession.py +++ b/bid_main/migrations/0046_usersession.py @@ -17,19 +17,8 @@ class Migration(migrations.Migration): name='UserSession', fields=[ ('id', models.BigAutoField(primary_key=True, serialize=False)), - ('agent_is_trusted', models.BooleanField(default=False)), ('created_at', models.DateTimeField(auto_now_add=True)), ('last_active_at', models.DateTimeField()), - ('ip', models.GenericIPAddressField(blank=True, null=True)), - ('user_agent', models.CharField(blank=True, max_length=255)), - ( - 'session', - models.OneToOneField( - editable=False, - on_delete=django.db.models.deletion.CASCADE, - to='sessions.session', - ), - ), ( 'user', models.ForeignKey( @@ -38,6 +27,17 @@ class Migration(migrations.Migration): to=settings.AUTH_USER_MODEL, ), ), + ('agent_is_trusted', models.BooleanField(default=False)), + ('ip', models.GenericIPAddressField(blank=True, null=True)), + ( + 'session', + models.OneToOneField( + editable=False, + on_delete=django.db.models.deletion.CASCADE, + to='sessions.session', + ), + ), + ('user_agent', models.CharField(blank=True, max_length=255)), ], ), ] -- 2.30.2 From a208437b940c6c40fd2df3d6d81306185a32841b Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Thu, 1 Aug 2024 12:35:37 +0200 Subject: [PATCH 06/17] active sessions page --- .../templates/bid_main/active_sessions.html | 44 +++++++++++++++++++ bid_main/templates/bid_main/index.html | 5 +++ bid_main/tests/test_user_sessions.py | 29 +++++++++++- bid_main/urls.py | 6 +++ bid_main/views/normal_pages.py | 28 +++++++++++- blenderid/settings.py | 1 + 6 files changed, 110 insertions(+), 3 deletions(-) create mode 100644 bid_main/templates/bid_main/active_sessions.html diff --git a/bid_main/templates/bid_main/active_sessions.html b/bid_main/templates/bid_main/active_sessions.html new file mode 100644 index 0000000..4127f85 --- /dev/null +++ b/bid_main/templates/bid_main/active_sessions.html @@ -0,0 +1,44 @@ +{% extends 'layout.html' %} +{% load humanize pipeline static %} +{% block page_title %} +Active Sessions +{% endblock %} + +{% block body %} +
+

Active Sessions

+ + + + + + + + + + + + {% for session in sessions %} + + + + + + + + {% endfor %} + +
CreatedLast ActiveIPDevice
{{ session.created_at|naturaltime }} + {% if session.is_current %} + Current Session + {% else %} + {{ session.last_active_at|naturaltime }} + {% endif %} + {{ session.ip }}{{ session.device }} +
+ {% csrf_token %} + +
+
+
+{% endblock %} diff --git a/bid_main/templates/bid_main/index.html b/bid_main/templates/bid_main/index.html index b49e268..4ab17a3 100644 --- a/bid_main/templates/bid_main/index.html +++ b/bid_main/templates/bid_main/index.html @@ -135,6 +135,11 @@ Profile

Account

+
+ + Active Sessions + +
Change Password diff --git a/bid_main/tests/test_user_sessions.py b/bid_main/tests/test_user_sessions.py index a6beb8e..fe40cd4 100644 --- a/bid_main/tests/test_user_sessions.py +++ b/bid_main/tests/test_user_sessions.py @@ -1,5 +1,6 @@ -from django.test import TestCase from django.test.client import Client +from django.test import TestCase +from django.urls import reverse from bid_main.tests.factories import UserFactory @@ -14,3 +15,29 @@ class TestUserSessions(TestCase): self.assertEqual(user.sessions.count(), 2) client2.logout() self.assertEqual(user.sessions.count(), 1) + + +class TestActiveSessions(TestCase): + def test_active_sessions(self): + user = UserFactory() + client1 = Client() + client2 = Client() + client1.force_login(user) + first_session_pk = user.sessions.first().pk + client2.force_login(user) + response = client1.get(reverse('bid_main:active_sessions')) + self.assertContains(response, 'Current Session') + self.assertContains(response, 'Terminate Session') + + response = client2.post( + reverse('bid_main:terminate_session', kwargs={'pk': first_session_pk}) + ) + self.assertEqual(response.status_code, 302) + + response = client2.get(reverse('bid_main:active_sessions')) + self.assertEqual(response.status_code, 200) + + # got logged out, redirect to login + response = client1.get(reverse('bid_main:active_sessions')) + self.assertEqual(response.status_code, 302) + self.assertEqual(response['Location'], '/login?next=/active-sessions/') diff --git a/bid_main/urls.py b/bid_main/urls.py index f860894..7d5ca44 100644 --- a/bid_main/urls.py +++ b/bid_main/urls.py @@ -139,6 +139,12 @@ urlpatterns = [ registration_email.ConfirmEmailPollView.as_view(), name="confirm-email-poll", ), + path('active-sessions/', normal_pages.ActiveSessionsView.as_view(), name='active_sessions'), + path( + 'terminate-session//', + normal_pages.TerminateSessionView.as_view(), + name='terminate_session', + ), ] # Only enable this on a dev server: diff --git a/bid_main/views/normal_pages.py b/bid_main/views/normal_pages.py index 759698f..81586c4 100644 --- a/bid_main/views/normal_pages.py +++ b/bid_main/views/normal_pages.py @@ -14,8 +14,8 @@ from django.core.exceptions import ValidationError from django.utils.translation import gettext as _ from django.db import transaction, IntegrityError from django.db.models import Count -from django.http import HttpResponseRedirect -from django.shortcuts import resolve_url, render +from django.http import HttpResponseNotFound, HttpResponseRedirect +from django.shortcuts import redirect, resolve_url, render from django.urls import reverse_lazy from django.utils import timezone from django.utils.decorators import method_decorator @@ -23,6 +23,7 @@ from django.views.decorators.cache import never_cache from django.views.decorators.csrf import csrf_exempt from django.views.decorators.debug import sensitive_post_parameters from django.views.generic import TemplateView, FormView +from django.views.generic.base import View from django.views.generic.edit import UpdateView import loginas.utils import oauth2_provider.models as oauth2_models @@ -429,3 +430,26 @@ class DeleteUserView( if not ok: log.error("Failed to send an email about deletion of account %s", user.pk) return render(self.request, "bid_main/delete_user/confirm.html", context=ctx) + + +class ActiveSessionsView(LoginRequiredMixin, TemplateView): + template_name = "bid_main/active_sessions.html" + + def get_context_data(self, **kwargs): + user_sessions = self.request.user.sessions.order_by('-last_active_at') + for session in user_sessions: + if session.session.session_key == self.request.session.session_key: + session.is_current = True + return { + **super().get_context_data(**kwargs), + 'sessions': user_sessions, + } + + +class TerminateSessionView(LoginRequiredMixin, View): + def post(self, request, *args, **kwargs): + if user_session := self.request.user.sessions.filter(pk=kwargs.get('pk', 0)).first(): + user_session.session.delete() + user_session.delete() + return redirect('bid_main:active_sessions') + return HttpResponseNotFound("session not found") diff --git a/blenderid/settings.py b/blenderid/settings.py index 49b502e..99e75e4 100644 --- a/blenderid/settings.py +++ b/blenderid/settings.py @@ -50,6 +50,7 @@ INSTALLED_APPS = [ "django.contrib.admindocs", "django.contrib.auth", "django.contrib.contenttypes", + "django.contrib.humanize", "django.contrib.sessions", "django.contrib.messages", "django.contrib.staticfiles", -- 2.30.2 From 392952e6bd37340cb74cc78a2c83cbd091a6f9fd Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Thu, 1 Aug 2024 13:41:54 +0200 Subject: [PATCH 07/17] delete UserSession records on user.anonymize --- bid_main/models.py | 1 + 1 file changed, 1 insertion(+) diff --git a/bid_main/models.py b/bid_main/models.py index 4fc25cc..6dd59b0 100644 --- a/bid_main/models.py +++ b/bid_main/models.py @@ -495,6 +495,7 @@ class User(AbstractBaseUser, PermissionsMixin): is_active=False, avatar=None, ) + self.sessions.all().delete() @classmethod def generate_nickname(cls, email: Optional[str] = '', full_name: Optional[str] = '') -> str: -- 2.30.2 From 5b787841446e380c2715a95ca8c372a103ec23ff Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Thu, 1 Aug 2024 14:49:57 +0200 Subject: [PATCH 08/17] Check that email is confirmed before sending a new user session email --- bid_main/signals.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/bid_main/signals.py b/bid_main/signals.py index 61b40c9..468898b 100644 --- a/bid_main/signals.py +++ b/bid_main/signals.py @@ -28,19 +28,19 @@ def process_new_login(sender, request, user, **kwargs): user.login_count = F("login_count") + 1 fields = {"login_count"} + user_session, _ = models.UserSession.update_or_create_from_request(request, user) # Only move 'current' to 'last' login IP if the IP address is different. request_ip = utils.get_client_ip(request) if request_ip and user.current_login_ip != request_ip: user.last_login_ip = F("current_login_ip") user.current_login_ip = request_ip fields.update({"last_login_ip", "current_login_ip"}) - try: - email.send_new_user_session(request.session.create_model_instance({})) - except Exception: - log.exception('failed to send a new user session email') - + if user.has_confirmed_email: + try: + email.send_new_user_session(user_session) + except Exception: + log.exception('failed to send a new user session email') user.save(update_fields=fields) - models.UserSession.update_or_create_from_request(request, user) @receiver(m2m_changed) -- 2.30.2 From 1f4e4db0d6c4b6fce4dde44cbde443d83d1f32f5 Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Fri, 2 Aug 2024 11:46:54 +0200 Subject: [PATCH 09/17] improve readability --- bid_main/models.py | 3 +-- bid_main/views/normal_pages.py | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bid_main/models.py b/bid_main/models.py index 6dd59b0..620af0c 100644 --- a/bid_main/models.py +++ b/bid_main/models.py @@ -689,5 +689,4 @@ class UserSession(models.Model): def device(self): if self.user_agent: return user_agents.parse(self.user_agent) - else: - return None + return None diff --git a/bid_main/views/normal_pages.py b/bid_main/views/normal_pages.py index 81586c4..b836079 100644 --- a/bid_main/views/normal_pages.py +++ b/bid_main/views/normal_pages.py @@ -448,7 +448,8 @@ class ActiveSessionsView(LoginRequiredMixin, TemplateView): class TerminateSessionView(LoginRequiredMixin, View): def post(self, request, *args, **kwargs): - if user_session := self.request.user.sessions.filter(pk=kwargs.get('pk', 0)).first(): + user_session_pk = kwargs.get('pk') + if user_session := self.request.user.sessions.filter(pk=user_session_pk).first(): user_session.session.delete() user_session.delete() return redirect('bid_main:active_sessions') -- 2.30.2 From d3a6bc2556d8a57c1b0b951e3fdb17c784c33ec4 Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Fri, 2 Aug 2024 15:21:22 +0200 Subject: [PATCH 10/17] move email notification to a background task --- bid_main/email.py | 31 ++--------------- bid_main/signals.py | 15 +++++--- bid_main/tasks.py | 28 +++++++++++++++ .../bid_main/emails/new_user_session.txt | 4 +-- bid_main/tests/test_user_sessions.py | 34 +++++++++++++++++++ 5 files changed, 77 insertions(+), 35 deletions(-) create mode 100644 bid_main/tasks.py diff --git a/bid_main/email.py b/bid_main/email.py index 4aa8646..816ede6 100644 --- a/bid_main/email.py +++ b/bid_main/email.py @@ -259,35 +259,10 @@ def check_verification_payload( return VerificationResult.OK, payload -def send_new_user_session(session): - if not hasattr(session, 'user'): - log.error('programming error: called for a session without a user') - return False - user = session.user - - # sending only a text/plain email to reduce the room for look-alike phishing emails - email_body_txt, subject = construct_new_user_session(session) - - email = user.email - try: - send_mail( - subject, - message=email_body_txt, - from_email=None, # just use the configured default From-address. - recipient_list=[email], - fail_silently=False, - ) - except (smtplib.SMTPException, OSError): - log.exception("failed to send a new user session email for account %s", user.pk) - return False - log.info("sent a new user session email for account %s", user.pk) - return True - - -def construct_new_user_session(session): +def construct_new_user_session(user, session_data): context = { - "session": session, - "user": session.user, + "session_data": session_data, + "user": user, "subject": "Blender ID new sign-in", } diff --git a/bid_main/signals.py b/bid_main/signals.py index 468898b..787e521 100644 --- a/bid_main/signals.py +++ b/bid_main/signals.py @@ -6,9 +6,10 @@ from django.db.models import F from django.db.models.signals import m2m_changed, post_delete from django.dispatch import receiver -from . import email, models +from . import models import bid_main.utils as utils import bid_main.file_utils +import bid_main.tasks log = logging.getLogger(__name__) @@ -35,11 +36,15 @@ def process_new_login(sender, request, user, **kwargs): user.last_login_ip = F("current_login_ip") user.current_login_ip = request_ip fields.update({"last_login_ip", "current_login_ip"}) + if user.has_confirmed_email: - try: - email.send_new_user_session(user_session) - except Exception: - log.exception('failed to send a new user session email') + bid_main.tasks.send_new_user_session( + user_pk=user.pk, + session_data={ + 'device': str(user_session.device or 'Unknown'), + 'ip': user_session.ip, + }, + ) user.save(update_fields=fields) diff --git a/bid_main/tasks.py b/bid_main/tasks.py new file mode 100644 index 0000000..d247cc2 --- /dev/null +++ b/bid_main/tasks.py @@ -0,0 +1,28 @@ +import logging + +from background_task import background +from background_task.tasks import TaskSchedule +from django.core.mail import send_mail + +from bid_main.models import User +import bid_main.email + + +log = logging.getLogger(__name__) + + +@background(schedule={'action': TaskSchedule.RESCHEDULE_EXISTING}) +def send_new_user_session(user_pk, session_data): + user = User.objects.get(pk=user_pk) + log.info("sending a new user session email for account %s", user.pk) + + # sending only a text/plain email to reduce the room for look-alike phishing emails + email_body_txt, subject = bid_main.email.construct_new_user_session(user, session_data) + + email = user.email + send_mail( + subject=subject, + message=email_body_txt, + from_email=None, # just use the configured default From-address. + recipient_list=[email], + ) diff --git a/bid_main/templates/bid_main/emails/new_user_session.txt b/bid_main/templates/bid_main/emails/new_user_session.txt index a3cdb06..32e9d3a 100644 --- a/bid_main/templates/bid_main/emails/new_user_session.txt +++ b/bid_main/templates/bid_main/emails/new_user_session.txt @@ -3,8 +3,8 @@ Dear {{ user.full_name|default:user.email }}! A new sign-in for your Blender ID account {{ user.email }} -IP address: {{ session.ip }} -Device: {{ session.device }} +IP address: {{ session_data.ip }} +Device: {{ session_data.device }} If this was you, you can ignore this message. If this wasn't you, please change or reset your password. diff --git a/bid_main/tests/test_user_sessions.py b/bid_main/tests/test_user_sessions.py index fe40cd4..088277f 100644 --- a/bid_main/tests/test_user_sessions.py +++ b/bid_main/tests/test_user_sessions.py @@ -1,8 +1,13 @@ +from unittest.mock import patch + +from django.core import mail from django.test.client import Client from django.test import TestCase from django.urls import reverse +from django.utils import timezone from bid_main.tests.factories import UserFactory +import bid_main.tasks class TestUserSessions(TestCase): @@ -41,3 +46,32 @@ class TestActiveSessions(TestCase): response = client1.get(reverse('bid_main:active_sessions')) self.assertEqual(response.status_code, 302) self.assertEqual(response['Location'], '/login?next=/active-sessions/') + + +class TestNewUserSessionEmail(TestCase): + @patch( + 'bid_main.tasks.send_new_user_session', + new=bid_main.tasks.send_new_user_session.task_function, + ) + @patch( + 'django.contrib.auth.base_user.AbstractBaseUser.check_password', + new=lambda _, pwd: pwd == 'hunter2', + ) + def test_new_user_session_email(self): + user = UserFactory(confirmed_email_at=timezone.now()) + client1 = Client() + client1.force_login(user) + + # force_login doesn't set custom env for HttpRequest, so sending a real POST instead + client2 = Client(REMOTE_ADDR='127.0.1.1') + ua = 'Mozilla/5.0 (X11; Linux x86_64; rv:128.0) Gecko/20100101 Firefox/128.0' + response = client2.post( + '/login', + {'username': user.email, 'password': 'hunter2'}, + headers={'user-agent': ua}, + ) + self.assertEqual(response.status_code, 302) + sent_email = mail.outbox.pop() + self.assertEqual(sent_email.to[0], user.email) + self.assertIn('IP address: 127.0.1.1', sent_email.body) + self.assertIn('Device: PC / Linux / Firefox 128.0', sent_email.body) -- 2.30.2 From e48b15c7c9e72b5883e6016140e6f22c885a3f5d Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Fri, 2 Aug 2024 15:37:15 +0200 Subject: [PATCH 11/17] remove agent_is_trusted from this branch --- bid_main/migrations/0046_usersession.py | 1 - bid_main/models.py | 2 -- blenderid/settings.py | 2 -- requirements.txt | 1 - 4 files changed, 6 deletions(-) diff --git a/bid_main/migrations/0046_usersession.py b/bid_main/migrations/0046_usersession.py index dcd3a46..9c5e2eb 100644 --- a/bid_main/migrations/0046_usersession.py +++ b/bid_main/migrations/0046_usersession.py @@ -27,7 +27,6 @@ class Migration(migrations.Migration): to=settings.AUTH_USER_MODEL, ), ), - ('agent_is_trusted', models.BooleanField(default=False)), ('ip', models.GenericIPAddressField(blank=True, null=True)), ( 'session', diff --git a/bid_main/models.py b/bid_main/models.py index 620af0c..6e458d1 100644 --- a/bid_main/models.py +++ b/bid_main/models.py @@ -654,7 +654,6 @@ class UserNote(models.Model): class UserSession(models.Model): id = models.BigAutoField(primary_key=True) - agent_is_trusted = models.BooleanField(default=False, null=False) created_at = models.DateTimeField(auto_now_add=True) last_active_at = models.DateTimeField() ip = models.GenericIPAddressField(null=True, blank=True) @@ -674,7 +673,6 @@ class UserSession(models.Model): return cls.objects.update_or_create( session_id=request.session.session_key, defaults={ - 'agent_is_trusted': hasattr(request, 'agent') and request.agent.is_trusted, 'ip': bid_main.utils.get_client_ip(request), 'user': user, 'last_active_at': timezone.now(), diff --git a/blenderid/settings.py b/blenderid/settings.py index 99e75e4..1ce1500 100644 --- a/blenderid/settings.py +++ b/blenderid/settings.py @@ -65,7 +65,6 @@ INSTALLED_APPS = [ "bid_api", "bid_addon_support", "background_task", - "django_agent_trust", ] MIDDLEWARE = [ @@ -74,7 +73,6 @@ MIDDLEWARE = [ "django.middleware.common.CommonMiddleware", "django.middleware.csrf.CsrfViewMiddleware", "django.contrib.auth.middleware.AuthenticationMiddleware", - "django_agent_trust.middleware.AgentMiddleware", "bid_main.middleware.user_session_middleware", "django.contrib.messages.middleware.MessageMiddleware", "django.middleware.clickjacking.XFrameOptionsMiddleware", diff --git a/requirements.txt b/requirements.txt index 6820b25..c972bb7 100644 --- a/requirements.txt +++ b/requirements.txt @@ -10,7 +10,6 @@ csscompressor==0.9.5 ; python_version >= "3.8" and python_version < "4" deprecated==1.2.14 ; python_version >= "3.8" and python_version < "4" dj-database-url==2.2.0 django-admin-select2==1.0.1 ; python_version >= "3.8" and python_version < "4" -django-agent-trust==1.1.0 django-background-tasks-updated @ git+https://projects.blender.org/infrastructure/django-background-tasks.git@1.2.10 django-compat==1.0.15 ; python_version >= "3.8" and python_version < "4" django-loginas==0.3.11 ; python_version >= "3.8" and python_version < "4" -- 2.30.2 From 623c7c4cdbc8611eb7228313d686a33fa07a0369 Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Fri, 2 Aug 2024 15:41:08 +0200 Subject: [PATCH 12/17] change email subject --- bid_main/email.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bid_main/email.py b/bid_main/email.py index 816ede6..291a715 100644 --- a/bid_main/email.py +++ b/bid_main/email.py @@ -263,7 +263,7 @@ def construct_new_user_session(user, session_data): context = { "session_data": session_data, "user": user, - "subject": "Blender ID new sign-in", + "subject": "Security alert: new sign-in", } email_body_txt = loader.render_to_string( -- 2.30.2 From 6345c8316b997a6172ec2aef36b069c3657a3487 Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Fri, 2 Aug 2024 15:43:23 +0200 Subject: [PATCH 13/17] subject is not used in template --- bid_main/email.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/bid_main/email.py b/bid_main/email.py index 291a715..c2c8e86 100644 --- a/bid_main/email.py +++ b/bid_main/email.py @@ -263,11 +263,10 @@ def construct_new_user_session(user, session_data): context = { "session_data": session_data, "user": user, - "subject": "Security alert: new sign-in", } email_body_txt = loader.render_to_string( "bid_main/emails/new_user_session.txt", context ) - return email_body_txt, context["subject"] + return email_body_txt, "Security alert: new sign-in" -- 2.30.2 From 72a75023c88f4eda9daa2650d639dbb3d86ee340 Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Fri, 2 Aug 2024 15:48:31 +0200 Subject: [PATCH 14/17] remore redundant atomic decorator --- bid_main/models.py | 1 - 1 file changed, 1 deletion(-) diff --git a/bid_main/models.py b/bid_main/models.py index 6e458d1..e428345 100644 --- a/bid_main/models.py +++ b/bid_main/models.py @@ -666,7 +666,6 @@ class UserSession(models.Model): ) @classmethod - @transaction.atomic def update_or_create_from_request(cls, request, user=None): if not user: user = request.user -- 2.30.2 From b6fa9192ed8e77c1b4b578fb301662e51809adc2 Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Fri, 2 Aug 2024 15:52:41 +0200 Subject: [PATCH 15/17] improve message grammar --- bid_main/templates/bid_main/emails/new_user_session.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bid_main/templates/bid_main/emails/new_user_session.txt b/bid_main/templates/bid_main/emails/new_user_session.txt index 32e9d3a..bac22b1 100644 --- a/bid_main/templates/bid_main/emails/new_user_session.txt +++ b/bid_main/templates/bid_main/emails/new_user_session.txt @@ -1,7 +1,7 @@ {% autoescape off %} Dear {{ user.full_name|default:user.email }}! -A new sign-in for your Blender ID account {{ user.email }} +There was a new sign-in to your Blender ID account {{ user.email }} IP address: {{ session_data.ip }} Device: {{ session_data.device }} -- 2.30.2 From 8aed6094bec9c86ff3fba2b8a96e1dd74f3ce74c Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Fri, 2 Aug 2024 15:55:51 +0200 Subject: [PATCH 16/17] readability --- bid_main/email.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bid_main/email.py b/bid_main/email.py index c2c8e86..66e0305 100644 --- a/bid_main/email.py +++ b/bid_main/email.py @@ -264,9 +264,9 @@ def construct_new_user_session(user, session_data): "session_data": session_data, "user": user, } - email_body_txt = loader.render_to_string( "bid_main/emails/new_user_session.txt", context ) + subject = "Security alert: new sign-in" - return email_body_txt, "Security alert: new sign-in" + return email_body_txt, subject -- 2.30.2 From 5177bf3b518ee3e8567d9bd08fadda5ab436c361 Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Fri, 2 Aug 2024 15:59:35 +0200 Subject: [PATCH 17/17] improve naming --- bid_main/signals.py | 2 +- bid_main/tasks.py | 2 +- bid_main/tests/test_user_sessions.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/bid_main/signals.py b/bid_main/signals.py index 787e521..1e9b443 100644 --- a/bid_main/signals.py +++ b/bid_main/signals.py @@ -38,7 +38,7 @@ def process_new_login(sender, request, user, **kwargs): fields.update({"last_login_ip", "current_login_ip"}) if user.has_confirmed_email: - bid_main.tasks.send_new_user_session( + bid_main.tasks.send_new_user_session_email( user_pk=user.pk, session_data={ 'device': str(user_session.device or 'Unknown'), diff --git a/bid_main/tasks.py b/bid_main/tasks.py index d247cc2..ebcfe17 100644 --- a/bid_main/tasks.py +++ b/bid_main/tasks.py @@ -12,7 +12,7 @@ log = logging.getLogger(__name__) @background(schedule={'action': TaskSchedule.RESCHEDULE_EXISTING}) -def send_new_user_session(user_pk, session_data): +def send_new_user_session_email(user_pk, session_data): user = User.objects.get(pk=user_pk) log.info("sending a new user session email for account %s", user.pk) diff --git a/bid_main/tests/test_user_sessions.py b/bid_main/tests/test_user_sessions.py index 088277f..d4034d4 100644 --- a/bid_main/tests/test_user_sessions.py +++ b/bid_main/tests/test_user_sessions.py @@ -50,8 +50,8 @@ class TestActiveSessions(TestCase): class TestNewUserSessionEmail(TestCase): @patch( - 'bid_main.tasks.send_new_user_session', - new=bid_main.tasks.send_new_user_session.task_function, + 'bid_main.tasks.send_new_user_session_email', + new=bid_main.tasks.send_new_user_session_email.task_function, ) @patch( 'django.contrib.auth.base_user.AbstractBaseUser.check_password', -- 2.30.2