Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better verification to ReCaptcha #15761

Open
intellimedhu opened this issue Apr 15, 2024 · 7 comments
Open

Better verification to ReCaptcha #15761

intellimedhu opened this issue Apr 15, 2024 · 7 comments

Comments

@intellimedhu
Copy link

We're currently ignoring other attributes in the response (only "Success" we have), such as 'Score.' I'd like to request that all attributes be included, and the application be able to handle how it behaves accordingly.

This is how I intend to use it:

// Even a robot can submit a valid token; the score determines the likelihood:
// based on: https://developers.google.com/recaptcha/docs/v3#interpreting_the_score
if (!response.Success || response.Score < 0.5)
{
    throw ...
}
@MikeAlhayek
Copy link
Member

MikeAlhayek commented Apr 15, 2024

@intellimedhu I do not think reCaptcha v3 is supported yet. Feel free to submit a PR adding the support for v3 and adding the accepted minimum score to the settings so that the users is able to change the default values in the settings.

@MatthijsKrempel is this a correct that there still no v3 support?

@intellimedhu
Copy link
Author

It's supported, I've been using v3 for years, we just need some enhancements. I will create a PR. Thanks!

@intellimedhu
Copy link
Author

I'm about to creating another method similar to ReCaptchaService.VerifyCaptchaResponseAsync, but the result will be of type ReCaptchaResponse, allowing developers to use it as they see fit.

Of course the current VerifyCaptchaResponseAsync method will remain.

@MikeAlhayek
Copy link
Member

@intellimedhu if you like to submit a PR, I would suggest adding a score settings to the ReCaptchaSettings with a reasonable default value. This will be part of the UI settings so the user can change it when they configure the site-key and secret-key. In the ReCaptchaResponse you can add mapping for the following properties

  "score": number             // the score for this request (0.0 - 1.0)
  "action": string            // the action name for this request (important to verify)
  "challenge_ts": timestamp,  // timestamp of the challenge load (ISO format yyyy-MM-dd'T'HH:mm:ssZZ)
  "hostname": string,         // the hostname of the site where the reCAPTCHA was solved
  "error-codes": [...]        // optional

Then in the private VerifyAsync line 133, you can return result.Success && result.Score >_reCaptchaSettings.Score.

@MikeAlhayek MikeAlhayek added this to the 1.x milestone Apr 19, 2024
@intellimedhu
Copy link
Author

Okay, but I'm a bit confused, because v3 is only partially supported by OC. There's no problem on the server side; the API works with both v2 and v3, but on the client side, only v2 is implemented. I've been using the backend code with v3 for years, with a custom v3 implementation on the frontend.

So, I think we need to extend the settings with 2 properties:

"API version": v2 or v3 (and also implement client-side logic for v3, e.g., tag helper)
RiskScore: In the case of v3
In the case of v2, the VerifyAsync should ignore the score.

@MikeAlhayek
Copy link
Member

Yes that sounds good. Are you planning to submit a PR for this?

@intellimedhu
Copy link
Author

Yes, but I'm still working on it (and tag helper is new for me).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants