Merge lp:~maxiberta/canonical-identity-provider/drop-account-registration-captcha-web into lp:canonical-identity-provider/release

[Maximiliano Bertacchini]

Drop captcha from account registration via web.

Account registration captcha has been disabled for a long time (years?). And the implementation depends on reCaptcha v1 which is dead since March 2018. So, let's just drop all captcha bits from there.

[Matias Bordese (matiasb)]

LGTM

review: Approve

[Otto Co-Pilot (otto-copilot)]

Running landing tests failed
https://jenkins.ols.canonical.com/online-services/job/canonical-identity-provider/201/

=== modified file 'src/webui/templates/registration/_create_account_form.html'
--- src/webui/templates/registration/_create_account_form.html	2018-07-04 17:51:43 +0000
+++ src/webui/templates/registration/_create_account_form.html	2018-10-22 16:30:41 +0000
@@ -50,17 +50,6 @@
 
     {% include "widgets/passwords.html" with fields=create_form %}
 
-    {% if captcha_required %}
-    <div class="captcha" id="captcha">
-        {% if captcha_error_message %}
-            <span class="error">
-                {{ captcha_error_message }}
-            </span>
-        {% endif %}
-        {% include "widgets/recaptcha.html" %}
-    </div>
-    {% endif %}
-
     <div class="input-row{% if create_form.accept_tos.errors %} haserrors{% endif %} accept-tos-input">
 
         {% if create_form.accept_tos.errors %}

=== removed file 'src/webui/templates/widgets/recaptcha.html'
--- src/webui/templates/widgets/recaptcha.html	2014-12-11 17:44:29 +0000
+++ src/webui/templates/widgets/recaptcha.html	1970-01-01 00:00:00 +0000
@@ -1,43 +0,0 @@
-{% comment %}
-Copyright 2010 Canonical Ltd.  This software is licensed under the
-GNU Affero General Public License version 3 (see the file  LICENSE).
-{% endcomment %}
-
-{% load i18n %}
-{% load static_url %}
-<script type="text/javascript">
-    var RecaptchaOptions = {
-        theme: 'white',
-        custom_translations: {
-          visual_challenge : "{% trans "Get a visual challenge" %}",
-          audio_challenge : "{% trans "Get an audio challenge" %}",
-          refresh_btn : "{% trans "Get a new challenge" %}",
-          instructions_visual : "{% trans "Type the two words:" %}",
-          instructions_audio : "{% trans "Type what you hear:" %}",
-          help_btn : "{% trans "Help" %}",
-          play_again : "{% trans "Play sound again" %}",
-          cant_hear_this : "{% trans "Download sound as MP3" %}",
-          incorrect_try_again : "{% trans "Incorrect. Try again." %}"
-        },
-    };
-</script>
-<div {% if captcha_error %}class='captchaError'{% endif %}>
-{% ifequal captcha_error "&error=no-challenge" %}
-<p>
-{% blocktrans with "support_form"|static_url as support_form_url %}
-It appears that our captcha service was unable to load on this page.
-This may be caused by a plugin on your browser.
-Please correct this and try again. If the problem persists, please <a href="{{ support_form_url }}">contact support</a>
-{% endblocktrans %}
-</p>
-{% endifequal %}
-<script type="text/javascript" src="{{ CAPTCHA_API_URL_SECURE }}/challenge?k={{ CAPTCHA_PUBLIC_KEY }}{{ captcha_error }}">
-</script>
-<noscript>
-    <iframe src="{{ CAPTCHA_API_URL_SECURE }}/noscript?k={{ CAPTCHA_PUBLIC_KEY }}" height="300" width="500" frameborder="0" class="recaptcha-noscript">
-    </iframe>
-    <textarea class="recaptcha-challenge-field" name="recaptcha_challenge_field" rows="3" cols="40">
-    </textarea>
-    <input type="hidden" name="recaptcha_response_field" value="manual_challenge">
-</noscript>
-</div>

=== modified file 'src/webui/tests/test_views_registration.py'
--- src/webui/tests/test_views_registration.py	2018-05-28 20:15:33 +0000
+++ src/webui/tests/test_views_registration.py	2018-10-22 16:30:41 +0000
@@ -161,16 +161,6 @@
         ctx = response.context_data
         self.assertEqual(ctx['form']['email'].value(), '[e-mail]')
 
