Merge ~maxiberta/canonical-identity-provider:forgot-password-captcha into canonical-identity-provider:master

[Maximiliano Bertacchini]

Add captcha to the forgotten password view

And a couple of related cleanups

Changes are neatly split in separate commits, including:
- Unify rendering of captcha error message in a single template, with more consistent vanilla style.
- Drop unused code in webui.views.utils.display_email_sent().
- Drop duplicate assertions in the old, captcha-agnostic forgotten password tests.
- Refactor/cleanup the captcha logic in the email confirmation view to make it similar to the forgotten password.

As a result, both the email confirmation and the forgotten password views now require Captcha; both behind the CAPTCHA feature flag. We could use separate feature flags for each view if needed. Also, I think the captcha logic should probably go into form validation to make it more djangoish. But otoh the current implementation at the view level is quite compact and has less moving parts. That said, we could consider just dropping our custom recaptcha implementation (hi NIH!) and use e.g. https://pypi.org/project/django-recaptcha/ :D

[Maximiliano Bertacchini (maxiberta)]

Comments for reviewers.

[Maximiliano Bertacchini (maxiberta)]

Updated comments for reviewers.

[Guillermo Gonzalez (verterok)]

LGTM

review: Approve

[Hasan Ammar (hasanammar)]

Looks good, but see an inline suggestion and a comment. Thanks.

[Maximiliano Bertacchini (maxiberta)]

Thanks! Will push a improvements shortly.

[Hasan Ammar (hasanammar)]

Looks great, thanks for the improvements!

review: Approve

diff --git a/src/identityprovider/models/captcha.py b/src/identityprovider/models/captcha.py
index 0e5f68f..ef7a234 100644
--- a/src/identityprovider/models/captcha.py
+++ b/src/identityprovider/models/captcha.py
@@ -107,11 +107,12 @@ class CaptchaBase(object):
 
         return CaptchaResponse(response.code, response, None)
 
-    def check_whitelist(self, email):
-        for pattern in settings.EMAIL_WHITELIST_REGEXP_LIST:
-            if re.match(pattern, email):
-                return True
-        return False
+    @staticmethod
+    def check_whitelist(email):
+        return any(
+            re.match(pattern, email)
+            for pattern in settings.EMAIL_WHITELIST_REGEXP_LIST
+        )
 
 
 class CaptchaV2(CaptchaBase):
@@ -123,11 +124,11 @@ class CaptchaV2(CaptchaBase):
         with maybe_contextmanager(timer, 'captcha-new', None):
             super(CaptchaV2, self).__init__()
 
-    def verify(self, captcha_solution, remote_ip, email, timer=None):
+    def verify(self, captcha_solution, remote_ip, email=None, timer=None):
         if self._verified is not None:
             return self._verified
 
-        if self.check_whitelist(email):
+        if email and self.check_whitelist(email):
             return True
 
         if not settings.CAPTCHA_PRIVATE_KEY:
diff --git a/src/identityprovider/tests/test_models_captcha.py b/src/identityprovider/tests/test_models_captcha.py
index 2730468..f8fe023 100644
--- a/src/identityprovider/tests/test_models_captcha.py
+++ b/src/identityprovider/tests/test_models_captcha.py
@@ -95,6 +95,16 @@ class CaptchaV2VerifyTestCase(CaptchaBaseTestCase):
 
         captcha.check_whitelist.assert_called_with('foo')
 
+    def test_verify_with_no_email(self):
+        captcha = CaptchaV2(None)
+        captcha.check_whitelist = Mock(return_value=False)
+
+        result = captcha.verify(None, 'localhost')
+        self.assertFalse(result)
+        self.assertTrue(self.mock_captcha_open.called)
+
+        captcha.check_whitelist.assert_not_called()
+
     def test_check_whitelist_match(self):
         captcha = CaptchaV2(None)
 
