fix(captcha): correctly deal with valid submission after invalid one

Fixes #13335

docs/changes.rst

@@ -15,6 +15,7 @@ Not yet released.
 * Avoid query parser crash in multi-threaded environments.
 * Avoid :ref:`autofix` crash on multi-value strings.
 * Make project tokens work when :ref:`2fa` or :ref:`component-agreement` are enforced.
+* Captcha solution were sometimes not accepted.
 
 **Compatibility**
 

weblate/accounts/captcha.py

@@ -5,10 +5,14 @@
 """Simple mathematical captcha."""
 
 import ast
+import base64
+import json
 import operator
 import time
+import urllib.parse
 from random import SystemRandom
 
+from altcha import Challenge, Solution, solve_challenge
 from django.utils.html import format_html
 
 from weblate.utils.templatetags.icons import icon
@@ -106,3 +110,30 @@ def eval_node(node):
         # binary operation
         return eval_node(node.op)(eval_node(node.left), eval_node(node.right))
     raise ValueError(node)
+
+
+def solve_altcha(challenge: Challenge, number: int | None = None) -> str:
+    solution: Solution = solve_challenge(
+        challenge=challenge.challenge,
+        salt=challenge.salt,
+        algorithm=challenge.algorithm,
+        max_number=challenge.maxnumber,
+        start=0,
+    )
+    # Make sure the challenge expiry is in past
+    split_salt = challenge.salt.split("?")
+    params = urllib.parse.parse_qs(split_salt[1])
+    expires = int(params["expires"][0])
+    while time.time() == expires:
+        time.sleep(0.1)
+    return base64.b64encode(
+        json.dumps(
+            {
+                "algorithm": challenge.algorithm,
+                "challenge": challenge.challenge,
+                "number": solution.number if number is None else number,
+                "salt": challenge.salt,
+                "signature": challenge.signature,
+            }
+        ).encode("utf-8")
+    ).decode("utf-8")

weblate/accounts/forms.py

@@ -542,8 +542,9 @@ def clean_altcha(self) -> None:
 
     def is_valid(self) -> bool:
         result = super().is_valid()
-        self.cleanup_session()
-        if not result and self.has_captcha:
+        if result:
+            self.cleanup_session()
+        elif self.has_captcha:
             self.store_challenge()
         return result
 

weblate/accounts/tests/test_captcha.py

@@ -6,7 +6,11 @@
 
 from unittest import TestCase
 
-from weblate.accounts.captcha import MathCaptcha
+from django.http import HttpRequest
+from django.test.utils import override_settings
+
+from weblate.accounts.captcha import MathCaptcha, solve_altcha
+from weblate.accounts.forms import CaptchaForm
 
 
 class CaptchaTest(TestCase):
@@ -24,3 +28,89 @@ def test_generate(self) -> None:
         for operator in MathCaptcha.operators:
             captcha.operators = (operator,)
             self.assertIn(operator, captcha.generate_question())
