fix: remove reCAPTCHA for Woo from modal checkout #1984

laurelfulford · 2024-12-03T20:11:17Z

All Submissions:

Have you followed the Newspack Contributing guideline?
Does your code follow the WordPress' coding standards and VIP Go coding standards?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

This PR removes and dequeues code from the reCAPTCHA for WooCommerce plugin from our modal checkout.

See 1207817176293825-as-1208873550136264

Note: the way I went about removing the scripts might be terrible; it seemed kind of duplicative, but I couldn't think of a better way that didn't require spelling out each allowed script.

How to test the changes in this Pull Request:

Install reCAPTCHA for WooCommerce and enable it on your site.
Try running through a modal checkout; note you get an error.
Apply this PR.
Confirm that you can complete the modal checkout.
View source in the modal checkout and confirm you don't have anything referencing recaptcha from the plugin (a g-recaptcha div, plus the script, rcfwc.js).
Confirm that those elements still exist in the regular checkout and that the reCAPTCHA there doesn't throw errors.

Other information:

Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes, as applicable?
Have you successfully ran tests with your changes locally?

…seem required

includes/class-modal-checkout.php

chickenn00dle · 2024-12-03T20:51:07Z

includes/class-modal-checkout.php

+	/**
+	 * Remove hooks related to reCAPTCHA for WooCommerce
+	 */
+	public static function remove_recaptcha_hooks() {


Question: How common is this extension? It feels strange to have to account for specific 3rd party plugin hooks like this within our own plugin. It also doesn't seem very sustainable for other cases that might be impacting checkout down the line.

Is it possible to do this more dynamically using an approach similar to how we handle allowed scripts?

How common is this extension?

It's active on 28 sites now (and installed but inactive on others) -- definitely more than I would have expected!

Is it possible to do this more dynamically using an approach similar to how we handle allowed scripts?

Like pass a filterable array of hooks to remove? I can dig into it!

laurelfulford · 2024-12-04T00:28:59Z

Ok I think I have everything sorted, @chickenn00dle!

As an extra precaution, I did some error logging for the scripts/styles getting removed before and after this change, and re-added a couple -- one (the woocommerce styles one) was causing issues. I didn't see any issues caused by selectWoo, but better safe than sorry! (It's Woo's accessible fork of select2, which was also being enqueued [I think] as a fallback).

For the remove_action part, I added an array with the hook and callback that can be filtered using newspack_blocks_modal_checkout_remove_hooks.

Just let me know if anything needs changing -- thanks!

chickenn00dle

Hmm, I tested with the changes but am still seeing the recaptcha error on my end:

Not sure if its the same extension but I am testing with this one: https://woocommerce.com/products/recaptcha-for-woocommerce/

I also left two small comments that are non-blocking (sort of). Let me know what you think!

chickenn00dle · 2024-12-04T16:04:55Z

includes/class-modal-checkout.php

+			],
+			[
+				'hook'     => 'woocommerce_review_order_before_submit',
+				'callback' => 'rcfwc_field_checkout',


Another (non-blocking question) I had a look through the code and am seeing we have this array of fields to filter out during validation in the modal checkout js:

newspack-blocks/src/modal-checkout/index.js

Lines 618 to 620 in f398d8c

const removeFromValidation = [

'save_user_in_woopay',

];

For this hook, which I'm assuming is adding a checkout field, is this something that can be removed using the same strategy?

We'd still need to remove hooks associated with any backend logic of course, but if we can stick to the same strategy for bypassing field validation that'd be great!

For this hook, which I'm assuming is adding a checkout field, is this something that can be removed using the same strategy?

I'm not sure of the ins and outs of how this works, but it's adding a div, like:

<div class="g-recaptcha" data-sitekey="6LfrhJEqAAAAAPuv8JoMdAqEM6G-TKQxc2Es_37A"></div>

I double-checked, and it doesn't look like anything related to reCAPTCHA is included in $form.serializeArray(), which is what we're checking removeFromValidation for.

chickenn00dle · 2024-12-04T16:08:50Z

includes/class-modal-checkout.php

+		 *
+		 * @param string[] $remove_list Array of hooks to remove.
+		 */
+		$remove_list = apply_filters( 'newspack_blocks_modal_checkout_remove_hooks', $remove_list );


Another non-blocking comment. This is definitely better. But in the end we are still needing to specify the rcfwc_ callbacks here. This is fine if this is the only one, but I can't imagine we don't run into similar issues in the future with other extensions.

I haven't dug too deep into it, but it's possible to view all hooks associated with a specific action via the $wp_filter global. Is it possible to do something like create an allow list of plugins, and check the added filters for the checkout processing hook(s), then remove if not in the allow list?

I'm happy to go with this quick fix specific to recaptcha for woocommerce, but really feel this should be improved so we don't have to keep accounting for one-off plugins like this.

I think for now (and in the interest of getting this fixed and rolled out during alphas), my vote would be to keep it simple and revisit when we need to add more plugins 😅 I know something more abstracted would be ideal but I'm worried about cutting this too close to the wire.

I'm going to spin up a separate issue to capture this and see what I can do with it! 🙂

laurelfulford · 2024-12-04T18:42:18Z

I've updated the code to actually work with the correct ReCaptcha for WooCommerce plugin 😆

Anything related to the plugin should be dequeued/removed in the modal checkout, but still be present in the regular checkout. The plugin uses the prefix i13 for scripts and elements it inserts.

chickenn00dle

Looks good. Thanks @laurelfulford

chickenn00dle · 2024-12-04T19:13:30Z

includes/class-modal-checkout.php

+		$remove_list = [];
+
+		// reCaptcha for WooCommerce.
+		if ( method_exists( 'I13_Woo_Recpatcha', '__construct' ) ) {


non-blocking nit: we can just do a check for the class with class_exists.

Thanks Raz! Fixed in ad577d6.

github-actions · 2024-12-04T20:04:17Z

Hey @laurelfulford, good job getting this PR merged! 🎉

Now, the needs-changelog label has been added to it.

Please check if this PR needs to be included in the "Upcoming Changes" and "Release Notes" doc. If it doesn't, simply remove the label.

If it does, please add an entry to our shared document, with screenshots and testing instructions if applicable, then remove the label.

Thank you! ❤️

# [4.5.0-alpha.2](v4.5.0-alpha.1...v4.5.0-alpha.2) (2024-12-04) ### Bug Fixes * remove reCaptcha for WooCommere code from modal checkout ([#1984](#1984)) ([8e250eb](8e250eb))

matticbot · 2024-12-04T20:10:02Z

🎉 This PR is included in version 4.5.0-alpha.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

# [4.5.0](v4.4.0...v4.5.0) (2024-12-09) ### Bug Fixes * also search for coauthor posts by term slug ([#1954](#1954)) ([49357ff](49357ff)) * **modal-checkout:** allow all gateway assets ([#1988](#1988)) ([e371e30](e371e30)) * **modal-checkout:** handle paypal ([#1985](#1985)) ([9bb2b8c](9bb2b8c)) * **ras-acc:** correct spacing issue around saved credit cards ([#1980](#1980)) ([52a5c57](52a5c57)) * **ras-acc:** fix display issues with Additional Fields ([#1979](#1979)) ([b9390ef](b9390ef)) * **ras-acc:** remove space caused by empty divs ([#1978](#1978)) ([8cb6ead](8cb6ead)) * remove reCaptcha for WooCommere code from modal checkout ([#1984](#1984)) ([8e250eb](8e250eb)) ### Features * add Bluesky support to the Author Profile, List blocks ([#1969](#1969)) ([d26a7e4](d26a7e4)) * add CSS class to variation buttons for tracking ([#1989](#1989)) ([910e6b1](910e6b1)) * add support for Newspack Guest Contributor in HPP blocks ([#1934](#1934)) ([c16849e](c16849e)) * merge RAS-ACC work into trunk ([#1977](#1977)) ([2eeaa89](2eeaa89))

matticbot · 2024-12-09T17:38:48Z

🎉 This PR is included in version 4.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

laurelfulford added 3 commits December 3, 2024 12:03

fix: remove reCAPTCHA for Woo from modal checkout

2fc0ddd

fix: don't remove recaptcha script; this seems dangerous and doesn't …

c3b6b6f

…seem required

fix: remove formatting change

d225247

laurelfulford requested a review from a team as a code owner December 3, 2024 20:11

laurelfulford added [Status] Needs Review ras-acc testing labels Dec 3, 2024

chickenn00dle requested changes Dec 3, 2024

View reviewed changes

laurelfulford added 5 commits December 3, 2024 13:28

fix: updating approach to removing reCAPTCHA JS

9cb4f16

fix: update approach to stripping out reCAPTCHA

1b8371a

fix: remove unneeded 0s

b10dfd4

fix: change up the formatting

7e0b99e

fix: add selectWoo to allowed scripts

f398d8c

laurelfulford requested a review from chickenn00dle December 4, 2024 00:29

chickenn00dle requested changes Dec 4, 2024

View reviewed changes

laurelfulford added 2 commits December 4, 2024 10:29

fix: update filters to match official reCaptcha for Woo plugin

79d9edb

fix: add metadata to the list of hooks to remove

25d6760

laurelfulford requested a review from chickenn00dle December 4, 2024 18:42

chickenn00dle approved these changes Dec 4, 2024

View reviewed changes

github-actions bot added [Status] Approved and removed [Status] Needs Review labels Dec 4, 2024

fix: improve class check

ad577d6

laurelfulford merged commit 8e250eb into trunk Dec 4, 2024
8 checks passed

laurelfulford deleted the fix/remove-recaptcha-for-woo-from-modal branch December 4, 2024 20:04

github-actions bot added the needs-changelog label Dec 4, 2024

matticbot added the released on @alpha label Dec 4, 2024

laurelfulford removed the needs-changelog label Dec 4, 2024

matticbot added the released label Dec 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: remove reCAPTCHA for Woo from modal checkout #1984

fix: remove reCAPTCHA for Woo from modal checkout #1984

laurelfulford commented Dec 3, 2024 •

edited

Loading

chickenn00dle Dec 3, 2024

laurelfulford Dec 3, 2024

laurelfulford commented Dec 4, 2024 •

edited

Loading

chickenn00dle left a comment

chickenn00dle Dec 4, 2024

laurelfulford Dec 4, 2024

chickenn00dle Dec 4, 2024

laurelfulford Dec 4, 2024

laurelfulford Dec 4, 2024

laurelfulford commented Dec 4, 2024

chickenn00dle left a comment

chickenn00dle Dec 4, 2024

laurelfulford Dec 4, 2024

github-actions bot commented Dec 4, 2024

matticbot commented Dec 4, 2024

matticbot commented Dec 9, 2024

fix: remove reCAPTCHA for Woo from modal checkout #1984

fix: remove reCAPTCHA for Woo from modal checkout #1984

Conversation

laurelfulford commented Dec 3, 2024 • edited Loading

All Submissions:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Other information:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laurelfulford commented Dec 4, 2024 • edited Loading

chickenn00dle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laurelfulford commented Dec 4, 2024

chickenn00dle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Dec 4, 2024

matticbot commented Dec 4, 2024

matticbot commented Dec 9, 2024

laurelfulford commented Dec 3, 2024 •

edited

Loading

laurelfulford commented Dec 4, 2024 •

edited

Loading