Merge pull request #3402 from rebeccacremona/another-trap

Tweak form submission flow

perma_web/conftest.py

@@ -108,6 +108,7 @@ def cleanup_storage():
     'homepage': reverse('landing'),
     'login': reverse('user_management_limited_login'),
     'about': reverse('about'),
+    'contact': reverse('contact'),
     'folders': reverse('create_link'),
 }
 

perma_web/functional_tests/test_contact.py

@@ -0,0 +1,63 @@
+
+def test_contact(page, urls, mailoutbox) -> None:
+    """The Contact form should submit"""
+    msg = "I've got important things to say."
+    email = "functional_test_user@example.com"
+
+    page.goto(urls.contact)
+
+    email_field = page.locator('#id_email')
+    email_field.focus()
+    email_field.type(email)
+    message_field = page.locator('#id_box2')
+    message_field.focus()
+    message_field.type(msg)
+
+    page.locator('.contact-form button[type=submit]').click()
+
+    assert page.title() == "Perma.cc | Thanks"
+    assert len(mailoutbox) == 1
+    m = mailoutbox[0]
+    assert msg in m.body
+
+
+def test_contact_no_js(page, urls, mailoutbox, caplog) -> None:
+    """The Contact form should submit, but be rejected."""
+    msg = "I've got important things to say."
+    email = "functional_test_user@example.com"
+
+    page.goto(urls.contact)
+    # Remove the form's submit event listener
+    page.evaluate('() => {let elem = document.querySelector(".contact-form"); elem.innerHTML = elem.innerHTML;}')
+
+    email_field = page.locator('#id_email')
+    email_field.focus()
+    email_field.type(email)
+    message_field = page.locator('#id_box2')
+    message_field.focus()
+    message_field.type(msg)
+
+    page.locator('.contact-form button[type=submit]').click()
+
+    assert page.title() == "Perma.cc | Thanks"
+    assert any("Suppressing invalid form submission" in msg for msg in caplog.messages)
+    assert len(mailoutbox) == 0
+
+
+def test_contact_no_js_logged_in(logged_in_user, urls, mailoutbox, caplog) -> None:
+    """The Contact form should submit, and not be rejected despite no JS."""
+    msg = "I've got important things to say."
+
+    logged_in_user.goto(urls.contact)
+    # Remove the form's submit event listener
+    logged_in_user.evaluate('() => {let elem = document.querySelector(".contact-form"); elem.innerHTML = elem.innerHTML;}')
+
+    message_field = logged_in_user.locator('#id_box2')
+    message_field.focus()
+    message_field.type(msg)
+
+    logged_in_user.locator('.contact-form button[type=submit]').click()
+
+    assert len(mailoutbox) == 1
+    m = mailoutbox[0]
+    assert msg in m.body

perma_web/perma/forms.py

@@ -1,6 +1,7 @@
 from axes.utils import reset as reset_login_attempts
 
 from django import forms
+from django.conf import settings
 from django.contrib.auth.forms import SetPasswordForm
 from django.db.models.fields import BLANK_CHOICE_DASH
 from django.forms import ModelForm
@@ -16,13 +17,22 @@
 
 ### HELPERS ###
 
-def check_honeypot(request, redirect_to_view, honey_pot_fieldname='telephone'):
-    # the honeypot field should be display: none, so should never be filled out except by spam bots.
-    if request.POST.get(honey_pot_fieldname):
+def check_honeypot(request, redirect_to_view, honey_pot_fieldname='telephone', check_js=False):
+    def reject_request():
         user_ip = get_client_ip(request)
         logger.info(f"Suppressing invalid form submission from {user_ip}: {request.POST}")
         return HttpResponseRedirect(reverse(redirect_to_view))
 
