Automated Payment Events: Stripe #5908
Conversation
|
|
simon-id
changed the title
simon-id
changed the title
simon-id
requested review from
claponcet,
mabdinur,
manuel-alvarez-alvarez and
wantsui
and removed request for
a team
|
|
||
|
|
||
| def make_webhook_request(data: dict, secret: bytes = WEBHOOK_SECRET): | ||
| timestamp = int(time.time()) |
There was a problem hiding this comment.
Why do you need a mutable value for this ? If it's only about getting different payload, would it be possible to use an incrementor instead ?
There was a problem hiding this comment.
you mean for the time ? it's how the Stripe API signs its webhook payloads, it's made so you can't replay an event in the future, the signed payload is only valid around the time it was signed. This is how the Stripe API behave, I just mirrored the logic, and I think we should keep it.
If you worry it will be flaky, we can always adjust the time offset tolerance config in the libraries
There was a problem hiding this comment.
Nah, that's fine. We'll consider that iif it turns out to be flaky.
Do you mind just adding a small comment explaining this ?
There was a problem hiding this comment.
| timestamp = int(time.time()) | |
| timestamp = int(time.time()) # the stripe backend signs the payload with a timestamp for security reasons |
cbeauchesne
left a comment
cbeauchesne
left a comment
There was a problem hiding this comment.
Just a small non blocking comment, the rest is AGTM !
|
Updating the PR to fix CI issues |
Add tests for the new Automated Payment Events feature for the Stripe SDK. Added on the RASP scenario because it needs to mock the Stripe API, and i'm reusing the
internal_servercontainer as a mock. It also includes hardcoded rules copied from this PR: https://github.com/DataDog/appsec-event-rules/pull/264Also includes the Node.js weblog update.
[RFC link]