Page MenuHomePhabricator

Improve UI/UX when users try to add an invalid card with Stripe
ClosedPublic

Authored by epriestley on Fri, Feb 8, 3:18 PM.

Details

Summary

Ref T13244. See PHI1052. Our error handling for Stripe errors isn't great right now. We can give users a bit more information, and a less jarring UI.

Test Plan

Before (this is in developer mode, production doesn't get a stack trace):

After:

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.Fri, Feb 8, 3:18 PM
epriestley requested review of this revision.Fri, Feb 8, 3:19 PM
epriestley added inline comments.Fri, Feb 8, 3:23 PM
src/applications/fund/storage/FundBacker.php
95–96

This is a fix for Fund after T13186, since this policy is now checked explicitly in more places.

src/applications/phortune/controller/payment/PhortunePaymentMethodCreateController.php
112–115

(No other payment method can currently actually raise any errors today -- the "Pile of Wealth" test provider can't fail, and Paypal doesn't go through this flow.)

src/applications/policy/filter/PhabricatorPolicyFilter.php
178–182

Just making this (almost always internal/debugging) exception easier to figure out since it wasn't immediately obvious which object was complaining when I hit an issue in Fund.

amckinley accepted this revision.Fri, Feb 8, 7:36 PM

Unrelated to this diff, but we rate limit users attempting to add new payment methods, right?

This revision is now accepted and ready to land.Fri, Feb 8, 7:36 PM

Unrelated to this diff, but we rate limit users attempting to add new payment methods, right?

Not explicitly, but (on admin, at least) there's no way to generate a charge without waiting 30 days and no way to add a payment method without generating a charge first, so we're not very attractive for validating a big list of stolen cards.

You can also register as many accounts as you want, so even if each account has a limit I'm not sure that's really stopping anything.

Is there some other abuse vector here I'm not aware of? The only one I know of is "validate a big list of stolen cards to see which ones actually work", which I believe to be hard even without any rate limiting since: you have to wait 30 days for us to create a charge for you; you can't generate a charge of less than $20; and once you find a good card it clears the charge and you have to wait another 30 days to get another one so you can keep validating.

Is there some other abuse vector here I'm not aware of?

Nope, validating a list of stolen CC's was the thing I was worrying about.

We do let you add as many cards as you want, and Stripe does some amount of validation on add, at least based on the possible errors the API can return. Let me add some rate limiting while I'm in here since it can't hurt, just wanted to make sure I wasn't missing some kind of weird abuse I've never heard of.

This revision was automatically updated to reflect the committed changes.