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

Revert route validation #9009

Merged
merged 2 commits into from
Jun 25, 2024
Merged

Conversation

brucealdridge
Copy link
Contributor

Fixes #9008

Changes proposed in this Pull Request

This PR reverts #8901 which introduced stricter REST route validation of unique object IDs. Restoring the validation of these URLs to the state in WooPayments 7.7.0.

The original PR increased validation with stricter object ID's checking moving from \w+ to [A-Za-z0-9]+ regex.

Charges and disputes also had specific prefixes required (ch|py|pi)_ and (dispute|dp)_.

After the release of WooPayments 7.8.0 reports came in that confirmed dispute objects can also have a prefix of du_. Due to the original PR, these disputes were unable to viewed or responded to.

As per Stripe docs, changing ID prefixes is considered backwards compatible:

Stripe considers the following changes to be backwards-compatible:
Changing the length or format of opaque strings, such as object IDs, error messages, and other human-readable strings.
This includes adding or removing fixed prefixes (such as ch_ on charge IDs). ⚠️

Testing instructions

Test the various routes that are affected by this PR.

  • Charge authorization
  • View charge
  • View, update and close dispute
  • Get file, content, and detail
  • View fraud detail
  • In person payments
  • Charge timeline

NB: As the validation state is reverting to a known working state in WooPayments 7.7, not all of these scenarios need to be tested.


  • Run npm run changelog to add a changelog file, choose patch to 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.
  • Covered with tests (or have a good reason not to test in description ☝️)
  • Tested on mobile (or does not apply)

Post merge

@brucealdridge brucealdridge changed the base branch from trunk to develop June 24, 2024 23:47
@botwoo
Copy link
Collaborator

botwoo commented Jun 24, 2024

Test the build

Option 1. Jetpack Beta

  • Install and activate Jetpack Beta.
  • Use this build by searching for PR number 9009 or branch name revert-8901-update/route-param-validation in your-test.site/wp-admin/admin.php?page=jetpack-beta&plugin=woocommerce-payments

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:

  • Latest commit: 7764c42
  • Build time: 2024-06-25 01:21:00 UTC

Note: the build is updated when a new commit is pushed to this PR.

@Jinksi Jinksi requested review from a team and Jinksi June 24, 2024 23:50
Copy link
Contributor

github-actions bot commented Jun 24, 2024

Size Change: 0 B

Total Size: 1.25 MB

