Extract Captcha logic and robot detection to an abstractions library

[chaaboah]

Hi there :)
I know this is related to an old issue (#2078) and it was solved by adding an Orchard Core ReCAPTCHA module (pull request #2192).
But i think that extracting the Robot detection and the Captcha behavior, will help the Community to build other Captcha modules with other providers (like hCaptcha who gained popularity thanks to cloudflare).
I'm working on a multi provider Captcha module but it's not ready yet :)
This pull request shouldn't introduce breaking changes, hope my work will help :)

P.S : even if my pull request is rejected i think the method "ProcessAsync" from the RecaptchaTagHelper should be optimized because it has currently an avoidable call to the ReCAPTCHA API

[dnfclas]

All CLA requirements met.

[infofromca]

Hi there :)
I know this is related to an old issue (#2078) and it was solved by adding an Orchard Core ReCAPTCHA module (pull request #2192).
But i think that extracting the Robot detection and the Captcha behavior, will help the Community to build other Captcha modules with other providers (like hCaptcha who gained popularity thanks to cloudflare).
I'm working on a multi provider Captcha module but it's not ready yet :)
This pull request shouldn't introduce breaking changes, hope my work will help :)
P.S : even if my pull request is rejected i think the method "ProcessAsync" from the RecaptchaTagHelper should be optimized because it has currently an avoidable call to the ReCAPTCHA API

hi great. this is what I need.
#2192 (comment)

[Skrypt]

I think the method "ProcessAsync" from the RecaptchaTagHelper should be optimized because it has currently an avoidable call to the ReCAPTCHA API

Elaborate?

[chaaboah]

@Skrypt first of all an erratum: it's an avoidable call to the Robot detection API (IDetectRobot interface) and not ReCaptcha API.
So the first problem happens in line 55:
var robotDetected = robotDetectors.Invoke(d => d.DetectRobot(), _logger).Any(d => d.IsRobot) && Mode == ReCaptchaMode.PreventRobots;
Because the && is "short-circuiting" it's better to evaluate the mode before the invokation of robotDetectors, which gives us this ne line :
var robotDetected =Mode == ReCaptchaMode.PreventRobots && robotDetectors.Invoke(d => d.DetectRobot(), _logger).Any(d => d.IsRobot) ;
We can also add more optimizations to "ProcessAsync" by avoiding service resolution if the mode is 'AlwaysDisplay'.
Last point, the check "if the Recaptcha is configured" happens too late, it should be at the beginning so we avoid any further processing if it's FALSE.
All the points are implemented in the commit of this pull request :)
Hope this helped to clarify my PS :)

[Skrypt]

I agree with the effort of abstracting these services. I will try to review the code to make sure everything is fine. But I think from a first quick glance it looked good.

[agriffard]

Please fix the conflicts.

[chaaboah]

Conflict fixed :)

[Piedone]

I sincerely apologize for us taking so much time here. I checked out the PR, going over old ones, and this would be quite useful, though with some adjustments, and updated to the latest source.

Is this something you'd like to revisit any time soon @chaaboah or should we close?

[github-actions]

This pull request has merge conflicts. Please resolve those before requesting a review.

[Piedone]

Closing due to inactivity.