[#2831] Store contactform captcha values in session

Author: pi-sigma

src/open_inwoner/components/templates/components/Contact/ContactForm.html

@@ -31,7 +31,7 @@
                             </p>
                             <label class="label captcha__label">
                                 <span class="captcha__check">
-                                    {{ form_object.captcha.field.question }}
+                                    {{ form_object.captcha.field.request_session.captcha_question }}
                                     <span class="captcha__input">{% field_as_widget form_object.captcha "input" form_id %}</span>
                                 </span>
                                 {% if form_object.captcha.errors %}

src/open_inwoner/openklant/forms.py

@@ -52,12 +52,13 @@ class ContactForm(forms.Form):
 
     user: User
 
-    def __init__(self, user, *args, **kwargs):
+    def __init__(self, user, request_session, *args, **kwargs):
         super().__init__(*args, **kwargs)
         self.user = user
 
         config = OpenKlantConfig.get_solo()
         self.fields["subject"].queryset = config.contactformsubject_set.all()
+        self.fields["captcha"].request_session = request_session
 
         if self.user.is_authenticated:
             del self.fields["first_name"]

src/open_inwoner/openklant/views/contactform.py

@@ -1,3 +1,5 @@
+import secrets
+
 from django.conf import settings
 from django.contrib import messages
 from django.urls import reverse
@@ -21,9 +23,36 @@
 from open_inwoner.utils.views import CommonPageMixin, LogMixin
 
 
+def generate_question_answer_pair(
+    range_: tuple[int, int],
+    operators: list[str],
+) -> tuple[str, int]:
+    lower, upper = range_
+    num1 = secrets.choice(range(lower, upper))
+    num2 = secrets.choice(range(lower, upper))
+    operator = secrets.choice(operators)
+
+    # exclude negative results
+    num1, num2 = max(num1, num2), min(num1, num2)
+
+    question = _("What is {num1} {operator} {num2}?").format(
+        num1=num1, operator=operator, num2=num2
+    )
+
+    match operator:
+        case "+":
+            answer = num1 + num2
+        case "-":
+            answer = num1 - num2
+
+    return question, answer
+
+
 class ContactFormView(CommonPageMixin, LogMixin, BaseBreadcrumbMixin, FormView):
     form_class = ContactForm
-    template_name = "pages/contactform/form_wrap.html"  # inner ("structure") template rendered by CMS plugin
+    template_name = (
+        "pages/contactform/form_wrap.html"  # inner ("structure") template rendered by CMS plugin
+    )
 
     @cached_property
     def crumbs(self):
@@ -44,7 +73,18 @@ def get_success_url(self):
 
     def get_form_kwargs(self):
         kwargs = super().get_form_kwargs()
+
+        captcha_question = self.request.session.get("captcha_question")
+        captcha_answer = self.request.session.get("captcha_answer")
+
+        if not captcha_question or not captcha_answer:
+            captcha_question, captcha_answer = generate_question_answer_pair((1, 10), ["+", "-"])
+
+        self.request.session["captcha_question"] = captcha_question
+        self.request.session["captcha_answer"] = captcha_answer
+
         kwargs["user"] = self.request.user
+        kwargs["request_session"] = self.request.session
         return kwargs
 
     def get_initial(self):
@@ -106,9 +146,7 @@ def form_valid(self, form):
         user_email = api_user_email or user_email or form.cleaned_data.get("email")
 
         if send_confirmation:
-            send_contact_confirmation_mail(
-                user_email, form.cleaned_data["subject"].subject
-            )
+            send_contact_confirmation_mail(user_email, form.cleaned_data["subject"].subject)
 
         self.set_result_message(email_success or api_success)
 
@@ -133,9 +171,7 @@ def register_by_email(self, form, recipient_email):
         success = template.send_email([recipient_email], context)
 
         if success:
-            self.log_system_action(
-                "registered contactmoment by email", user=self.request.user
-            )
+            self.log_system_action("registered contactmoment by email", user=self.request.user)
             return True
         else:
             self.log_system_action(
@@ -156,9 +192,7 @@ def register_by_api(self, form, config: OpenKlantConfig) -> tuple[bool, str]:
             if self.request.user.is_authenticated and (
                 self.request.user.bsn or self.request.user.kvk
             ):
-                klant = klanten_client.retrieve_klant(
-                    **get_fetch_parameters(self.request)
-                )
+                klant = klanten_client.retrieve_klant(**get_fetch_parameters(self.request))
                 if klant:
                     self.log_system_action(
                         "retrieved klant for BSN or KVK user", user=self.request.user
@@ -216,9 +250,7 @@ def register_by_api(self, form, config: OpenKlantConfig) -> tuple[bool, str]:
             parts = [form.cleaned_data[k] for k in ("first_name", "infix", "last_name")]
             full_name = " ".join(p for p in parts if p)
 
-            text = _("{text}\n\nNaam: {full_name}").format(
-                text=text, full_name=full_name
-            )
+            text = _("{text}\n\nNaam: {full_name}").format(text=text, full_name=full_name)
 
             self.log_system_action(
                 "could not retrieve or create klant for user, appended info to message",
@@ -253,9 +285,7 @@ def register_by_api(self, form, config: OpenKlantConfig) -> tuple[bool, str]:
             contactmoment = contactmoment_client.create_contactmoment(data, klant=klant)
 
         if contactmoment:
-            self.log_system_action(
-                "registered contactmoment by API", user=self.request.user
-            )
+            self.log_system_action("registered contactmoment by API", user=self.request.user)
             return True, user_email
         else:
             self.log_system_action(

src/open_inwoner/utils/forms.py

@@ -1,5 +1,4 @@
 import mimetypes
-import secrets
 
 from django import forms
 from django.conf import settings
@@ -11,9 +10,7 @@
 
 class ErrorMessageMixin:
     default_error_messages = {
-        "required": _(
-            "Het verplichte veld {field_name} is niet (goed) ingevuld. Vul het veld in."
-        )
+        "required": _("Het verplichte veld {field_name} is niet (goed) ingevuld. Vul het veld in.")
     }
 
     def __init__(self, *args, **kwargs):
@@ -30,9 +27,7 @@ def __init__(self, *args, **kwargs):
             error_messages = {}
             for key in self.default_error_messages.keys():
                 if key in field_error_messages.keys():
-                    error_message = field_error_messages[key].format(
-                        field_name=f'"{field.label}"'
-                    )
+                    error_message = field_error_messages[key].format(field_name=f'"{field.label}"')
                 else:
                     error_message = self.default_error_messages[key].format(
                         field_name=f'"{field.label}"'
@@ -56,9 +51,7 @@ def get_context(self, name, value, attrs):
         """
         context = super().get_context(name, value, attrs)
         if self.is_initial(value):
-            context["url"] = reverse(
-                self.url_name, kwargs={"uuid": value.instance.uuid}
-            )
+            context["url"] = reverse(self.url_name, kwargs={"uuid": value.instance.uuid})
         return context
 
 
@@ -129,10 +122,7 @@ def allowed_mime_types(self):
 
     @property
     def allowed_extensions(self):
-        return [
-            mimetypes.guess_extension(mt)[1:]
-            for mt in self.allowed_mime_types.split(",")
-        ]
+        return [mimetypes.guess_extension(mt)[1:] for mt in self.allowed_mime_types.split(",")]
 
     def widget_attrs(self, widget):
         attrs = super().widget_attrs(widget)
@@ -169,51 +159,17 @@ def clean(self, *args, **kwargs):
 class MathCaptchaField(forms.Field):
     def __init__(
         self,
-        range_: tuple = (1, 10),
-        operators: list[str] | None = None,
         *args,
         **kwargs,
     ):
         super().__init__(*args, **kwargs)
         self.widget = forms.NumberInput()
-        self.range_ = range_
-        self.operators = operators or ["+", "-"]
-        self.question, self.answer = self.generate_question_answer_pair(
-            self.range_, self.operators
-        )
-
-    @staticmethod
-    def generate_question_answer_pair(
-        range_: tuple[int, int],
-        operators: list[str],
-    ) -> tuple[str, int]:
-        lower, upper = range_
-        num1 = secrets.choice(range(lower, upper))
-        num2 = secrets.choice(range(lower, upper))
-        operator = secrets.choice(operators)
-
-        # exclude negative results
-        num1, num2 = max(num1, num2), min(num1, num2)
-
-        question = _("What is {num1} {operator} {num2}?").format(
-            num1=num1, operator=operator, num2=num2
-        )
-
-        match operator:
-            case "+":
-                answer = num1 + num2
-            case "-":
-                answer = num1 - num2
-
-        return question, answer
 
     def clean(self, value: str) -> str:
         if not value:
             raise forms.ValidationError(_("Dit veld is vereist."))
-        if not isinstance(value, str):
-            raise forms.ValidationError(_("Voer een geheel getal in."))
-        if value.isspace():
+        if not isinstance(value, int):
             raise forms.ValidationError(_("Voer een geheel getal in."))
-        if int(value) != self.answer:
+        if int(value) != self.request_session["captcha_answer"]:
             raise forms.ValidationError(_("Fout antwoord, probeer het opnieuw."))
         return value

src/open_inwoner/utils/tests/test_form_fields.py

@@ -2,12 +2,21 @@
 from django.test import TestCase
 from django.utils.translation import gettext as _
 
+from open_inwoner.openklant.views.contactform import generate_question_answer_pair
+
 from ..forms import MathCaptchaField
 
 
 class MockForm(forms.Form):
-    captcha = MathCaptchaField(range_=(4, 5), operators=["+"])
-    captcha_2 = MathCaptchaField(range_=(4, 5), operators=["-"])
+    captcha = MathCaptchaField()
+
+    def __init__(self, question, answer, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+
+        self.fields["captcha"].request_session = {
+            "captcha_question": question,
+            "captcha_answer": answer,
+        }
 
 
 class MathCaptchaFieldUnitTest(TestCase):
@@ -24,29 +33,34 @@ def test_captcha_invalid(self):
                 "reason": "wrong input type",
             },
             {
-                "captcha": 42,
+                "captcha": "42",
                 "message": _("Voer een geheel getal in."),
                 "reason": "wrong input type",
             },
             {
-                "captcha": "42",  # captcha only computes 2 numbers between 1 and 10
+                "captcha": 42,  # captcha only computes 2 numbers between 1 and 10
                 "message": _("Fout antwoord, probeer het opnieuw."),
                 "reason": "wrong answer",
             },
         ]
+        question, answer = generate_question_answer_pair((1, 10), ["+", "-"])
         for test_case in test_cases:
             with self.subTest(reason=test_case["reason"]):
                 form = MockForm(
+                    question=question,
+                    answer=answer,
                     data={
                         "captcha": test_case["captcha"],
-                        "captcha_2": test_case["captcha"],
                     },
                 )
                 self.assertFalse(form.is_valid())
                 self.assertEqual(form.errors["captcha"], [test_case["message"]])
 
     def test_captcha_valid(self):
+        question, answer = generate_question_answer_pair((1, 10), ["+", "-"])
         form = MockForm(
-            data={"captcha": "8", "captcha_2": "0"},
+            question=question,
+            answer=answer,
+            data={"captcha": answer},
         )
         self.assertTrue(form.is_valid())