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.

Sign up for GitHub

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

Jump to bottom

dev/core#2150 Update checking of response function to work with v2 api #311

Merged
merged 1 commit into from
Oct 29, 2020
Merged

dev/core#2150 Update checking of response function to work with v2 api #311

merged 1 commit into from
Oct 29, 2020

Conversation

seamuslee001
Copy link
Contributor

This updates the check answer function to work with the v2 API as per https://developers.google.com/recaptcha/docs/verify

@civibot
Copy link

civibot bot commented Oct 29, 2020

(Standard links)

@civibot civibot bot added the master label Oct 29, 2020
@seamuslee001
Copy link
Contributor Author

@MikeyMJCO @johntwyman @a-cormick-dockery @eileenmcnaughton

@johntwyman
Copy link

@seamuslee001 I think there's another task to modify CRM/Utils/ReCAPTCHA.php in core to make use of this updated library function, right?

@seamuslee001 seamuslee001 force-pushed the dev_core_2150 branch 2 times, most recently from 724ba4a to 8efb303 Compare October 29, 2020 04:45
@johntwyman johntwyman mentioned this pull request Oct 29, 2020
Merged
@johntwyman
Copy link

Noting that @andrew-cormick-dockery has been able to verify that this PR and this one work as expected.

andrew-cormick-dockery
andrew-cormick-dockery reviewed Oct 29, 2020
View reviewed changes
recaptcha/recaptchalib.php Outdated Show resolved Hide resolved
recaptcha/recaptchalib.php Outdated Show resolved Hide resolved
recaptcha/recaptchalib.php Outdated Show resolved Hide resolved
@seamuslee001
dev/core#2150 Update checking of response function to work with v2 api
48aaf9a 
Use Guzzle instead of fsockopen

Rename variables as per Andrew and also return string all the time from the post and drop port param as not needed any more
@mattwire
Copy link
Contributor

This looks good to me. Merging based on @andrew-cormick-dockery/@johntwyman review and test.

@mattwire mattwire merged commit 4380321 into civicrm:master Oct 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrew-cormick-dockery andrew-cormick-dockery andrew-cormick-dockery left review comments

Assignees
No one assigned
Labels
master
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@seamuslee001 @johntwyman @mattwire @andrew-cormick-dockery