+    # the honeypot field should be display: none, so should never be filled out except by spam bots.
+    if request.POST.get(honey_pot_fieldname):
+        return reject_request()
+
+    # and if we are being particular... you have to have submitted this form via JS
+    if check_js and request.user.is_anonymous and settings.REQUIRE_JS_FORM_SUBMISSIONS:
+        if not request.POST.get('javascript'):
+            return reject_request()
+
+
 class OrganizationField(forms.ModelMultipleChoiceField):
     def __init__(self,
                  queryset=Organization.objects.order_by('name'),

perma_web/perma/settings/deployments/settings_common.py

@@ -182,6 +182,7 @@
 SESSION_COOKIE_SAMESITE = 'Lax'
 # This defaults to 'SAMEORIGIN' w/ Django 2, but was changed to 'DENY' in Django 3
 X_FRAME_OPTIONS = 'DENY'
+REQUIRE_JS_FORM_SUBMISSIONS = True
 
 #
 # Authentication

perma_web/perma/tests/test_views_common.py

@@ -435,6 +435,7 @@ def test_contact_params_regular(self):
     ###   Does the contact form submit as expected?
     ###
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_contact_standard_submit_fail(self):
         '''
             Blank submission should fail and, for most users, request
@@ -447,6 +448,7 @@ def test_contact_standard_submit_fail(self):
                                      error_keys = ['email', 'box2'])
         self.assertEqual(response.request['PATH_INFO'], reverse('contact'))
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_contact_org_user_submit_fail(self):
         '''
             Org users are special. Blank submission should fail
@@ -460,6 +462,7 @@ def test_contact_org_user_submit_fail(self):
                                      error_keys = ['email', 'box2', 'registrar'])
         self.assertEqual(response.request['PATH_INFO'], reverse('contact'))
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_contact_standard_submit_required(self):
         '''
             All fields, including custom subject and referer
@@ -482,6 +485,7 @@ def test_contact_standard_submit_required(self):
         self.assertEqual(message.recipients(), [self.our_address])
         self.assertDictEqual(message.extra_headers, {'Reply-To': self.from_email})
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_contact_standard_submit_required_with_spam_catcher(self):
         '''
             All fields, including custom subject and referer
@@ -496,6 +500,7 @@ def test_contact_standard_submit_required_with_spam_catcher(self):
 
         self.assertEqual(len(mail.outbox), 0)
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_contact_standard_submit_no_optional(self):
         '''
             All fields except custom subject and referer
@@ -515,6 +520,7 @@ def test_contact_standard_submit_no_optional(self):
         self.assertEqual(message.recipients(), [self.our_address])
         self.assertDictEqual(message.extra_headers, {'Reply-To': self.from_email})
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_contact_org_user_submit_invalid(self):
         '''
             Org users get extra fields. Registrar must be a valid choice.
@@ -528,6 +534,7 @@ def test_contact_org_user_submit_invalid(self):
                                      error_keys = ['registrar'])
         self.assertEqual(response.request['PATH_INFO'], reverse('contact'))
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_contact_org_user_submit(self):
         '''
             Should be sent to registrar users.
@@ -554,6 +561,7 @@ def test_contact_org_user_submit(self):
             self.assertEqual(message.cc, [self.our_address, self.from_email] )
             self.assertEqual(message.reply_to, [self.from_email])
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_contact_org_user_affiliation_string(self):
         '''
             Verify org affiliations are printed correctly