diff --git a/src/webui/templates/account/confirm_new_email.html b/src/webui/templates/account/confirm_new_email.html
index b71e8c6..79552b5 100644
--- a/src/webui/templates/account/confirm_new_email.html
+++ b/src/webui/templates/account/confirm_new_email.html
@@ -20,14 +20,7 @@ GNU Affero General Public License version 3 (see the file  LICENSE).
   <form action="" method="post">
     {% csrf_token %}
     {% if captcha_required %}
-    <p class="captcha" id="captcha">
-      {% if captcha_error_message %}
-      <span class="error">
-        {{ captcha_error_message }}
-      </span>
-      {% endif %}
-      {% include "widgets/recaptcha-v2.html" %}
-    </p>
+    {% include "widgets/recaptcha-v2.html" %}
     {% endif %}
     <input type="hidden" name="post" value="yes" />
     <button type="submit" name="continue" class="p-button--positive" data-qa-id="confirm_email_validation">{% trans "Yes, I'm sure" %}</button>
diff --git a/src/webui/templates/registration/forgot_password.html b/src/webui/templates/registration/forgot_password.html
index 6c5c4d2..53d7128 100644
--- a/src/webui/templates/registration/forgot_password.html
+++ b/src/webui/templates/registration/forgot_password.html
@@ -43,6 +43,9 @@ GNU Affero General Public License version 3 (see the file  LICENSE).
           </p>
           {% endif %}
         </div>
+        {% if captcha_required %}
+        {% include "widgets/recaptcha-v2.html" %}
+        {% endif %}
         <button type="submit" name="continue" class="p-button--positive" data-qa-id="send_password_reset_token">{% trans "Continue" %}</button>
       </form>
     </div>
diff --git a/src/webui/templates/widgets/recaptcha-v2.html b/src/webui/templates/widgets/recaptcha-v2.html
index 67bb710..0daa4da 100644
--- a/src/webui/templates/widgets/recaptcha-v2.html
+++ b/src/webui/templates/widgets/recaptcha-v2.html
@@ -10,3 +10,10 @@ GNU Affero General Public License version 3 (see the file  LICENSE).
 <script src="https://www.google.com/recaptcha/api.js" async defer></script>
 <div id="g-recaptcha" class="g-recaptcha" data-sitekey="{{ CAPTCHA_PUBLIC_KEY }}" data-theme="light"></div>
 </div>
+<p class="captcha" id="captcha">
+  {% if captcha_error_message %}
+  <span class="p-form-validation__message">
+    <strong>Error:</strong> {{ captcha_error_message }}
+  </span>
+  {% endif %}
+</p>
diff --git a/src/webui/tests/test_loginservice.py b/src/webui/tests/test_loginservice.py
index 14a3a15..6ea96a3 100644
--- a/src/webui/tests/test_loginservice.py
+++ b/src/webui/tests/test_loginservice.py
@@ -10,6 +10,7 @@ from gargoyle.testutils import switches
 from identityprovider.fields import FIELD_REQUIRED, INVALID_EMAIL
 from identityprovider.login import LOGIN_INCORRECT
 from identityprovider.models.authtoken import AuthToken
+from identityprovider.models.captcha import CaptchaV2
 from identityprovider.tests.utils import SSOBaseTestCase
 
 
@@ -129,3 +130,34 @@ class ForgottenPasswordTest(SSOBaseTestCase):
         self.assertTemplateUsed(response, 'registration/email_sent.html')
 
         AuthToken.objects.all().delete()
+
+
+@switches(CAPTCHA=True)
+class ForgottenPasswordWithCaptchaTest(SSOBaseTestCase):
+
+    fixtures = ["test"]
+
+    def test_invalid_captcha(self):
+        self.patch(CaptchaV2, 'verify', return_value=False)
+        query = {'email': '[e-mail]', 'g-recaptcha-response': '0'}
+        self.preseed_cookie_check()
+        response = self.client.post('/+forgot_password', query)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(len(mail.outbox), 0)
+        self.assertTemplateUsed(response, 'registration/forgot_password.html')
+
+    def test_success(self):
+        self.patch(CaptchaV2, 'verify', return_value=True)
+        query = {'email': '[e-mail]', 'g-recaptcha-response': '0'}
+        self.preseed_cookie_check()
+        response = self.client.post('/+forgot_password', query)
+        self.assertContains(response, "Reset password")
+        self.assertContains(response, "[e-mail]")
+        self.assertEqual(len(mail.outbox), 1)
+        self.assertEqual(
+            mail.outbox[0].subject,
+            u"{}: Forgotten Password".format(settings.NOREPLY_FROM_NAME),
+        )
+        mail.outbox = []
+        self.assertTemplateUsed(response, 'registration/email_sent.html')
+        AuthToken.objects.all().delete()
diff --git a/src/webui/tests/test_views_registration.py b/src/webui/tests/test_views_registration.py
index df5ef51..92168a5 100644
--- a/src/webui/tests/test_views_registration.py
+++ b/src/webui/tests/test_views_registration.py
@@ -13,7 +13,7 @@ from django.test.utils import override_settings
 from django.urls import reverse
 
 from gargoyle.testutils import switches
-from mock import Mock, call
+from mock import ANY, Mock, call
 from oath import hotp
 from ssoclient.v2 import errors as api_errors
 
@@ -27,6 +27,7 @@ from identityprovider.forms import (
     TwoFactorSyncForm,
 )
 from identityprovider.models import Account
+from identityprovider.models.captcha import CaptchaV2
 from identityprovider.models.twofactor import (
     TWOFACTOR_LOGIN,
     TWOFACTOR_SYNC_ERROR,
@@ -485,6 +486,7 @@ class RegisterMessagesTestCase(SSOBaseTestCase, RegisterTestMixin):
         self.assertIsNone(Account.objects.get_by_email('[e-mail]'))
 
 
+@switches(CAPTCHA=False)
 class PasswordResetRequestTestCase(SSOBaseTestCase):
 
     url_name = 'forgot_password'
@@ -516,11 +518,11 @@ class PasswordResetRequestTestCase(SSOBaseTestCase):
             expected = call(name, key=key, rpconfig=rpconfig)
             self.assertEqual(expected, actual)
 
-    def assert_form_displayed(self, response, **kwargs):
+    def assert_form_displayed(self, response, errors=None):
         self.assertTemplateUsed(response, 'registration/forgot_password.html')
         form = response.context_data['form']
-        for field, value in kwargs.items():
-            self.assertIn(unicode(value), form.errors[field][0])
+        for field, value in (errors or {}).items():
+            self.assertEqual(value, form.errors[field])
 
     def do_request(self, method, token=None, data=None, **kwargs):
         # Before running any test, prepare the test cookie.
@@ -549,7 +551,6 @@ class PasswordResetRequestTestCase(SSOBaseTestCase):
         self.assertEqual(ctx['rpconfig'], None)
         self.assert_form_displayed(response)
         self.assert_stat_calls(['requested'])
-        self.assertTemplateUsed(response, 'registration/forgot_password.html')
 
     def test_get_with_email(self):
         response = self.get(data=dict(email='[e-mail]'))
@@ -579,17 +580,15 @@ class PasswordResetRequestTestCase(SSOBaseTestCase):
 
     def test_post_required_fields(self):
         response = self.post()
-        self.assert_form_displayed(response, email=FIELD_REQUIRED)
+        self.assert_form_displayed(
+            response, errors={'email': [FIELD_REQUIRED]}
+        )
         self.assert_stat_calls(['error.form'])
-        self.assertTemplateUsed(response, 'registration/forgot_password.html')
 
     def test_post_invalid_data(self):
         response = self.post(data=dict(email="111"))
-
-        self.assert_form_displayed(response, email=INVALID_EMAIL)
-        self.assert_form_displayed(response)
+        self.assert_form_displayed(response, errors={'email': [INVALID_EMAIL]})
         self.assert_stat_calls(['error.form'])
-        self.assertTemplateUsed(response, 'registration/forgot_password.html')
 
     def test_post_invalid_data_with_token(self):
         token = '12345678' * 2
@@ -600,7 +599,6 @@ class PasswordResetRequestTestCase(SSOBaseTestCase):
 
         self.assert_form_displayed(response)
         self.assert_stat_calls(['error.form'])
-        self.assertTemplateUsed(response, 'registration/forgot_password.html')
 
     def test_post_success(self):
         response = self.post(data=self.data)
@@ -625,18 +623,6 @@ class PasswordResetRequestTestCase(SSOBaseTestCase):
             self.client.session['token_email'], self.data['email'])
         self.assertTemplateUsed(response, 'registration/email_sent.html')
 
