Skip to content
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

Only accept webhooks with a matching livemode setting #547

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

polski-g
Copy link

What's this PR do?

Adds an optional configuration option to only accept events if the event "livemode" parameter is set to a value we're interested in.

Any background context you want to provide?

Per Stripe (https://stripe.com/docs/connect/webhooks):

For Connect webhooks, it’s important to note that while only test webhooks will be sent to your development webhook URLs, both live and test webhooks will be sent to your production webhook URLs. This is due to the fact that you can perform both live and test transactions under a production application. For this reason, we recommend you check the livemode value when receiving an event webhook to know what action, if any, should be taken.

So we need to be really sure that we want to accept a webhook event for the given environment.

What ticket or issue # does this fix?

None that I know of.

Definition of Done (check if considered and/or addressed):

Code is backward/forward compatible; default setting is None, which means do not check "livemode" parameter at all.

No new dependencies.

Documentation not updated in this PR.

I don't know how to write unit tests or run the test suite, so not done.

@codecov
Copy link

codecov bot commented Mar 11, 2018

Codecov Report

Merging #547 into master will decrease coverage by 0.16%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #547      +/-   ##
==========================================
- Coverage    99.4%   99.23%   -0.17%     
==========================================
  Files          33       33              
  Lines        1838     1841       +3     
  Branches      168      170       +2     
==========================================
  Hits         1827     1827              
- Misses          5        7       +2     
- Partials        6        7       +1
Flag Coverage Δ
#py27dj110 98.91% <0%> (-0.17%) ⬇️
#py27dj111 98.91% <0%> (-0.17%) ⬇️
#py27dj18 99.18% <0%> (-0.17%) ⬇️
#py34dj110 98.91% <0%> (-0.17%) ⬇️
#py34dj111 98.91% <0%> (-0.17%) ⬇️
#py34dj18 99.18% <0%> (-0.17%) ⬇️
#py34dj20 98.91% <0%> (-0.17%) ⬇️
#py35dj110 98.91% <0%> (-0.17%) ⬇️
#py35dj111 98.91% <0%> (-0.17%) ⬇️
#py35dj18 99.18% <0%> (-0.17%) ⬇️
#py35dj20 98.91% <0%> (-0.17%) ⬇️
#py36dj111 98.91% <0%> (-0.17%) ⬇️
#py36dj20 98.91% <0%> (-0.17%) ⬇️
#py36dj20psql 98.91% <0%> (-0.17%) ⬇️
Impacted Files Coverage Δ
pinax/stripe/webhooks.py 98.17% <0%> (-0.91%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fafa715...9618f78. Read the comment docs.

@paltman
Copy link
Member

paltman commented Nov 26, 2021

Good idea, but I think we should probably just discard in the view before even creating the Event

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants