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

[Maximiliano Bertacchini]

Drop captcha from account registration API v1.x.

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.

[Guillermo Gonzalez (verterok)]

LGTM

review: Approve

=== modified file 'src/api/templates/api/wadl1.0.xml'
--- src/api/templates/api/wadl1.0.xml	2017-06-12 16:00:39 +0000
+++ src/api/templates/api/wadl1.0.xml	2018-10-22 16:11:43 +0000
@@ -562,14 +562,14 @@
                         required="true" fixed="register">
               <wadl:doc>The name of the operation being invoked.</wadl:doc>
             </wadl:param>
-            <wadl:param style="query" required="true"
+            <wadl:param style="query" required="false"
                         name="captcha_solution">
              <wadl:doc xmlns="http://www.w3.org/1999/xhtml">
 Solution for the generated captcha.
 </wadl:doc>
               
             </wadl:param>
-            <wadl:param style="query" required="true"
+            <wadl:param style="query" required="false"
                         name="captcha_id">
              <wadl:doc xmlns="http://www.w3.org/1999/xhtml">
 ID for the generated captcha

=== modified file 'src/api/templates/api/wadl1.1.xml'
--- src/api/templates/api/wadl1.1.xml	2017-06-12 16:00:39 +0000
+++ src/api/templates/api/wadl1.1.xml	2018-10-22 16:11:43 +0000
@@ -568,14 +568,14 @@
                         required="true" fixed="register">
               <wadl:doc>The name of the operation being invoked.</wadl:doc>
             </wadl:param>
-            <wadl:param style="query" required="true"
+            <wadl:param style="query" required="false"
                         name="captcha_solution">
              <wadl:doc xmlns="http://www.w3.org/1999/xhtml">
 Solution for the generated captcha.
 </wadl:doc>
               
             </wadl:param>
-            <wadl:param style="query" required="true"
+            <wadl:param style="query" required="false"
                         name="captcha_id">
              <wadl:doc xmlns="http://www.w3.org/1999/xhtml">
 ID for the generated captcha

=== modified file 'src/api/v10/forms.py'
--- src/api/v10/forms.py	2015-05-08 19:25:27 +0000
+++ src/api/v10/forms.py	2018-10-22 16:11:43 +0000
@@ -5,17 +5,12 @@
 
 from __future__ import unicode_literals
 
-from django import forms
 from django.forms import fields
-from django.utils.translation import ugettext_lazy as _
 
 from identityprovider.forms import NewAccountForm
-from identityprovider.models.captcha import Captcha
 
 
 class WebserviceCreateAccountForm(NewAccountForm):
-    captcha_id = fields.CharField(max_length=1024)
-    captcha_solution = fields.CharField(max_length=256)
     remote_ip = fields.CharField(max_length=256)
     platform = fields.TypedChoiceField(choices=[
         ('web', 'Web'), ('desktop', 'Desktop'), ('mobile', 'Mobile')],
@@ -38,21 +33,3 @@
         if not validate_redirect_to:
             validate_redirect_to = None
         return validate_redirect_to
-
-    def clean(self):
-        cleaned_data = super(WebserviceCreateAccountForm, self).clean()
-        captcha_id = cleaned_data.get('captcha_id')
-        captcha_solution = cleaned_data.get('captcha_solution')
-
-        # The remote IP address is absolutely required, and comes from
-        # SSO itself, not from the client.  If it's missing, it's a
-        # programming error, and should not be returned to the client
-        # as a validation error.  So, we use a normal key lookup here.
-        remote_ip = cleaned_data['remote_ip']
-
-        captcha = Captcha(captcha_id)
-        email = cleaned_data.get('email', '')
-        if captcha.verify(captcha_solution, remote_ip, email):
-            return cleaned_data
-        # not verified
-        raise forms.ValidationError(_("Wrong captcha solution."))

=== modified file 'src/api/v10/handlers.py'
--- src/api/v10/handlers.py	2018-10-22 15:10:24 +0000
+++ src/api/v10/handlers.py	2018-10-22 16:11:43 +0000
@@ -36,9 +36,6 @@
     EmailAddress,
     get_team_memberships_for_user,
 )