ℹ️ View Unchanged
Filename Size
release/woocommerce-payments/assets/css/admin.css 1.08 kB
release/woocommerce-payments/assets/css/admin.rtl.css 1.08 kB
release/woocommerce-payments/assets/css/success.css 172 B
release/woocommerce-payments/assets/css/success.rtl.css 172 B
release/woocommerce-payments/dist/blocks-checkout-rtl.css 2.07 kB
release/woocommerce-payments/dist/blocks-checkout.css 2.07 kB
release/woocommerce-payments/dist/blocks-checkout.js 51.1 kB
release/woocommerce-payments/dist/bnpl-announcement-rtl.css 530 B
release/woocommerce-payments/dist/bnpl-announcement.css 531 B
release/woocommerce-payments/dist/bnpl-announcement.js 20 kB
release/woocommerce-payments/dist/cart-block.js 15.3 kB
release/woocommerce-payments/dist/cart.js 4.57 kB
release/woocommerce-payments/dist/checkout-rtl.css 599 B
release/woocommerce-payments/dist/checkout.css 599 B
release/woocommerce-payments/dist/checkout.js 31.5 kB
release/woocommerce-payments/dist/express-checkout-rtl.css 155 B
release/woocommerce-payments/dist/express-checkout.css 155 B
release/woocommerce-payments/dist/express-checkout.js 3.59 kB
release/woocommerce-payments/dist/index-rtl.css 41.1 kB
release/woocommerce-payments/dist/index.css 41.1 kB
release/woocommerce-payments/dist/index.js 294 kB
release/woocommerce-payments/dist/multi-currency-analytics.js 1.05 kB
release/woocommerce-payments/dist/multi-currency-rtl.css 3.28 kB
release/woocommerce-payments/dist/multi-currency-switcher-block.js 59.5 kB
release/woocommerce-payments/dist/multi-currency.css 3.29 kB
release/woocommerce-payments/dist/multi-currency.js 54.7 kB
release/woocommerce-payments/dist/order-rtl.css 733 B
release/woocommerce-payments/dist/order.css 735 B
release/woocommerce-payments/dist/order.js 41.8 kB
release/woocommerce-payments/dist/payment-gateways-rtl.css 1.21 kB
release/woocommerce-payments/dist/payment-gateways.css 1.21 kB
release/woocommerce-payments/dist/payment-gateways.js 38.6 kB
release/woocommerce-payments/dist/payment-request-rtl.css 155 B
release/woocommerce-payments/dist/payment-request.css 155 B
release/woocommerce-payments/dist/payment-request.js 5.92 kB
release/woocommerce-payments/dist/plugins-page-rtl.css 388 B
release/woocommerce-payments/dist/plugins-page.css 388 B
release/woocommerce-payments/dist/plugins-page.js 19.3 kB
release/woocommerce-payments/dist/product-details-rtl.css 398 B
release/woocommerce-payments/dist/product-details.css 402 B
release/woocommerce-payments/dist/product-details.js 11.2 kB
release/woocommerce-payments/dist/settings-rtl.css 11 kB
release/woocommerce-payments/dist/settings.css 10.9 kB
release/woocommerce-payments/dist/settings.js 201 kB
release/woocommerce-payments/dist/subscription-edit-page.js 669 B
release/woocommerce-payments/dist/subscription-product-onboarding-modal-rtl.css 527 B
release/woocommerce-payments/dist/subscription-product-onboarding-modal.css 527 B
release/woocommerce-payments/dist/subscription-product-onboarding-modal.js 19.4 kB
release/woocommerce-payments/dist/subscription-product-onboarding-toast.js 693 B
release/woocommerce-payments/dist/subscriptions-empty-state-rtl.css 120 B
release/woocommerce-payments/dist/subscriptions-empty-state.css 120 B
release/woocommerce-payments/dist/subscriptions-empty-state.js 18.5 kB
release/woocommerce-payments/dist/tokenized-payment-request-rtl.css 155 B
release/woocommerce-payments/dist/tokenized-payment-request.css 155 B
release/woocommerce-payments/dist/tokenized-payment-request.js 6.56 kB
release/woocommerce-payments/dist/tos-rtl.css 235 B
release/woocommerce-payments/dist/tos.css 236 B
release/woocommerce-payments/dist/tos.js 21 kB
release/woocommerce-payments/dist/woopay-direct-checkout.js 4.94 kB
release/woocommerce-payments/dist/woopay-express-button-rtl.css 155 B
release/woocommerce-payments/dist/woopay-express-button.css 155 B
release/woocommerce-payments/dist/woopay-express-button.js 15.2 kB
release/woocommerce-payments/dist/woopay-rtl.css 4.25 kB
release/woocommerce-payments/dist/woopay.css 4.22 kB
release/woocommerce-payments/dist/woopay.js 69.4 kB
release/woocommerce-payments/includes/subscriptions/assets/css/plugin-page.css 622 B
release/woocommerce-payments/includes/subscriptions/assets/js/plugin-page.js 815 B
release/woocommerce-payments/vendor/automattic/jetpack-assets/build/i18n-loader.js 2.44 kB
release/woocommerce-payments/vendor/automattic/jetpack-assets/src/js/i18n-loader.js 1.01 kB
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/jetpack-sso-admin-create-user.css 196 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/jetpack-sso-admin-create-user.js 20 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/jetpack-sso-admin-create-user.rtl.css 196 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/jetpack-sso-login.css 627 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/jetpack-sso-login.js 20 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/jetpack-sso-login.rtl.css 628 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/jetpack-sso-users.js 390 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/tracks-ajax.js 522 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/tracks-callables.js 581 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/src/sso/jetpack-sso-admin-create-user.css 214 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/src/sso/jetpack-sso-admin-create-user.js 523 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/src/sso/jetpack-sso-login.css 722 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/src/sso/jetpack-sso-login.js 408 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/src/sso/jetpack-sso-users.js 517 B
release/woocommerce-payments/vendor/automattic/jetpack-identity-crisis/babel.config.js 160 B
release/woocommerce-payments/vendor/automattic/jetpack-identity-crisis/build/index.css 2.36 kB
release/woocommerce-payments/vendor/automattic/jetpack-identity-crisis/build/index.js 13.5 kB
release/woocommerce-payments/vendor/automattic/jetpack-identity-crisis/build/index.rtl.css 2.36 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/about.css 1.03 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/admin-empty-state.css 291 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/admin-order-statuses.css 403 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/admin.css 3.6 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/checkout.css 299 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/modal.css 742 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/view-subscription.css 572 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/wcs-upgrade.css 411 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/admin-pointers.js 544 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/admin.js 9.4 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/jstz.js 6.8 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/jstz.min.js 3.83 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/meta-boxes-coupon.js 544 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/meta-boxes-subscription.js 2.52 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/moment.js 22.1 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/moment.min.js 11.6 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/payment-method-restrictions.js 1.29 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/wcs-meta-boxes-order.js 502 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/frontend/payment-methods.js 355 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/frontend/single-product.js 429 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/frontend/view-subscription.js 1.38 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/frontend/wcs-cart.js 781 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/modal.js 1.1 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/wcs-upgrade.js 1.27 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/build/index.css 392 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/build/index.js 3.05 kB

compressed-size-action

Copy link
Member

@Jinksi Jinksi left a comment

Choose a reason for hiding this comment

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

Looks good @brucealdridge 🌟

I've tested the following on a live (non-local) site in test mode against production WCPay-server.

  • Charge authorization & capture
  • View charge
  • View, update (challenged) and close (accept) dispute.
    • Note: I have not tested against a du_ dispute ID – unsure how to generate one of these
    • Looking at the code, however, this PR should fix the issue.
  • Get file, content, and detail (Upload and view logo in card reader receipt settings)
  • View fraud detail (blocked payment transaction details)
  • In person payments
  • Charge timeline

@brucealdridge brucealdridge force-pushed the revert-8901-update/route-param-validation branch from 6d96965 to 56f080e Compare June 25, 2024 01:10
@brucealdridge brucealdridge changed the base branch from develop to trunk June 25, 2024 01:10
@dmvrtx dmvrtx merged commit 92ff80d into trunk Jun 25, 2024
22 checks passed
@dmvrtx dmvrtx deleted the revert-8901-update/route-param-validation branch June 25, 2024 08:09
dmvrtx pushed a commit that referenced this pull request Jun 25, 2024
This PR reverts #8901 which introduced stricter REST route validation of unique object IDs. Restoring the validation of these URLs to the state in WooPayments 7.7.0.

The original PR increased validation with stricter object ID's checking moving from `\w+` to `[A-Za-z0-9]+` regex.

Charges and disputes also had specific prefixes required `(ch|py|pi)_` and `(dispute|dp)_`. 

After the release of WooPayments 7.8.0 reports came in that confirmed dispute objects can also have a prefix of `du_`. Due to the original PR, these disputes were unable to viewed or responded to.

As per [Stripe docs](https://docs.stripe.com/upgrades#what-changes-does-stripe-consider-to-be-backwards-compatible), changing ID prefixes is considered backwards compatible:
> Stripe considers the following changes to be backwards-compatible:
> Changing the length or format of opaque strings, such as object IDs, error messages, and other human-readable strings.
> This includes adding or removing fixed prefixes (such as ch_ on charge IDs). ⚠️
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Try challenging a dispute will result in "Dispute not loaded" error
4 participants