Page MenuHomePhabricator

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

Authored by epriestley on Feb 8 2019, 3:18 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Apr 5, 9:18 PM
Unknown Object (File)
Tue, Mar 26, 1:36 PM
Unknown Object (File)
Tue, Mar 26, 1:35 PM
Unknown Object (File)
Tue, Mar 26, 1:35 PM
Unknown Object (File)
Tue, Mar 26, 1:34 PM
Unknown Object (File)
Feb 15 2024, 8:37 PM
Unknown Object (File)
Feb 3 2024, 9:16 PM
Unknown Object (File)
Jan 23 2024, 7:54 PM
Subscribers
None

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):

Screen Shot 2019-02-08 at 7.12.49 AM.png (546×2 px, 136 KB)

After:

Screen Shot 2019-02-08 at 7.12.23 AM.png (389×604 px, 34 KB)

Diff Detail

Repository
rP Phabricator
Branch
stripe1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 21907
Build 29908: Run Core Tests
Build 29907: arc lint + arc unit

Event Timeline

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–181

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.

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.Feb 8 2019, 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.