-from identityprovider.models.captcha import (
-    VerifyCaptchaError,
-)
 from identityprovider.models.const import AuthTokenType, EmailStatus
 from identityprovider.signals import (
     account_created,
@@ -189,25 +186,16 @@
         data = request.data
         data['remote_ip'] = request.environ['REMOTE_ADDR']
         form = WebserviceCreateAccountForm(data)
-        try:
-            if not form.is_valid():
-                errors = dict((k, map(unicode, v))
-                              for (k, v) in form.errors.items())
-                # XXX: cope with client trimming error messages
-                if unicode(PASSWORD_LEAKED) in errors.get('password', ''):
-                    errors['password'] = unicode(_(
-                        'unsafe: leaked by security breach '
-                        'on another website'))
-                result = {'status': 'error', 'errors': errors}
-                return result
-        except VerifyCaptchaError:
-            logger.exception("reCaptcha connection error")
-            msg = unicode(
-                _('Unable to verify captcha. Please try again shortly.'))
-            return {
-                'status': 'error',
-                'errors': {'captcha_solution': [msg]}
-            }
+        if not form.is_valid():
+            errors = dict((k, map(unicode, v))
+                          for (k, v) in form.errors.items())
+            # XXX: cope with client trimming error messages
+            if unicode(PASSWORD_LEAKED) in errors.get('password', ''):
+                errors['password'] = unicode(_(
+                    'unsafe: leaked by security breach '
+                    'on another website'))
+            result = {'status': 'error', 'errors': errors}
+            return result
 
         cleaned_data = form.cleaned_data
         requested_email = cleaned_data['email']

=== modified file 'src/api/v10/tests/test_forms.py'
--- src/api/v10/tests/test_forms.py	2015-11-24 14:38:04 +0000
+++ src/api/v10/tests/test_forms.py	2018-10-22 16:11:43 +0000
@@ -5,23 +5,12 @@
 
 from __future__ import unicode_literals
 
-from django.test.utils import override_settings
-
 from api.v10.forms import WebserviceCreateAccountForm
 from identityprovider.tests.utils import SSOBaseTestCase
 
 
-@override_settings(CAPTCHA_PUBLIC_KEY='public', CAPTCHA_PRIVATE_KEY='private')
 class WebServiceCreateAccountFormTestCase(SSOBaseTestCase):
 
-    def setUp(self):
-        super(WebServiceCreateAccountFormTestCase, self).setUp()
-        # patch Captcha so it never hits the real network
-        self.mock_captcha_open = self.patch(
-            'identityprovider.models.captcha.Captcha._open')
-        self.mock_captcha_open.return_value.is_error = False
-        self.mock_captcha_open.return_value.data.return_value = 'true\nyey'
-
     def test_nonascii_password(self):
         data = {'password': 'Curuzú Cuatiá',
                 'remote_ip': '127.0.0.1'}
@@ -46,19 +35,6 @@
         self.assertTrue(form.is_valid())
         self.assertEqual(form.cleaned_data['platform'], 'desktop')
 
-    def test_captcha_checked_for_whitelist(self):
-        data = {
-            'email': '[e-mail]',
-            'password': 'password1A',
-            'captcha_id': '1',
-            'captcha_solution': '2',
-            'remote_ip': '127.0.0.1',
-        }
-        pattern = '^canonicaltest(?:\+.+)?@gmail\.com$'
-        with self.settings(EMAIL_WHITELIST_REGEXP_LIST=[pattern]):
-            form = WebserviceCreateAccountForm(data)
-            self.assertTrue(form.is_valid())
-
     def test_default_cleaned_validate_redirect_to(self):
         data = {
             'email': '[e-mail]',

=== modified file 'src/api/v10/tests/test_handlers.py'
--- src/api/v10/tests/test_handlers.py	2018-10-22 15:10:24 +0000
+++ src/api/v10/tests/test_handlers.py	2018-10-22 16:11:43 +0000
@@ -173,8 +173,7 @@
                                   email_validated=False)
 
         response = self.api.registrations.register(
-            email='[e-mail]', password='blogdf3Daa',
-            captcha_solution='solution', captcha_id='id')
+            email='[e-mail]', password='blogdf3Daa')
 
         self.assertEqual(response['errors'],
                          {'email': ["Email already registered"]})
@@ -184,8 +183,7 @@
         email = '[e-mail]'
 
         response = self.api.registrations.register(
-            email=email, password='MySecretPassword1',
-            captcha_solution='foobar', captcha_id='id')
+            email=email, password='MySecretPassword1')
         self.assertEqual(response['status'], 'ok')
 
         account = Account.objects.get_by_email(email)
