Catch a generic CaptchaError and make captcha service usage more generic (#18651)

* Adjust configuration to avoid circular import

We shouldn't default to a ReCaptcha service either

* Use a generic form field

* Return a generic error message

* Use a generic CaptchaError instead

Author: di

dev/environment

@@ -81,7 +81,7 @@ HCAPTCHA_SECRET_KEY=0x0000000000000000000000000000000000000000
 # HCAPTCHA_SITE_KEY=30000000-ffff-ffff-ffff-000000000003
 
 # Example of Captcha backend configuration
-# CAPTCHA_BACKEND=warehouse.captcha.hcaptcha.Service
+CAPTCHA_BACKEND=warehouse.captcha.hcaptcha.Service
 
 # Example of HelpScout configuration
 # HELPSCOUT_WAREHOUSE_APP_ID="an insecure helpscout app id"

tests/conftest.py

@@ -346,6 +346,7 @@ def get_app_config(database, nondefaults=None):
         "oidc.jwk_cache_url": "redis://localhost:0/",
         "warehouse.oidc.audience": "pypi",
         "oidc.backend": "warehouse.oidc.services.NullOIDCPublisherService",
+        "captcha.backend": "warehouse.captcha.hcaptcha.Service",
     }
 
     if nondefaults:

tests/unit/accounts/test_forms.py

@@ -433,7 +433,7 @@ def test_validate(self, metrics):
                     "new_password": "mysupersecurepassword1!",
                     "password_confirm": "mysupersecurepassword1!",
                     "email": "[email protected]",
-                    "g_recaptcha_reponse": "",
+                    "captcha_reponse": "",
                 }
             ),
             user_service=user_service,
@@ -634,12 +634,12 @@ def test_recaptcha_disabled(self):
         assert not form.validate()
         # there shouldn't be any errors for the recaptcha field if it's
         # disabled
-        assert not form.g_recaptcha_response.errors
+        assert not form.captcha_response.errors
 
     def test_recaptcha_required_error(self):
         form = forms.RegistrationForm(
             request=pretend.stub(),
-            formdata=MultiDict({"g_recaptcha_response": ""}),
+            formdata=MultiDict({"captcha_response": ""}),
             user_service=pretend.stub(),
             captcha_service=pretend.stub(
                 enabled=True,
@@ -648,12 +648,12 @@ def test_recaptcha_required_error(self):
             breach_service=pretend.stub(check_password=lambda pw, tags=None: False),
         )
         assert not form.validate()
-        assert form.g_recaptcha_response.errors.pop() == "Recaptcha error."
+        assert form.captcha_response.errors.pop() == "Captcha error."
 
     def test_recaptcha_error(self):
         form = forms.RegistrationForm(
             request=pretend.stub(),
-            formdata=MultiDict({"g_recaptcha_response": "asd"}),
+            formdata=MultiDict({"captcha_response": "asd"}),
             user_service=pretend.stub(),
             captcha_service=pretend.stub(
                 verify_response=pretend.raiser(recaptcha.RecaptchaError),
@@ -662,7 +662,7 @@ def test_recaptcha_error(self):
             breach_service=pretend.stub(check_password=lambda pw, tags=None: False),
         )
         assert not form.validate()
-        assert form.g_recaptcha_response.errors.pop() == "Recaptcha error."
+        assert form.captcha_response.errors.pop() == "Captcha error."
 
     def test_username_exists(self, pyramid_config):
         form = forms.RegistrationForm(

tests/unit/accounts/test_views.py

@@ -1752,7 +1752,7 @@ def _find_service(service=None, name=None, context=None):
                 "password_confirm": "MyStr0ng!shP455w0rd",
                 "email": "[email protected]",
                 "full_name": "full_name",
-                "g_recaptcha_response": "captchavalue",
+                "captcha_response": "captchavalue",
             }
         )
 

tests/unit/captcha/test_init.py

@@ -2,26 +2,7 @@
 
 import pretend
 
-from warehouse.captcha import includeme, interfaces, recaptcha
-
-
-def test_includeme_defaults_to_recaptcha():
-    config = pretend.stub(
-        registry=pretend.stub(settings={}),
-        maybe_dotted=lambda i: i,
-        register_service_factory=pretend.call_recorder(
-            lambda factory, iface, name: None
-        ),
-    )
-    includeme(config)
-
-    assert config.register_service_factory.calls == [
-        pretend.call(
-            recaptcha.Service.create_service,
-            interfaces.ICaptchaService,
-            name="captcha",
-        ),
-    ]
+from warehouse.captcha import includeme, interfaces
 
 
 def test_include_with_custom_backend():

warehouse/accounts/forms.py

@@ -29,7 +29,7 @@
 )
 from warehouse.accounts.models import DisableReason, ProhibitedEmailDomain
 from warehouse.accounts.services import RECOVERY_CODE_BYTES
