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

Add HMAC header for verification #10599

Open
Tracked by #9615
dacook opened this issue Mar 22, 2023 · 4 comments
Open
Tracked by #9615

Add HMAC header for verification #10599

dacook opened this issue Mar 22, 2023 · 4 comments

Comments

@dacook
Copy link
Member

dacook commented Mar 22, 2023

As suggested by Olivier:

How can you verify the order cycle open event is sent by the OFN instance (and not someone else) ? HMAC would be a nice and resilient feature (like github https://docs.github.com/en/webhooks-and-events/webhooks/securing-your-webhooks#validating-payloads-from-github) not too difficult to implement

https://openfoodnetwork.slack.com/archives/C026JMVK1DK/p1679388404086199?thread_ts=1679352493.562299&cid=C026JMVK1DK

@dacook
Copy link
Member Author

dacook commented Mar 22, 2023

I think we could use the user's API key as the secret key.

@olivier5741
Copy link

I think we could use the user's API key as the secret key.

This could be a temporary workaround :). But it's interesting to have both separate because the way you can protect both is different and who is in charge is different.

The generation of the API key is OFN responsibility, the hash secret is the integrator responsibility (a bad hash secret will only affect the integrator).

For protection, on OFN side you only need a hash of the API key. Someone send you a request, you take the api key, you make a hash of it and compare it to the value of what you have in database. So if someone steels the database or manage to get access to the api key stored in there, they can't use them because they are hashed (ideal world ... their need to be some thinking on how to hash api keys) -> you don't need to ask to everybody to change its api key ... This can't be achieved for a webhook secret, you need the secret to create a hash.

Side note :) : for me, the api key should only be showed once since you would store a hash of it ... (I would consider it mandatory by law since it's an equivalent of a password ...)

@RachL
Copy link
Contributor

RachL commented Mar 22, 2023

@dacook is this a tech-debt issue?

@dacook
Copy link
Member Author

dacook commented Mar 22, 2023

Thanks Olivier, I definitely didn't think about it that much!

@RachL I don't consider it technically tech debt or a bug, but rather a security enhancement.

I guess if you have business-critical tasks relying on the webhook, and especially if you're performing actions based on a user's order details once paid, I can now see that this would be important. You wouldn't want somebody to fake the webhook to say that they paid for certain products that they didn't.

So I'd consider this a pre-requisite for any webhooks based on order data.

@sigmundpetersen sigmundpetersen mentioned this issue Feb 20, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: All the things 💤
Development

No branches or pull requests

3 participants