From 5ab2bf5ae8686c76a6e11e3334fe7251215d87ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C4=8Ciha=C5=99?= Date: Wed, 18 Dec 2024 20:11:30 +0100 Subject: [PATCH] fix(captcha): correctly deal with valid submission after invalid one Fixes #13335 --- docs/changes.rst | 1 + weblate/accounts/captcha.py | 31 +++++++ weblate/accounts/forms.py | 5 +- weblate/accounts/tests/test_captcha.py | 92 ++++++++++++++++++++- weblate/accounts/tests/test_registration.py | 27 ++---- 5 files changed, 132 insertions(+), 24 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index 454a5c2bb279..fbb51989539c 100644 --- a/docs/changes.rst +++ b/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** diff --git a/weblate/accounts/captcha.py b/weblate/accounts/captcha.py index bc0515af2e94..08334a75af47 100644 --- a/weblate/accounts/captcha.py +++ b/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") diff --git a/weblate/accounts/forms.py b/weblate/accounts/forms.py index c2c816a94fcf..6bb42bfc13da 100644 --- a/weblate/accounts/forms.py +++ b/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 diff --git a/weblate/accounts/tests/test_captcha.py b/weblate/accounts/tests/test_captcha.py index 4b47e596ab20..5d63748f026a 100644 --- a/weblate/accounts/tests/test_captcha.py +++ b/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, {}) diff --git a/weblate/accounts/tests/test_registration.py b/weblate/accounts/tests/test_registration.py index 40c7fcf6045e..bad449957689 100644 --- a/weblate/accounts/tests/test_registration.py +++ b/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": "noreply-weblate@example.org", @@ -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"]