collective / volto-form-block

Volto addon for a customizable form block
MIT License
9 stars 8 forks source link

Captcha #26

Closed giuliaghisini closed 2 years ago

giuliaghisini commented 2 years ago

https://github.com/collective/collective.volto.formsupport/issues/13#issuecomment-1102486914

erral commented 2 years ago

I will look at this during the Beethoven Sprint

erral commented 2 years ago

I am testing this branch and I see strange behaviors here... The first time I checked everything was OK, now I am not able to get the token and the backend returns always "Not available token" :confused:

I keep looking at it.

giuliaghisini commented 2 years ago

@mamico i've tested both recaptcha and hcaptcha and seems working well! good. I'm not only fully convinced of new formSchema 'captcha' field... This means user have to choose in each form which type of captcha use.. I think a site will use recaptcha or hcaptcha and not both..

In my opinion this 'captcha' type value need to be moved to the config of addon, or even better, in volto.form.support. volto.form.support could look if it's installed plone.formwidget.recaptcha or plone.formwidget.hcaptcha and returns correct hcaptcha or recaptcha data config into form block. I also think if type of captcha is 'hcaptcha', block need's to know if render 'invisible hcaptcha' or not, based on the config value setted in hcaptcha controlpanel..

What do you think?

@cekk @erral

erral commented 2 years ago

I think that the option of selecting the captcha format is OK in the form schema. I think that it should be something up to the form and not to the whole site.

I am thinking on extending this for a site of ours to use another kind of captcha, and we should be able to choose one captcha kind in one form and some other in another form.

What I would do is remove the "hcaptcha" or "recaptcha" options if the corresponding env var is not available, I don't know if that's feasible though.

giuliaghisini commented 2 years ago

What I would do is remove the "hcaptcha" or "recaptcha" options if the corresponding env var is not available, I don't know if that's feasible though.

in this implementation, env var expires! Everithing is configured in Hcaptcha or Recaptcha control panel and is returned from volto.form.support in form block data

giuliaghisini commented 2 years ago

I am thinking on extending this for a site of ours to use another kind of captcha, and we should be able to choose one captcha kind in one form and some other in another form.

so.. proposal:

captcha types could be configured in volto-form-block configuration. If types length is equal to '1' user doesn't have to select the type for each form, else we let user select the type of the captcha.

@mamico

erral commented 2 years ago

And can the block get the available captcha options from the backend? Depending on the relevant adapter registration from https://github.com/collective/collective.volto.formsupport/blob/main/src/collective/volto/formsupport/captcha/configure.zcml ?

giuliaghisini commented 2 years ago

And can the block get the available captcha options from the backend? Depending on the relevant adapter registration from https://github.com/collective/collective.volto.formsupport/blob/main/src/collective/volto/formsupport/captcha/configure.zcml ?

my proposal is to configure captcha types in volto's side.. not in plone's side.. if you have a different type of captcha that isn't recaptcha or hacaptcha, io also have to implement the widget in volto side..

erral commented 2 years ago

I am playing the "angry customer" role here :wink: But if you don't have the widgets enabled in the backend it's useless to try to configure a captcha, it will fail

giuliaghisini commented 2 years ago

I am playing the "angry customer" role here 😉 But if you don't have the widgets enabled in the backend it's useless to try to configure a captcha, it will fail

🤷‍♀️ also other million of things you are not able to use if you don't configure it in the backend.. i think it is mainly the developer's responsability to configure correctly the addons

mamico commented 2 years ago

@giuliaghisini @erral My proposal. A vocabulary in the backend that returns the captcha adapters that works (defined/configured), an optional settings/env in the frontend to choice the default captcha method (that might simplify the works of editors).

giuliaghisini commented 2 years ago

@giuliaghisini @erral My proposal. A vocabulary in the backend that returns the captcha adapters that works (defined/configured), an optional settings/env in the frontend to choice the default captcha method (that might simplify the works of editors).

ok. and in the volto.form.support we also need to know if captcha is invisible or not based on controlpanel config value

erral commented 2 years ago

LGTM

mamico commented 2 years ago

