modxcms / Quip

A commenting component for MODX Revolution
https://modx.com/extras/package/quip
4 stars 16 forks source link

Updated reCaptcha integration to use the 2.0 API. #21

Open benk-cogapp opened 7 years ago

Waivor commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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:

benk-cogapp commented 7 years ago

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 commented 7 years ago

The $fields array exists with the following data:

reCaptchaResponse Object ( [is_valid] => [error] => ) Array ( [nospam] => [thread] => insightNr24 [parent] => 0 [name] => My Name [email] => test@test.de [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 commented 7 years ago

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 commented 7 years ago

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

benk-cogapp commented 7 years ago

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 commented 7 years ago

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] => test@test.de [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 commented 7 years ago

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 commented 6 years ago

Hi - does this actually work?