diff --git a/corporate/lib/stripe.py b/corporate/lib/stripe.py index c554052bae..e4fc350e67 100644 --- a/corporate/lib/stripe.py +++ b/corporate/lib/stripe.py @@ -194,16 +194,23 @@ def do_create_stripe_customer(user: UserProfile, stripe_token: Optional[str]=Non return customer @catch_stripe_errors -def do_replace_payment_source(user: UserProfile, stripe_token: str) -> stripe.Customer: +def do_replace_payment_source(user: UserProfile, stripe_token: str, + pay_invoices: bool=False) -> stripe.Customer: stripe_customer = stripe_get_customer(Customer.objects.get(realm=user.realm).stripe_customer_id) stripe_customer.source = stripe_token # Deletes existing card: https://stripe.com/docs/api#update_customer-source - # This can also have other side effects, e.g. it will try to pay certain past-due - # invoices: https://stripe.com/docs/api#update_customer updated_stripe_customer = stripe.Customer.save(stripe_customer) RealmAuditLog.objects.create( realm=user.realm, acting_user=user, event_type=RealmAuditLog.STRIPE_CARD_CHANGED, event_time=timezone_now()) + if pay_invoices: + for stripe_invoice in stripe.Invoice.list( + billing='charge_automatically', customer=stripe_customer.id, status='open'): + # The user will get either a receipt or a "failed payment" email, but the in-app + # messaging could be clearer here (e.g. it could explictly tell the user that there + # were payment(s) and that they succeeded or failed). + # Worth fixing if we notice that a lot of cards end up failing at this step. + stripe.Invoice.pay(stripe_invoice) return updated_stripe_customer # event_time should roughly be timezone_now(). Not designed to handle diff --git a/corporate/tests/stripe_fixtures/replace_payment_source--Customer.retrieve.2.json b/corporate/tests/stripe_fixtures/replace_payment_source--Customer.retrieve.2.json index e4ccdecce6..74cd489a96 100644 Binary files a/corporate/tests/stripe_fixtures/replace_payment_source--Customer.retrieve.2.json and b/corporate/tests/stripe_fixtures/replace_payment_source--Customer.retrieve.2.json differ diff --git a/corporate/tests/stripe_fixtures/replace_payment_source--Customer.retrieve.3.json b/corporate/tests/stripe_fixtures/replace_payment_source--Customer.retrieve.3.json index e4ccdecce6..74cd489a96 100644 Binary files a/corporate/tests/stripe_fixtures/replace_payment_source--Customer.retrieve.3.json and b/corporate/tests/stripe_fixtures/replace_payment_source--Customer.retrieve.3.json differ diff --git a/corporate/tests/stripe_fixtures/replace_payment_source--Customer.retrieve.4.json b/corporate/tests/stripe_fixtures/replace_payment_source--Customer.retrieve.4.json index e4ccdecce6..9d152f6a46 100644 Binary files a/corporate/tests/stripe_fixtures/replace_payment_source--Customer.retrieve.4.json and b/corporate/tests/stripe_fixtures/replace_payment_source--Customer.retrieve.4.json differ diff --git a/corporate/tests/stripe_fixtures/replace_payment_source--Customer.retrieve.5.json b/corporate/tests/stripe_fixtures/replace_payment_source--Customer.retrieve.5.json new file mode 100644 index 0000000000..9d152f6a46 Binary files /dev/null and b/corporate/tests/stripe_fixtures/replace_payment_source--Customer.retrieve.5.json differ diff --git a/corporate/tests/stripe_fixtures/replace_payment_source--Customer.retrieve.6.json b/corporate/tests/stripe_fixtures/replace_payment_source--Customer.retrieve.6.json new file mode 100644 index 0000000000..636afc3747 Binary files /dev/null and b/corporate/tests/stripe_fixtures/replace_payment_source--Customer.retrieve.6.json differ diff --git a/corporate/tests/stripe_fixtures/replace_payment_source--Customer.save.1.json b/corporate/tests/stripe_fixtures/replace_payment_source--Customer.save.1.json index 92e6750bfe..03f37a1c34 100644 Binary files a/corporate/tests/stripe_fixtures/replace_payment_source--Customer.save.1.json and b/corporate/tests/stripe_fixtures/replace_payment_source--Customer.save.1.json differ diff --git a/corporate/tests/stripe_fixtures/replace_payment_source--Customer.save.2.json b/corporate/tests/stripe_fixtures/replace_payment_source--Customer.save.2.json index be642ba9d3..46b79b7b95 100644 Binary files a/corporate/tests/stripe_fixtures/replace_payment_source--Customer.save.2.json and b/corporate/tests/stripe_fixtures/replace_payment_source--Customer.save.2.json differ diff --git a/corporate/tests/stripe_fixtures/replace_payment_source--Customer.save.3.json b/corporate/tests/stripe_fixtures/replace_payment_source--Customer.save.3.json new file mode 100644 index 0000000000..cc69346f3f Binary files /dev/null and b/corporate/tests/stripe_fixtures/replace_payment_source--Customer.save.3.json differ diff --git a/corporate/tests/stripe_fixtures/replace_payment_source--Invoice.create.2.json b/corporate/tests/stripe_fixtures/replace_payment_source--Invoice.create.2.json new file mode 100644 index 0000000000..efff6a82ea Binary files /dev/null and b/corporate/tests/stripe_fixtures/replace_payment_source--Invoice.create.2.json differ diff --git a/corporate/tests/stripe_fixtures/replace_payment_source--Invoice.finalize_invoice.2.json b/corporate/tests/stripe_fixtures/replace_payment_source--Invoice.finalize_invoice.2.json new file mode 100644 index 0000000000..756a91e914 Binary files /dev/null and b/corporate/tests/stripe_fixtures/replace_payment_source--Invoice.finalize_invoice.2.json differ diff --git a/corporate/tests/stripe_fixtures/replace_payment_source--Invoice.list.1.json b/corporate/tests/stripe_fixtures/replace_payment_source--Invoice.list.1.json new file mode 100644 index 0000000000..5e5dd98a6d Binary files /dev/null and b/corporate/tests/stripe_fixtures/replace_payment_source--Invoice.list.1.json differ diff --git a/corporate/tests/stripe_fixtures/replace_payment_source--Invoice.list.2.json b/corporate/tests/stripe_fixtures/replace_payment_source--Invoice.list.2.json new file mode 100644 index 0000000000..82f6d5eeba Binary files /dev/null and b/corporate/tests/stripe_fixtures/replace_payment_source--Invoice.list.2.json differ diff --git a/corporate/tests/stripe_fixtures/replace_payment_source--Invoice.list.3.json b/corporate/tests/stripe_fixtures/replace_payment_source--Invoice.list.3.json new file mode 100644 index 0000000000..82f6d5eeba Binary files /dev/null and b/corporate/tests/stripe_fixtures/replace_payment_source--Invoice.list.3.json differ diff --git a/corporate/tests/stripe_fixtures/replace_payment_source--Invoice.list.4.json b/corporate/tests/stripe_fixtures/replace_payment_source--Invoice.list.4.json new file mode 100644 index 0000000000..995f52e9cf Binary files /dev/null and b/corporate/tests/stripe_fixtures/replace_payment_source--Invoice.list.4.json differ diff --git a/corporate/tests/stripe_fixtures/replace_payment_source--Invoice.pay.1.json b/corporate/tests/stripe_fixtures/replace_payment_source--Invoice.pay.1.json new file mode 100644 index 0000000000..2b4b216f31 Binary files /dev/null and b/corporate/tests/stripe_fixtures/replace_payment_source--Invoice.pay.1.json differ diff --git a/corporate/tests/stripe_fixtures/replace_payment_source--Invoice.pay.2.json b/corporate/tests/stripe_fixtures/replace_payment_source--Invoice.pay.2.json new file mode 100644 index 0000000000..b56f1da2bf Binary files /dev/null and b/corporate/tests/stripe_fixtures/replace_payment_source--Invoice.pay.2.json differ diff --git a/corporate/tests/stripe_fixtures/replace_payment_source--InvoiceItem.create.3.json b/corporate/tests/stripe_fixtures/replace_payment_source--InvoiceItem.create.3.json new file mode 100644 index 0000000000..3d10e5c8ca Binary files /dev/null and b/corporate/tests/stripe_fixtures/replace_payment_source--InvoiceItem.create.3.json differ diff --git a/corporate/tests/stripe_fixtures/replace_payment_source--Token.create.2.json b/corporate/tests/stripe_fixtures/replace_payment_source--Token.create.2.json index af2fed62d5..ee7547e104 100644 Binary files a/corporate/tests/stripe_fixtures/replace_payment_source--Token.create.2.json and b/corporate/tests/stripe_fixtures/replace_payment_source--Token.create.2.json differ diff --git a/corporate/tests/stripe_fixtures/replace_payment_source--Token.create.3.json b/corporate/tests/stripe_fixtures/replace_payment_source--Token.create.3.json index adebaf8cc4..64f08ed5a1 100644 Binary files a/corporate/tests/stripe_fixtures/replace_payment_source--Token.create.3.json and b/corporate/tests/stripe_fixtures/replace_payment_source--Token.create.3.json differ diff --git a/corporate/tests/stripe_fixtures/replace_payment_source--Token.create.4.json b/corporate/tests/stripe_fixtures/replace_payment_source--Token.create.4.json new file mode 100644 index 0000000000..a0a35dd8cb Binary files /dev/null and b/corporate/tests/stripe_fixtures/replace_payment_source--Token.create.4.json differ diff --git a/corporate/tests/test_stripe.py b/corporate/tests/test_stripe.py index 565dc2ad82..69fc424707 100644 --- a/corporate/tests/test_stripe.py +++ b/corporate/tests/test_stripe.py @@ -153,6 +153,8 @@ def normalize_fixture_data(decorated_function: CallableT, file_content = file_content.replace(match, normalized_values[pattern][match]) file_content = re.sub(r'(?<="risk_score": )(\d+)', '00', file_content) file_content = re.sub(r'(?<="times_redeemed": )(\d+)', '00', file_content) + file_content = re.sub(r'(?<="idempotency-key": )"([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f-]*)"', + '"00000000-0000-0000-0000-000000000000"', file_content) # Dates file_content = re.sub(r'(?<="Date": )"(.* GMT)"', '"NORMALIZED DATETIME"', file_content) file_content = re.sub(r'[0-3]\d [A-Z][a-z]{2} 20[1-2]\d', 'NORMALIZED DATE', file_content) @@ -168,7 +170,7 @@ MOCKED_STRIPE_FUNCTION_NAMES = ["stripe.{}".format(name) for name in [ "Charge.create", "Charge.list", "Coupon.create", "Customer.create", "Customer.retrieve", "Customer.save", - "Invoice.create", "Invoice.finalize_invoice", "Invoice.list", "Invoice.upcoming", + "Invoice.create", "Invoice.finalize_invoice", "Invoice.list", "Invoice.pay", "Invoice.upcoming", "InvoiceItem.create", "InvoiceItem.list", "Plan.create", "Product.create", @@ -819,35 +821,57 @@ class StripeTest(StripeTestCase): user = self.example_user("hamlet") self.login(user.email) self.upgrade() - # Try replacing with a valid card - stripe_token = stripe_create_token(card_number='5555555555554444').id - response = self.client_post("/json/billing/sources/change", - {'stripe_token': ujson.dumps(stripe_token)}) - self.assert_json_success(response) - number_of_sources = 0 - for stripe_source in stripe_get_customer(Customer.objects.first().stripe_customer_id).sources: - self.assertEqual(cast(stripe.Card, stripe_source).last4, '4444') - number_of_sources += 1 - self.assertEqual(number_of_sources, 1) - audit_log_entry = RealmAuditLog.objects.order_by('-id') \ - .values_list('acting_user', 'event_type').first() - self.assertEqual(audit_log_entry, (user.id, RealmAuditLog.STRIPE_CARD_CHANGED)) - RealmAuditLog.objects.filter(acting_user=user).delete() + # Create an open invoice + stripe_customer_id = Customer.objects.first().stripe_customer_id + stripe.InvoiceItem.create(amount=5000, currency='usd', customer=stripe_customer_id) + stripe_invoice = stripe.Invoice.create(customer=stripe_customer_id) + stripe.Invoice.finalize_invoice(stripe_invoice) + RealmAuditLog.objects.filter(event_type=RealmAuditLog.STRIPE_CARD_CHANGED).delete() - # Try replacing with an invalid card + # Replace with an invalid card stripe_token = stripe_create_token(card_number='4000000000009987').id + with patch("corporate.lib.stripe.billing_logger.error") as mock_billing_logger: + with patch("stripe.Invoice.list") as mock_invoice_list: + response = self.client_post("/json/billing/sources/change", + {'stripe_token': ujson.dumps(stripe_token)}) + mock_billing_logger.assert_called() + mock_invoice_list.assert_not_called() + self.assertEqual(ujson.loads(response.content)['error_description'], 'card error') + self.assert_json_error_contains(response, 'Your card was declined') + for stripe_source in stripe_get_customer(stripe_customer_id).sources: + self.assertEqual(cast(stripe.Card, stripe_source).last4, '4242') + self.assertFalse(RealmAuditLog.objects.filter(event_type=RealmAuditLog.STRIPE_CARD_CHANGED).exists()) + + # Replace with a card that's valid, but charging the card fails + stripe_token = stripe_create_token(card_number='4000000000000341').id with patch("corporate.lib.stripe.billing_logger.error") as mock_billing_logger: response = self.client_post("/json/billing/sources/change", {'stripe_token': ujson.dumps(stripe_token)}) mock_billing_logger.assert_called() self.assertEqual(ujson.loads(response.content)['error_description'], 'card error') self.assert_json_error_contains(response, 'Your card was declined') + for stripe_source in stripe_get_customer(stripe_customer_id).sources: + self.assertEqual(cast(stripe.Card, stripe_source).last4, '0341') + self.assertEqual(len(list(stripe.Invoice.list(customer=stripe_customer_id, status='open'))), 1) + self.assertEqual(1, RealmAuditLog.objects.filter( + event_type=RealmAuditLog.STRIPE_CARD_CHANGED).count()) + + # Replace with a valid card + stripe_token = stripe_create_token(card_number='5555555555554444').id + response = self.client_post("/json/billing/sources/change", + {'stripe_token': ujson.dumps(stripe_token)}) + self.assert_json_success(response) number_of_sources = 0 - for stripe_source in stripe_get_customer(Customer.objects.first().stripe_customer_id).sources: + for stripe_source in stripe_get_customer(stripe_customer_id).sources: self.assertEqual(cast(stripe.Card, stripe_source).last4, '4444') number_of_sources += 1 + # Verify that we replaced the previous card, rather than adding a new one self.assertEqual(number_of_sources, 1) - self.assertFalse(RealmAuditLog.objects.filter(event_type=RealmAuditLog.STRIPE_CARD_CHANGED).exists()) + # Ideally we'd also test that we don't pay invoices with billing=='send_invoice' + for stripe_invoice in stripe.Invoice.list(customer=stripe_customer_id): + self.assertEqual(stripe_invoice.status, 'paid') + self.assertEqual(2, RealmAuditLog.objects.filter( + event_type=RealmAuditLog.STRIPE_CARD_CHANGED).count()) class RequiresBillingAccessTest(ZulipTestCase): def setUp(self) -> None: diff --git a/corporate/views.py b/corporate/views.py index bd29cf5ef2..8add45a0d5 100644 --- a/corporate/views.py +++ b/corporate/views.py @@ -218,7 +218,7 @@ def downgrade(request: HttpRequest, user: UserProfile) -> HttpResponse: # nocov def replace_payment_source(request: HttpRequest, user: UserProfile, stripe_token: str=REQ("stripe_token", validator=check_string)) -> HttpResponse: try: - do_replace_payment_source(user, stripe_token) + do_replace_payment_source(user, stripe_token, pay_invoices=True) except BillingError as e: return json_error(e.message, data={'error_description': e.description}) return json_success() diff --git a/stubs/stripe/__init__.pyi b/stubs/stripe/__init__.pyi index 17c6b3d987..cc9b85f71d 100644 --- a/stubs/stripe/__init__.pyi +++ b/stubs/stripe/__init__.pyi @@ -67,7 +67,8 @@ class Invoice: ... @staticmethod - def list(customer: str=..., limit: Optional[int]=...) -> List[Invoice]: + def list(billing: str=..., customer: str=..., limit: Optional[int]=..., + status: str=...) -> List[Invoice]: ... @staticmethod @@ -79,6 +80,10 @@ class Invoice: def finalize_invoice(invoice: Invoice) -> Invoice: ... + @staticmethod + def pay(invoice: Invoice) -> Invoice: + ... + def get(self, key: str) -> Any: ...