Implemented the backend part: https://github.com/collective/collective.volto.formsupport/commit/c9cc1297453b40d9388c516d09689b4a41b473e9

A volunteer for the frontend ;) ?

mamico commented 2 years ago

and in the volto.form.support we also need to know if captcha is invisible or not based on controlpanel config value

If hcaptcha is configured, the vocabulary returns two terms: hcaptcha and hcaptcha_invisible. The editor make the choice and the frontend code has to do the rest. Eventually the "hcaptcha invisible" checkbox in frontend should be removed.

https://github.com/collective/collective.volto.formsupport/commit/c9cc1297453b40d9388c516d09689b4a41b473e9#diff-e3d05a628091291b87d1e6b8f4a04a0350c887d0a27f9ec7e0522cb5528a7e97R13

giuliaghisini commented 2 years ago

Implemented the backend part: collective/collective.volto.formsupport@c9cc129

A volunteer for the frontend ;) ?

me!

giuliaghisini commented 2 years ago

@mamico implemented frontend for the new part. Only one thing: on form submit, backend will receive in captcha.provider the value selected from the vocabulary. For example

{
   ...data,
   captcha:{
           provider:'hcaptcha_invisible'
           token:'xyzxyzxyz'
   }
}

is it ok?

mamico commented 2 years ago

is it ok?

Should be ok. But please, make a test.

mamico commented 2 years ago

@giuliaghisini previously we said to add a default captcha provider in the volto settings / environment. What do you think about it?

giuliaghisini commented 2 years ago

is it ok?

Should be ok. But please, make a test.

i'm not able to make a test for hcaptcha in localhost because hcaptcha doesn't validate location localhost 😩

giuliaghisini commented 2 years ago

@giuliaghisini previously we said to add a default captcha provider in the volto settings / environment. What do you think about it?

probably it is useless, because without the configuration of the keys on the server it is not possible to do anything..

erral commented 2 years ago

I am testing the latest changes and I am unable to make this work with Recaptcha. I have installed and configured a correctly key in plone.formwidget.recaptcha, added a form block and configured to be protected with Google Recaptcha.

I can see that the public key information is coming from the backend in the API key but the form button is not enabled. If I remove the disabled property using the HTML inspector and send the form to the backend I receive the 400 Bad Request - No captcha token provided. error :confused:

erral commented 2 years ago

Debugging the FormView I see that captchaToken.current is undefined in my case and that's why the button is disabled. Perhaps am I a robot? :laughing:

erral commented 2 years ago

The same happens using hcaptcha, I can't get the button enabled.

giuliaghisini commented 2 years ago

The same happens using hcaptcha, I can't get the button enabled.

have you generated recaptcha and hcaptcha keys enabled for 'localhost'?

mamico commented 2 years ago

@erral @giuliaghisini Some troubles, and solutions, that I experimented during my tests:

mamico commented 2 years ago

I correct myself. Doesn't work also for me. Upon loading the page the token is generated, but there is not a new token generation in frontend before form is submitted, so the token could be expired. (tested with google recaptcha).

erral commented 2 years ago

I would say I tested with recaptcha 3, but I will do it again. I bypassed the localhost issue using a nip.io domain, they are very handy to situations like this.

erral commented 2 years ago

I have just tested with recaptcha 3 and it works as expected! I would document the requirement of using Recaptcha 3 keys and we're done.

With hcaptcha, it does not matter invisible or visible, the "Submit" button is never enabled so I can't continue.

mamico commented 2 years ago

I have just tested with recaptcha 3 and it works as expected! I would document the requirement of using Recaptcha 3 keys and we're done.

@erral ... wait, as I said before, I think there is still a problem (with both recaptcha and hcaptcha). The user token expires, and the frontend does not attempt to update it before submit. Give it a try: open a form, wait (2/3 minutes should be enough) and try to send, does it work for you?

erral commented 2 years ago

@mamico according to recaptcha docs:

Note: reCAPTCHA tokens expire after two minutes. If you're protecting an action with reCAPTCHA, make sure to call execute when the user takes the action rather than on page load.

So perhaps we need to refresh the token when the user hits the Submit button.

