feat(accounts,ToU): add version date check, refactor and fix tests

Signed-off-by: David Wallace <[email protected]>

rdmo/accounts/adapter.py

@@ -68,5 +68,5 @@ def signup(self, request, user):
 
         # store the consent field
         if settings.ACCOUNT_TERMS_OF_USE:
-            consent = ConsentFieldValue(user=user, consent=self.cleaned_data['consent'])
-            consent.save()
+            if self.cleaned_data['consent']:
+                ConsentFieldValue.create_consent(user=user, session=request.session)

rdmo/accounts/forms.py

@@ -89,7 +89,7 @@ def __init__(self, *args, **kwargs):
     consent.label = _("I confirm that I want my profile to be completely removed. This can not be undone!")
 
 
-class UpdateConsentForm(forms.Form):
+class AcceptConsentForm(forms.Form):
 
     consent = forms.BooleanField(
         label="I agree to the terms of use",
@@ -103,20 +103,10 @@ def __init__(self, *args, user=None, **kwargs):
         if not self.user:
             raise ValueError("A user instance is required to initialize the form.")
 
-        # pre-fill the 'consent' field based on the user's existing consent
-        # Ensure the user cannot uncheck the consent
-        has_consented = ConsentFieldValue.objects.filter(user=self.user).exists()
-        if has_consented:
-            self.fields["consent"].disabled = True  # Disable field after acceptance
-            self.fields["consent"].initial = True
-        else:
-            self.fields["consent"].initial = False
-
-    def save(self) -> bool:
-        if not ConsentFieldValue.objects.filter(user=self.user).exists():
-            if self.cleaned_data.get("consent"):
-                ConsentFieldValue.objects.create(user=self.user, consent=True)
-                # session consent is updated in the view
-                return True  # consent accepted
-
-        return False  # No changes made
+    def save(self, session) -> bool:
+        if self.cleaned_data['consent']:
+            success = ConsentFieldValue.create_consent(user=self.user, session=session)
+            if not success:
+                self.add_error(None, "Consent could not be saved. Please try again.")  # Add non-field error
+            return success
+        return False

rdmo/accounts/middleware.py

@@ -5,8 +5,9 @@
 from django.http import HttpResponseRedirect
 from django.urls import reverse
 
-from .utils import user_has_accepted_terms
+from .models import ConsentFieldValue
 
+# these exclude url settings are optional
 TERMS_EXCLUDE_URL_PREFIX_LIST = getattr(
     settings,
     "TERMS_EXCLUDE_URL_PREFIX_LIST",
@@ -27,41 +28,22 @@ def __init__(self, get_response):
         self.get_response = get_response
 
     def __call__(self, request):
-        # Skip processing if ACCOUNT_TERMS_OF_USE is disabled
-        if not getattr(settings, "ACCOUNT_TERMS_OF_USE", False):
-            return self.get_response(request)
-
-        # check if the current path is protected
-        accept_terms_path = reverse("terms_of_use_accept")
         if (
-            request.user.is_authenticated
-            and not request.path.startswith(accept_terms_path)
+            settings.ACCOUNT_TERMS_OF_USE  # Terms enforcement enabled
+            and request.user.is_authenticated
+            and request.path != reverse("terms_of_use_accept")
             and self.is_path_protected(request.path)
-            and not user_has_accepted_terms(request.user, request.session)
+            and not ConsentFieldValue.has_accepted_terms(request.user, request.session)
         ):
-            return HttpResponseRedirect(accept_terms_path)
+            return HttpResponseRedirect(reverse("terms_of_use_accept"))
 
         # Proceed with the response for non-protected paths or accepted terms
         return self.get_response(request)
 
     @staticmethod
     def is_path_protected(path):
-        """
-        Determine if a given path is protected by the middleware.
-
-        Paths are excluded if they match any of the following:
-        - Start with a prefix in TERMS_EXCLUDE_URL_PREFIX_LIST
-        - Contain a substring in TERMS_EXCLUDE_URL_CONTAINS_LIST
-        - Are explicitly listed in TERMS_EXCLUDE_URL_LIST
-        - Start with the ACCEPT_TERMS_PATH
-        """
-        if any(path.startswith(prefix) for prefix in TERMS_EXCLUDE_URL_PREFIX_LIST):
-            return False
-
-        if any(substring in path for substring in TERMS_EXCLUDE_URL_CONTAINS_LIST):
-            return False
-
-        if path in TERMS_EXCLUDE_URL_LIST:
-            return False
-
-        return True
+        return not (
+                any(path.startswith(prefix) for prefix in TERMS_EXCLUDE_URL_PREFIX_LIST) or
+                any(substring in path for substring in TERMS_EXCLUDE_URL_CONTAINS_LIST) or
+                path in TERMS_EXCLUDE_URL_LIST
+        )

rdmo/accounts/models.py

@@ -1,13 +1,19 @@
+from datetime import datetime
+
 from django.conf import settings
 from django.contrib.sites.models import Site
 from django.db import models
 from django.db.models.signals import post_save
 from django.dispatch import receiver
+from django.utils.dateparse import parse_date
+from django.utils.formats import get_format
 from django.utils.translation import gettext_lazy as _
 
 from rdmo.core.models import Model as RDMOTimeStampedModel
 from rdmo.core.models import TranslationMixin
 
+CONSENT_SESSION_KEY = "user_has_consented"
+
 
 class AdditionalField(models.Model, TranslationMixin):
 
@@ -124,6 +130,64 @@ class Meta:
     def __str__(self):
         return self.user.username
 
+    @classmethod
+    def create_consent(cls, user, session=None) -> bool:
+        obj, _created = cls.objects.update_or_create(user=user, defaults={"consent": True})
+
+        # Validate consent before storing in session
+        if obj.is_consent_valid():
+            if session:
+                session[CONSENT_SESSION_KEY] = True
+            return True
+
+        obj.delete()  # Remove when consent is outdated
+        return False
+
+
+    @classmethod
+    def has_accepted_terms(cls, user, session) -> bool:
+        if not settings.ACCOUNT_TERMS_OF_USE:
+            return True  # If terms are disabled, assume accepted.
+
+        # Check session cache first
+        if CONSENT_SESSION_KEY in session:
+            return session[CONSENT_SESSION_KEY]
+
+        # Query the database if not cached
+        consent = cls.objects.filter(user=user).first()
+        has_valid_consent = bool(consent and consent.is_consent_valid())
+
+        session[CONSENT_SESSION_KEY] = has_valid_consent  # Cache result
+        return has_valid_consent
+
+    def is_consent_valid(self) -> bool:
+        # optionally enable terms to be outdated
+        terms_version_date = getattr(settings, 'TERMS_VERSION_DATE', None)
+
+        if terms_version_date is None:
+            return True
+
+        # First, try standard ISO format (YYYY-MM-DD)
+        latest_terms_version_date = parse_date(terms_version_date)
+
+        # If ISO parsing fails, try localized formats
+        if not latest_terms_version_date:
+            for fmt in get_format('DATE_INPUT_FORMATS'):
+                try:
+                    latest_terms_version_date = datetime.strptime(terms_version_date, fmt).date()
+                    break  # Stop if parsing succeeds
+                except ValueError:
+                    continue  # Try the next format
+
+        # If still not parsed, raise an error
+        if not latest_terms_version_date:
+            raise ValueError(
+                f"Invalid date format for TERMS_VERSION_DATE: {terms_version_date}. Valid formats {get_format('DATE_INPUT_FORMATS')}"  # noqa: E501
+            )
+
+        # Compare only dates (ignores time)
+        return self.updated and (self.updated.date() >= latest_terms_version_date)
+
 
 class Role(models.Model):
 

rdmo/accounts/templates/account/terms_of_use_accept_form.html

@@ -33,6 +33,14 @@ <h2>{% trans 'Terms of use' %}</h2>
                 {% trans "You have accepted the terms of use." %}
             </p>
         {% endif %}
+
+        {% if form.non_field_errors %}
+          <ul class="list-unstyled text-danger">
+            {% for error in form.non_field_errors %}
+                <li>{{ error }}</li>
+            {% endfor %}
+          </ul>
+        {% endif %}
     </div>
 
 {% endblock %}

rdmo/accounts/tests/test_views.py

@@ -1,4 +1,5 @@
 import re
+from datetime import datetime, timedelta
 
 import pytest
 
@@ -10,9 +11,8 @@
 
 from pytest_django.asserts import assertContains, assertNotContains, assertRedirects, assertTemplateUsed, assertURLEqual
 
-from rdmo.accounts.models import ConsentFieldValue
+from rdmo.accounts.models import CONSENT_SESSION_KEY, ConsentFieldValue
 from rdmo.accounts.tests.utils import reload_urls
-from rdmo.accounts.utils import CONSENT_SESSION_KEY
 
 users = (
     ('editor', 'editor'),
@@ -799,17 +799,10 @@ def test_shibboleth_logout_username_pattern(db, client, settings, shibboleth, us
             assertURLEqual(response.url, reverse('account_logout'))
 
 
-@pytest.mark.parametrize('post_consent', [True, False])
 @pytest.mark.parametrize('username,password', users)
-def test_terms_of_use_middleware_redirect_and_update(
-    db, client, settings, username, password, post_consent
+def test_terms_of_use_middleware_redirect_and_accept(
+    db, client, settings, username, password
 ):
-    """
-    Test the behavior of TermsAndConditionsRedirectMiddleware and terms_of_use_update view:
-    - When the middleware is enabled, users are redirected to the terms update page if not consented.
-    - Valid POST saves consent to the session and database.
-    - Invalid POST does not save consent to the session or database.
-    """
     # Arrange
     settings.ACCOUNT_TERMS_OF_USE = True
     reload_urls('accounts')  # needs to reload before the middleware
@@ -826,56 +819,81 @@ def test_terms_of_use_middleware_redirect_and_update(
     client.login(username=username, password=password)
     response = client.get(reverse('projects'))
 
-    # Assert - Middleware should redirect to terms_of_use_update
+    # Assert - Middleware should redirect to terms_of_use_accept
     if password is not None:
-        assert response.status_code == 302
-        assert response.url == reverse('terms_of_use_update')
+        assertRedirects(response, reverse('terms_of_use_accept'))
 
         # Session should not yet have consent
         assert not client.session.get(CONSENT_SESSION_KEY, False)
     else:
         # Anonymous user is redirected to login
-        assert response.status_code == 302
-        assert response.url == reverse('account_login') + '?next=' + reverse('projects')
+        assertRedirects(response, reverse('account_login') + '?next=' + reverse('projects'))
         return  # Exit test for anonymous users
 
-    # Act - Make a POST request to terms_of_use_update
-    terms_update_url = reverse('terms_of_use_update')
-    data = {
-        'consent': post_consent,  # Set based on parameterization
-    }
-    response = client.post(terms_update_url, data)
+    # Act - Make a POST request to terms_of_use_accept
+    response = client.post(reverse('terms_of_use_accept'), {'consent': True}, follow=True)
+    assertRedirects(response, reverse('projects'))
 
-    assert response.status_code == 302
-    assert response.url == reverse('home')
+    # Assert POST behavior, ToU accepted
+    # Consent should be stored in the session and database
+    assert client.session[CONSENT_SESSION_KEY] is True
+    assert ConsentFieldValue.objects.filter(user=user).exists()
 
-    # Assert POST behavior
-    if post_consent:  # accepted you
-        # Consent should be stored in the session and database
-        assert client.session[CONSENT_SESSION_KEY] is True
-        assert ConsentFieldValue.objects.filter(user=user).exists()
+    response = client.get(reverse('projects'))
+    assert response.status_code == 200
 
-        response = client.get(reverse('projects'))
-        assert response.status_code == 200
 
-        # also test the revoke consent
-        response = client.post(terms_update_url, {'delete': ''})
-        assert response.status_code == 302
-        assert response.url == reverse('home')
-        assert client.session.get(CONSENT_SESSION_KEY, False) is False
-        assert not ConsentFieldValue.objects.filter(user=user).exists()
+def test_terms_of_use_middleware_invalidate_terms_version(
+    db, client, settings
+):
+    # Arrange constants, settings and user
+    past_datetime = (datetime.now() - timedelta(days=10)).strftime(format="%Y-%m-%d")
+    future_datetime = (datetime.now() + timedelta(days=10)).strftime(format="%Y-%m-%d")
 
-        response = client.get(reverse('projects'))
-        assert response.status_code == 302
-        assert response.url == reverse('terms_of_use_update')
+    settings.ACCOUNT_TERMS_OF_USE = True
+    reload_urls('accounts')  # needs to reload before the middleware
+    settings.MIDDLEWARE += [
+        "rdmo.accounts.middleware.TermsAndConditionsRedirectMiddleware"
+    ]
+    terms_accept_url = reverse('terms_of_use_accept')
+    # Arrange user object
+    username = password = 'user'
+    user = get_user_model().objects.get(username=username)
+    _consent = ConsentFieldValue.objects.create(user=user, consent=True)
+
+    # Assert - Access the home page, user has a valid consent
+    # settings.TERMS_VERSION_DATE is not set
+    client.login(username=username, password=password)
+    response = client.get(reverse('projects'))
+    assert response.status_code == 200
+    client.logout()
 
-    else:  # did not accept you
-        # Invalid POST
-        # assertTemplateUsed(response, "account/terms_of_use_update_form.html")
-        # Consent should not be stored in the session or database
-        assert client.session.get(CONSENT_SESSION_KEY, False) is False
-        assert not ConsentFieldValue.objects.filter(user=user).exists()
+    # Act - change the version date setting to a distant future
+    settings.TERMS_VERSION_DATE = future_datetime
+    # Arrange - new login
+    client.login(username=username, password=password)
+    response = client.get(reverse('projects'))
 
-        response = client.get(reverse('projects'))
-        assert response.status_code == 302
-        assert response.url == reverse('terms_of_use_update')
+    # Assert - consent is now invalid and should redirect to terms_of_use_accept
+    assertRedirects(response, terms_accept_url)
+
+    # Act - Try to make a POST request to terms_of_use_accept
+    response = client.post(terms_accept_url, {'consent': True})
+    # Assert - consent was not saved because version date is in the future
+    assert not ConsentFieldValue.objects.filter(user=user).exists()
+    assertContains(response, 'could not be saved')
+    client.logout()
+
+    # Act - change the version date setting to a past datetime
+    settings.TERMS_VERSION_DATE = past_datetime
+    # Arrange - new login without consent
+    client.login(username=username, password=password)
+    response = client.get(reverse('projects'))
+    assertRedirects(response, terms_accept_url)
+
+    # Act - post the consent
+    response = client.post(terms_accept_url, {'consent': True}, follow=True)
+    # Assert - the consent should now be updated since the version date is valid
+    assert ConsentFieldValue.objects.filter(user=user).exists()
+    assert client.session[CONSENT_SESSION_KEY] is True
+    assertRedirects(response, reverse('projects'))