WIP

src/core/logic.py

@@ -11,13 +11,14 @@
 import operator
 import re
 from functools import reduce
+from urllib.parse import unquote, urlparse
 
 from django.conf import settings
 from django.contrib.auth import logout
 from django.contrib import messages
 from django.template.loader import get_template
 from django.db.models import Q
-from django.http import JsonResponse
+from django.http import JsonResponse, QueryDict
 from django.forms.models import model_to_dict
 from django.shortcuts import reverse
 from django.utils import timezone
@@ -35,81 +36,117 @@
 logger = get_logger(__name__)
 
 
+def get_raw_next_url(next_url, request):
+    """
+    Get the next_url passed in or the 'next' on the request, as raw unicode.
+    :param next_url: an optional string with the path and query parts of
+                     a destination URL -- overrides any 'next' in request data
+    :param request: HttpRequest, optionally containing 'next' in GET or POST
+    """
+    if not next_url:
+        next_url = request.GET.get('next', '') or request.POST.get('next', '')
+    return unquote(next_url)
+
+
+def reverse_with_next(url_name, request, next_url='', args=None, kwargs=None):
+    """
+    Reverse a URL but keep the 'next' parameter that exists on the request
+    or that the caller wants to introduce.
+    The value of 'next' on the request or 'next_url' can be in raw unicode or
+    it can have been percent-encoded one time.
+    :param request: HttpRequest, optionally containing 'next' in GET or POST
+    :param next_url: an optional string with the path and query parts of
+                     a destination URL -- overrides any 'next' in request data
+    :param args: args to pass to django.shortcuts.reverse, if no kwargs
+    :param kwargs: kwargs to pass to django.shortcuts.reverse, if no args
+    """
+    # reverse can only accept either args or kwargs
+    if args:
+        reversed_url = reverse(url_name, args=args)
+    elif kwargs:
+        reversed_url = reverse(url_name, kwargs=kwargs)
+    else:
+        reversed_url = reverse(url_name)
+
+    raw_next_url = get_raw_next_url(next_url, request)
+
+    if not raw_next_url:
+        return reversed_url
+
+    if reversed_url == raw_next_url:
+        # Avoid circular next URLs
+        return reversed_url
+
+    # Parse the reversed URL string enough to safely update the query parameters.
+    # Then re-encode them into a query string and generate the final URL.
+    parsed_url = urlparse(reversed_url) # ParseResult
+    parsed_query = QueryDict(parsed_url.query, mutable=True) # mutable QueryDict
+    parsed_query.update({'next': raw_next_url})
+    # We treat / as safe to match the default behavior
+    # of the |urlencode template filter,
+    # which is where many next URLs are created
+    new_query_string = parsed_query.urlencode(safe="/") # Full percent-encoded query string
+    return parsed_url._replace(query=new_query_string).geturl()
+
+
 def send_reset_token(request, reset_token):
     core_reset_password_url = request.site_type.site_url(
         reverse(
             'core_reset_password',
             kwargs={'token': reset_token.token},
-        )
+        ),
+        query={'next': get_raw_next_url('', request)},
     )
     context = {
         'reset_token': reset_token,
         'core_reset_password_url': core_reset_password_url,
     }
     log_dict = {'level': 'Info', 'types': 'Reset Token', 'target': None}
