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

Fix fraud prevention token not showing up on checkout pages created via the site editor #8142

Merged
merged 20 commits into from
Feb 13, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
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
4 changes: 4 additions & 0 deletions changelog/fix-8141-fraud-prevention-token-site-editor-problem
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: fix

Fix fraud prevention token not showing up on site editor checkout page
4 changes: 1 addition & 3 deletions client/checkout/blocks/generate-payment-method.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,7 @@ const generatePaymentMethod = async (
paymentMethod: { id },
} = await request.send();

const fraudPreventionToken = document
.querySelector( '#wcpay-fraud-prevention-token' )
?.getAttribute( 'value' );
const fraudPreventionToken = window.wcpayFraudPreventionToken ?? null;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should fallback to false here instead of null. I've included the reasoning in the main review comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching this! It is being converted to an empty string when sending to the server. No changes needed on this one.


return {
type: 'success',
Expand Down
4 changes: 1 addition & 3 deletions client/checkout/blocks/payment-processor.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,7 @@ const getBillingDetails = ( billingData ) => {
};

const getFraudPreventionToken = () => {
return document
.querySelector( '#wcpay-fraud-prevention-token' )
?.getAttribute( 'value' );
return window.wcpayFraudPreventionToken ?? null;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should fallback to false here instead of null. I've included the reasoning in the main review comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, this was the one that's causing 400 to be returned. Nice catch! Addressed in 008817f.

};

const PaymentProcessor = ( {
Expand Down
5 changes: 2 additions & 3 deletions client/checkout/blocks/saved-token-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,8 @@ export const SavedTokenHandler = ( {

useEffect( () => {
return onPaymentSetup( () => {
const fraudPreventionToken = document
.querySelector( '#wcpay-fraud-prevention-token' )
?.getAttribute( 'value' );
const fraudPreventionToken =
window.wcpayFraudPreventionToken ?? null;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should fallback to false here instead of null. I've included the reasoning in the main review comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching this! It is being converted to an empty string when sending to the server. No changes needed on this one too.


return {
type: 'success',
Expand Down
6 changes: 2 additions & 4 deletions client/payment-request/utils/normalize.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,7 @@ export const normalizeOrderData = ( paymentData ) => {
const phone = paymentData?.paymentMethod?.billing_details?.phone ?? '';
const billing = paymentData?.paymentMethod?.billing_details?.address ?? {};
const shipping = paymentData?.shippingAddress ?? {};
const fraudPreventionTokenInput = document.querySelector(
'input[name="wcpay-fraud-prevention-token"]'
);
const fraudPreventionTokenValue = window.wcpayFraudPreventionToken ?? '';

let paymentRequestType = 'payment_request_api';
if ( paymentData?.walletName === 'applePay' ) {
Expand Down Expand Up @@ -78,7 +76,7 @@ export const normalizeOrderData = ( paymentData ) => {
terms: 1,
'wcpay-payment-method': paymentData?.paymentMethod?.id,
payment_request_type: paymentRequestType,
'wcpay-fraud-prevention-token': fraudPreventionTokenInput?.value ?? '',
'wcpay-fraud-prevention-token': fraudPreventionTokenValue,
};
};

Expand Down
24 changes: 2 additions & 22 deletions includes/class-wc-payments-blocks-payment-method.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ public function initialize() {
$this->name = WC_Payment_Gateway_WCPay::GATEWAY_ID;
$this->gateway = WC_Payments::get_gateway();
$this->wc_payments_checkout = WC_Payments::get_wc_payments_checkout();

add_filter( 'the_content', [ $this, 'maybe_add_card_testing_token' ] );
}

/**
Expand Down Expand Up @@ -72,6 +70,8 @@ public function get_payment_method_script_handles() {
WC_Payments::register_script_with_dependencies( 'WCPAY_BLOCKS_CHECKOUT', 'dist/blocks-checkout', [ 'stripe' ] );
wp_set_script_translations( 'WCPAY_BLOCKS_CHECKOUT', 'woocommerce-payments' );

Fraud_Prevention_Service::maybe_append_fraud_prevention_token();

return [ 'WCPAY_BLOCKS_CHECKOUT' ];
}

Expand Down Expand Up @@ -101,24 +101,4 @@ public function get_payment_method_data() {
$this->wc_payments_checkout->get_payment_fields_js_config()
);
}

/**
* Adds the hidden input containing the card testing prevention token to the blocks checkout page.
*
* @param string $content The content that's going to be flushed to the browser.
*
* @return string
*/
public function maybe_add_card_testing_token( $content ) {
if ( ! wp_script_is( 'WCPAY_BLOCKS_CHECKOUT' ) || ! WC()->session ) {
return $content;
}

$fraud_prevention_service = Fraud_Prevention_Service::get_instance();
// phpcs:ignore WordPress.Security.NonceVerification.Missing,WordPress.Security.ValidatedSanitizedInput.MissingUnslash,WordPress.Security.ValidatedSanitizedInput.InputNotSanitized
if ( $fraud_prevention_service->is_enabled() ) {
$content .= '<input type="hidden" name="wcpay-fraud-prevention-token" id="wcpay-fraud-prevention-token" value="' . esc_attr( Fraud_Prevention_Service::get_instance()->get_token() ) . '">';
}
return $content;
}
}
6 changes: 2 additions & 4 deletions includes/class-wc-payments-checkout.php
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ public function register_scripts() {
$script_dependencies[] = 'woocommerce-tokenization-form';
}

Fraud_Prevention_Service::maybe_append_fraud_prevention_token();

$script = 'dist/checkout';

WC_Payments::register_script_with_dependencies( 'wcpay-upe-checkout', $script, $script_dependencies );
Expand Down Expand Up @@ -414,10 +416,6 @@ function() use ( $payment_fields ) {

</fieldset>

<?php if ( WC()->session && Fraud_Prevention_Service::get_instance()->is_enabled() ) : ?>
<input type="hidden" name="wcpay-fraud-prevention-token" value="<?php echo esc_attr( Fraud_Prevention_Service::get_instance()->get_token() ); ?>">
<?php endif; ?>

<?php

do_action( 'wcpay_payment_fields_upe', $this->gateway->id );
Expand Down
6 changes: 3 additions & 3 deletions includes/class-wc-payments-payment-request-button-handler.php
Original file line number Diff line number Diff line change
Expand Up @@ -722,6 +722,8 @@ public function scripts() {

wp_enqueue_script( 'WCPAY_PAYMENT_REQUEST' );

Fraud_Prevention_Service::maybe_append_fraud_prevention_token();

$gateways = WC()->payment_gateways->get_available_payment_gateways();
if ( isset( $gateways['woocommerce_payments'] ) ) {
WC_Payments::get_wc_payments_checkout()->register_scripts();
Expand All @@ -735,9 +737,7 @@ public function display_payment_request_button_html() {
if ( ! $this->should_show_payment_request_button() ) {
return;
}
if ( WC()->session && Fraud_Prevention_Service::get_instance()->is_enabled() ) : ?>
<input type="hidden" name="wcpay-fraud-prevention-token" value="<?php echo esc_attr( Fraud_Prevention_Service::get_instance()->get_token() ); ?>">
<?php endif; ?>
?>
<div id="wcpay-payment-request-button">
<!-- A Stripe Element will be inserted here. -->
</div>
Expand Down
34 changes: 34 additions & 0 deletions includes/fraud-prevention/class-fraud-prevention-service.php
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,40 @@ public static function get_instance( $session = null, $gateway = null ): self {
return self::$instance;
}

/**
* Appends the fraud prevention token to the JS context if the protection is enabled, and a session exists.
*
* @return void
*/
public static function maybe_append_fraud_prevention_token() {
// Check session first before trying to append the token.
if ( ! WC()->session ) {
return;
}

$instance = self::get_instance();

// Don't add the token if the prevention is not enabled.
if ( ! $instance->is_enabled() ) {
return;
}

// Don't add the token if the user isn't on the cart or checkout page.
// Checking the cart page too because the user can pay quickly via the payment buttons on that page.
if ( ! is_checkout() && ! is_cart() ) {
return;
}

wp_register_script( self::TOKEN_NAME, '', [], time(), true );
wp_enqueue_script( self::TOKEN_NAME );
// Add the fraud prevention token to the checkout configuration.
wp_add_inline_script(
self::TOKEN_NAME,
"window.wcpayFraudPreventionToken = '" . esc_js( $instance->get_token() ) . "';",
'after'
);
}

/**
* Sets a instance to be used in request cycle.
* Introduced primarily for supporting unit tests.
Expand Down
65 changes: 0 additions & 65 deletions tests/unit/test-class-wc-payments-checkout.php
Original file line number Diff line number Diff line change
Expand Up @@ -139,71 +139,6 @@ public function tear_down() {
WC_Payments::set_gateway( $this->default_gateway );
}

public function test_fraud_prevention_token_added_when_prevention_service_enabled() {
$token_value = 'test-token';
$fraud_prevention_service_mock = $this->getMockBuilder( Fraud_Prevention_Service::class )
->disableOriginalConstructor()
->getMock();

$fraud_prevention_service_mock
->expects( $this->once() )
->method( 'is_enabled' )
->willReturn( true );

$fraud_prevention_service_mock
->expects( $this->once() )
->method( 'get_token' )
->willReturn( $token_value );

Fraud_Prevention_Service::set_instance( $fraud_prevention_service_mock );

$this->mock_wcpay_gateway
->expects( $this->any() )
->method( 'get_payment_method_ids_enabled_at_checkout' )
->willReturn( [] );

// Use a callback to get and test the output (also suppresses the output buffering being printed to the CLI).
$this->setOutputCallback(
function ( $output ) use ( $token_value ) {
$result = preg_match_all( '/<input[^>]*type="hidden"[^>]*name="wcpay-fraud-prevention-token"[^>]*value="' . preg_quote( $token_value, '/' ) . '"[^>]*>/', $output );

$this->assertSame( 1, $result );
}
);

$this->system_under_test->payment_fields();
}

public function test_fraud_prevention_token_not_added_when_prevention_service_disabled() {
$token_value = 'test-token';
$fraud_prevention_service_mock = $this->getMockBuilder( Fraud_Prevention_Service::class )
->disableOriginalConstructor()
->getMock();

$fraud_prevention_service_mock
->expects( $this->once() )
->method( 'is_enabled' )
->willReturn( true );

Fraud_Prevention_Service::set_instance( $fraud_prevention_service_mock );

$this->mock_wcpay_gateway
->expects( $this->any() )
->method( 'get_payment_method_ids_enabled_at_checkout' )
->willReturn( [] );

// Use a callback to get and test the output (also suppresses the output buffering being printed to the CLI).
$this->setOutputCallback(
function ( $output ) use ( $token_value ) {
$result = preg_match_all( '/<input[^>]*type="hidden"[^>]*name="wcpay-fraud-prevention-token"[^>]*value="' . preg_quote( $token_value, '/' ) . '"[^>]*>/', $output );

$this->assertSame( 0, $result );
}
);

$this->system_under_test->payment_fields();
}

public function test_save_payment_method_checkbox_not_called_when_saved_cards_disabled() {
// given: prepare the dependencies.
wp_set_current_user( 1 );
Expand Down
Loading