From 9f8bcd5d4e2a3cf9205b77b7ac3fd69ee1cf7f51 Mon Sep 17 00:00:00 2001 From: Paul Schilling Date: Mon, 30 Sep 2024 16:49:28 +0200 Subject: [PATCH] [#2752] Add math captcha to contactform --- .../components/Contact/ContactForm.html | 11 ++++ src/open_inwoner/openklant/forms.py | 10 +++- .../openklant/tests/test_contactform.py | 53 +++++++++++++----- src/open_inwoner/utils/forms.py | 54 +++++++++++++++++++ .../utils/tests/test_form_fields.py | 52 ++++++++++++++++++ 5 files changed, 164 insertions(+), 16 deletions(-) create mode 100644 src/open_inwoner/utils/tests/test_form_fields.py diff --git a/src/open_inwoner/components/templates/components/Contact/ContactForm.html b/src/open_inwoner/components/templates/components/Contact/ContactForm.html index fad8315e41..2675369eec 100644 --- a/src/open_inwoner/components/templates/components/Contact/ContactForm.html +++ b/src/open_inwoner/components/templates/components/Contact/ContactForm.html @@ -24,7 +24,18 @@ {% input form_object.phonenumber %} {% endif %} {% input form_object.question %} + + {% if form_object.captcha %} + + {{ form_object.captcha.label }} * + + {{ form_object.captcha_prompt }} + {{ form_object.captcha.question }} + {% field_as_widget form_object.captcha "input" form_id %} + {% endif %} + {% form_actions primary_text=_("Verzenden") primary_icon="arrow_forward" %} + {% endrender_form %} {% else %} diff --git a/src/open_inwoner/openklant/forms.py b/src/open_inwoner/openklant/forms.py index 78239ef3e2..ae22d9c327 100644 --- a/src/open_inwoner/openklant/forms.py +++ b/src/open_inwoner/openklant/forms.py @@ -1,13 +1,13 @@ from django import forms -from django.forms import Form from django.utils.translation import gettext_lazy as _ from open_inwoner.accounts.models import User from open_inwoner.openklant.models import ContactFormSubject, OpenKlantConfig +from open_inwoner.utils.forms import MathCaptchaField from open_inwoner.utils.validators import DutchPhoneNumberValidator -class ContactForm(Form): +class ContactForm(forms.Form): subject = forms.ModelChoiceField( label=_("Onderwerp"), required=True, @@ -45,6 +45,10 @@ class ContactForm(Form): widget=forms.Textarea(attrs={"rows": "5"}), required=True, ) + captcha = MathCaptchaField( + label=_("Beantwoord deze rekensom"), + required=True, + ) user: User @@ -54,11 +58,13 @@ def __init__(self, user, *args, **kwargs): config = OpenKlantConfig.get_solo() self.fields["subject"].queryset = config.contactformsubject_set.all() + self.captcha_prompt = self.fields["captcha"].question if self.user.is_authenticated: del self.fields["first_name"] del self.fields["last_name"] del self.fields["infix"] + del self.fields["captcha"] if self.user.email: del self.fields["email"] if self.user.phonenumber: diff --git a/src/open_inwoner/openklant/tests/test_contactform.py b/src/open_inwoner/openklant/tests/test_contactform.py index 5f49024801..c3011318f0 100644 --- a/src/open_inwoner/openklant/tests/test_contactform.py +++ b/src/open_inwoner/openklant/tests/test_contactform.py @@ -16,6 +16,7 @@ from open_inwoner.openklant.tests.data import MockAPICreateData from open_inwoner.openklant.tests.factories import ContactFormSubjectFactory from open_inwoner.openzaak.tests.factories import ServiceFactory +from open_inwoner.utils.forms import MathCaptchaField from open_inwoner.utils.test import ClearCachesMixin, DisableRequestLogMixin from open_inwoner.utils.tests.helpers import AssertFormMixin, AssertTimelineLogMixin @@ -24,6 +25,7 @@ @modify_settings( MIDDLEWARE={"remove": ["open_inwoner.kvk.middleware.KvKLoginMiddleware"]} ) +@patch.object(MathCaptchaField, "clean", autospec=True) @patch( "open_inwoner.openklant.views.contactform.send_contact_confirmation_mail", autospec=True, @@ -55,7 +57,9 @@ def setUp(self): config.send_email_confirmation = True config.save() - def test_singleton_has_configuration_method(self, m, mock_send_confirm): + def test_singleton_has_configuration_method( + self, m, mock_send_confirm, mock_captcha + ): # use cleared (from setUp() config = OpenKlantConfig.get_solo() self.assertFalse(config.has_form_configuration()) @@ -81,7 +85,9 @@ def test_singleton_has_configuration_method(self, m, mock_send_confirm): mock_send_confirm.assert_not_called() - def test_no_form_shown_if_not_has_configuration(self, m, mock_send_confirm): + def test_no_form_shown_if_not_has_configuration( + self, m, mock_send_confirm, mock_captcha + ): # set nothing config = OpenKlantConfig.get_solo() self.assertFalse(config.has_form_configuration()) @@ -90,7 +96,9 @@ def test_no_form_shown_if_not_has_configuration(self, m, mock_send_confirm): self.assertContains(response, _("Contact formulier niet geconfigureerd.")) self.assertEqual(0, len(response.pyquery("#contactmoment-form"))) - def test_anon_form_requires_either_email_or_phonenumber(self, m, mock_send_confirm): + def test_anon_form_requires_either_email_or_phonenumber( + self, m, mock_send_confirm, mock_captcha + ): config = OpenKlantConfig.get_solo() config.register_email = "example@example.com" config.save() @@ -108,6 +116,7 @@ def test_anon_form_requires_either_email_or_phonenumber(self, m, mock_send_confi "email", "phonenumber", "question", + "captcha", # captcha present for anon user ), ) form["subject"].select(text=subject.subject) @@ -123,7 +132,9 @@ def test_anon_form_requires_either_email_or_phonenumber(self, m, mock_send_confi ) mock_send_confirm.assert_not_called() - def test_regular_auth_form_fills_email_and_phonenumber(self, m, mock_send_confirm): + def test_regular_auth_form_fills_email_and_phonenumber( + self, m, mock_send_confirm, mock_captcha + ): config = OpenKlantConfig.get_solo() config.register_email = "example@example.com" config.save() @@ -146,7 +157,9 @@ def test_regular_auth_form_fills_email_and_phonenumber(self, m, mock_send_confir response = form.submit(status=302) mock_send_confirm.assert_called_once_with(user.email, subject.subject) - def test_expected_ordered_subjects_are_shown(self, m, mock_send_confirm): + def test_expected_ordered_subjects_are_shown( + self, m, mock_send_confirm, mock_captcha + ): config = OpenKlantConfig.get_solo() config.register_email = "example@example.com" config.save() @@ -183,7 +196,7 @@ def test_expected_ordered_subjects_are_shown(self, m, mock_send_confirm): ) mock_send_confirm.assert_not_called() - def test_submit_and_register_via_email(self, m, mock_send_confirm): + def test_submit_and_register_via_email(self, m, mock_send_confirm, mock_captcha): config = OpenKlantConfig.get_solo() config.register_email = "example@example.com" config.has_form_configuration = True @@ -223,7 +236,9 @@ def test_submit_and_register_via_email(self, m, mock_send_confirm): mock_send_confirm.assert_called_once_with("foo@example.com", subject.subject) - def test_submit_and_register_anon_via_api_with_klant(self, m, mock_send_confirm): + def test_submit_and_register_anon_via_api_with_klant( + self, m, mock_send_confirm, mock_captcha + ): MockAPICreateData.setUpServices() config = OpenKlantConfig.get_solo() @@ -306,7 +321,9 @@ def test_submit_and_register_anon_via_api_with_klant(self, m, mock_send_confirm) mock_send_confirm.assert_called_once_with("foo@example.com", subject.subject) - def test_submit_and_register_anon_via_api_without_klant(self, m, mock_send_confirm): + def test_submit_and_register_anon_via_api_without_klant( + self, m, mock_send_confirm, mock_captcha + ): MockAPICreateData.setUpServices() config = OpenKlantConfig.get_solo() @@ -381,7 +398,9 @@ def test_submit_and_register_anon_via_api_without_klant(self, m, mock_send_confi self.assertTimelineLog("registered contactmoment by API") mock_send_confirm.assert_called_once_with("foo@example.com", subject.subject) - def test_register_bsn_user_via_api_without_id(self, m, mock_send_confirm): + def test_register_bsn_user_via_api_without_id( + self, m, mock_send_confirm, mock_captcha + ): MockAPICreateData.setUpServices() config = OpenKlantConfig.get_solo() @@ -432,7 +451,9 @@ def test_register_bsn_user_via_api_without_id(self, m, mock_send_confirm): }, ) - def test_submit_and_register_bsn_user_via_api(self, m, mock_send_confirm): + def test_submit_and_register_bsn_user_via_api( + self, m, mock_send_confirm, mock_captcha + ): MockAPICreateData.setUpServices() config = OpenKlantConfig.get_solo() @@ -506,7 +527,9 @@ def test_submit_and_register_bsn_user_via_api(self, m, mock_send_confirm): self.assertTimelineLog("registered contactmoment by API") mock_send_confirm.assert_called_once_with("foo@example.com", subject.subject) - def test_submit_and_register_kvk_or_rsin_user_via_api(self, _m, mock_send_confirm): + def test_submit_and_register_kvk_or_rsin_user_via_api( + self, _m, mock_send_confirm, mock_captcha + ): MockAPICreateData.setUpServices() config = OpenKlantConfig.get_solo() @@ -602,7 +625,7 @@ def test_submit_and_register_kvk_or_rsin_user_via_api(self, _m, mock_send_confir mock_send_confirm.reset_mock() def test_submit_and_register_bsn_user_via_api_and_update_klant( - self, m, mock_send_confirm + self, m, mock_send_confirm, mock_captcha ): MockAPICreateData.setUpServices() @@ -686,7 +709,7 @@ def test_submit_and_register_bsn_user_via_api_and_update_klant( mock_send_confirm.reset_mock() def test_submit_and_register_kvk_or_rsin_user_via_api_and_update_klant( - self, _m, mock_send_confirm + self, m, mock_send_confirm, mock_captcha ): self.maxDiff = None MockAPICreateData.setUpServices() @@ -789,7 +812,9 @@ def test_submit_and_register_kvk_or_rsin_user_via_api_and_update_klant( ) mock_send_confirm.reset_mock() - def test_send_email_confirmation_is_configurable(self, m, mock_send_confirm): + def test_send_email_confirmation_is_configurable( + self, m, mock_send_confirm, mock_captcha + ): MockAPICreateData.setUpServices() config = OpenKlantConfig.get_solo() diff --git a/src/open_inwoner/utils/forms.py b/src/open_inwoner/utils/forms.py index ddb58266ef..0705b38fcb 100644 --- a/src/open_inwoner/utils/forms.py +++ b/src/open_inwoner/utils/forms.py @@ -1,4 +1,5 @@ import mimetypes +import random from django import forms from django.conf import settings @@ -163,3 +164,56 @@ def clean(self, *args, **kwargs): ) return f + + +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.TextInput() + 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 = random.randint(lower, upper) # nosec + num2 = random.randint(lower, upper) # nosec + operator = random.choice(operators) # nosec + + # exclude negative results + num1, num2 = max(num1, num2), min(num1, num2) + + question = _("What is {num1} {operator_str} {num2}?").format( + num1=num1, operator_str=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(): + raise forms.ValidationError(_("Voer een geheel getal in.")) + if int(value) != self.answer: + raise forms.ValidationError(_("Fout antwoord, probeer het opnieuw.")) + return value diff --git a/src/open_inwoner/utils/tests/test_form_fields.py b/src/open_inwoner/utils/tests/test_form_fields.py new file mode 100644 index 0000000000..51c9bdea14 --- /dev/null +++ b/src/open_inwoner/utils/tests/test_form_fields.py @@ -0,0 +1,52 @@ +from django import forms +from django.test import TestCase +from django.utils.translation import gettext as _ + +from ..forms import MathCaptchaField + + +class MockForm(forms.Form): + captcha = MathCaptchaField(range_=(4, 4), operators=["+"]) + captcha_2 = MathCaptchaField(range_=(4, 4), operators=["-"]) + + +class MathCaptchaFieldUnitTest(TestCase): + def test_captcha_invalid(self): + test_cases = [ + { + "captcha": "", + "message": _("Dit veld is vereist."), + "reason": "field required", + }, + { + "captcha": " ", + "message": _("Voer een geheel getal in."), + "reason": "wrong input type", + }, + { + "captcha": 42, + "message": _("Voer een geheel getal in."), + "reason": "wrong input type", + }, + { + "captcha": "42", # captcha only computes 2 numbers between 1 and 10 + "message": _("Fout antwoord, probeer het opnieuw."), + "reason": "wrong answer", + }, + ] + for test_case in test_cases: + with self.subTest(reason=test_case["reason"]): + form = MockForm( + 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): + form = MockForm( + data={"captcha": "8", "captcha_2": "0"}, + ) + self.assertTrue(form.is_valid())