-    def test_post_with_unknown_email(self):
-        response = self.post(data=self.data)
-
-        self.mock_api_request_password_reset.assert_called_once_with(
-            email=self.data['email'], token=None)
-        self.assertEqual(response.status_code, 200)
-        self.assertEqual(
-            self.client.session['token_email'], self.data['email'])
-        self.assertEqual(response.context_data['email_heading'],
-                         'Reset password')
-        self.assertTemplateUsed(response, 'registration/email_sent.html')
-
     def test_post_email_invalidated(self):
         exc = api_errors.EmailInvalidated(Mock())
         self.mock_api_request_password_reset.side_effect = exc
@@ -682,7 +668,10 @@ class PasswordResetRequestTestCase(SSOBaseTestCase):
         self.mock_api_request_password_reset.side_effect = exc
         response = self.post(data=self.data)
         self.assertEqual(response.status_code, 200)
-        self.assert_form_displayed(response, __all__=ERROR_TOO_MANY_REQUESTS)
+        self.assert_form_displayed(
+            response,
+            errors={'__all__': [ERROR_TOO_MANY_REQUESTS]},
+        )
         self.assert_stat_calls(['error.too_many_requests'])
 
     def test_post_disabled_when_readonly(self):
@@ -696,6 +685,142 @@ class PasswordResetRequestTestCase(SSOBaseTestCase):
         self.assertEqual(response.status_code, 405)
 
 
