-
Notifications
You must be signed in to change notification settings - Fork 0
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.
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
Fix processing of account.application.deauthorized events #33
Conversation
Codecov Report
@@ Coverage Diff @@
## next #33 +/- ##
==========================================
+ Coverage 99.05% 99.06% +<.01%
==========================================
Files 34 34
Lines 1799 1814 +15
Branches 159 160 +1
==========================================
+ Hits 1782 1797 +15
Misses 9 9
Partials 8 8
Continue to review full report at Codecov.
|
7858405
to
31b4db7
Compare
pinax/stripe/webhooks.py
Outdated
) | ||
except stripe.error.PermissionError as exc: | ||
if stripe_account_id not in str(exc): | ||
raise exc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be more specific, and needs to cover the re-raise.
pinax/stripe/webhooks.py
Outdated
""" | ||
Validate incoming events. | ||
|
||
We don't fetch remote the event as we lost access to the account where the event belongs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doc needs to be adjusted.
pinax/stripe/webhooks.py
Outdated
@@ -144,8 +144,30 @@ class AccountApplicationDeauthorizeWebhook(Webhook): | |||
name = "account.application.deauthorized" | |||
description = "Occurs whenever a user deauthorizes an application. Sent to the related application only." | |||
|
|||
def validate(self): | |||
""" | |||
Validate incoming events. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*"Specialized validation of incoming events."
a067097
to
a951d06
Compare
pinax/stripe/webhooks.py
Outdated
self.event.validated_message = self.event.webhook_message | ||
self.event.stripe_account = self.stripe_account | ||
else: | ||
raise ValueError("The remote account is still valid. This might be a hostile event.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed the message, please pull.
d49106b
to
5ed86af
Compare
don't try to fetch the event as we lost access to remote account
webhook_message=data, | ||
) | ||
RetrieveMock.side_effect = stripe.error.PermissionError( | ||
"The provided key 'sk_test_********************abcd' does not have access to account 'acc_aa'") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That wasn't the full message, was it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
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() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.get()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to blow up in case we are not in sync ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in #35
self.event.validated_message = self.event.webhook_message | ||
self.event.stripe_account = self.stripe_account | ||
else: | ||
raise ValueError("The remote account still valid. this might be an hostile event") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried twice to fix this: c962eaf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushing to my fork is fine when I don't work on it.
it means I have time to read the notification.
thank you anyway
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. There is --force-with-lease
for git-push to only force-push is what you expect it to be.
don't try to fetch the event as we lost access to remote account