+
+    @override_settings(
+        REGISTRATION_CAPTCHA=True,
+        ALTCHA_MAX_NUMBER=100,
+    )
+    def test_form(self) -> None:
+        def create_request(session):
+            request = HttpRequest()
+            request.method = "POST"
+            request.session = session
+            return request
+
+        session_store = {}
+
+        # Successful submission
+        form = CaptchaForm(request=create_request(session_store))
+        self.assertIn("captcha_challenge", session_store)
+        self.assertIn("captcha", session_store)
+        math = MathCaptcha.unserialize(session_store["captcha"])
+        form = CaptchaForm(
+            request=create_request(session_store),
+            data={"captcha": math.result, "altcha": solve_altcha(form.challenge)},
+        )
+        self.assertTrue(form.is_valid())
+        self.assertEqual(session_store, {})
+
+        # Wrong captcha
+        form = CaptchaForm(request=create_request(session_store))
+        self.assertIn("captcha_challenge", session_store)
+        self.assertIn("captcha", session_store)
+        form = CaptchaForm(
+            request=create_request(session_store),
+            data={"captcha": -1, "altcha": solve_altcha(form.challenge)},
+        )
+        self.assertFalse(form.is_valid())
+        self.assertIn("captcha_challenge", session_store)
+        self.assertIn("captcha", session_store)
+        math = MathCaptcha.unserialize(session_store["captcha"])
+        form = CaptchaForm(
+            request=create_request(session_store),
+            data={"captcha": math.result, "altcha": solve_altcha(form.challenge)},
+        )
+        self.assertTrue(form.is_valid())
+        self.assertEqual(session_store, {})
+
+        # Wrong altcha
+        form = CaptchaForm(request=create_request(session_store))
+        self.assertIn("captcha_challenge", session_store)
+        self.assertIn("captcha", session_store)
+        math = MathCaptcha.unserialize(session_store["captcha"])
+        form = CaptchaForm(
+            request=create_request(session_store),
+            data={
+                "captcha": math.result,
+                "altcha": solve_altcha(form.challenge, number=-1),
+            },
+        )
+        self.assertFalse(form.is_valid())
+        self.assertIn("captcha_challenge", session_store)
+        self.assertIn("captcha", session_store)
+        math = MathCaptcha.unserialize(session_store["captcha"])
+        form = CaptchaForm(
+            request=create_request(session_store),
+            data={"captcha": math.result, "altcha": solve_altcha(form.challenge)},
+        )
+        self.assertTrue(form.is_valid())
+        self.assertEqual(session_store, {})
+
+        # Wrong both
+        form = CaptchaForm(request=create_request(session_store))
+        self.assertIn("captcha_challenge", session_store)
+        self.assertIn("captcha", session_store)
+        form = CaptchaForm(
+            request=create_request(session_store),
+            data={"captcha": -1, "altcha": solve_altcha(form.challenge, number=-1)},
+        )
+        self.assertFalse(form.is_valid())
+        self.assertIn("captcha_challenge", session_store)
+        self.assertIn("captcha", session_store)
+        math = MathCaptcha.unserialize(session_store["captcha"])
+        form = CaptchaForm(
+            request=create_request(session_store),
+            data={"captcha": math.result, "altcha": solve_altcha(form.challenge)},
+        )
+        self.assertTrue(form.is_valid())
+        self.assertEqual(session_store, {})

weblate/accounts/tests/test_registration.py

@@ -6,18 +6,17 @@
 
 from __future__ import annotations
 
-import base64
-import json
+from typing import TYPE_CHECKING
 from urllib.parse import parse_qs, urlparse
 
 import responses
-from altcha import Challenge, Solution, solve_challenge
 from django.conf import settings
 from django.core import mail
 from django.test import Client, TestCase
 from django.test.utils import override_settings
 from django.urls import reverse
 
+from weblate.accounts.captcha import solve_altcha
 from weblate.accounts.models import VerifiedEmail
 from weblate.accounts.tasks import cleanup_social_auth
 from weblate.auth.models import User
@@ -26,6 +25,9 @@
 from weblate.utils.django_hacks import immediate_on_commit, immediate_on_commit_leave
 from weblate.utils.ratelimit import reset_rate_limit
 
+if TYPE_CHECKING:
+    from altcha import Challenge
+
 REGISTRATION_DATA = {
     "username": "username",
     "email": "[email protected]",
@@ -162,24 +164,7 @@ def test_register_captcha_fail(self) -> None:
     def solve_altcha(self, response, data: dict):
         form = response.context["form"]
         challenge: Challenge = form.challenge
-        solution: Solution = solve_challenge(
-            challenge=challenge.challenge,
-            salt=challenge.salt,
-            algorithm=challenge.algorithm,
-            max_number=challenge.maxnumber,
-            start=0,
-        )
-        data["altcha"] = base64.b64encode(
-            json.dumps(
-                {
-                    "algorithm": challenge.algorithm,
-                    "challenge": challenge.challenge,
-                    "number": solution.number,
-                    "salt": challenge.salt,
-                    "signature": challenge.signature,
-                }
-            ).encode("utf-8")
-        ).decode("utf-8")
+        data["altcha"] = solve_altcha(challenge)
 
     def solve_math(self, response, data: dict):
         form = response.context["form"]