+@switches(CAPTCHA=True)
+@override_settings(CAPTCHA_PUBLIC_KEY='42')
+class PasswordResetRequestWithCaptchaTestCase(SSOBaseTestCase):
+
+    url_name = 'forgot_password'
+
+    def setUp(self):
+        super(PasswordResetRequestWithCaptchaTestCase, self).setUp()
+        self.data = {'email': '[e-mail]', 'g-recaptcha-response': 'foo'}
+        self.mock_get_api_client = self.patch(
+            'webui.views.registration.get_api_client'
+        )
+        self.mock_api_request_password_reset = (
+            self.mock_get_api_client.return_value.request_password_reset
+        )
+        self.mock_captcha_verify = self.patch(CaptchaV2, 'verify')
+
+    def assert_form_displayed(self, response, errors=None):
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'registration/forgot_password.html')
+        self.assertTemplateUsed(response, 'widgets/recaptcha-v2.html')
+        self.assertTrue(response.context_data['captcha_required'])
+        self.assertEqual('42', response.context_data['CAPTCHA_PUBLIC_KEY'])
+        form = response.context_data['form']
+        for field, value in (errors or {}).items():
+            if field == 'captcha':
+                self.assertIn(value, response.content)
+            else:
+                self.assertEqual(value, form.errors[field])
+
+    def do_request(self, method, token=None, data=None, **kwargs):
+        # Before running any test, prepare the test cookie.
+        self.preseed_cookie_check()
+        url = reverse(self.url_name, kwargs={'token': token} if token else {})
+        return getattr(self.client, method.lower())(
+            url, data=data or {}, **kwargs
+        )
+
+    def get(self, **kwargs):
+        return self.do_request(method='GET', **kwargs)
+
+    def post(self, **kwargs):
+        return self.do_request(method='POST', **kwargs)
+
+    def test_get(self):
+        response = self.get()
+        self.assert_form_displayed(response)
+
+    def test_post_required_fields(self):
+        response = self.post()
+        self.assert_form_displayed(
+            response,
+            errors={
+                'email': [FIELD_REQUIRED],
+                'captcha': 'A captcha challenge is required',
+            },
+        )
+
+    def test_post_invalid_email(self):
+        response = self.post(data={'email': '111'})
+        self.assert_form_displayed(
+            response,
+            errors={
+                'email': [INVALID_EMAIL],
+                'captcha': 'A captcha challenge is required',
+            },
+        )
+
+    def test_post_invalid_captcha(self):
+        self.mock_captcha_verify.return_value = False
+        response = self.post(data=self.data)
+        self.assert_form_displayed(
+            response,
+            errors={'captcha': 'A captcha challenge is required'},
+        )
+
+    def test_post_success(self):
+        self.mock_captcha_verify.return_value = True
+        response = self.post(data=self.data)
+        self.mock_api_request_password_reset.assert_called_once_with(
+            email=self.data['email'], token=None
+        )
+        self.mock_captcha_verify.assert_called_once_with(
+            'foo', '127.0.0.1', timer=ANY
+        )
+        self.assertEqual(
+            self.client.session['token_email'], self.data['email']
+        )
+        self.assertTemplateUsed(response, 'registration/email_sent.html')
+
+    def test_post_email_invalidated(self):
+        self.mock_captcha_verify.return_value = True
+        exc = api_errors.EmailInvalidated(Mock())
+        self.mock_api_request_password_reset.side_effect = exc
+        response = self.post(data=self.data)
+        self.assert_form_displayed(response)
+
+    def test_post_account_suspended(self):
+        self.mock_captcha_verify.return_value = True
+        exc = api_errors.AccountSuspended(Mock())
+        self.mock_api_request_password_reset.side_effect = exc
+        response = self.post(data=self.data)
+        self.assert_form_displayed(response)
+
+    def test_post_account_deactivated(self):
+        self.mock_captcha_verify.return_value = True
+        exc = api_errors.AccountDeactivated(Mock())
+        self.mock_api_request_password_reset.side_effect = exc
+        response = self.post(data=self.data)
+        self.assert_form_displayed(response)
+
+    def test_post_can_not_reset_password(self):
+        self.mock_captcha_verify.return_value = True
+        exc = api_errors.CanNotResetPassword(Mock())
+        self.mock_api_request_password_reset.side_effect = exc
+        response = self.post(data=self.data)
+        self.assert_form_displayed(response)
+
+    def test_post_too_many_tokens(self):
+        self.mock_captcha_verify.return_value = True
+        exc = api_errors.TooManyTokens(Mock())
+        self.mock_api_request_password_reset.side_effect = exc
+        response = self.post(data=self.data)
+        self.assert_form_displayed(response)
+
+    def test_post_too_many_requests(self):
+        self.mock_captcha_verify.return_value = True
+        exc = api_errors.TooManyRequests(Mock())
+        self.mock_api_request_password_reset.side_effect = exc
+        response = self.post(data=self.data)
+        self.assert_form_displayed(
+            response,
+            errors={'__all__': [ERROR_TOO_MANY_REQUESTS]},
+        )
+
+
 @switches(TWOFACTOR=True)
 class TwoFactorSyncTestCase(SSOBaseTestCase):
     """Tests for 2fa OTP synchronization."""
