From 7c55920d7c89eebd4b8b09d1c6a7912669841671 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Thu, 16 Nov 2017 12:59:03 +0100 Subject: [PATCH 1/8] Fix AccountApplicationDeauthorizeWebhook for non-account events --- pinax/stripe/tests/test_webhooks.py | 18 +++++++++++++++++- pinax/stripe/webhooks.py | 26 +++++++++++++------------- 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/pinax/stripe/tests/test_webhooks.py b/pinax/stripe/tests/test_webhooks.py index 788a016d1..56e424d02 100644 --- a/pinax/stripe/tests/test_webhooks.py +++ b/pinax/stripe/tests/test_webhooks.py @@ -578,12 +578,14 @@ def test_process_deauthorize_fake_response(self, RetrieveMock): @patch("stripe.Event.retrieve") def test_process_deauthorize_with_authorizes_account(self, RetrieveMock): + stripe_account_id = self.account.stripe_id data = {"data": {"object": {"id": "evt_002"}}, - "account": self.account.stripe_id} + "account": stripe_account_id} event = Event.objects.create( kind=AccountApplicationDeauthorizeWebhook.name, webhook_message=data, ) + RetrieveMock.return_value.to_dict.return_value = data with self.assertRaises(ValueError): AccountApplicationDeauthorizeWebhook(event).process() @@ -601,3 +603,17 @@ def test_process_deauthorize_with_delete_account(self, RetrieveMock): self.assertTrue(event.valid) self.assertTrue(event.processed) self.assertIsNone(event.stripe_account) + + @patch("stripe.Event.retrieve") + def test_process_deauthorize_without_account(self, RetrieveMock): + data = {"data": {"object": {"id": "evt_001"}}} + event = Event.objects.create( + kind=AccountApplicationDeauthorizeWebhook.name, + webhook_message=data, + ) + RetrieveMock.return_value.to_dict.return_value = data + AccountApplicationDeauthorizeWebhook(event).process() + self.assertTrue(event.valid) + self.assertTrue(event.processed) + self.account.refresh_from_db() + self.assertTrue(self.account.authorized) diff --git a/pinax/stripe/webhooks.py b/pinax/stripe/webhooks.py index 75beb3459..eac7d5ffa 100644 --- a/pinax/stripe/webhooks.py +++ b/pinax/stripe/webhooks.py @@ -151,27 +151,27 @@ def validate(self): """ Specialized validation of incoming events. - We try to retrieve the event: + When this event is for a connected account we should not be able to + fetch the event anymore (since we have been disconnected). + + Therefore we try to retrieve the event: - in case of PermissionError exception, everything is perfectly normal. It means the account has been deauthorized. - In case no exception has been caught, then, most likely, the event has been forged to make you believe the account has been disabled despite it is still functioning. """ - stripe_account_id = self.event.webhook_message["account"] - self.stripe_account = models.Account.objects.filter(stripe_id=stripe_account_id).first() try: - stripe.Event.retrieve( - self.event.stripe_id, - stripe_account=stripe_account_id, - ) + super(AccountApplicationDeauthorizeWebhook, self).validate() except stripe.error.PermissionError as exc: - if not(stripe_account_id in str(exc) and obfuscate_secret_key(settings.PINAX_STRIPE_SECRET_KEY) in str(exc)): - raise exc - self.event.valid = True - self.event.validated_message = self.event.webhook_message - self.event.stripe_account = self.stripe_account + if self.stripe_account: + stripe_account_id = self.stripe_account.stripe_id + if not(stripe_account_id in str(exc) and obfuscate_secret_key(settings.PINAX_STRIPE_SECRET_KEY) in str(exc)): + raise exc else: - raise ValueError("The remote account is still valid. This might be a hostile event") + if self.stripe_account: + raise ValueError("The remote account is still valid. This might be a hostile event") + self.event.valid = True + self.event.validated_message = self.event.webhook_message def process_webhook(self): if self.stripe_account is not None: From 2ba464f83da45d20b9c221ad4cef9c18413b4060 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Tue, 21 Nov 2017 17:38:56 +0100 Subject: [PATCH 2/8] Add UniquePerAccountStripeObject for Plans The `stripe_id` for plans is not unique by itself globally (it is not generated by Stripe), and therefore needs to be unique per account. --- .../migrations/0011_auto_20171121_1648.py | 24 +++++++++++++++++++ .../migrations/0011_auto_20171123_2016.py | 2 +- pinax/stripe/models.py | 20 ++++++++++++++-- pinax/stripe/tests/test_models.py | 6 +++++ 4 files changed, 49 insertions(+), 3 deletions(-) create mode 100644 pinax/stripe/migrations/0011_auto_20171121_1648.py diff --git a/pinax/stripe/migrations/0011_auto_20171121_1648.py b/pinax/stripe/migrations/0011_auto_20171121_1648.py new file mode 100644 index 000000000..5782b3ed6 --- /dev/null +++ b/pinax/stripe/migrations/0011_auto_20171121_1648.py @@ -0,0 +1,24 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.6 on 2017-11-21 16:48 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('pinax_stripe', '0010_connect'), + ] + + operations = [ + migrations.AlterField( + model_name='plan', + name='stripe_id', + field=models.CharField(max_length=191), + ), + migrations.AlterUniqueTogether( + name='plan', + unique_together=set([('stripe_id', 'stripe_account')]), + ), + ] diff --git a/pinax/stripe/migrations/0011_auto_20171123_2016.py b/pinax/stripe/migrations/0011_auto_20171123_2016.py index 171003b48..4f3ed3707 100644 --- a/pinax/stripe/migrations/0011_auto_20171123_2016.py +++ b/pinax/stripe/migrations/0011_auto_20171123_2016.py @@ -8,7 +8,7 @@ class Migration(migrations.Migration): dependencies = [ - ('pinax_stripe', '0010_connect'), + ('pinax_stripe', '0011_auto_20171121_1648'), ] operations = [ diff --git a/pinax/stripe/models.py b/pinax/stripe/models.py index 9c3adde87..104182157 100644 --- a/pinax/stripe/models.py +++ b/pinax/stripe/models.py @@ -26,7 +26,7 @@ class Meta: abstract = True -class AccountRelatedStripeObject(StripeObject): +class AccountRelatedStripeObjectMixin(models.Model): stripe_account = models.ForeignKey( "pinax_stripe.Account", @@ -44,6 +44,22 @@ class Meta: abstract = True +class AccountRelatedStripeObject(AccountRelatedStripeObjectMixin, StripeObject): + """Uses a mixin to support Django 1.8 (name clash for stripe_id)""" + + class Meta: + abstract = True + + +class UniquePerAccountStripeObject(AccountRelatedStripeObjectMixin): + stripe_id = models.CharField(max_length=191) + created_at = models.DateTimeField(default=timezone.now) + + class Meta: + abstract = True + unique_together = ("stripe_id", "stripe_account") + + class StripeAccountFromCustomerMixin(object): @property def stripe_account(self): @@ -56,7 +72,7 @@ def stripe_account_stripe_id(self): @python_2_unicode_compatible -class Plan(AccountRelatedStripeObject): +class Plan(UniquePerAccountStripeObject): amount = models.DecimalField(decimal_places=2, max_digits=9) currency = models.CharField(max_length=15, blank=False) interval = models.CharField(max_length=15) diff --git a/pinax/stripe/tests/test_models.py b/pinax/stripe/tests/test_models.py index 888cf15ca..21056d834 100644 --- a/pinax/stripe/tests/test_models.py +++ b/pinax/stripe/tests/test_models.py @@ -62,6 +62,12 @@ def test_plan_stripe_plan_with_account(self, RetrieveMock): self.assertTrue(RetrieveMock.call_args_list, [ call("plan", stripe_account="acct_A")]) + def test_plan_per_account(self): + Plan.objects.create(stripe_id="plan", amount=decimal.Decimal("100"), interval="monthly", interval_count=1) + account = Account.objects.create(stripe_id="acct_A") + Plan.objects.create(stripe_id="plan", stripe_account=account, amount=decimal.Decimal("100"), interval="monthly", interval_count=1) + self.assertEquals(Plan.objects.count(), 2) + def test_event_processing_exception_str(self): e = EventProcessingException(data="hello", message="hi there", traceback="fake") self.assertTrue("Event=" in str(e)) From d3731c54cc883d0fd926fb028564be0a2042b247 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Wed, 22 Nov 2017 16:33:13 +0100 Subject: [PATCH 3/8] Add Charge.outcome --- pinax/stripe/actions/charges.py | 1 + .../stripe/migrations/0013_charge_outcome.py | 21 +++++++++++++++++++ pinax/stripe/models.py | 1 + pinax/stripe/tests/test_actions.py | 1 + 4 files changed, 24 insertions(+) create mode 100644 pinax/stripe/migrations/0013_charge_outcome.py diff --git a/pinax/stripe/actions/charges.py b/pinax/stripe/actions/charges.py index ee0857403..5e267134c 100644 --- a/pinax/stripe/actions/charges.py +++ b/pinax/stripe/actions/charges.py @@ -203,6 +203,7 @@ def sync_charge_from_stripe_data(data): ) obj.fee_currency = balance_transaction["currency"] obj.transfer_group = data.get("transfer_group") + obj.outcome = data.get("outcome") obj.save() return obj diff --git a/pinax/stripe/migrations/0013_charge_outcome.py b/pinax/stripe/migrations/0013_charge_outcome.py new file mode 100644 index 000000000..bcb9183e8 --- /dev/null +++ b/pinax/stripe/migrations/0013_charge_outcome.py @@ -0,0 +1,21 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.6 on 2017-11-22 15:40 +from __future__ import unicode_literals + +from django.db import migrations +import jsonfield.fields + + +class Migration(migrations.Migration): + + dependencies = [ + ('pinax_stripe', '0014_blank_with_null'), + ] + + operations = [ + migrations.AddField( + model_name='charge', + name='outcome', + field=jsonfield.fields.JSONField(blank=True, null=True), + ), + ] diff --git a/pinax/stripe/models.py b/pinax/stripe/models.py index 29489307b..6b57d67f6 100644 --- a/pinax/stripe/models.py +++ b/pinax/stripe/models.py @@ -468,6 +468,7 @@ class Charge(StripeAccountFromCustomerMixin, StripeObject): fee_currency = models.CharField(max_length=10, null=True, blank=True) transfer_group = models.TextField(null=True, blank=True) + outcome = JSONField(null=True, blank=True) objects = ChargeManager() diff --git a/pinax/stripe/tests/test_actions.py b/pinax/stripe/tests/test_actions.py index f8b3bcf44..11ed7ce27 100644 --- a/pinax/stripe/tests/test_actions.py +++ b/pinax/stripe/tests/test_actions.py @@ -2155,6 +2155,7 @@ def test_sync_charge_from_stripe_data_failed(self): charge = Charge.objects.get(stripe_id=data["id"]) self.assertEqual(charge.amount, decimal.Decimal("2")) self.assertEqual(charge.customer, None) + self.assertEqual(charge.outcome["risk_level"], "normal") @patch("stripe.Subscription.retrieve") def test_retrieve_stripe_subscription(self, RetrieveMock): From 79c61fd8b31d49d58842fe992c8de0ff8171e079 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Mon, 27 Nov 2017 20:29:44 +0100 Subject: [PATCH 4/8] Charge: add pk to __repr__ --- pinax/stripe/models.py | 3 ++- pinax/stripe/tests/test_models.py | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/pinax/stripe/models.py b/pinax/stripe/models.py index 29489307b..bad902e94 100644 --- a/pinax/stripe/models.py +++ b/pinax/stripe/models.py @@ -472,7 +472,8 @@ class Charge(StripeAccountFromCustomerMixin, StripeObject): objects = ChargeManager() def __repr__(self): - return "Charge(customer={!r}, source={!r}, amount={!r}, captured={!r}, paid={!r}, stripe_id={!r})".format( + return "Charge(pk={!r}, customer={!r}, source={!r}, amount={!r}, captured={!r}, paid={!r}, stripe_id={!r})".format( + self.pk, self.customer, self.source, self.amount, diff --git a/pinax/stripe/tests/test_models.py b/pinax/stripe/tests/test_models.py index f515daf6a..6bc85cd25 100644 --- a/pinax/stripe/tests/test_models.py +++ b/pinax/stripe/tests/test_models.py @@ -131,9 +131,9 @@ def test_connected_customer_str_and_repr(self): def test_charge_repr(self): charge = Charge() if PY2: - self.assertEquals(repr(charge), "Charge(customer=None, source=u'', amount=None, captured=None, paid=None, stripe_id=u'')") + self.assertEquals(repr(charge), "Charge(pk=None, customer=None, source=u'', amount=None, captured=None, paid=None, stripe_id=u'')") else: - self.assertEquals(repr(charge), "Charge(customer=None, source='', amount=None, captured=None, paid=None, stripe_id='')") + self.assertEquals(repr(charge), "Charge(pk=None, customer=None, source='', amount=None, captured=None, paid=None, stripe_id='')") def test_plan_display_invoiceitem(self): p = Plan(amount=decimal.Decimal("5"), name="My Plan", interval="monthly", interval_count=1) From a83cbe413b32331f16dd18f3649d011dd557ff50 Mon Sep 17 00:00:00 2001 From: Patrick Altman Date: Fri, 1 Dec 2017 16:22:46 -0600 Subject: [PATCH 5/8] Update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 9aa1a8ccc..3c3f754eb 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,7 @@ avoid namespace collisions and to have more consistency with Pinax. ## Pinax -Pinax is an open-source platform built on the Django Web Framework. It is an ecosystem of reusable Django apps, themes, and starter project templates. +Pinax is an open-source platform built on the Django Web Framework. It is an ecosystem of reusable Django apps and starter project templates. This collection can be found at http://pinaxproject.com. This app was developed as part of the Pinax ecosystem but is just a Django app and can be used independently of other Pinax apps. From 986a019c6d91a6faecb5150473c2dfc3868276b0 Mon Sep 17 00:00:00 2001 From: Patrick Altman Date: Fri, 1 Dec 2017 16:27:23 -0600 Subject: [PATCH 6/8] testing context --- .circleci/config.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index 57d196ff1..16aefa9ad 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -133,6 +133,7 @@ workflows: test: jobs: - lint + - context: org-global - py27dj18 - py27dj110 - py27dj111 From ba595d6de304e1db27be6c73bf3a5cbbacb8f3a1 Mon Sep 17 00:00:00 2001 From: Patrick Altman Date: Fri, 1 Dec 2017 16:28:07 -0600 Subject: [PATCH 7/8] fix typo --- .circleci/config.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 16aefa9ad..db8e680dd 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -132,8 +132,8 @@ workflows: version: 2 test: jobs: - - lint - - context: org-global + - lint: + context: org-global - py27dj18 - py27dj110 - py27dj111 From 51bbea358495b9724da2bcd68878ab96d1d41eaf Mon Sep 17 00:00:00 2001 From: Patrick Altman Date: Fri, 1 Dec 2017 16:30:50 -0600 Subject: [PATCH 8/8] remove test --- .circleci/config.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index db8e680dd..57d196ff1 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -132,8 +132,7 @@ workflows: version: 2 test: jobs: - - lint: - context: org-global + - lint - py27dj18 - py27dj110 - py27dj111