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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated reCaptcha integration to use the 2.0 API. #21

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

benk-cogapp
Copy link

No description provided.

@Waivor
Copy link

Waivor commented Sep 28, 2016

Thanks! Am I right in assuming that ReCaptcha v2 will be displayed but there are some final steps missing? After all, I constantly get the message that the recaptcha was not filled in correctly after clicking the post button.

@benk-cogapp
Copy link
Author

This should just work out-of-the-box - I've tried it on a fresh install of MODx and it seems to be fine. It may be worth checking that you're using a v2 compatible key. What's the exact error you're seeing?

@Waivor
Copy link

Waivor commented Sep 29, 2016

I created a new ReCaptcha key to ensure that it is v2 comaptible but the error still exists. The exact error is: "The reCAPTCHA wasn't entered correctly. Go back and try it again." (lexicon-key: recaptcha.incorrect). Additionally, I have checked all file changes line by line. The files in core/components should be identical.

@benk-cogapp
Copy link
Author

Scanning through for that error, it looks like that will get invoked if you're using comment previews. I hadn't spotted that was also using reCaptcha, and I've updated the logic there to match create.php. Give that a try, and if it's still giving you issues let me know what your snippet call for QuipReply looks like.

@Waivor
Copy link

Waivor commented Sep 29, 2016

Unfortunately nothing changes. The QuipReply looks like this:

[[!QuipReply? 
     &thread=`[[!getTemplateName]]Nr[[*id]]` 
     &dateFormat=`%d.%m.%Y` 
     &tplAddComment=`quipAddComment` 
     &requirePreview=`0`
     &closeAfter=`0` 
     &moderate=`1` 
     &recaptcha=`1` 
     &disableRecaptchaWhenLoggedIn=`1` 
     &moderators=`[[*publishedby:userinfo=`username]]` 
     &moderatorGroup=`Administrator` 
]]

Some notes:

  • &disableRecaptchaWhenLoggedIn doesn't disable reCaptcha.
  • It doesn't matter whether I'm logged in or not. The error is always the same.

@benk-cogapp
Copy link
Author

Hmm, I'm still not able to replicate this one. The issue with &disableRecaptchaWhenLoggedIn looks like it's unrelated - from looking at the code in create.php the user needs to have access to the web context for this to work.

The only other debugging step I can see is to actually take a look at what response you're getting back from reCaptcha - could you get the values of $response and $fields from ~line 58 in create.php (before the check for if (!$response->is_valid))?

@Waivor
Copy link

Waivor commented Sep 29, 2016

The $fields array exists with the following data:

reCaptchaResponse Object ( [is_valid] => [error] => ) Array ( [nospam] => [thread] => insightNr24 [parent] => 0 [name] => My Name [email] => [email protected] [website] => [g-recaptcha-response] => 03AHJ_VusifAs10-okxT-S3oz85VXF84ESxOBGOdtCc_yoPPz5y_UbyZMH_4Hjfmk4e2IkJ6V1tdIPfZihb4TGdA ... Puams [comment] => Test [quip-post] => 1 )

The $response array contains: reCaptchaResponse Object ( [is_valid] => [error] => )

@benk-cogapp
Copy link
Author

Okay, that's interesting - the fact that it's returning the $response with an empty error code suggests it's falling into the failed block in checkAnswers() when there isn't actually an error. I suspect this might be to do with how json_decode() is handling your response - if you change line 147 in recaptcha.class.php to:

if ((bool) $answers['success'] === true) {

does this make any difference? If not, it would be useful to know what value $answers has at that point.

@Waivor
Copy link

Waivor commented Sep 30, 2016

No changes so far: The $response and $fields outputs are identical. $answers has the following value: NULL.

@benk-cogapp
Copy link
Author

That...sounds wrong. $response and $fields should never have the same value (unless they're not yet set) since they come from different places. Where are you checking the value of $answers? If you capture it just before line 147 in recaptcha.class.php it should have a value like:

Array(
    [success] => 1
    [challenge_ts] => 2016-10-03T15:22:19Z
    [hostname] => modx.example.com
)

I can't see any set of circumstances that would obviously cause $answer to be NULL unless json_decode() was having serious issues with the response.

@Waivor
Copy link

Waivor commented Oct 4, 2016

In order to guard against misunderstandings: $response and $fields didn't change. Their output is still:

reCaptchaResponse Object ( [is_valid] => [error] => ) Array ( [nospam] => [thread] => insightNr24 [parent] => 0 [name] => My Name [email] => [email protected] [website] => [g-recaptcha-response] => 23HGI... [comment] => Test [quip-post] => 1 )

and

reCaptchaResponse Object ( [is_valid] => [error] => ).

It seems that json_decode() has a serious issue with the response because the output for $answers right before line 147 is NULL for var_dump($answers);.

@Waivor
Copy link

Waivor commented Oct 12, 2016

It seems that the error is in this code snippet:
$getResponse = $this->httpGet($verifyServer,array ( 'remoteip' => $remoteIp, 'secret' => $this->config[reCaptcha::OPT_PRIVATE_KEY], 'response' => $response, ));.

$getResponse isn't generated probably and that's why json_decode() has a serious issue with the response. $remoteIp, $this->config[reCaptcha::OPT_PRIVATE_KEY] and $response are clean.

@dubbsdubblin
Copy link

Hi - does this actually work?

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.

3 participants