diff --git a/src/webui/tests/test_views_ui.py b/src/webui/tests/test_views_ui.py
index e40f2d4..8bca417 100644
--- a/src/webui/tests/test_views_ui.py
+++ b/src/webui/tests/test_views_ui.py
@@ -828,6 +828,32 @@ class ForgotPasswordTestCase(LogHandlerTestCase, UIViewsBaseTestCase):
 
         self.assertEqual(self.mock_send_email.call_count, 1)
 
+    @switches(CAPTCHA=True)
+    @patch.object(CaptchaV2, 'verify', return_value=True)
+    def test_forgot_password_email_with_valid_captcha(self, mock_verify):
+        self.client.post(
+            reverse('forgot_password'),
+            {'email': self.email, 'g-recaptcha-response': 'valid'},
+        )
+        self.assertEqual(self.mock_send_email.call_count, 1)
+
+    @switches(CAPTCHA=True)
+    @patch.object(CaptchaV2, 'verify', return_value=False)
+    def test_forgot_password_email_with_invalid_captcha(self, mock_verify):
+        r = self.client.post(
+            reverse('forgot_password'),
+            {'email': self.email, 'g-recaptcha-response': 'invalid'},
+        )
+        self.assertEqual(self.mock_send_email.call_count, 0)
+        self.assertTemplateUsed(r, 'registration/forgot_password.html')
+
+    @switches(CAPTCHA=True)
+    @patch.object(CaptchaV2, 'verify', return_value=False)
+    def test_forgot_password_email_with_no_captcha(self, mock_verify):
+        r = self.client.post(reverse('forgot_password'), {'email': self.email})
+        self.assertEqual(self.mock_send_email.call_count, 0)
+        self.assertTemplateUsed(r, 'registration/forgot_password.html')
+
     def test_forgot_password_email_unknown_email(self):
         self.client.post(
             reverse('forgot_password'), {'email': "[e-mail]"})
diff --git a/src/webui/views/registration.py b/src/webui/views/registration.py
index 095258a..4e6a08c 100644
--- a/src/webui/views/registration.py
+++ b/src/webui/views/registration.py
@@ -4,6 +4,8 @@
 
 from __future__ import unicode_literals
 
+import logging
+
 from django.conf import settings
 from django.contrib import auth, messages
 from django.contrib.auth.decorators import login_required
@@ -30,8 +32,10 @@ from identityprovider.forms import (
     TwoFactorSyncForm,
 )
 from identityprovider.login import AuthenticationError
+from identityprovider.models.captcha import CaptchaV2
 from identityprovider.models.twofactor import login, sync
 from identityprovider.stats import stats
+from identityprovider.timeline_helpers import get_request_timing_function
 from identityprovider.utils import (
     add_non_field_error,
     polite_form_errors,
@@ -82,6 +86,9 @@ ERROR_TOO_MANY_REQUESTS = _('Too many requests. Please try again later.')
 WEB_CREATION_SOURCE = 'web-flow'
 
 
+logger = logging.getLogger(__name__)
+
+
 @redirect_home_if_logged_in
 @check_readonly
 @requires_cookies
@@ -182,6 +189,9 @@ def new_account(request, token=None):
 def forgot_password(request, token=None):
     # Need this for displaying metrics per RP
     rpconfig = get_rpconfig_from_request(request, token)
+    captcha_required = gargoyle.is_active('CAPTCHA', request)
+    captcha_verified = False
+    captcha_error_message = ''
 
     def collect_stats(key):
         stats.increment('flows.forgot_password', key=key, rpconfig=rpconfig)
@@ -193,8 +203,27 @@ def forgot_password(request, token=None):
         form = GenericEmailForm(initial={'email': email})
 
     elif request.method == 'POST':
+        if captcha_required:
+            captcha_solution = request.POST.get('g-recaptcha-response')
+            if captcha_solution:
+                captcha = CaptchaV2()
+                remote_addr = request.environ['REMOTE_ADDR']
+                timer = get_request_timing_function(request)
+                try:
+                    # ignore arbitrary email from unauthenticated user
+                    captcha_verified = captcha.verify(
+                        captcha_solution, remote_addr, timer=timer
+                    )
+                except Exception:
+                    logger.exception("reCaptcha error")
+
+            if not captcha_solution or not captcha_verified:
+                captcha_error_message = _(
+                    'A captcha challenge is required to complete the request.'
+                )
+
         form = GenericEmailForm(request.POST)
-        if form.is_valid():
+        if form.is_valid() and (captcha_verified or not captcha_required):
             email = form.cleaned_data['email']
 
             try:
@@ -249,13 +278,7 @@ def forgot_password(request, token=None):
                     'click on the link in your email.') % msgs
 
                 return display_email_sent(
-                    request,
-                    email,
-                    heading,
-                    reason,
-                    _("Check that you&rsquo;ve actually "
-                      "entered a subscribed email address."),
-                    rpconfig=rpconfig,
+                    request, email, heading, reason, rpconfig=rpconfig
                 )
 
         else:
