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

Add support for ReCaptcha #20

Closed
vg opened this Issue Aug 27, 2016 · 46 comments

Comments

Projects
None yet
5 participants
@vg

vg commented Aug 27, 2016

Might be a good idea if support for recaptcha is added. Might serve as another layer of protection against spam bots.

@vg vg changed the title from Add support for ReCapcha to Add support for ReCaptcha Aug 27, 2016

@eduardoboucas

This comment has been minimized.

Owner

eduardoboucas commented Sep 11, 2016

I like this. Adding a help wanted label as I don't have a chance to implement this right away.

@eduardoboucas

This comment has been minimized.

Owner

eduardoboucas commented Oct 7, 2016

To use reCAPTCHA, you need to sign up for an API key pair for your site. The key pair consists of a site key and secret. The site key is used to display the widget on your site. The secret authorizes communication between your application backend and the reCAPTCHA server to verify the user's response. The secret needs to be kept safe for security purposes.

(https://developers.google.com/recaptcha/docs/start)

If I understand correctly, Staticman would need to know each site's secret in order to communicate with the reCAPTCHA back-end to verify if a request is legit. This isn't possible at the moment, as we don't story any data.

Unless I'm missing anything?

@zburgermeiszter

This comment has been minimized.

Contributor

zburgermeiszter commented Oct 9, 2016

I think it could be fairly easily sorted by using public/private key encryption/decryption.
You could give an option to encrypt the reCAPTCHA API key that the Staticman API would receive with the comment request, and you could decrypt it, and check it with reCAPTCHA.

@eduardoboucas

This comment has been minimized.

Owner

eduardoboucas commented Oct 10, 2016

But that would mean Staticman having access to your private key to decrypt the message, right? How would you store this?

@zburgermeiszter

This comment has been minimized.

Contributor

zburgermeiszter commented Oct 10, 2016

Not my private key, but each Staticman service would have its own private key, and publish the public key for users to encrypt data that the server could decrypt with the private key.
So in this case your api.staticman.net service would have it's own private key, and you would publish the pubkey in the docs or repo.

Travis using this solution to encrypt environment variables in the config.

@eduardoboucas

This comment has been minimized.

Owner

eduardoboucas commented Oct 10, 2016

That makes sense. I'll look into it. Thanks!

@eduardoboucas

This comment has been minimized.

Owner

eduardoboucas commented Oct 10, 2016

(Also, this relates to #21)

@zburgermeiszter

This comment has been minimized.

Contributor

zburgermeiszter commented Oct 22, 2016

I have made some additional research on it.

So basically reCAPTCHA has a public token called site key and a secret key.

Secret Key
The secret key is required to check the CAPTCHA result with reCAPTCHA.
Normally this stored only on the server, but in this case it should be encrypted in the repo config, and the service can decrypt it after pulling the config.

Site Key
The site key is required to show the CAPTCHA box, so it must not be encrypted.
It must be written to the HTML output, so it is might be easier to save it to the site generator config (or Hugo, etc...) config, and let the service pull it from there.

The other way is to save it to the Staticman branch config, but then it is more difficult to extract a value from a file in these generators, and would require plugins or other techniques that the user would need to configure.

For me it would make sense to do the key extraction on the service side.
If you want to support different site generators (Jekyll,Hugo, etc), the service would need to know how to pull the value from each config.

Or save the public site key to a simple file as it is usually possible to read files in these site generators.

Verify CAPTCHA
The reCAPTCHA JS plugin inserts a new field to the form which will keep the response of the reCAPTCHA service. It is like a token that you need to send to the verification API together with the decrypted secret key.

It will return a JSON object with the true or false result of the CAPTCHA check.

If false, you need to stop further processing of the comment request.

@zburgermeiszter

This comment has been minimized.

Contributor

zburgermeiszter commented Oct 22, 2016

As soon as you have an implementation to decrypt data from the repo config on the service side, I can try to start to work on it and send a PR when it's done.
Normally I'm a PHP guy, but interested in Node, and it would be a good challenge for me :)

@zburgermeiszter

This comment has been minimized.

Contributor

zburgermeiszter commented Oct 22, 2016

For future reference, here's the reCAPTCHA integration documentation.

@eduardoboucas

This comment has been minimized.

Owner

eduardoboucas commented Oct 22, 2016

This is great, thanks for your help. I started a significant refactoring of the library, which you can find in the dev branch, which includes an early stage implementation of encryption. I will keep working on this, though, and will let you know once it's stable enough so that you can jump in.

@zburgermeiszter

This comment has been minimized.

Contributor

zburgermeiszter commented Nov 26, 2016

Here's a proof-of-concept code.
For some reason I get the following error:

Error: Error during decryption (probably incorrect key). Original error: Error: error:04065084:rsa routines:RSA_EAY_PRIVATE_DECRYPT:data too large for modulus
   at Error (native)
   at NodeRSA.module.exports.NodeRSA.$$decryptKey (/app/node_modules/node-rsa/src/NodeRSA.js:301:19)
   at NodeRSA.module.exports.NodeRSA.decrypt (/app/node_modules/node-rsa/src/NodeRSA.js:249:21)
   at /app/app.js:36:40
   at Layer.handle [as handle_request] (/app/node_modules/express/lib/router/layer.js:95:5)
   at next (/app/node_modules/express/lib/router/route.js:131:13)
   at Route.dispatch (/app/node_modules/express/lib/router/route.js:112:3)
   at Layer.handle [as handle_request] (/app/node_modules/express/lib/router/layer.js:95:5)
   at /app/node_modules/express/lib/router/index.js:277:22
   at Function.process_params (/app/node_modules/express/lib/router/index.js:330:12)

It used to be working fine, but I suspect I have changed something with the key generator which breaks the things.

If you have seen this error before, or have any suggestion, please ket me know, and I hope this code helps you to integrate it to Staticman.

@zburgermeiszter

This comment has been minimized.

Contributor

zburgermeiszter commented Dec 29, 2016

I had some time to move my experiments to the Staticman code.
Submitted a PR with the implementation of this feature request.
#67

@eduardoboucas

This comment has been minimized.

Owner

eduardoboucas commented Mar 2, 2017

This feature has been deployed to the preview instance. For anyone kind enough to help us test it, all you need to do is replace api.staticman.net with dev.staticman.net in the URL you submit content to.

Many thanks to @zburgermeiszter for the huge effort he put in making this happen!

@zburgermeiszter

This comment has been minimized.

Contributor

zburgermeiszter commented Mar 2, 2017

It's my pleasure. It is my first bigger contribution to an open source project, and I work with node since a few months.
Thanks for accepting the PR @eduardoboucas.
I hope, the issue opener (@vg) will provide us positive feedback.
I also owe you with the integration documentation for the staticman.net

@mmistakes

This comment has been minimized.

mmistakes commented Mar 3, 2017

@zburgermeiszter Gave this a go trying to add reCAPTCHA following these steps:

  1. Applied for reCAPTCHA API keys.
  2. Added recaptcha/api.js script.
  3. Added <input type="text" name="options[reCaptcha][siteKey]" value="mySiteKey"> and
    <input type="text" name="options[reCaptcha][encryptedSecret]" value="myEncryptedSecret"> to my comment form.
  4. Added the following to my staticman.yml config. I have a comments property in there so I nested it under that like all of the other settings.
     reCaptcha:
       enabled: true
       siteKey: "6LdRBykTAAAAAFB46MnIu6ixuxwu9W1ihFF8G60Q"
       secret: "vMgZExAgTmtHky/4h9xda7W9bDroQ56qWQqb1BVbcl5MRo7Uw81fJxAIUYeUk0iztpcNIzvAnJbb6qY1slMgH9VoMwLfCTldHjS82ynvvOCIKp9+F/lngvPW3xaHI+LoKRVtjUajejcacLdqD1JS1VI6xmVrJlhG1mB5k+Q0370="
  5. Changed POST on the form from https://api.staticman.net to https://dev.staticman.net
  6. Submitted a comment.

In the console the error was Failed to load resource: the server responded with a status of 500 () trying to POST to https://dev.staticman.net. I believe the exact problem was on the recaptcha object as it appeared to be empty.

I also tried adding the following to my form as instructed by the client side instructions on reCAPTCHA's site which made no difference:

Paste this snippet at the end of the

where you want the reCAPTCHA widget to appear:
<div class="g-recaptcha" data-sitekey="mySiteKey"></div>

@eduardoboucas

This comment has been minimized.

Owner

eduardoboucas commented Mar 4, 2017

I made some changes to the implementation a put together a simple demo site using Jekyll and Staticman with reCAPTCHA (using dev.staticman.net for now).

@mmistakes Can you have a look at this implementation and see if it helps?

@mmistakes

This comment has been minimized.

mmistakes commented Mar 4, 2017

Thanks @eduardoboucas. The example is pretty much how I did it on my site. Keeping it in production if it helps troubleshoot. I'm getting this in the console.

Failed to load resource: the server responded with a status of 500 ()

Object
  responseText: "{"success":false,"data":{},"errorCode":"RECAPTCHA_ERROR"}"

Here's a post with the comment form. And my staticman.yml config.

@zburgermeiszter

This comment has been minimized.

Contributor

zburgermeiszter commented Mar 4, 2017

@mmistakes
This happens when there's an exception somewhere in the recaptcha validation.
Are you sure you have encrypted the correct secret?

Also make sure you have added your website domain to the allowed domains list on the reCAPTCHA page.

@mmistakes

This comment has been minimized.

mmistakes commented Mar 4, 2017

@zburgermeiszter As far as I can tell my secret key is correct. Took what reCAPTCHA supplied and encrypted it with this endpoint https://api.staticman.net/v2/encrypt/{TEXT TO BE ENCRYPTED}... then added to my staticman.yml file and options[reCaptcha][encryptedSecret] on the form.

My allowed domain list also has the correct domain.

@zburgermeiszter

This comment has been minimized.

Contributor

zburgermeiszter commented Mar 5, 2017

@eduardoboucas
We should have some kind of accessible error logs to debug this. Maybe there's something useful in them.
It is not possible to use @mmistakes credentials locally as they has been encrypted by the staticman.net instance private key.

Or configure the test captcha to accept requests from any domain, then @mmistakes can use that to see if the error has gone.

@eduardoboucas

This comment has been minimized.

Owner

eduardoboucas commented Mar 5, 2017

@mmistakes I added mademistakes.com to the list of allowed domains for the reCAPTCHA account I'm using in the test site. Could you try copying the site key and secret from there and see if you still get the same error?

@mmistakes

This comment has been minimized.

mmistakes commented Mar 5, 2017

Still no dice. I added the site/secret keys from the test site to my form and staticman.yml config and still get the same error. You can see the form on this page.

I even tried simplify my form and ordering the inputs and script to mimic the test site in case it was having issues identifying things.

<form id="comment-form" class="page__form js-form form" method="post" action="https://dev.staticman.net/v2/entry/mmistakes/made-mistakes-jekyll/master/comments">
  <label for="comment-form-message"><strong>Comment</strong> <small>(<a href="https://daringfireball.net/projects/markdown/syntax">Markdown</a> is allowed)</small></label>
  <textarea type="text" rows="6" id="comment-form-message" name="fields[message]" spellcheck="true"></textarea>
  <label for="comment-form-name"><strong>Name</strong></label>
  <input type="text" id="comment-form-name" name="fields[name]" spellcheck="false">
  <label for="comment-form-email"><strong>Email</strong> <small>(used for <a href="http://gravatar.com">Gravatar</a> image and reply notifications)</small></label>
  <input type="email" id="comment-form-email" name="fields[email]" spellcheck="false">
  <input class="hidden" style="display: none;" type="hidden" name="options[origin]" value="https://mademistakes.com/articles/365-days-of-drawing/">
  <input type="hidden" id="comment-parent" name="options[parent]" value="">
  <input type="hidden" id="comment-post-id" name="options[slug]" value="365-days-of-drawing">
  <p class="hidden js-notice"><span class="js-notice-text"></span></p>
  <input type="hidden" name="options[reCaptcha][siteKey]" value="6LdimBcUAAAAANX016gDfwSdk0CXduIsYGj9VhSY">
  <input type="hidden" name="options[reCaptcha][secret]" value="z9d1HgPjpT4S6V28tRMylatjh80fWSR6CW94GaEbU1FLO9RUELEiL3W91xrrKXAyb6PpWe485OmF/FtMnbNnIjZjHZtf/1Zlb0adjT582qq3m+YvEhUVHrDf0w74PJ+AoaLM8MAMVU/ZM0mRNp/WnzVeVx2I+ROs5mTMqbeyKfo=">
  <div class="g-recaptcha" data-sitekey="6LdimBcUAAAAANX016gDfwSdk0CXduIsYGj9VhSY"></div>
  <button type="submit" id="comment-form-submit" class="btn btn--large">Submit Comment</button>
</form>
<script src="https://www.google.com/recaptcha/api.js"></script>

staticman.yml

 reCaptcha:
    enabled: true
    siteKey: "6LdimBcUAAAAANX016gDfwSdk0CXduIsYGj9VhSY"
    secret: "z9d1HgPjpT4S6V28tRMylatjh80fWSR6CW94GaEbU1FLO9RUELEiL3W91xrrKXAyb6PpWe485OmF/FtMnbNnIjZjHZtf/1Zlb0adjT582qq3m+YvEhUVHrDf0w74PJ+AoaLM8MAMVU/ZM0mRNp/WnzVeVx2I+ROs5mTMqbeyKfo="

The interesting thing is if I change the POST url to the test site I get a completely different reCaptcha error. I'd expect it to give me errors on the fields since we're using different ones (which it does after checking the captcha box), but very strange that a blank form from both give different results.

MM post url:

action="https://dev.staticman.net/v2/entry/mmistakes/made-mistakes-jekyll/master/comments"


{"success":false,"data":{},"errorCode":"RECAPTCHA_ERROR"}

And using the test site:

action="https://dev.staticman.net/v2/entry/eduardoboucas/staticman-recaptcha/master/comments"

{"success":false,"data":"reCAPTCHA: The response parameter is invalid or malformed","errorCode":"RECAPTCHA_ERROR"}

And checking the captcha box with the test site's post URL:

action="https://dev.staticman.net/v2/entry/eduardoboucas/staticman-recaptcha/master/comments"

{"success":false,"data":[{"code":"MISSING_REQUIRED_FIELDS","data":["name","language","message"]}],"errorCode":"ENTRY_ERROR"}

Which I'd expect since the fields are different. But no fail on the reCaptcha bit.

@zburgermeiszter

This comment has been minimized.

Contributor

zburgermeiszter commented Mar 5, 2017

reCAPTCHA: The response parameter is invalid or malformed is returned when the g-recaptcha-response POST field contains incorrect data.

@zburgermeiszter

This comment has been minimized.

Contributor

zburgermeiszter commented Mar 5, 2017

Update:
Insert a console.log(reason) into the Staticman.js:498.

It will give the following:

TypeError: Cannot read property 'origin' of undefined
   at Staticman._validateConfig (/app/lib/Staticman.js:349:21)
   at github.readFile.then.data (/app/lib/Staticman.js:225:35)
   at process._tickCallback (internal/process/next_tick.js:103:7)

It is some kind of config loading error, which need some investigation.
I can reproduce it using @mmistakes details in Postman sending to a local modified instance.

@mmistakes

This comment has been minimized.

mmistakes commented Mar 5, 2017

@zburgermeiszter That's exactly it. I disabled allowedOrigins and notifications in my config and it went through. So the issue is definitely with that.

@eduardoboucas

This comment has been minimized.

Owner

eduardoboucas commented Mar 5, 2017

Thanks both for your help. I was able to replicate the issue and track down the bug. I'll have a fix shortly.

@zburgermeiszter

This comment has been minimized.

Contributor

zburgermeiszter commented Mar 6, 2017

@eduardoboucas
I already submitted a PR. #88

@eduardoboucas

This comment has been minimized.

Owner

eduardoboucas commented Mar 6, 2017

@mmistakes Could you try now, please?

@mmistakes

This comment has been minimized.

mmistakes commented Mar 6, 2017

@eduardoboucas I think that did it. Added back allowedOrigins and notifications and everything seems to be working with reCAPTCHA.

@eduardoboucas

This comment has been minimized.

Owner

eduardoboucas commented Mar 6, 2017

Great, thanks!

Btw, there are still some things to consider, but allowedOrigins will probably be removed in the next release as it's a bit pointless now.

@eduardoboucas

This comment has been minimized.

Owner

eduardoboucas commented Mar 7, 2017

This has been deployed to the live API, so you can now switch back to using api.staticman.net to submit your entries.

Please file an issue if something is not working properly.

Thanks again to @zburgermeiszter for all the effort he put into making this happen.

@mmistakes

This comment has been minimized.

mmistakes commented Mar 8, 2017

Not sure if something broke with another commit or merge but I'm getting errors. This time it says Missing reCAPTCHA API credentials.

Same keys that were working before, only change I can see was flipping back to api.staticman.net from dev. Reverting back to dev.staticman.net doesn't appear to help either. Nothing else on my end changed between now and when it was working a few days ago.

@eduardoboucas eduardoboucas reopened this Mar 8, 2017

@eduardoboucas

This comment has been minimized.

Owner

eduardoboucas commented Mar 8, 2017

@mmistakes This is odd, I can't replicate the issue. Can you try switching back to the keys used in the demo, to see if that helps?

<input type="hidden" name="options[reCaptcha][siteKey]" value="6LdimBcUAAAAANX016gDfwSdk0CXduIsYGj9VhSY">
<input type="hidden" name="options[reCaptcha][secret]" value="z9d1HgPjpT4S6V28tRMylatjh80fWSR6CW94GaEbU1FLO9RUELEiL3W91xrrKXAyb6PpWe485OmF/FtMnbNnIjZjHZtf/1Zlb0adjT582qq3m+YvEhUVHrDf0w74PJ+AoaLM8MAMVU/ZM0mRNp/WnzVeVx2I+ROs5mTMqbeyKfo=">

<div class="g-recaptcha" data-sitekey="6LdimBcUAAAAANX016gDfwSdk0CXduIsYGj9VhSY"></div>
@mmistakes

This comment has been minimized.

mmistakes commented Mar 8, 2017

Will give the demo keys a try and report back.

Related, I just got a comment on my site from someone having the exact same issue so perhaps something broke with the merge.

@eduardoboucas

This comment has been minimized.

Owner

eduardoboucas commented Mar 8, 2017

If the demo keys work, could you try adding staticman.net to the list of allowed domains for your reCAPTCHA account and trying with yours again?

@zburgermeiszter

This comment has been minimized.

Contributor

zburgermeiszter commented Mar 8, 2017

@eduardoboucas The allowed domain list is only for restricting domains from which reCAPTCHA accept user clicks.

@eduardoboucas

This comment has been minimized.

Owner

eduardoboucas commented Mar 8, 2017

I understand that, just trying to rule everything out.

@zburgermeiszter

This comment has been minimized.

Contributor

zburgermeiszter commented Mar 8, 2017

@mmistakes Make sure you use options[reCaptcha][secret] field instead of options[reCaptcha][encryptedSecret] as it is in your form. Keep the encrypted value, simply rename it.
@eduardoboucas has changed it recently.

@mmistakes

This comment has been minimized.

mmistakes commented Mar 8, 2017

@zburgermeiszter I bet that is it. If it changed from [encryptedSecret] to [secret] that would explain why it broke. Will try that and report back.

@mmistakes

This comment has been minimized.

mmistakes commented Mar 8, 2017

That was it @zburgermeiszter! I made the change to the form and used my reCAPTCHA keys and its working again.

@eduardoboucas

This comment has been minimized.

Owner

eduardoboucas commented Mar 8, 2017

I'm confused. That changed 5 days ago and it was working fine for you 3 days ago.

🤔

@mmistakes

This comment has been minimized.

mmistakes commented Mar 8, 2017

@eduardoboucas I hadn't checked it in days, so it's entirely possible when you made the switch from encryptedSecret to secret it happened shortly after i got things working. I had no idea anything changed in the API so I didn't bother to check things until yesterday when I switched from dev.staticman.net to api.staticman.net.

@eduardoboucas

This comment has been minimized.

Owner

eduardoboucas commented Mar 8, 2017

My bad, I should've mentioned that. I just changed so it's consistent with other fields in the config, as we don't usually prefix encrypted fields with encrypted.

Sorry for the bumpy ride, but I think we're getting there now!

I'll close this again, but feel free to reopen if any other issues arise.

@mmistakes

This comment has been minimized.

mmistakes commented Mar 8, 2017

No worries. I don't mind being a guinea pig 😄

@sukiletxe

This comment has been minimized.

Contributor

sukiletxe commented Mar 13, 2017

Does this also support the invissible recaptcha?

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