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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: Recaptcha validation #2

Merged
merged 2 commits into from
Aug 21, 2022

Conversation

Meldiron
Copy link
Contributor

Hey there 馃憢 I have created an implementation for Google reCAPTCHA V2 so we can start the code review process.

When we agree on the structure, let's keep the PR open and I will implement more providers before the final review and merge.

I did manual QA with solution in this PR and was able to get it working on my demo application:

CleanShot 2022-06-12 at 10 47 29@2x

CleanShot 2022-06-12 at 10 47 18@2x

Related to issue #1

@Meldiron
Copy link
Contributor Author

Meldiron commented Jun 12, 2022

One concern you might have is why I named the parameter g-recaptcha-response (in readme, step 6. also Code.js line 105).

This name comes from Google Docs, and while we could easily rename it to something simpler, I find it important to stay as close to the native solution as possible. That will help people experienced with Google reCAPTCHA to easily use it in FormEasy without having to scan docs.

On the other hand, this will create inconsistency between different captcha providers. This could be a valid reason to try to use specific words for all of them, like captcha. But with that, if some provider requires more than just a string response, we would make it a mess and people would have to learn how to pass it into FormEasy.


I would love to hear your feedback on this.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/Code.js Outdated Show resolved Hide resolved
@Meldiron Meldiron requested a review from Basharath June 13, 2022 15:34
@Basharath Basharath closed this Jun 14, 2022
@Basharath Basharath reopened this Jun 14, 2022
@Meldiron
Copy link
Contributor Author

Hey @Basharath 馃憢
What do you think about PR - all good? Can I start implementing more adapters to support more captcha providers?

@Basharath
Copy link
Owner

Hey @Meldiron
I was waiting to hear from you about that.

It is all good so far, I haven't merged yet.
Please add all the providers you want and then tell me so we can finalize the merge. Thanks!

@Meldiron
Copy link
Contributor Author

Meldiron commented Jun 21, 2022

Awesome, I'll do so in the upcoming days. Ill also take my time with research to see what Captcha providers are most commonly used.

@Basharath
Copy link
Owner

Yeah, that's fine, you can take your time. Once done just update here.

@scottydelta
Copy link

any ETA on merging of this PR?

I am looking to use FormEasy with reCAPTCHA. Without CAPTCHA, it's just too much spam.

@Meldiron
Copy link
Contributor Author

I won't be able to research other providers any time soon. With that said, reCAPTCHA is properly implemented and tested. This PR can be merged, in my opinion. In the future, I will open a new PR to add new providers.

Let me ping @Basharath about this.

@Basharath
Copy link
Owner

Hey, @Meldiron I thought to ask you about this a few days ago, meanwhile, this happened.

I'll try merging this by this weekend and publish the new update.

@Basharath Basharath merged commit c5e1be5 into Basharath:master Aug 21, 2022
ericfesta2 pushed a commit to ericfesta2/formstar that referenced this pull request May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants