meteor-useraccounts / core

Meteor sign up and sign in templates' core functionalities
http://useraccounts.meteor.com/
MIT License
529 stars 279 forks source link

Adding reCAPTCHA form? #238

Closed theplatapi closed 9 years ago

theplatapi commented 9 years ago

How would I go about adding a reCAPTCHA form? I need to add a <div> element at the end of the form, and then intercept the form when it's submitted. Is this possible with useraccounts?

splendido commented 9 years ago

Hi there!

I'd say you could easily redefine the atPwdForm Template as described here with something like:

<template name="atPwdChaptchaForm">
  <div class="at-pwd-form">
    <form role="form" id="at-pwd-form" class="{{disabled}}" novalidate action="#" method="POST">
      {{#each fields}}
        {{> atInput}}
      {{/each}}
      <div class="re-chaptcha">
         ...
      </div>
      {{#if showForgotPasswordLink}}
        {{> atPwdLink}}
      {{/if}}
      {{> atPwdFormBtn}}
    </form>
  </div>
</template>

and make sure you add the 'chaptcha' field as the last one so to have the input for it right above the chaptcha picture. This way the chaptcha value will be automatically collected and submitted on form submission. (differently is see it hard right now to intercept the form submission, unless you wish to override the current submission event making sure to preserve what's in there...)

Then I think the problem would be having some other helper to get the chaptcha picture to be later validated via a custom asynchronous validation method as described in the Custom Validation paragraph of the Form Fields Configuration section.

I'd suggest to see what you can do with a method call to get the link of a chaptcha picture leveraging the connection object you can access from inside the server-side method (or much simpler the userId...) to store the actual chaptcha value inside some custom collection to be later validated on form submission.

To this aim, also this issue could give you some hint about how to write the custom validation procedure.

Btw, I think this is a very interesting topic, and I' like to able to add this as a plugin-package for the 2.0 Milestone.

Lets try to see how can we get this to work! ....even if I have no experience with chaptcha generation (how would you go about this?)

theplatapi commented 9 years ago

Thank you for all this information! I'll see how overwriting atPwdForm and the submission event goes.

For the captcha generation, I'm planning on using Google's reCAPTCHA service. There's a Meteor plugin here that makes it pretty easy to integrate.

splendido commented 9 years ago

Wow! Seems so easy :)

Yeah, now I defenitely think it should be easy to integrate it directly into AT.

If you wish I can give you some pointer to let you know where to play with this and see if you can make it directly into AT.

Let me know!

theplatapi commented 9 years ago

Yeah, I'm up for the challenge. What pointers do you have?

splendido commented 9 years ago

So, I'd say...

1) Configuration: see this method which uses this template to check provided options. I'd say we'd need a reChaptcha option to be something like this:

reChaptcha: {
 publickey: String,
 privatekey: String
}

plus, possibly another showReChaptcha boolean to be used to choose whether to display the reChaptcha input or not. (Otherwise the presence of reChaptcha among the configured options might be enough to interpret it as a showReChaptcha: true. Up to you...

2) Add an helper to let the atPwdForm know whether to show the new template. It should enter at this line

    showReChaptcha: function() {
        var parentData = Template.currentData();
        var state = (parentData && parentData.state) || AccountsTemplates.getState();
        return state === "signUp" && AccountsTemplates.options.showReChaptcha;
    },

3) change the atPwdForm template (see this file) (for each of the available front-end frameworks :( ) to be something like this:

<template name="atPwdForm">
  <div class="at-pwd-form">
    <form role="form" id="at-pwd-form" class="{{disabled}}" novalidate action="#" method="POST">
      {{#each fields}}
        {{> atInput}}
      {{/each}}
      {{#if showReChaptcha}}
        {{> atReChaptcha}}
      {{/if}}
      {{#if showForgotPasswordLink}}
        {{> atPwdLink}}
      {{/if}}
      {{> atPwdFormBtn}}
    </form>
  </div>
</template>

4) create the new template files (re_chatpcha.* ??) inside this folder, but actually putting its helpers here. This is basicaly copy/pasting of the package you linked above from this folder... plus taking public/private keys from AccountsTemplates.options.reChaptcha. ...

5) modify the form submission event to pick up the chaptcha value in case the configuration option says it should be visible and add them to the profile data or as a separate options there...

6) modify the ATCreateUserServer method to validate also the reChaptcha value in case it was shown...

7) update the README file putting it clear the configuration for the private key should be made within a separate file to be put under the server folder in order not to send the private key to the client!

I'd say this is all. But might be I'm forgetting somethig...

Thanks for jumping into the challenge! Let me know how it goes ;-)

theplatapi commented 9 years ago

Very comprehensive! I just have two questions:

1) Instead of copying the template files from the reCAPATCHA package, why don't I just add it as a dependecy and use it?

2) With adding the keys, AT.prototype.configure is on the server AND client, right? That means that the private key will be leaked into the client. A separate file in the server/ directory is needed to keep it private, as well as a separate user config just in the server. What do you think?

To avoid forcing the user to create a separate server config file, they can provide the private key in a Meteor settings file or an Environment Variable and I can pick it up from there. It makes things less customizable however.

splendido commented 9 years ago

Answers:

1) I'm not against adding it as a dependency, but I guess it will be easier to go raw. For example you could put a:

Accounts.Templates.prototype.reCaptchaRendered = function() {
    $.getScript('//www.google.com/recaptcha/api/js/recaptcha_ajax.js', function() {
        Recaptcha.create(reCAPTCHA.settings.publickey, 'rendered-captcha-container', {
            theme: reCAPTCHA.settings.theme,
            callback: function() {
                return;
            }
        });
    });
};

somewhere inside this folder and then do

Template.reChaptcha.rendered = Accounts.Templates.reCaptchaRendered;

for each front-end framework (i.e. inside the file re_chaptcha.js to be put into this folder) This would allow to have the code in only one place without having to duplicate it for each styled version (alike to what it's done for all other helpers...)

2) you're right. you can call AccountsTemplate.configure on both client and server. Actually it is suggested to keep the configuration file on a shared folder so to be sure to make the same calls on both sides... This is why I wrote to put it clear on the README.md that the configuration call to set the provate key should be done on a file which is under the server folder in order not to make the call on the server and keep the private key safe on the server side.

Basically you should do

AccountsTemplates.configure({
  reChaptcha: {
    privateKey: "XXXXXXXXXXXXXXXX"
  }
});

on the server ONLY and

AccountsTemplates.configure({
  reChaptcha: {
    publicKey: "XXXXXXXXXXXXXXXX"
  }
});

at least on the client (both doing it together with the other calls on a shared file wouldn't be a problem at all).

Actually letting the user configure reChaptcha through the AT interface is another reason not to rely on the other package, so you could implement different way to pick up the key (as you're well suggesting).

On the settings.json file you could have:

{
  "reChaptcha": {
    "privateKey": "XXXXXXXXXXXXXX"
  },
  "public": {
    "reChaptcha": {
      "publicKey": "XXXXXXXXXXXXXX"
    },
  }
}

so to be able to pick up Meteor.settings.reChaptcha.privateKey || AccountsTemplates.options.reChaptcha.privateKey" on the server andMeteor.settings.public.reChaptcha.publicKey || AccountsTemplates.options.reChaptcha.publicKey" on the client. ...and possibly throw a warning on both is one or the other are undefined.

As for the environment variables, keep in mind they're not available on the client side, so you could pick up the private key from the process.env but not the public one. For this reason I'd say direct configuration or Meteor.settings are the two only options...

What do you think?

theplatapi commented 9 years ago

I agree with everything you said. For the second point, I wasn't aware that AccountsTemplates.configure could be called just on the server.

I'll work on this over the next few weeks. Thanks for all the information!

splendido commented 9 years ago

:+1:

theplatapi commented 9 years ago

Hey, I'm almost done with this! The last thing I need to add is error handling. I'm throwing a Meteor.Error if the reCaptcha verification fails, but how do I handle it on the client? I'm confused how to use this method here.

splendido commented 9 years ago

Good news then @theplatapi!

I think all the magic happens in these lines at the moment.

And I'd say the reChaptcha verification should be placed here or here.

I'd also say that something like this:

validReChaptch = something();
if (!validReChaptch) {
  validationErrors['reChaptcha'] = "Incorrect Value";
  someError = true;
}

should be enough to do the trick and get the error propagated down to the client!

...the only missing this might be some wrapper div put around the reChaptcha div/field to show up the error accordingly. Lets see how this is done for the normal text input.

theplatapi commented 9 years ago

Right now it's crashing here, saying TypeError: Cannot read property 'setError' of undefined. So yeah, it needs it's own field/wrapper. How do I go about doing that?

splendido commented 9 years ago

mmm right this could be trickier than what I tought... :( perhaps for now it's easier to signal it as a general error like the Login Forbidden. so a simple:

validReChaptcha = something();
if (!validReChaptcha) {
  throw new Meteor.Error(403, "Invalid reChaptcha code", "Invalid reChaptcha code");
}

should do the job.

...I'd say we'll need to wait for v2.0 to be able to do a better integration.

splendido commented 9 years ago

Done, thanks!