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

fix: remove reCAPTCHA for Woo from modal checkout #1984

Merged
merged 11 commits into from
Dec 4, 2024
Merged

fix: remove reCAPTCHA for Woo from modal checkout #1984

Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
2fc0ddd
fix: remove reCAPTCHA for Woo from modal checkout
laurelfulford Dec 3, 2024
c3b6b6f
fix: don't remove recaptcha script; this seems dangerous and doesn't …
laurelfulford Dec 3, 2024
d225247
fix: remove formatting change
laurelfulford Dec 3, 2024
9cb4f16
fix: updating approach to removing reCAPTCHA JS
laurelfulford Dec 3, 2024
1b8371a
fix: update approach to stripping out reCAPTCHA
laurelfulford Dec 3, 2024
b10dfd4
fix: remove unneeded 0s
laurelfulford Dec 3, 2024
7e0b99e
fix: change up the formatting
laurelfulford Dec 4, 2024
f398d8c
fix: add selectWoo to allowed scripts
laurelfulford Dec 4, 2024
79d9edb
fix: update filters to match official reCaptcha for Woo plugin
laurelfulford Dec 4, 2024
25d6760
fix: add metadata to the list of hooks to remove
laurelfulford Dec 4, 2024
ad577d6
fix: improve class check
laurelfulford Dec 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 55 additions & 2 deletions includes/class-modal-checkout.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,9 @@ public static function init() {
}
add_filter( 'woocommerce_subscriptions_product_limited_for_user', [ __CLASS__, 'subscriptions_product_limited_for_user' ], 10, 3 );
add_filter( 'woocommerce_get_privacy_policy_text', [ __CLASS__, 'woocommerce_get_privacy_policy_text' ], 10, 2 );

// Remove any hooks that aren't supported by the modal checkout.
add_action( 'plugins_loaded', [ __CLASS__, 'remove_hooks' ] );
}

/**
Expand Down Expand Up @@ -731,6 +734,7 @@ public static function dequeue_scripts() {
'newspack-ui',
'newspack-style',
'newspack-recaptcha',
'newspack-woocommerce-style',
// Woo.
'woocommerce',
'WCPAY',
Expand All @@ -740,6 +744,7 @@ public static function dequeue_scripts() {
'wcs-',
'stripe',
'select2',
'selectWoo',
// Metorik.
'metorik',
];
Expand All @@ -756,7 +761,7 @@ public static function dequeue_scripts() {
foreach ( $wp_scripts->queue as $handle ) {
$allowed = false;
foreach ( $allowed_assets as $allowed_asset ) {
if ( false !== strpos( $handle, $allowed_asset ) ) {
if ( 0 === strpos( $handle, $allowed_asset ) ) {
$allowed = true;
break;
}
Expand All @@ -768,7 +773,7 @@ public static function dequeue_scripts() {
foreach ( $wp_styles->queue as $handle ) {
$allowed = false;
foreach ( $allowed_assets as $allowed_asset ) {
if ( false !== strpos( $handle, $allowed_asset ) ) {
if ( 0 === strpos( $handle, $allowed_asset ) ) {
$allowed = true;
break;
}
Expand All @@ -779,6 +784,54 @@ public static function dequeue_scripts() {
}
}

/**
* Remove any hooks that may not work nicely with the modal checkout.
*/
public static function remove_hooks() {
if ( ! self::is_modal_checkout() ) {
return;
}
$remove_list = [
// reCAPTCHA for WooCommerce.
[
'hook' => 'woocommerce_review_order_before_payment',
'callback' => 'rcfwc_field_checkout',
],
[
'hook' => 'woocommerce_review_order_after_payment',
'callback' => 'rcfwc_field_checkout',
],
[
'hook' => 'woocommerce_before_checkout_billing_form',
'callback' => 'rcfwc_field_checkout',
],
[
'hook' => 'woocommerce_after_checkout_billing_form',
'callback' => 'rcfwc_field_checkout',
],
[
'hook' => 'woocommerce_review_order_before_submit',
'callback' => 'rcfwc_field_checkout',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

],
[
'hook' => 'woocommerce_checkout_process',
'callback' => 'rcfwc_checkout_check',
],
];

/**
* Filters the hooks to remove from the modal checkout.
*
* @param string[] $remove_list Array of hooks to remove.
*/
$remove_list = apply_filters( 'newspack_blocks_modal_checkout_remove_hooks', $remove_list );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


foreach ( $remove_list as $remove ) {
$priority = has_action( $remove['hook'], $remove['callback'] );
remove_action( $remove['hook'], $remove['callback'], $priority );
}
}

/**
* Enqueue script for triggering modal checkout.
*
Expand Down
Loading