@@ -266,7 +289,13 @@ def forgot_password(request, token=None):
     else:
         return HttpResponseNotAllowed(['GET', 'POST'])
 
-    context = dict(form=form, rpconfig=rpconfig)
+    context = {
+        'form': form,
+        'rpconfig': rpconfig,
+        'captcha_required': captcha_required,
+        'captcha_error_message': captcha_error_message,
+        'CAPTCHA_PUBLIC_KEY': settings.CAPTCHA_PUBLIC_KEY,
+    }
     template = 'registration/forgot_password.html'
     return TemplateResponse(request, template, context)
 
diff --git a/src/webui/views/ui.py b/src/webui/views/ui.py
index 373347c..fa9c42e 100644
--- a/src/webui/views/ui.py
+++ b/src/webui/views/ui.py
@@ -614,23 +614,20 @@ def confirm_email(request, authtoken, email_address, token=None):
             )
 
     if request.method == 'POST':
-        captcha_solution = request.POST.get('g-recaptcha-response')
         verified = False
-
-        if not captcha_solution and captcha_required:
-            captcha_error_message = _('A captcha challenge is required'
-                                      ' to complete the request.')
-
-        elif captcha_solution:
-            remote_addr = request.environ['REMOTE_ADDR']
-            try:
-                timer = get_request_timing_function(request)
-                verified = captcha.verify(
-                    captcha_solution, remote_addr, email_address, timer=timer)
-            except Exception:
-                logger.exception("reCaptcha error")
-
-            if not verified:
+        if captcha_required:
+            captcha_solution = request.POST.get('g-recaptcha-response')
+            if captcha_solution:
+                remote_addr = request.environ['REMOTE_ADDR']
+                try:
+                    timer = get_request_timing_function(request)
+                    verified = captcha.verify(
+                        captcha_solution, remote_addr, email_address, timer
+                    )
+                except Exception:
+                    logger.exception("reCaptcha error")
+
+            if not captcha_solution or not verified:
                 captcha_error_message = _('A captcha challenge is required'
                                           ' to complete the request.')
 
diff --git a/src/webui/views/utils.py b/src/webui/views/utils.py
index 63984f7..18f40fe 100644
--- a/src/webui/views/utils.py
+++ b/src/webui/views/utils.py
@@ -26,13 +26,13 @@ def allow_only(*methods):
     return decorator
 
 
-def display_email_sent(request, email, heading, reason, extra=None, token=None,
-                       rpconfig=None):
+def display_email_sent(
+    request, email, heading, reason, token=None, rpconfig=None
+):
     context = {
         'email_feedback': settings.FEEDBACK_TO_ADDRESS,
         'email_heading': heading,
         'email_reason': reason,
-        'email_notreceived_extra': extra,
         'email': email,
         'token': token,
         'rpconfig': rpconfig,