Fix fraud prevention token not showing up on checkout pages created via the site editor #8142
Conversation
tpaksu
changed the title
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
|
Size Change: +18 B (0%) Total Size: 1.26 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Thanks for working on this fix, Taha! I've reviewed the code and manually tested it a bit and I've found an issue. I left inline comments in the three JS files where const fraudPreventionToken = window.wcpayFraudPreventionToken ?? null; is used. That is where the issue is.
If card_testing_prevention_enabled is enabled for the account and if the fraudPreventionToken token is null then the POST request returns a 400 response and the following error is returned on the checkout:
payment_data[2][value] is not of type string,boolean.
If null is replaced with false then the POST request returns a 402 response and we get the following:
We're not able to process this payment. Please refresh the page and try again.
I think this is the better outcome in a rare instance where the token could not be generated or retrieved. That way the shopper will hopefully at least refresh and retry the order and (also hopefully) the error that prevented the token from being generated or retrieved was just a one-off anomaly that won't occur a second time.
All that said, I ran into a bigger issue related to the null value. After testing null versus false with card_testing_prevention_enabled enabled, I then disabled card_testing_prevention_enabled and refreshed my account cache on the client. Testing with card_testing_prevention_enabled disabled then null is always passed as the value of fraudPreventionToken and the POST request always returns a 400 and the error:
payment_data[2][value] is not of type string,boolean.
The checkout is broken for shoppers when card_testing_prevention_enabled meta is disabled for the account as a result. However if false is passed instead the order is processed successfully.
| const fraudPreventionToken = document | ||
| .querySelector( '#wcpay-fraud-prevention-token' ) | ||
| ?.getAttribute( 'value' ); | ||
| const fraudPreventionToken = window.wcpayFraudPreventionToken ?? null; |
There was a problem hiding this comment.
I think we should fallback to false here instead of null. I've included the reasoning in the main review comment.
There was a problem hiding this comment.
Thanks for catching this! It is being converted to an empty string when sending to the server. No changes needed on this one.
| return document | ||
| .querySelector( '#wcpay-fraud-prevention-token' ) | ||
| ?.getAttribute( 'value' ); | ||
| return window.wcpayFraudPreventionToken ?? null; |
There was a problem hiding this comment.
I think we should fallback to false here instead of null. I've included the reasoning in the main review comment.
There was a problem hiding this comment.
Yep, this was the one that's causing 400 to be returned. Nice catch! Addressed in 008817f.
| .querySelector( '#wcpay-fraud-prevention-token' ) | ||
| ?.getAttribute( 'value' ); | ||
| const fraudPreventionToken = | ||
| window.wcpayFraudPreventionToken ?? null; |
There was a problem hiding this comment.
I think we should fallback to false here instead of null. I've included the reasoning in the main review comment.
There was a problem hiding this comment.
Thanks for catching this! It is being converted to an empty string when sending to the server. No changes needed on this one too.
There was a problem hiding this comment.
Thanks for fixing the issues that I reported earlier. The latest code changes look good. I've run through the testing and everything is working except for card payments on the twentytwentyfour theme with card_testing_prevention_enabled set to true. I also skipped testing Apple Pay as I don't have an iPhone. I think we can get a fellow team member to test that.
Testing Results:
Storefront theme with card testing prevention disabled:
| Payment Method | Result |
|---|---|
| Regular checkout using test card | ✅ |
| Regular checkout using saved card | ✅ |
| Regular checkout using one UPE method | ✅ |
| ApplePay | ⏭️ |
| GooglePay | ✅ |
| WooPay | ✅ |
Storefront theme with card testing prevention enabled:
| Payment Method | Result |
|---|---|
| Regular checkout using test card | ✅ |
| Regular checkout using saved card | ✅ |
| Regular checkout using one UPE method | ✅ |
| ApplePay | ⏭️ |
| GooglePay | ✅ |
| WooPay | ✅ |
TwentyTwentyFour theme with card testing prevention disabled:
| Payment Method | Result |
|---|---|
| Regular checkout using test card | ✅ |
| Regular checkout using saved card | ✅ |
| Regular checkout using one UPE method | ✅ |
| ApplePay | ⏭️ |
| GooglePay | ✅ |
| WooPay | ✅ |
TwentyTwentyFour theme with card testing prevention enabled:
| Payment Method | Result |
|---|---|
| Regular checkout using test card | ❌ |
| Regular checkout using saved card | ❌ |
| Regular checkout using one UPE method | ✅ |
| ApplePay | ⏭️ |
| GooglePay | ✅ |
| WooPay | ✅ |
Debugging TwentyTwentyFour theme with card testing prevention enabled:
POST https://my-test-site-domain/wp-json/wc/store/v1/checkout?_locale=user
Response: 402 (Payment Required)
Console error:
VM474:1 Uncaught (in promise) SyntaxError: Unexpected token '<', "<!DOCTYPE "... is not valid JSON
Response body:
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Site has been temporarily disabled</title>
<meta name="description" content="">
<meta name="author" content="">
<style>
body {
background-color: #f8f8f8;
font-family: "Helvetica Neue", Helvetica, Arial, sans-serif;
color: #333;
padding-top: 20px;
}
div {
display: block;
}
.container {
width: 1100px;
padding-top: 50px;
margin-left: auto;
margin-right: auto;
}
.error-msg, .support-msg {
text-align: center;
}
.error-msg {
margin-bottom: 40px;
}
.error-msg h1 {
font-size: 52px;
display: block;
margin: 0px;
}
.error-msg p {
font-size: 20px;
display: block;
margin: 10px 0;
}
.support-msg p {
font-size: 14px;
color: #888;
}
</style>
</head>
<body>
<div class="container">
<div class="error-msg">
<p>This site has been disabled pending subscription renewal.</p>
</div>
<div class="support-msg">
<p>Website owner? If you think you have reached this message in error, please contact support.</p>
</div>
</div>
</body>
</html>
The console error and response body above seem to be a symptom and not the cause. This highjacked response is a known issue and there's an open PR to deal with it: #8102.
That said, I checked the POST request payload and the data looks good. I don't know why Stripe is returning it as a failed payment. In all tests I was using the 4242 card.
I'll keep digging on this on my end but wanted to share it now to get it on your radar as well.
There was a problem hiding this comment.
I've re-tested the card and saved card payment flows on the JN test site using the twentytwentyfour theme and the payments are now going through. I'm not sure why Stripe was returning a 402 response on those yesterday, but it seems to have been an intermittent issue. With those two tests passing, now all tests are passing. So I think we can ship this.
LGTM
tpaksu
deleted the
fix/8141-fraud-prevention-token-site-editor-problem
branch
Fixes #8141
Changes proposed in this Pull Request
This PR changes the way the fraud prevention token is delivered to the browser, and retrieved by the payment methods to be sent to the checkout endpoints. The token will now be present as a global JS variable
window.wcpayFraudPreventionTokenwhen the user is viewing the cart page, or the checkout page, when the fraud mitigations are activated for the merchant, instead of being appended as a value in a hidden input.Testing instructions
All the possible checkout types that use our server needs to be tested to see if the payment is still working as intended, with the fraud mitigations disabled, and enabled.
A way to test those in a single installation:
card_testing_prevention_enabledaccount meta to1.Important
Repeat the tests after completing all tests with switching to the Twenty Twenty Four theme. This will test it with a Site Editor enabled theme, which was the main problem in the issue.
npm run changelogto add a changelog file, choosepatchto leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge