-
Notifications
You must be signed in to change notification settings - Fork 8
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
Added an easy way to add captcha to any form #5
base: master
Are you sure you want to change the base?
Conversation
@@ -25,6 +25,12 @@ | |||
<tag name="kernel.event_subscriber" /> | |||
<argument type="service" id="request"/> | |||
</service> | |||
|
|||
<service id="thelia.form_validator" class="ReCaptcha\FormValidator\ReCaptchaFormValidator"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is dangerous to do this because this will not work if another module override the thelia form validator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, but it makes the module usable almost out of the box (inserting a hook déclaration in a template is enough), instead of having to code something.
Additionally, the module could not be used easily for base Thelia forms. This change make it usable in these forms too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I understand your PR and this is very great but like I said it's dangerous because you can think you are protected but you are not because another module override the form validator.
Maybe we can add a warning message in this case ? (I don't know if we can check if another module override this service)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nowadays, ReCaptcha is a must-have Thelia module (it should be included directly in Thelia) so any solution for making module's integration easier would be appreciated 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry we are not going to include a module who is connected to a proprietary service in Thelia.
I agreed we need a more simple way to add this module to a form but like I explain this PR is not a good solution,
the service can be override by another module, so it will be inefficient and if a bot remove the field g-recaptcha-response
the module will not check the captcha and will be inefficient too.
This PR adds a specific form validator to the module , which extends the base TheliaFormValidator:
ReCaptchaFormValidator
.This form validator dispatches the
CHECK_CAPTCHA_EVENT
event if theg-recaptcha-response
form field is defined in the request, and throw an exception if a suspicious action is detected.Thus, adding
{hook name="recaptcha.check"}
to any form will add a ReCaptcha check to this form without the need to update the form validation code.