@@ -568,6 +576,7 @@ def test_contact_org_user_affiliation_string(self):
         self.assertIn("Affiliations: Another Library's Journal (Another Library), A Third Journal (Test Library)", message.body)
         self.assertIn("Logged in: true", message.body)
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_contact_reg_user_affiliation_string(self):
         '''
             Verify registrar affiliations are printed correctly
@@ -589,6 +598,7 @@ def test_report_with_no_guid_renders(self):
         response = self.get('report').content
         self.assertIn(b"Report Inappropriate Content", response)
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_report_empty_post(self):
         self.submit_form(
             'report',
@@ -600,6 +610,7 @@ def test_report_empty_post(self):
             ]
         )
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_report_post_with_spam_catcher1(self):
         self.submit_form(
             'report',
@@ -614,7 +625,7 @@ def test_report_post_with_spam_catcher1(self):
         )
         self.assertEqual(len(mail.outbox), 0)
 
-
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_report_post_with_spam_catcher2(self):
         self.submit_form(
             'report',
@@ -631,6 +642,7 @@ def test_report_post_with_spam_catcher2(self):
     # Report form, as a person
     #
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_report_post_with_basic_fields(self):
         self.submit_form(
             'report',
@@ -650,6 +662,7 @@ def test_report_post_with_basic_fields(self):
         self.assertIn('some-string-that-could-be-a-guid', message.body)
         self.assertIn("Logged in: false", message.body)
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_report_email_prepopulated_for_logged_in_user(self):
         response = self.get('report', user='test_another_library_org_user@example.com').content
         soup = BeautifulSoup(response, 'html.parser')

perma_web/perma/tests/test_views_user_management.py

@@ -3,6 +3,7 @@
 from django.core import mail
 from django.conf import settings
 from django.db import IntegrityError
+from django.test import override_settings
 from django.utils import timezone
 
 from mock import patch, sentinel
@@ -1833,6 +1834,7 @@ def check_lib_email(self, message, new_lib, user):
         self.assertEqual(message.recipients(), [our_address])
         self.assertDictEqual(message.extra_headers, {'Reply-To': user['raw_email']})
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_new_library_render(self):
         '''
            Does the library signup form display as expected?
@@ -1904,6 +1906,7 @@ def test_new_library_render(self):
             else:
                 self.assertFalse(input.get('value', ''))
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_new_library_submit_success(self):
         '''
            Does the library signup form submit as expected? Success cases.
@@ -1956,6 +1959,7 @@ def test_new_library_submit_success(self):
         self.assertEqual(len(mail.outbox), expected_emails_sent)
         self.check_lib_email(mail.outbox[expected_emails_sent - 1], new_lib, existing_lib_user)
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_new_library_form_honeypot(self):
         new_lib = self.new_lib()
         new_lib_user = self.new_lib_user()
@@ -1971,6 +1975,7 @@ def test_new_library_form_honeypot(self):
         self.assertEqual(len(mail.outbox), 0)
         self.assertFalse(Registrar.objects.filter(name=new_lib['name']).exists())
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_new_library_submit_failure(self):
         '''
            Does the library signup form submit as expected? Failures.
@@ -2044,6 +2049,7 @@ def check_court_email(self, message, court_email):
         self.assertEqual(message.recipients(), [our_address])
         self.assertDictEqual(message.extra_headers, {'Reply-To': court_email})
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_new_court_success(self):
         '''
             Does the court signup form submit as expected? Success cases.
@@ -2122,6 +2128,7 @@ def test_new_court_success(self):
         self.assertEqual(len(mail.outbox), expected_emails_sent)
         self.check_court_email(mail.outbox[expected_emails_sent - 1], existing_user['email'])
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_new_court_form_honeypot(self):
         new_court = self.new_court()
         new_user = self.new_court_user()
@@ -2134,6 +2141,7 @@ def test_new_court_form_honeypot(self):
         self.assertEqual(len(mail.outbox), 0)
         self.assertFalse(LinkUser.objects.filter(email__iexact=new_user['raw_email']).exists())
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_new_court_failure(self):
         '''
             Does the court signup form submit as expected? Failure cases.
@@ -2176,6 +2184,7 @@ def check_firm_email(self, message, firm_email):
         self.assertEqual(message.recipients(), [our_address])
         self.assertDictEqual(message.extra_headers, {'Reply-To': firm_email})
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_new_firm_success(self):
         '''
             Does the firm signup form submit as expected? Success cases.
@@ -2254,6 +2263,7 @@ def test_new_firm_success(self):
         self.assertEqual(len(mail.outbox), expected_emails_sent)
         self.check_firm_email(mail.outbox[expected_emails_sent - 1], existing_user['email'])
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_new_firm_form_honeypot(self):
         new_firm = self.new_firm()
         new_user = self.new_firm_user()
@@ -2266,6 +2276,7 @@ def test_new_firm_form_honeypot(self):
         self.assertEqual(len(mail.outbox), 0)
         self.assertFalse(LinkUser.objects.filter(email__iexact=new_user['raw_email']).exists())
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_new_firm_failure(self):
         '''
             Does the firm signup form submit as expected? Failure cases.
@@ -2298,6 +2309,7 @@ def new_journal_user(self):
                  'first': 'Joe',
                  'last': 'Yacobówski' }
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_new_journal_success(self):
         '''
             Does the journal signup form submit as expected? Success cases.
@@ -2332,6 +2344,7 @@ def test_new_journal_success(self):
         self.assertEqual(len(mail.outbox), expected_emails_sent)
         self.check_new_activation_email(mail.outbox[expected_emails_sent - 1], new_user['raw_email'])
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_new_journal_form_honeypot(self):
         new_journal = self.new_journal()
         new_user = self.new_journal_user()
@@ -2343,6 +2356,7 @@ def test_new_journal_form_honeypot(self):
         self.assertEqual(len(mail.outbox), 0)
         self.assertFalse(LinkUser.objects.filter(email__iexact=new_user['raw_email']).exists())
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_new_journal_failure(self):
         '''
             Does the journal signup form submit as expected? Failure cases.
@@ -2393,6 +2407,7 @@ def new_faculty_user(self):
                  'last': 'Yacobówski',
                  'requested_account_note': 'Journal {}'.format(rand) }
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_new_faculty_success(self):
         '''
             Does the faculty signup form submit as expected? Success cases.
@@ -2426,6 +2441,7 @@ def test_new_faculty_success(self):
         self.assertEqual(len(mail.outbox), expected_emails_sent)
         self.check_new_activation_email(mail.outbox[expected_emails_sent - 1], new_user['raw_email'])
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_new_faculty_form_honeypot(self):
         new_user = self.new_faculty_user()
         self.submit_form('sign_up_faculty',
@@ -2436,6 +2452,7 @@ def test_new_faculty_form_honeypot(self):
         self.assertEqual(len(mail.outbox), 0)
         self.assertFalse(LinkUser.objects.filter(email__iexact=new_user['raw_email']).exists())
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_new_faculty_failure(self):
         '''
             Does the faculty signup form submit as expected? Failure cases.
@@ -2484,6 +2501,7 @@ def check_new_activation_email(self, message, user_email):
         activation_url = next(line for line in message.body.rstrip().split("\n") if line.startswith('http'))
         return activation_url
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_account_creation_views(self):
         # user registration
         new_user_raw_email = self.randomize_capitalization("new_email@test.com")
@@ -2532,6 +2550,7 @@ def test_account_creation_views(self):
         response = self.client.post(reverse('user_management_limited_login'), {'username': new_user_raw_email, 'password': 'Anewpass1'}, follow=True, secure=True)
         self.assertContains(response, 'Enter any URL to preserve it forever')
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_signup_with_existing_email_rejected(self):
         self.assertEqual(LinkUser.objects.filter(email__iexact=self.registrar_user.email).count(), 1)
         self.submit_form('sign_up',
@@ -2542,6 +2561,7 @@ def test_signup_with_existing_email_rejected(self):
                  error_keys=['email'])
         self.assertEqual(LinkUser.objects.filter(email__iexact=self.registrar_user.email).count(), 1)
 
+    @override_settings(REQUIRE_JS_FORM_SUBMISSIONS=False)
     def test_new_user_form_honeypot(self):
         new_user_email = "new_email@test.com"
         self.submit_form('sign_up',