-    @switches(CAPTCHA=False)
-    def test_get_optional_captcha_switch_off(self):
-        response = self.get()
-        self.assertEqual(response.context_data['captcha_required'], False)
-
-    @switches(CAPTCHA=True, CAPTCHA_NEW_ACCOUNT=True)
-    def test_get_optional_captcha_switch_on(self):
-        response = self.get()
-        self.assertEqual(response.context_data['captcha_required'], True)
-
     def test_post_required_fields(self):
         response = self.post()
         self.assert_form_displayed(
@@ -213,9 +203,6 @@
             email=self.TESTDATA['email'],
             password=self.TESTDATA['password'],
             displayname=self.TESTDATA['displayname'],
-            captcha_id=None,
-            captcha_solution=None,
-            create_captcha=False,
             creation_source=WEB_CREATION_SOURCE,
         )
 
@@ -242,9 +229,6 @@
             password=self.TESTDATA['password'],
             displayname=self.TESTDATA['displayname'],
             username=self.TESTDATA['username'],
-            captcha_id=None,
-            captcha_solution=None,
-            create_captcha=False,
             creation_source=WEB_CREATION_SOURCE,
         )
         self.TESTDATA.pop('username')
@@ -337,9 +321,6 @@
         expected_args = dict(email=self.TESTDATA['email'],
                              password=self.TESTDATA['password'],
                              displayname=self.TESTDATA['displayname'],
-                             create_captcha=False,
-                             captcha_solution=None,
-                             captcha_id=None,
                              creation_source='web-flow',
                              oid_token=token)
         self.mock_api_register.assert_called_once_with(**expected_args)
@@ -351,38 +332,6 @@
         self.assert_form_displayed(response, email=VERIFY_EMAIL_MESSAGE)
         self.assert_stat_calls(['error.email'])
 
-    def test_post_captcha_required(self):
-        exc = api_errors.CaptchaRequired(Mock())
-        self.mock_api_register.side_effect = exc
-        response = self.post(**self.TESTDATA)
-        self.assert_form_displayed(response)
-        self.assertEqual(response.context_data['captcha_required'], True)
-
-    def test_post_captcha_failure(self):
-        mock_response = Mock()
-        body = {'extra': {'captcha_message': 'XXX'}}
-        exc = api_errors.CaptchaFailure(mock_response, body)
-        self.mock_api_register.side_effect = exc
-
-        response = self.post(**self.TESTDATA)
-        self.assert_form_displayed(response)
-        self.assertEqual(response.context_data['captcha_required'], True)
-        self.assertEqual(
-            response.context_data['captcha_error'],
-            '&error=XXX')
-        self.assert_stat_calls(['error.captcha'])
-
-    def test_post_captcha_error(self):
-        mock_response = Mock()
-        body = {}
-        exc = api_errors.CaptchaError(mock_response, body)
-        self.mock_api_register.side_effect = exc
-
-        response = self.post(**self.TESTDATA)
-        self.assert_form_displayed(response)
-        self.assertEqual(response.context_data['captcha_required'], True)
-        self.assert_stat_calls(['error.captcha'])
-
 
 class RegisterTimelineTestCase(
         SSOBaseTestCase, RegisterTestMixin, TimelineActionMixin):

=== modified file 'src/webui/tests/test_views_ui.py'
--- src/webui/tests/test_views_ui.py	2018-09-07 21:54:44 +0000
+++ src/webui/tests/test_views_ui.py	2018-10-22 16:30:41 +0000
@@ -10,7 +10,6 @@
 import urllib2
 from datetime import date
 from functools import partial
-from StringIO import StringIO
 from urlparse import urlsplit
 
 from django.conf import settings
@@ -29,7 +28,6 @@
 from django.test.utils import override_settings
 from django.urls import reverse
 from django.utils.html import escape
-from gargoyle import gargoyle
 from gargoyle.testutils import switches
 from mock import Mock, patch
 from pyquery import PyQuery
@@ -50,11 +48,7 @@
     OpenIDRPConfig,
     twofactor,
 )
-from identityprovider.models.captcha import (
-    Captcha,
-    CaptchaResponse,
-    CaptchaV2,
-)
+from identityprovider.models.captcha import CaptchaV2
 from identityprovider.models.const import (
     AccountStatus,
     AuthLogType,
@@ -65,7 +59,6 @@
 from identityprovider.tests import DEFAULT_USER_PASSWORD
 from identityprovider.tests.test_auth import AuthLogTestCaseMixin
 from identityprovider.tests.utils import (
-    MockHandler,
     SSOBaseTestCase,
     TimelineActionMixin,
 )
@@ -115,9 +108,6 @@
             'passwordconfirm': 'Testing123',
             'accept_tos': True
         }
-        if gargoyle.is_active('CAPTCHA'):
-            data['recaptcha_challenge_field'] = 'ignored'
-            data['recaptcha_response_field'] = 'ignored'
 
         return self.client.post(url, data, follow=follow)
 