-from warehouse.captcha import recaptcha
+from warehouse.captcha import CaptchaError
 from warehouse.constants import MAX_PASSWORD_SIZE
 from warehouse.email import (
     send_password_compromised_email_hibp,
@@ -407,24 +407,24 @@ class RegistrationForm(  # type: ignore[misc]
             PreventNullBytesValidator(),
         ]
     )
-    g_recaptcha_response = wtforms.StringField()
+    captcha_response = wtforms.StringField()
 
     def __init__(self, *args, captcha_service, user_service, **kwargs):
         super().__init__(*args, **kwargs)
         self.user_service = user_service
         self.user_id = None
         self.captcha_service = captcha_service
 
-    def validate_g_recaptcha_response(self, field):
+    def validate_captcha_response(self, field):
         # do required data validation here due to enabled flag being required
         if self.captcha_service.enabled and not field.data:
-            raise wtforms.validators.ValidationError("Recaptcha error.")
+            raise wtforms.validators.ValidationError("Captcha error.")
         try:
             self.captcha_service.verify_response(field.data)
-        except recaptcha.RecaptchaError:
+        except CaptchaError:
             # TODO: log error
             # don't want to provide the user with any detail
-            raise wtforms.validators.ValidationError("Recaptcha error.")
+            raise wtforms.validators.ValidationError("Captcha error.")
 
 
 class LoginForm(PasswordMixin, UsernameMixin, wtforms.Form):

warehouse/captcha/__init__.py

@@ -1,14 +1,15 @@
 # SPDX-License-Identifier: Apache-2.0
 
 from .interfaces import ICaptchaService
-from .recaptcha import Service
+
+
+class CaptchaError(ValueError):
+    pass
 
 
 def includeme(config):
     # Register our Captcha service
-    captcha_class = config.maybe_dotted(
-        config.registry.settings.get("captcha.backend", Service)
-    )
+    captcha_class = config.maybe_dotted(config.registry.settings["captcha.backend"])
     config.register_service_factory(
         captcha_class.create_service,
         ICaptchaService,

warehouse/captcha/hcaptcha.py

@@ -8,12 +8,13 @@
 from requests.exceptions import Timeout
 from zope.interface import implementer
 
+from . import CaptchaError
 from .interfaces import ChallengeResponse, ICaptchaService
 
 VERIFY_URL = "https://api.hcaptcha.com/siteverify"
 
 
-class HCaptchaError(ValueError):
+class HCaptchaError(CaptchaError):
     pass
 
 

warehouse/captcha/recaptcha.py

@@ -6,12 +6,13 @@
 
 from zope.interface import implementer
 
+from . import CaptchaError
 from .interfaces import ChallengeResponse, ICaptchaService
 
 VERIFY_URL = "https://www.google.com/recaptcha/api/siteverify"
 
 
-class RecaptchaError(ValueError):
+class RecaptchaError(CaptchaError):
     pass
 
 

warehouse/templates/includes/input-captcha.html

@@ -4,9 +4,9 @@
   {% if captcha_svc.enabled %}
     <div class="{{ captcha_svc.class_name }}"
          data-sitekey="{{ captcha_svc.site_key }}"></div>
-    {% if form.g_recaptcha_response.errors %}
+    {% if form.captcha_response.errors %}
       <ul class="form-errors">
-        {% for error in form.g_recaptcha_response.errors %}<li>{{ error }}</li>{% endfor %}
+        {% for error in form.captcha_response.errors %}<li>{{ error }}</li>{% endfor %}
       </ul>
     {% endif %}
   {% endif %}