[18.0][FIX] auth_signup_verify_email: verify captcha on signup

[szalatyzuzanna]

Port of #933 (17.0) to 18.0, with a stronger test. Original fix by @Cl0ut1eR, kept as the commit author.

Problem

auth_signup verifies the captcha token in web_auth_signup(), before do_signup() creates anything. This module overrides that route and, when a login arrives with no password, answers it with passwordless_signup() without calling super(), so that verification is never reached.

The result is that with google_recaptcha or website_cf_turnstile installed and configured, the widget is rendered on the signup page but the token is never checked on the server for this flow. We are getting automated signups in production because of it, and they are not harmless: each one triggers an activation email from our server to an address the bot chose, which is what damages the sending reputation of the domain.

There is a second effect, reported in #878: the captcha token stays in request.params and reaches res.users.signup(), where it raises an invalid field error for legitimate users.

Fix

Call ir.http._verify_request_recaptcha_token("signup") at the start of the passwordless flow, so the existing error handling renders the message.

No new dependency is added. This answers the question raised in #783 ("Isn't reCaptcha an extra module?"), which was never followed up: _verify_request_recaptcha_token is defined in base (ir.http) and returns True when no captcha module is installed, so a database without one behaves exactly as it does today. google_recaptcha and website_cf_turnstile override it, and both pop their own token from request.params — which is also what fixes #878.

About the test

test_failed_recaptcha patches the method on the registry class (type(self.env["ir.http"])) rather than on base's IrHttp. This matters: when a captcha module is installed, its own override sits in front of the one in base, so patching base leaves the real implementation running and the test does not exercise the rejection at all. Patching base was verified to fail on a database with website_cf_turnstile installed — precisely the setup the fix is meant to protect.

The test also asserts that no user is created, which is the property that actually matters here, rather than only that an error is rendered.

Related

#774, #783, #878, #901, #933

[jelenapoblet]

The issue and the proposed fix make sense to me.

[oliverg09]

LGTM!

[jelenapoblet]

@pedrobaeza I think this is a good fix, If you're ok with it, I would merge it

[pedrobaeza]

OK, but the problem may be using the new method passwordless_signup that replaces the whole logic of web_auth_signup instead of reusing it and only injecting the needed bits. Isn't possible to inject the verification inside the existing method and calling super afterwards for doing the rest of the standard setup?

And about the test, being so synthetic that is mocking the direct method, I don't think it's adding value.

[szalatyzuzanna]

@pedrobaeza Thanks for the review.

I tried it: reusing web_auth_signup() and only overriding _signup_with_values() to skip the session authentication makes test_good_email fail. The user ends up on the login page with a "Wrong login/password" error instead of the "check your email" screen, since the account is created without a password and the standard flow logs in right
after. So it's a behaviour change, not just a super call. Worth doing, but as this is a security fix I'd rather leave the refactor to its own PR. Tell me if you'd rather have it all here.

On the test, fair point. It's only a regression guard so the call can't be dropped again. Happy to remove it if you think it's not worth it.

[pedrobaeza]

OK, I was talking about the method web_auth_signup, if you see it not possible, I don't insist. About the regression test, I think it's not adding value in this case. A real regression tests would be a complete signup flow and only mocking the captcha failure.