@@ -208,8 +206,7 @@
         self.factory.make_leaked_credential(email=email, password=password)
 
         response = self.api.registrations.register(
-            email=email, password=password,
-            captcha_solution='foobar', captcha_id='id')
+            email=email, password=password)
         self.assertEqual(response['status'], 'error')
         self.assertEqual(
             response['errors'],
@@ -223,7 +220,6 @@
 
         response = self.api.registrations.register(
             email=email, password='MySecretPassword1',
-            captcha_solution='foobar', captcha_id='id',
             displayname='Test User')
         self.assertEqual(response['status'], 'ok')
 
@@ -242,7 +238,6 @@
 
         response = self.api.registrations.register(
             email=email, password='MySecretPassword1',
-            captcha_solution='foobar', captcha_id='id',
             displayname='Test User')
 
         self.assertEqual(response['status'], 'ok')
@@ -259,7 +254,6 @@
         # password: 8 characters long (no uppercase or numbers, only lowercase)
         response = self.api.registrations.register(
             email=email, password='abcdefgh',
-            captcha_solution='foobar', captcha_id='id',
             displayname='Test User')
         self.assertEqual(response['status'], 'ok')
 
@@ -271,8 +265,7 @@
         email = self.factory.make_email_address()
 
         response = self.api.registrations.register(
-            email=email, password='MySecretPassword1',
-            captcha_solution='foobar', captcha_id='id')
+            email=email, password='MySecretPassword1')
         self.assertEqual(response['status'], 'ok')
         self.assert_new_user_email_sent(email)
 
@@ -281,7 +274,6 @@
 
         response = self.api.registrations.register(
             email=email, password='MySecretPassword1',
-            captcha_solution='foobar', captcha_id='id',
             platform='mobile')
         self.assertEqual(response['status'], 'ok')
         self.assert_new_user_email_sent(email, 'mobile')
@@ -291,7 +283,6 @@
 
         response = self.api.registrations.register(
             email=email, password='MySecretPassword1',
-            captcha_solution='foobar', captcha_id='id',
             platform='foo')
 
         self.assertEqual(response['status'], 'error')
@@ -303,7 +294,6 @@
 
         response = self.api.registrations.register(
             email=email_address, password='MySecretPassword1',
-            captcha_solution='foobar', captcha_id='id',
             platform='desktop')
 
         self.assertEqual(response['status'], 'ok')
@@ -314,7 +304,6 @@
 
         response = self.api.registrations.register(
             email=email_address, password='MySecretPassword1',
-            captcha_solution='foobar', captcha_id='id',
             platform='desktop', validate_redirect_to='http://foo')
 
         self.assertEqual(response['status'], 'ok')
@@ -327,7 +316,6 @@
 
         response = self.api.registrations.register(
             email=email_address, password='MySecretPassword1',
-            captcha_solution='foobar', captcha_id='id',
             platform='mobile', validate_redirect_to='http://foo')
 
         self.assertEqual(response['status'], 'ok')
@@ -341,7 +329,6 @@
 
         response = self.api.registrations.register(
             email=email_address, password='MySecretPassword1',
-            captcha_solution='foobar', captcha_id='id',
             platform='mobile')
 
         self.assertEqual(response['status'], 'ok')
@@ -385,9 +372,7 @@
                 self.url,
                 {'ws.op': 'register',
                  'email': email_address,
-                 'password': 'MySecretPassword1',
-                 'captcha_solution': 'foobar',
-                 'captcha_id': 'id'})
+                 'password': 'MySecretPassword1'})
 
     @switches(USER_REGISTRATION_API_ENABLED=False)
     def test_feature_flag_disabled_with_internal_client(self):
@@ -396,7 +381,6 @@
         data = {
             'ws.op': 'register', 'displayname': 'Test User', 'email': email,
             'password': 'MySecretPassword1',
-            'captcha_solution': 'foobar', 'captcha_id': 'id',
         }
 
         response = self.client.get(self.url, data)
@@ -413,7 +397,6 @@
         data = {
             'ws.op': 'register', 'displayname': 'Test User', 'email': email,
             'password': 'MySecretPassword1',
-            'captcha_solution': 'foobar', 'captcha_id': 'id',
         }
         headers = {'HTTP_X_FORWARDED_FOR': '127.0.0.1'}
 
@@ -429,7 +412,6 @@
         data = {
             'ws.op': 'register', 'displayname': 'Test User', 'email': email,
             'password': 'MySecretPassword1',
-            'captcha_solution': 'foobar', 'captcha_id': 'id',
         }
 
         response = self.client.get(self.url, data)
@@ -446,7 +428,6 @@
         data = {
             'ws.op': 'register', 'displayname': 'Test User', 'email': email,
             'password': 'MySecretPassword1',
-            'captcha_solution': 'foobar', 'captcha_id': 'id',
         }
         headers = {'HTTP_X_FORWARDED_FOR': '127.0.0.1'}