-    if not request.journal:
-        message = render_template.get_message_content(
-            request,
-            context,
-            request.press.password_reset_text,
-            template_is_setting=True,
-        )
-    else:
-        message = render_template.get_message_content(
-            request,
-            context,
-            'password_reset',
-        )
-
-    subject = 'subject_password_reset'
-
-    notify_helpers.send_email_with_body_from_user(
+    notify_helpers.send_email_with_body_from_setting_template(
         request,
-        subject,
+        'password_reset',
+        'subject_password_reset',
         reset_token.account.email,
-        message,
+        context,
         log_dict=log_dict,
     )
 
 
-def send_confirmation_link(request, new_user):
-    core_confirm_account_url = request.site_type.site_url(
+def get_confirm_account_url(request, user, next_url=''):
+    return request.site_type.site_url(
         reverse(
             'core_confirm_account',
-            kwargs={'token': new_user.confirmation_code},
-        )
+            kwargs={'token': user.confirmation_code},
+        ),
+        query={'next': get_raw_next_url(next_url, request)},
     )
+
+
+def send_confirmation_link(request, new_user):
+    core_confirm_account_url = get_confirm_account_url(request, new_user)
+    if request.journal:
+        site_name = request.journal.name
+    elif request.repository:
+        site_name = request.repository.name
+    else:
+        site_name = request.press.name
     context = {
         'user': new_user,
+        'site_name': site_name,
         'core_confirm_account_url': core_confirm_account_url,
     }
-    if not request.journal:
-        message = render_template.get_message_content(
-            request,
-            context,
-            request.press.registration_text,
-            template_is_setting=True,
-        )
-    else:
-        message = render_template.get_message_content(
-            request,
-            context,
-            'new_user_registration',
-        )
-
-    subject = 'subject_new_user_registration'
-
     notify_helpers.send_slack(
         request,
         'New registration: {0}'.format(new_user.full_name()),
         ['slack_admins'],
     )
     log_dict = {'level': 'Info', 'types': 'Account Confirmation', 'target': None}
-    notify_helpers.send_email_with_body_from_user(
+    notify_helpers.send_email_with_body_from_setting_template(
         request,
-        subject,
+        'new_user_registration',
+        'subject_new_user_registration',
         new_user.email,
-        message,
+        context,
         log_dict=log_dict,
     )
 
@@ -644,20 +681,18 @@ def handle_article_thumb_image_file(uploaded_file, article, request):
         article.save()
 
 
-def handle_email_change(request, email_address):
+def handle_email_change(request, email_address, next_url=''):
     request.user.email = email_address
     request.user.is_active = False
     request.user.confirmation_code = uuid.uuid4()
     request.user.clean()
     request.user.save()
 
-    core_confirm_account_url = request.site_type.site_url(
-        reverse(
-            'core_confirm_account',
-            kwargs={'token': request.user.confirmation_code},
-        )
+    core_confirm_account_url = get_confirm_account_url(
+        request,
+        request.user,
+        next_url=next_url,
     )
-
     context = {
         'user': request.user,
         'core_confirm_account_url': core_confirm_account_url,
@@ -868,7 +903,7 @@ def check_for_bad_login_attempts(request):
     time = timezone.now() - timedelta(minutes=10)
 
     attempts = models.LoginAttempt.objects.filter(user_agent=user_agent, ip_address=ip_address, timestamp__gte=time)
-    print(time, attempts.count())
+    logger.debug(f'Bad login attempt {attempts.count()+1} at {time}')
     return attempts.count()
 
 

src/core/models.py

@@ -255,6 +255,7 @@ class Account(AbstractBaseUser, PermissionsMixin):
         validators=[plain_text_validator],
     )
 
+    # activation_code is deprecated
     activation_code = models.CharField(max_length=100, null=True, blank=True)
     salutation = models.CharField(
         max_length=10,
@@ -294,7 +295,14 @@ class Account(AbstractBaseUser, PermissionsMixin):
     profile_image = models.ImageField(upload_to=profile_images_upload_path, null=True, blank=True, storage=fs, verbose_name=("Profile Image"))
     email_sent = models.DateTimeField(blank=True, null=True)
     date_confirmed = models.DateTimeField(blank=True, null=True)
-    confirmation_code = models.CharField(max_length=200, blank=True, null=True, verbose_name=_("Confirmation Code"))
+    confirmation_code = models.CharField(
+        max_length=200,
+        blank=True,
+        null=True,
+        verbose_name=_("Confirmation Code"),
+        help_text='A UUID created upon registration and retrieved '
+                  'for authentication during account activation',
+    )
     signature = JanewayBleachField(
         blank=True,
         verbose_name=_("Signature"),

src/core/templatetags/next_url.py

@@ -0,0 +1,46 @@
+from django import template
+from django.urls import reverse
+from core.logic import reverse_with_next
+from urllib.parse import quote
+
+
+register = template.Library()
+
+@register.simple_tag(takes_context=True)
+def url_with_next(context, url_name, next_url_name='', *args, **kwargs):
+    """
+    A template tag to use instead of 'url' when you want
+    the reversed URL to include the same 'next' parameter that
+    already exists in the GET or POST data of the request,
+    or you want to introduce a new next url by Django URL name.
+    """
+    if next_url_name:
+        next_url = reverse(next_url_name)
+    else:
+        next_url = ''
+    request = context.get('request')
+    return reverse_with_next(
+        url_name,
+        request,
+        next_url=next_url,
+        *args,
+        **kwargs,
+    )
+
+
+@register.simple_tag(takes_context=True)
+def url_with_return(context, url_name, *args, **kwargs):
+    """
+    A template tag to use instead of 'url' when you want
+    the reversed URL to include a new 'next' parameter that
+    contains the full path of the current request.
+    """
+    request = context.get('request')
+    next_url = quote(request.get_full_path())
+    return reverse_with_next(
+        url_name,
+        request,
+        next_url=next_url,
+        *args,
+        **kwargs,
+    )

src/core/tests/test_logic.py

@@ -1,18 +1,37 @@
+__copyright__ = "Copyright 2024 Birkbeck, University of London"
+__author__ = "Open Library of Humanities"
+__license__ = "AGPL v3"
+__maintainer__ = "Open Library of Humanities"
+
+import uuid
+from mock import patch
+
 from django.test import TestCase
 
 from core import logic
-from core.models import SettingGroup
 from utils.testing import helpers
 
 class TestLogic(TestCase):
-    def setUp(self):
-        self.press = helpers.create_press()
-        self.press.save()
-        self.journal_one, self.journal_two = helpers.create_journals()
-        self.request = helpers.Request()
-        self.request.press = self.press
-        self.request.journal = self.journal_one
-        self.request.site_type = self.journal_one
+
+    @classmethod
+    def setUpTestData(cls):
+        cls.press = helpers.create_press()
+        cls.press.save()
+        cls.journal_one, cls.journal_two = helpers.create_journals()
+        cls.request = helpers.Request()
+        cls.request.press = cls.press
+        cls.request.journal = cls.journal_one
+        cls.request.site_type = cls.journal_one
+        cls.request.GET = {}
+        cls.request.POST = {}
+        cls.inactive_user = helpers.create_user('[email protected]')
+        cls.inactive_user.is_active = False
+        cls.inactive_user.confirmation_code = '8bd3cdc9-1c3c-4ec9-99bc-9ea0b86a3c55'
+        cls.inactive_user.clean()
+        cls.inactive_user.save()
+
+        # The result of passing a URL through the |urlencode template filter
+        cls.next_url_encoded = '/target/page/%3Fq%3Da'
 
     def test_render_nested_settings(self):
         expected_rendered_setting = "<p>For help with Janeway, contact <a href=\"mailto:--No support email set--\">--No support email set--</a>.</p>"
@@ -23,3 +42,44 @@ def test_render_nested_settings(self):
             nested_settings=[('support_email','general')],
         )
         self.assertEqual(expected_rendered_setting, rendered_setting)
+
+    @patch('core.logic.reverse')
+    def test_reverse_with_next_in_get_request(self, mock_reverse):
+        mock_reverse.return_value = '/my/path/?my=params'
+        self.request.GET = {'next': self.next_url_encoded}
+        reversed_url = logic.reverse_with_next('/test/', self.request)
+        self.assertIn(self.next_url_encoded, reversed_url)
+
+    @patch('core.logic.reverse')
+    def test_reverse_with_next_in_post_request(self, mock_reverse):
+        mock_reverse.return_value = '/my/path/?my=params'
+        self.request.POST = {'next': self.next_url_encoded}
+        reversed_url = logic.reverse_with_next('/test/', self.request)
+        self.assertIn(self.next_url_encoded, reversed_url)
+
+    @patch('core.logic.reverse')
+    def test_reverse_with_next_in_kwarg(self, mock_reverse):
+        mock_reverse.return_value = '/my/path/?my=params'
+        reversed_url = logic.reverse_with_next(
+            '/test/',
+            self.request,
+            next_url=self.next_url_encoded,
+        )
+        self.assertIn(self.next_url_encoded, reversed_url)
+
+    @patch('core.logic.reverse')
+    def test_reverse_with_next_no_next(self, mock_reverse):
+        mock_reverse.return_value = '/my/url/?my=params'
+        reversed_url = logic.reverse_with_next('/test/', self.request)
+        self.assertEqual(mock_reverse.return_value, reversed_url)
+
+    def test_get_confirm_account_url(self):
+        url = logic.get_confirm_account_url(
+            self.request,
+            self.inactive_user,
+            next_url=self.next_url_encoded,
+        )
+        self.assertIn(
+            f'/register/step/2/8bd3cdc9-1c3c-4ec9-99bc-9ea0b86a3c55/?next={ self.next_url_encoded }',
+            url,
+        )