@@ -132,24 +122,6 @@
         assert self.client.login(
             username=self.data['email'], password=self.data['password'])
 
-    def request_when_captcha_fails(self, url, data):
-        class MockCaptcha(object):
-            def __init__(self, *args):
-                pass
-
-            def verify(self, solution, ip_addr, email):
-                self.message = 'no-challenge'
-                return False
-
-            @classmethod
-            def new(cls, env):
-                return cls()
-
-        with patch.object(ui, 'Captcha', MockCaptcha):
-            r = self.client.post(url, data)
-
-        return r
-
 
 @override_settings(LANGUAGE_CODE='es')
 class SpanishUIViewsTestCase(BaseTestCase):
@@ -1348,62 +1320,6 @@
         self.assertFalse(email.is_verified)
 
 
-@override_settings(CAPTCHA_PRIVATE_KEY='some-private-key')
-class CaptchaVerificationTestCase(BaseTestCase):
-
-    success_status = 302
-
-    def setUp(self):
-        super(CaptchaVerificationTestCase, self).setUp()
-        mock_handler = MockHandler()
-        mock_handler.set_next_response(200, 'false\nno-challenge')
-        self.patch(Captcha, 'opener', new=urllib2.build_opener(mock_handler))
-
-        p = switches(CAPTCHA=True)
-        p.patch()
-        self.addCleanup(p.unpatch)
-
-    def test_new_account_when_form_validation_fails(self):
-        r = self.post_new_account()
-        self.assertTemplateUsed(r, 'registration/new_account.html')
-        msg = 'It appears that our captcha service was unable to load'
-        self.assertContains(r, msg)
-
-    def test_new_account_captcha_whitelist(self):
-        email = '[e-mail]'
-        pattern = '^canonicaltest(?:\+.+)?@gmail\.com$'
-        with self.settings(EMAIL_WHITELIST_REGEXP_LIST=[pattern]):
-            response = self.post_new_account(email=email)
-            self.assertEqual(response.status_code, self.success_status)
-
-    def test_new_account_captcha_whitelist_with_uuid(self):
-        email = '[e-mail]'
-        pattern = '^canonicaltest(?:\+.+)?@gmail\.com$'
-        with self.settings(EMAIL_WHITELIST_REGEXP_LIST=[pattern]):
-            response = self.post_new_account(email=email)
-            self.assertEqual(response.status_code, self.success_status)
-
-    def test_new_account_captcha_whitelist_fail(self):
-        email = '[e-mail]'
-        pattern = '^canonicaltest(?:\+.+)?@gmail\.com$'
-        with self.settings(EMAIL_WHITELIST_REGEXP_LIST=[pattern]):
-            response = self.post_new_account(email=email)
-            msg = 'It appears that our captcha service was unable to load'
-            self.assertContains(response, msg)
-
-    @patch.object(Captcha, '_open')
-    def test_uses_timeline_from_request(self, mock_open):
-        mock_open.return_value = CaptchaResponse(200, StringIO('true\na'))
-        request = Mock()
-        timeline = Timeline()
-        request.META = {'timeline.timeline': timeline}
-        request.POST = {'recaptcha_challenge_field': 'captcha-id'}
-        request.environ = {'REMOTE_ADDR': '127.0.0.1'}
-        ui._verify_captcha_response(None, request, None)
-        self.assertEqual(1, len(timeline.actions))
-        self.assertEqual('captcha-verify', timeline.actions[0].category)
-
-
 class CookiesTestCase(SSOBaseTestCase):
 
     def setUp(self):

=== modified file 'src/webui/views/registration.py'
--- src/webui/views/registration.py	2018-05-28 20:15:33 +0000
+++ src/webui/views/registration.py	2018-10-22 16:30:41 +0000
@@ -53,7 +53,6 @@
     requires_cookies,
 )
 from webui.views.utils import (
-    add_captcha_settings,
     display_email_sent,
     set_session_email,
 )
@@ -87,10 +86,6 @@
 @requires_cookies
 @require_http_methods(['GET', 'POST'])
 def new_account(request, token=None):
-    captcha_required = (gargoyle.is_active('CAPTCHA', request) and
-                        gargoyle.is_active('CAPTCHA_NEW_ACCOUNT', request))
-    captcha_error = ''
-    captcha_error_message = None
     rpconfig = get_rpconfig_from_request(request, token)
 
     def collect_stats(key):
@@ -108,14 +103,7 @@
             data = dict((k, v) for k, v in form.cleaned_data.items()
                         if k in ('email', 'password', 'displayname',
                                  'username'))
-            data['captcha_id'] = request.POST.get(
-                'recaptcha_challenge_field'
-            )
-            data['captcha_solution'] = request.POST.get(
-                'recaptcha_response_field'
-            )
             # we'll handle our own capture generation
-            data['create_captcha'] = False
             data['creation_source'] = WEB_CREATION_SOURCE
             if token:
                 data['oid_token'] = token
@@ -136,15 +124,6 @@
                 collect_stats('error.email')
                 form._errors['email'] = [VERIFY_EMAIL_MESSAGE]
 
-            except api_errors.CaptchaRequired as e:
-                captcha_required = True
-                collect_stats('captcha_required')
-
-            except (api_errors.CaptchaFailure, api_errors.CaptchaError) as e:
-                captcha_required = True
-                captcha_error = '&error=' + e.extra.get('captcha_message', '')
-                captcha_error_message = _('Incorrect captcha solution')
-                collect_stats('error.captcha')
             except Exception as e:
                 return HttpResponseServerError("exception: " + str(e))
             else:
@@ -175,12 +154,7 @@
         'form': form,
         'rpconfig': rpconfig,
         'token': token,
-        'captcha_required': captcha_required,
-        'captcha_error': captcha_error,
-        'captcha_error_message': captcha_error_message,
     }
-    if captcha_required:
-        context = add_captcha_settings(context)
 
     if form.errors:
         err = form.errors.get('email', [''])[0]

=== modified file 'src/webui/views/ui.py'
--- src/webui/views/ui.py	2018-08-24 15:30:53 +0000
+++ src/webui/views/ui.py	2018-10-22 16:30:41 +0000
@@ -56,11 +56,7 @@
 
 )
 from identityprovider.models import twofactor
-from identityprovider.models.captcha import (
-    Captcha,
-    CaptchaV2,
-    VerifyCaptchaError
-)
+from identityprovider.models.captcha import CaptchaV2
 from identityprovider.models.const import AccountStatus, AuthTokenType
 from identityprovider.signals import login_failed, login_succeeded
 from identityprovider.signed import BadSignedValue
@@ -94,7 +90,6 @@
     requires_cookies,
 )
 from webui.views import registration
-from webui.views.utils import add_captcha_settings
 
 
 ACCOUNT_CREATED = _("Your account was created successfully")
@@ -150,11 +145,6 @@
             self, request, token, rpconfig, form, create_account_form=None):
         context = super(LoginView, self).get_context(
             request, token=token, rpconfig=rpconfig, form=form)
-        # add captcha and account creation form
-        context['captcha_required'] = (
-            gargoyle.is_active('CAPTCHA', request) and
-            gargoyle.is_active('CAPTCHA_NEW_ACCOUNT', request))
-        context = add_captcha_settings(context)
         context['create_account_form'] = create_account_form
         return context
 
@@ -503,30 +493,6 @@
     return registration.new_account(request, token)
 
 
-def _verify_captcha_response(template, request, form):
-    captcha = Captcha(request.POST.get('recaptcha_challenge_field'))
-    captcha_solution = request.POST.get('recaptcha_response_field')
-    email = request.POST.get('email', '')
-    ip_addr = request.environ["REMOTE_ADDR"]
-    try:
-        timer_fn = get_request_timing_function(request)
-        verified = captcha.verify(captcha_solution, ip_addr, email,
-                                  timer=timer_fn)
-        if verified:
-            return None
-    except VerifyCaptchaError:
-        logger.exception("reCaptcha connection error")
-
-    # not verified
-    return render(
-        request,
-        template,
-        add_captcha_settings({
-            'form': form,
-            'captcha_error': ('&error=%s' % captcha.message),
-            'captcha_required': True}))
-
-
 @require_twofactor_authenticated(
     _("Please log in to use this confirmation code"))
 def confirm_email(request, authtoken, email_address, token=None):

=== modified file 'src/webui/views/utils.py'
--- src/webui/views/utils.py	2015-05-08 19:25:27 +0000
+++ src/webui/views/utils.py	2018-10-22 16:30:41 +0000
@@ -43,10 +43,3 @@
 def set_session_email(session, email):
     """Place information about the current token's email in the session"""
     session['token_email'] = email
-
-
-def add_captcha_settings(context):
-    d = {'CAPTCHA_PUBLIC_KEY': settings.CAPTCHA_PUBLIC_KEY,
-         'CAPTCHA_API_URL_SECURE': settings.CAPTCHA_API_URL_SECURE}
-    d.update(context)
-    return d