giuliaghisini commented 2 years ago

@mamico according to recaptcha docs:

Note: reCAPTCHA tokens expire after two minutes. If you're protecting an action with reCAPTCHA, make sure to call execute when the user takes the action rather than on page load.

So perhaps we need to refresh the token when the user hits the Submit button.

ok

erral commented 2 years ago

@mamico according to recaptcha docs: Note: reCAPTCHA tokens expire after two minutes. If you're protecting an action with reCAPTCHA, make sure to call execute when the user takes the action rather than on page load. So perhaps we need to refresh the token when the user hits the Submit button.

ok

According to the recaptcha plugin docs at https://www.npmjs.com/package/react-google-recaptcha-v3#user-content-withgooglerecaptcha we can use the withGoogleReCaptcha HOC to be able to do the validation whenever we want.

mamico commented 2 years ago

@erral @giuliaghisini I've started to refactoring some part of captcha/antispam implementation. It's still in progress, in my local tests works well with hcaptcha, but I need to do more checks.

@erral if you agree, when implementation will be more stable, I can adapt and move here your changes for norobots.

erral commented 2 years ago

Thanks @mamico I can look at it too if you do not have time, we have a project where we need the norobots stuff and that's why I am pushing :smile:

mamico commented 2 years ago

Thanks @mamico I can look at it too if you do not have time, we have a project where we need the norobots stuff and that's why I am pushing smile

I've just pushed the changes for hcaptcha/hcaptcha invisible and norobots (not yet the recaptcha implementation). @erral @giuliaghisini testing and review are welcome.

mamico commented 2 years ago

I will check it again on monday.

@erral you had time to check and review current implementation? Before starting with recaptcha implementation, your feedback would be very helpful

erral commented 2 years ago

I have checked the implementation again and I can make it work with Norobots but not with Hcaptcha. I always get "The entered code is wrong".

I am using the same site keys that I was using when it worked, in the same site with the same domain. I have debugged the captcha token and everything looks OK, but something seems to be missing here... I have checked both hcaptcha implementations the invisible and the standard one, and the result is the same for both :/

mamico commented 2 years ago

I have checked the implementation again and I can make it work with Norobots but not with Hcaptcha. I always get "The entered code is wrong".

I've just tried again, and it still works for me. (@giuliaghisini do you have tested?)

@erral, can you check if the same token/challenge is verified twice ?

erral commented 2 years ago

I see a initial load of hcaptcha when the form renders and I go to fill in the form, I select the correct cars, boats or trains, and it says OK, but when sending the form it fails, both for invisible and standard captcha.

But this happens only with hcaptcha, because with Recaptcha and no robots work fine for me.

If it works for you, perhaps it is something related to my configuration

erral commented 2 years ago

I have reconfigured hcaptcha and set the Passing Threshold to "Auto" instead of the previous "Moderate" and now I pass the captcha both in the invisible and in the standard flavours.

So a big :+1: on my side :sweat_smile:

giuliaghisini commented 2 years ago

ok for me!

mamico commented 2 years ago

I have reconfigured hcaptcha and set the Passing Threshold to "Auto" instead of the previous "Moderate" and now I pass the captcha both in the invisible and in the standard flavours.

@erral think it might be a good idea to open an issue on this? Is it a problem with this implementation or with the hcaptcha-react package?

erral commented 2 years ago

I don't know but it might worth perhaps a bit of docs. Let me add some docs in the README right?

erral commented 2 years ago

can we merge this? I think we all have tested it

giuliaghisini commented 2 years ago

can we merge this? I think we all have tested it

we could merge this next week. This pr needs also to merge https://github.com/collective/collective.volto.formsupport/issues/13#issuecomment-110248691 this one. We could make a minor for both

erral commented 2 years ago

Yes, the recpatcha, hcaptcha and norobots support is already merged in collective.volto.formsupport we only need a release of that.

giuliaghisini commented 2 years ago

Yes, the recpatcha, hcaptcha and norobots support is already merged in collective.volto.formsupport we only need a release of that.

released 2.5.0 version. need to release collective.volto.formsupport to version 2.4.0