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

Jump to bottom

feat: rate limit adding new payment methods by user #3679

Merged
merged 8 commits into from
Feb 19, 2025
Merged

feat: rate limit adding new payment methods by user #3679

merged 8 commits into from
Feb 19, 2025

Conversation

dkoo
Copy link
Contributor

@dkoo dkoo commented Jan 16, 2025 •
edited
Loading

All Submissions:

Changes proposed in this Pull Request:

Building on #3678, improves rate limiting for adding payment methods in My Account.

How to test the changes in this Pull Request:

  1. Check out this branch and enable/connect at least two different payment gateways.
  2. In wp-config.php, define NEWSPACK_CHECKOUT_RATE_LIMIT as something long enough for manual testing like 60 or 90.
  3. As a reader, log in and visit /my-account/add-payment-method to add a new payment method.
  4. Add a new payment method for Stripe using a test Stripe card which will be declined. Confirm that you get a "card was declined" error.
  5. Within the number of seconds defined in step 2, attempt to submit the same or a different card number and confirm that you get an error message:
Screenshot 2025-01-16 at 11 00 19 AM
  1. Within the number of seconds defined in step 2, attempt to submit a new payment method with a different payment gateway and confirm that you get the same error message again.
  2. Wait until after the time limit has elapsed, then try to add a valid card number once more with either payment gateway and confirm that it succeeds.

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?

@dkoo dkoo added the [Status] Needs Review The issue or pull request needs to be reviewed label Jan 16, 2025
@dkoo dkoo self-assigned this Jan 16, 2025
@dkoo dkoo requested a review from a team as a code owner January 16, 2025 18:01
@dkoo dkoo marked this pull request as draft January 23, 2025 15:54
Base automatically changed from feat/rate-limit-checkouts-payment-methods to trunk January 23, 2025 17:14
@dkoo dkoo marked this pull request as ready for review February 19, 2025 18:12
@dkoo
Copy link
Contributor Author

dkoo commented Feb 19, 2025

@Automattic/newspack-product this is now ready for review

chickenn00dle
chickenn00dle requested changes Feb 19, 2025
View reviewed changes
Copy link
Contributor

@chickenn00dle chickenn00dle left a comment

Choose a reason for hiding this comment

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

Nice work @dkoo!

Leaving this comment for posterity, but as discussed in p1739998359685289-slack-C04UKFHPB6U, this is almost good to go. Just need to address the get_current_user_id issue.

chickenn00dle
chickenn00dle approved these changes Feb 19, 2025
View reviewed changes
Copy link
Contributor

@chickenn00dle chickenn00dle left a comment

Choose a reason for hiding this comment

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

Nice! That did the trick. Looks good 👍

@github-actions github-actions bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Feb 19, 2025
@dkoo dkoo merged commit 0fd5ea5 into trunk Feb 19, 2025
8 checks passed
@dkoo dkoo deleted the feat/rate-limit-payment-methods branch February 19, 2025 22:52
@github-actions GitHub Actions
Copy link

Hey @dkoo, 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! ❤️

matticbot pushed a commit that referenced this pull request Feb 20, 2025
@semantic-release-bot
chore(release): 6.0.0-alpha.1 [skip ci]
e9a1602 
# [6.0.0-alpha.1](v5.14.2...v6.0.0-alpha.1) (2025-02-20)

### Bug Fixes

* **campaigns:** remove placeholder Analytics admin page ([#3729](#3729)) ([abd1bc2](abd1bc2))
* **corrections:** address feedbacks on improving code ([b84fab2](b84fab2))
* **corrections:** improve code formatting ([11ed58c](11ed58c))
* **donations:** handle trashed products and avoid creating dupes ([#3760](#3760)) ([5e78832](5e78832))
* **esp-sync:** transform outgoing dates to site timzeone ([#3728](#3728)) ([77dc361](77dc361))
* **perfmatters:** default for lazyload img setting parent selector ([#3753](#3753)) ([491e335](491e335))
* **recaptcha:** improvements for reCAPTCHA v2 + modal checkout ([#3692](#3692)) ([c4738a7](c4738a7))
* **recaptcha:** no need to scroll to top when showing v2 widget ([#3741](#3741)) ([882d55c](882d55c))
* **woocommerce:** add team name to checkouts for memberships-for-teams ([#3752](#3752)) ([e3661c6](e3661c6))

### Features

* **corrections-modal:** refactor corrections admin UI ([0adac63](0adac63))
* **corrections:** add date handling to corrections and enhance UI ([4c77020](4c77020))
* **corrections:** add site timezone handling logic for correction date ([28d3bb9](28d3bb9))
* **corrections:** update style of modal in the editor ([#3766](#3766)) ([0aee542](0aee542))
* **esp-sync:** queue data events sync to run once ([#3661](#3661)) ([dd2b499](dd2b499))
* fixes and improvements for WooCommerce Subscriptions Gifting ([#3747](#3747)) ([49c4b35](49c4b35))
* handle user nicename change ([#3725](#3725)) ([cb045a3](cb045a3))
* **my-account:** add email change feature flag ([#3758](#3758)) ([21f2c30](21f2c30))
* rate limit adding new payment methods by user ([#3679](#3679)) ([0fd5ea5](0fd5ea5))

### BREAKING CHANGES

* **recaptcha:** shippable product orders will auto-complete by default after this change.

* chore: undo unwanted change
@matticbot
Copy link
Contributor

🎉 This PR is included in version 6.0.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chickenn00dle chickenn00dle chickenn00dle approved these changes

Assignees

@dkoo dkoo

Labels
released on @alpha [Status] Approved The pull request has been reviewed and is ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dkoo @matticbot @chickenn00dle @leogermani